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

…4166)

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

* refactor: file_name might be None

* document the origin_path argument

* update test

* fix formatting

* update test

crates/rome_cli/tests/commands/check.rs

@@ -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,
 };
@@ -855,6 +856,119 @@ fn fs_error_unknown() {
     ));
 }
 
+// Symbolic link ignore pattern test
+//
+// Verifies, that ignore patterns to symbolic links are allowed.
+//
+// ├── rome.json
+// ├── 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
+// └── src
+//     ├── 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 src_path = root_path.join("src");
+
+    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(src_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();
+
+    // src/symlink_testcase1_1
+    let symlink_testcase1_1_path = src_path.join("symlink_testcase1_1");
+    // hidden_nested/test/symlink_testcase1_2
+    let symlink_testcase1_2_path = nested_sub_path.join("symlink_testcase1_2");
+    // src/symlink_testcase2
+    let symlink_testcase2_path = src_path.join("symlink_testcase2");
+
+    #[cfg(target_family = "unix")]
+    {
+        // src/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();
+        // src/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 = root_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(root_path.clone()),
+            OsString::from("--apply"),
+            OsString::from(src_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();

crates/rome_cli/tests/configs.rs

@@ -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"
+    ]
+  }
+}"#;

crates/rome_cli/tests/snapshots/main_commands_check/fs_files_ignore_symlink.snap

@@ -0,0 +1,18 @@
+---
+source: crates/rome_cli/tests/snap_test.rs
+assertion_line: 346
+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>
+```
+
+

crates/rome_fs/src/fs/os.rs

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