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 1 commit
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
105 changes: 102 additions & 3 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
use pico_args::Arguments;
use std::env;
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 +857,103 @@ fn fs_error_unknown() {
));
}

#[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();
}

// Change the current working directory
// TODO: Remove this once PR #4158 was merged
assert!(env::set_current_dir(&project_path).is_ok());

let result = run_cli(
DynRef::Owned(Box::new(OsFileSystem)),
&mut console,
Arguments::from_vec(vec![
OsString::from("check"),
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 @@ -245,3 +245,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,18 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 335
expression: content
---
# Emitted Messages

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

```

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


25 changes: 20 additions & 5 deletions crates/rome_fs/src/fs/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,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 @@ -157,7 +157,12 @@ 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,
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 @@ -181,7 +186,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 @@ -191,6 +196,7 @@ fn handle_dir_entry<'scope>(
scope: &Scope<'scope>,
ctx: &'scope dyn TraversalContext,
entry: DirEntry,
mut origin_path: Option<PathBuf>,
) {
let mut path = entry.path();

Expand Down Expand Up @@ -234,6 +240,10 @@ fn handle_dir_entry<'scope>(
}
};

if file_type.is_dir() {
origin_path = Some(path);
}

path = target_path;
};

Expand All @@ -251,7 +261,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 @@ -264,11 +274,16 @@ fn handle_dir_entry<'scope>(
return;
}

let rome_path = if let Some(origin_path) = origin_path {
RomePath::new(origin_path.join(path.file_name().unwrap()))
} 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);
if !ctx.can_handle(&rome_path) {
return;
}
Expand Down