Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_fs): Add support for symbolic links (symlinks) (#3706)
Browse files Browse the repository at this point in the history
* feat(rome_fs): Add support for symbolic links (symlinks)

* Rename PathInterner and update the FileSystemDiagnostic struct

* Update comments

* Replace PathBuf with String in FileSystemDiagnostic error_kind

* Add integration tests for Windows
  • Loading branch information
Filip Kieres authored Nov 16, 2022
1 parent 08e7e5b commit 00266da
Show file tree
Hide file tree
Showing 14 changed files with 408 additions and 172 deletions.
10 changes: 5 additions & 5 deletions crates/rome_cli/src/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rome_diagnostics::{
PrintDiagnostic, Severity, Visit,
},
};
use rome_fs::{AtomicInterner, FileSystem, OpenOptions, PathInterner, RomePath};
use rome_fs::{FileSystem, OpenOptions, PathInterner, RomePath};
use rome_fs::{TraversalContext, TraversalScope};
use rome_service::workspace::{SupportsFeatureResult, UnsupportedReason};
use rome_service::{
Expand Down Expand Up @@ -87,7 +87,7 @@ pub(crate) fn traverse(execution: Execution, mut session: CliSession) -> Result<
});
}

let (interner, recv_files) = AtomicInterner::new();
let (interner, recv_files) = PathInterner::new();
let (send_msgs, recv_msgs) = unbounded();
let (sender_reports, recv_reports) = unbounded();

Expand Down Expand Up @@ -589,8 +589,8 @@ struct TraversalOptions<'ctx, 'app> {
workspace: &'ctx dyn Workspace,
/// Determines how the files should be processed
execution: &'ctx Execution,
/// File paths interner used by the filesystem traversal
interner: AtomicInterner,
/// File paths interner cache used by the filesystem traversal
interner: PathInterner,
/// Shared atomic counter storing the number of processed files
processed: &'ctx AtomicUsize,
/// Shared atomic counter storing the number of skipped files
Expand Down Expand Up @@ -640,7 +640,7 @@ impl<'ctx, 'app> TraversalOptions<'ctx, 'app> {
}

impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> {
fn interner(&self) -> &dyn PathInterner {
fn interner(&self) -> &PathInterner {
&self.interner
}

Expand Down
103 changes: 95 additions & 8 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
use pico_args::Arguments;
use rome_cli::Termination;
use std::env::temp_dir;
use std::ffi::OsString;
use std::fs::{create_dir, create_dir_all, remove_dir_all};
#[cfg(target_family = "unix")]
use std::os::unix::fs::symlink;
#[cfg(target_os = "windows")]
use std::os::windows::fs::{symlink_dir, symlink_file};
use std::path::{Path, PathBuf};

use crate::configs::{
Expand All @@ -12,7 +18,7 @@ use crate::configs::{
use crate::snap_test::SnapshotPayload;
use crate::{assert_cli_snapshot, run_cli, FORMATTED, LINT_ERROR, PARSE_ERROR};
use rome_console::{BufferConsole, LogLevel};
use rome_fs::{ErrorEntry, FileSystemExt, MemoryFileSystem};
use rome_fs::{ErrorEntry, FileSystemExt, MemoryFileSystem, OsFileSystem};
use rome_service::DynRef;

const ERRORS: &str = r#"
Expand Down Expand Up @@ -666,23 +672,104 @@ fn no_lint_if_files_are_listed_in_ignore_option() {
}

#[test]
fn fs_error_symlink() {
let mut fs = MemoryFileSystem::default();
fn fs_error_dereferenced_symlink() {
let fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

fs.insert_error(PathBuf::from("prefix/ci.js"), ErrorEntry::SymbolicLink);
let root_path = temp_dir().join("rome_test_broken_symlink");
let subdir_path = root_path.join("prefix");

#[allow(unused_must_use)]
{
remove_dir_all(root_path.clone());
}
create_dir(PathBuf::from(root_path.clone())).unwrap();
create_dir(subdir_path.clone()).unwrap();

#[cfg(target_family = "unix")]
{
symlink(root_path.join("null"), root_path.join("broken_symlink")).unwrap();
}

#[cfg(target_os = "windows")]
{
symlink_file(root_path.join("null"), root_path.join("broken_symlink")).unwrap();
}

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Owned(Box::new(OsFileSystem)),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![OsString::from("check"), OsString::from("prefix")]),
Arguments::from_vec(vec![
OsString::from("check"),
OsString::from(root_path.clone()),
]),
);

remove_dir_all(root_path).unwrap();

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
#[cfg(target_family = "unix")]
"fs_error_dereferenced_symlink_unix",
#[cfg(target_os = "windows")]
"fs_error_dereferenced_symlink_windows",
fs,
console,
result,
));
}

#[test]
fn fs_error_infinite_symlink_exapansion() {
let fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let root_path = temp_dir().join("rome_test_infinite_symlink_exapansion");
let subdir1_path = root_path.join("prefix");
let subdir2_path = root_path.join("foo").join("bar");

#[allow(unused_must_use)]
{
remove_dir_all(root_path.clone());
}
create_dir(PathBuf::from(root_path.clone())).unwrap();
create_dir(subdir1_path.clone()).unwrap();

create_dir_all(subdir2_path.clone()).unwrap();

#[cfg(target_family = "unix")]
{
symlink(subdir1_path.clone(), root_path.join("self_symlink1")).unwrap();
symlink(subdir1_path, subdir2_path.join("self_symlink2")).unwrap();
}

#[cfg(target_os = "windows")]
{
symlink_dir(subdir1_path.clone(), root_path.join("self_symlink1")).unwrap();
symlink_dir(subdir1_path, subdir2_path.join("self_symlink2")).unwrap();
}

let result = run_cli(
DynRef::Owned(Box::new(OsFileSystem)),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![
OsString::from("check"),
OsString::from(root_path.clone()),
]),
);

remove_dir_all(root_path).unwrap();

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"fs_error_symlink",
#[cfg(target_family = "unix")]
"fs_error_infinite_symlink_exapansion_unix",
#[cfg(target_os = "windows")]
"fs_error_infinite_symlink_exapansion_windows",
fs,
console,
result,
Expand All @@ -694,7 +781,7 @@ fn fs_error_unknown() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

fs.insert_error(PathBuf::from("prefix/ci.js"), ErrorEntry::Unknown);
fs.insert_error(PathBuf::from("prefix/ci.js"), ErrorEntry::UnknownFileType);

let result = run_cli(
DynRef::Borrowed(&mut fs),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 223
expression: content
---
# Emitted Messages

```block
/tmp/rome_test_broken_symlink/broken_symlink internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Dereferenced symlink
i Rome encountered a file system entry that is a broken symbolic link: /tmp/rome_test_broken_symlink/broken_symlink
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 223
expression: content
---
# Emitted Messages

```block
C:\Users\User\AppData\Local\Temp\rome_test_broken_symlink\broken_symlink internalError/fs ━━━━━━━━━━
! Dereferenced symlink
i Rome encountered a file system entry that is a broken symbolic link: C:\Users\User\AppData\Local\Temp\rome_test_broken_symlink\broken_symlink
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 223
expression: content
---
# Emitted Messages

```block
/tmp/rome_test_infinite_symlink_exapansion/prefix internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Infinite symlink expansion
× Rome encountered a file system entry that leads to an infinite symbolic link expansion, causing an infinite cycle: /tmp/rome_test_infinite_symlink_exapansion/prefix
```

```block
/tmp/rome_test_infinite_symlink_exapansion/prefix internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Infinite symlink expansion
× Rome encountered a file system entry that leads to an infinite symbolic link expansion, causing an infinite cycle: /tmp/rome_test_infinite_symlink_exapansion/prefix
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 223
expression: content
---
# Emitted Messages

```block
C:\Users\User\AppData\Local\Temp\rome_test_infinite_symlink_exapansion\prefix internalError/fs ━━━━━━━━━━
! Infinite symlink expansion
× Rome encountered a file system entry that leads to an infinite symbolic link expansion, causing an infinite cycle: C:\Users\User\AppData\Local\Temp\rome_test_infinite_symlink_exapansion\prefix
```

```block
C:\Users\User\AppData\Local\Temp\rome_test_infinite_symlink_exapansion\prefix internalError/fs ━━━━━━━━━━
! Infinite symlink expansion
× Rome encountered a file system entry that leads to an infinite symbolic link expansion, causing an infinite cycle: C:\Users\User\AppData\Local\Temp\rome_test_infinite_symlink_exapansion\prefix
```


This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ expression: content
```block
prefix/ci.js internalError/fs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Encountered an unknown file type
! Unknown file type
i Rome encountered a file system entry that's neither a file, directory or symbolic link
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/src/v2/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl<P> FilePath<P> {
}

/// Converts a `FilePath<P>` to `FilePath<&P::Target>`.
pub(crate) fn as_deref(&self) -> FilePath<&<P as Deref>::Target>
pub fn as_deref(&self) -> FilePath<&<P as Deref>::Target>
where
P: Deref,
{
Expand Down
Loading

0 comments on commit 00266da

Please sign in to comment.