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

fix(rome_fs): Allow to ignore patterns to symbolic links (symlinks) #4166

Merged
merged 7 commits into from
Mar 5, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 117 additions & 3 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use pico_args::Arguments;
use std::env::temp_dir;
use std::ffi::OsString;
use std::fs::{create_dir, create_dir_all, remove_dir_all};
use std::fs::{create_dir, create_dir_all, remove_dir_all, File};
use std::io::Write;
#[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::{
CONFIG_FILE_SIZE_LIMIT, CONFIG_LINTER_AND_FILES_IGNORE, CONFIG_LINTER_DISABLED,
CONFIG_LINTER_DOWNGRADE_DIAGNOSTIC, CONFIG_LINTER_IGNORED_FILES,
CONFIG_FILE_SIZE_LIMIT, CONFIG_IGNORE_SYMLINK, CONFIG_LINTER_AND_FILES_IGNORE,
CONFIG_LINTER_DISABLED, CONFIG_LINTER_DOWNGRADE_DIAGNOSTIC, CONFIG_LINTER_IGNORED_FILES,
CONFIG_LINTER_SUPPRESSED_GROUP, CONFIG_LINTER_SUPPRESSED_RULE,
CONFIG_LINTER_UPGRADE_DIAGNOSTIC, CONFIG_RECOMMENDED_GROUP,
};
Expand Down Expand Up @@ -855,6 +856,119 @@ fn fs_error_unknown() {
));
}

// Symbolic link ignore pattern test
//
// Verifies, that ignore patterns to symbolic links are allowed.
//
// ├── hidden_nested
// │   └── test
// │   └── symlink_testcase1_2 -> hidden_testcase1
// ├── hidden_testcase1
// │   └── test
// │   └── test.js // ok
// ├── hidden_testcase2
// │   ├── test1.ts // ignored
// │   ├── test2.ts // ignored
// │   └── test.js // ok
// └── project
// ├── rome.json
// ├── symlink_testcase1_1 -> hidden_nested
// └── symlink_testcase2 -> hidden_testcase2
#[test]
fn fs_files_ignore_symlink() {
let fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let root_path = temp_dir().join("rome_test_files_ignore_symlink");
let project_path = root_path.join("project");

let testcase1_path = root_path.join("hidden_testcase1");
let testcase1_sub_path = testcase1_path.join("test");
let testcase2_path = root_path.join("hidden_testcase2");

let nested_path = root_path.join("hidden_nested");
let nested_sub_path = nested_path.join("test");

#[allow(unused_must_use)]
{
remove_dir_all(root_path.clone());
}
create_dir(root_path.clone()).unwrap();
create_dir(project_path.clone()).unwrap();
create_dir_all(testcase1_sub_path.clone()).unwrap();
create_dir(testcase2_path.clone()).unwrap();
create_dir_all(nested_sub_path.clone()).unwrap();

// project/symlink_testcase1_1
let symlink_testcase1_1_path = project_path.join("symlink_testcase1_1");
// hidden_nested/test/symlink_testcase1_2
let symlink_testcase1_2_path = nested_sub_path.join("symlink_testcase1_2");
// project/symlink_testcase2
let symlink_testcase2_path = project_path.join("symlink_testcase2");

#[cfg(target_family = "unix")]
{
// project/test/symlink_testcase1_1 -> hidden_nested
symlink(nested_path, symlink_testcase1_1_path).unwrap();
// hidden_nested/test/symlink_testcase1_2 -> hidden_testcase1
symlink(testcase1_path, symlink_testcase1_2_path).unwrap();
// project/symlink_testcase2 -> hidden_testcase2
symlink(testcase2_path.clone(), symlink_testcase2_path).unwrap();
}

#[cfg(target_os = "windows")]
{
check_windows_symlink!(symlink_dir(nested_path.clone(), symlink_testcase1_1_path));
check_windows_symlink!(symlink_dir(
testcase1_path.clone(),
symlink_testcase1_2_path
));
check_windows_symlink!(symlink_dir(testcase2_path.clone(), symlink_testcase2_path));
}

let config_path = project_path.join("rome.json");
let mut config_file = File::create(config_path).unwrap();
config_file
.write_all(CONFIG_IGNORE_SYMLINK.as_bytes())
.unwrap();

let files: [PathBuf; 4] = [
testcase1_sub_path.join("test.js"), // ok
testcase2_path.join("test.js"), // ok
testcase2_path.join("test1.ts"), // ignored
testcase2_path.join("test2.ts"), // ignored
];

for file_path in files {
let mut file = File::create(file_path).unwrap();
file.write_all(APPLY_SUGGESTED_BEFORE.as_bytes()).unwrap();
}

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

remove_dir_all(root_path).unwrap();

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

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"fs_files_ignore_symlink",
fs,
console,
result,
));
}

#[test]
fn file_too_large() {
let mut fs = MemoryFileSystem::default();
Expand Down
8 changes: 8 additions & 0 deletions crates/rome_cli/tests/configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,11 @@ pub const CONFIG_FILE_SIZE_LIMIT: &str = r#"{
"maxSize": 16
}
}"#;

pub const CONFIG_IGNORE_SYMLINK: &str = r#"{
"files": {
"ignore": [
"symlink_testcase2/**/*.ts"
]
}
}"#;
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 346
expression: content
---
# Emitted Messages

```block
<TEMP_DIR>/rome_test_files_ignore_symlink/project/rome.json lint ━━━━━━━━━━━━━━━━━━━━

no diagnostic message provided


```

```block
Skipped 4 suggested fixes.
If you wish to apply the suggested (unsafe) fixes, use the command rome check --apply-unsafe

```

```block
Fixed 3 file(s) in <TIME>
```

```block
Skipped 1 file(s)
```


42 changes: 36 additions & 6 deletions crates/rome_fs/src/fs/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {

if file_type.is_dir() {
self.scope.spawn(move |scope| {
handle_dir(scope, ctx, &path);
handle_dir(scope, ctx, &path, None);
});
return;
}
Expand All @@ -162,7 +162,13 @@ impl<'scope> TraversalScope<'scope> for OsTraversalScope<'scope> {
const DEFAULT_IGNORE: &[&str; 5] = &[".git", ".svn", ".hg", ".yarn", "node_modules"];

/// Traverse a single directory
fn handle_dir<'scope>(scope: &Scope<'scope>, ctx: &'scope dyn TraversalContext, path: &Path) {
fn handle_dir<'scope>(
scope: &Scope<'scope>,
ctx: &'scope dyn TraversalContext,
path: &Path,
// The unresolved origin path in case the directory is behind a symbolic link
origin_path: Option<PathBuf>,
) {
if let Some(file_name) = path.file_name().and_then(OsStr::to_str) {
if DEFAULT_IGNORE.contains(&file_name) {
return;
Expand All @@ -186,7 +192,7 @@ fn handle_dir<'scope>(scope: &Scope<'scope>, ctx: &'scope dyn TraversalContext,
}
};

handle_dir_entry(scope, ctx, entry);
handle_dir_entry(scope, ctx, entry, origin_path.clone());
}
}

Expand All @@ -196,6 +202,8 @@ fn handle_dir_entry<'scope>(
scope: &Scope<'scope>,
ctx: &'scope dyn TraversalContext,
entry: DirEntry,
// The unresolved origin path in case the directory is behind a symbolic link
mut origin_path: Option<PathBuf>,
) {
let mut path = entry.path();

Expand Down Expand Up @@ -239,6 +247,11 @@ fn handle_dir_entry<'scope>(
}
};

if file_type.is_dir() {
// Override the origin path of the symbolic link
origin_path = Some(path);
}

path = target_path;
};

Expand All @@ -256,7 +269,7 @@ fn handle_dir_entry<'scope>(

if file_type.is_dir() {
scope.spawn(move |scope| {
handle_dir(scope, ctx, &path);
handle_dir(scope, ctx, &path, origin_path);
});
return;
}
Expand All @@ -269,11 +282,28 @@ fn handle_dir_entry<'scope>(
return;
}

// In case the file is inside a directory that is behind a symbolic link,
// the unresolved origin path is used to construct a new path.
// This is required to support ignore patterns to symbolic links.
let rome_path = if let Some(origin_path) = origin_path {
if let Some(file_name) = path.file_name() {
RomePath::new(origin_path.join(file_name))
} else {
ctx.push_diagnostic(Error::from(FileSystemDiagnostic {
path: path.to_string_lossy().to_string(),
error_kind: ErrorKind::UnknownFileType,
}));
return;
}
} else {
RomePath::new(&path)
};

// Performing this check here let's us skip skip unsupported
// files entirely, as well as silently ignore unsupported files when
// doing a directory traversal, but printing an error message if the
// user explicitly requests an unsupported file to be handled
let rome_path = RomePath::new(&path);
// user explicitly requests an unsupported file to be handled.
// This check also works for symbolic links.
if !ctx.can_handle(&rome_path) {
return;
}
Expand Down