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

Commit

Permalink
fix(rome_fs): Allow to ignore patterns to symbolic links (symlinks)
Browse files Browse the repository at this point in the history
  • Loading branch information
realtimetodie committed Jan 21, 2023
1 parent ced6f5b commit 2d3fbbd
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 8 deletions.
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

0 comments on commit 2d3fbbd

Please sign in to comment.