address review: rescan a directory reached from a shallower path, skipping files already collected

apps/gridcraft-cli/src/check.rs

@@ -214,7 +214,7 @@ pub fn main(args: &[String]) -> ExitCode {
             Err(_) => return usage(format!("--max-report needs a whole number, not `{n}`")),
         }
     }
-    let Discovery { files, failures: mut reports } = collect_inputs(&a.positional);
+    let Discovery { files, failures: mut reports, .. } = collect_inputs(&a.positional);
     reports.extend(files.iter().map(|p| {
         let path = p.to_string_lossy().into_owned();
         match gridcraft_engine::io::read_file(&path) {
@@ -233,6 +233,9 @@ pub fn main(args: &[String]) -> ExitCode {
 #[derive(Default)]
 struct Discovery {
     files: Vec<PathBuf>,
+    /// Identity (dev+inode) of each collected file, so a rescan reached by a shallower path doesn't add it twice.
+    #[cfg(unix)]
+    file_ids: std::collections::HashSet<(u64, u64)>,
     failures: Vec<FileReport>,
 }
 
@@ -257,11 +260,11 @@ impl Discovery {
 /// (`~$…`) are skipped.
 fn collect_inputs(inputs: &[String]) -> Discovery {
     let mut out = Discovery::default();
-    let mut seen = std::collections::HashSet::new();
+    let mut seen = std::collections::HashMap::new();
     for i in inputs {
         let p = PathBuf::from(i);
         if p.is_dir() {
-            if !walk(&p, 0, &mut out, &mut seen) {
+            if !walk(&p, 0, false, &mut out, &mut seen) {
                 break;
             }
         } else if !out.add_file(p) {
@@ -277,23 +280,28 @@ fn collect_inputs(inputs: &[String]) -> Discovery {
     out
 }
 
-/// Returns false only when the global file budget stops traversal.
-fn walk(dir: &Path, depth: usize, out: &mut Discovery, seen: &mut std::collections::HashSet<(u64, u64)>) -> bool {
-    // A directory is visited once by identity (dev+inode on unix): several links to the same
-    // directory are aliases, not exponential branches of the scan. Other platforms keep the
-    // depth cap only.
+/// Returns false only when the global file budget stops traversal. `seen` holds the shallowest
+/// depth each directory was scanned from; `rescan` is set below a directory reached again from a
+/// shallower path (its depth-limited subtree may now fit), where already collected files are skipped.
+#[cfg_attr(not(unix), allow(unused_mut))]
+fn walk(dir: &Path, depth: usize, mut rescan: bool, out: &mut Discovery, seen: &mut std::collections::HashMap<(u64, u64), usize>) -> bool {
+    // A directory is visited once by identity (dev+inode on unix) unless reached from a shallower
+    // path: several links to the same directory are aliases, not exponential branches of the scan
+    // (each directory is scanned at most MAX_DEPTH + 1 times). Other platforms keep the depth cap only.
     #[cfg(unix)]
     {
         use std::os::unix::fs::MetadataExt as _;
         if let Ok(m) = std::fs::metadata(dir) {
             let identity = (m.dev(), m.ino());
-            // Already scanned: an alias. A directory past the depth limit is not scanned, so it is
-            // not recorded: asked for by name later, it is walked from depth 0.
-            if seen.contains(&identity) {
-                return true;
+            // A directory past the depth limit is not scanned, so it is not recorded: asked for
+            // by name later, it is walked from depth 0.
+            match seen.get(&identity) {
+                Some(&shallowest) if shallowest <= depth => return true,
+                Some(_) => rescan = true,
+                None => {}
             }
             if depth <= MAX_DEPTH {
-                seen.insert(identity);
+                seen.insert(identity, depth);
             }
         }
     }
@@ -329,13 +337,20 @@ fn walk(dir: &Path, depth: usize, out: &mut Discovery, seen: &mut std::collectio
             }
         };
         if metadata.is_dir() {
-            if !walk(&p, depth + 1, out, seen) {
+            if !walk(&p, depth + 1, rescan, out, seen) {
+                return false;
+            }
+        } else if p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) {
+            #[cfg(unix)]
+            {
+                use std::os::unix::fs::MetadataExt as _;
+                if !out.file_ids.insert((metadata.dev(), metadata.ino())) && rescan {
+                    continue;
+                }
+            }
+            if !out.add_file(p) {
                 return false;
             }
-        } else if p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm"))
-            && !out.add_file(p)
-        {
-            return false;
         }
     }
     true

apps/gridcraft-cli/src/check/tests.rs

@@ -11,8 +11,8 @@ fn discovery_reports_a_directory_read_error() {
     let path = std::env::temp_dir().join(format!("gridcraft-check-not-a-directory-{}", std::process::id()));
     std::fs::write(&path, "not a directory").unwrap();
     let mut discovery = Discovery::default();
-    let mut seen = std::collections::HashSet::new();
-    assert!(walk(&path, 0, &mut discovery, &mut seen));
+    let mut seen = std::collections::HashMap::new();
+    assert!(walk(&path, 0, false, &mut discovery, &mut seen));
     assert!(discovery.files.is_empty());
     assert_eq!(discovery.failures.len(), 1);
     assert_eq!(discovery.failures[0].path, path.to_string_lossy());

apps/gridcraft-cli/tests/it/cli.rs

@@ -223,6 +223,26 @@ fn a_directory_past_the_depth_limit_is_still_scanned_when_named_as_an_input() {
     let _ = std::fs::remove_dir_all(dir);
 }
 
+#[cfg(unix)]
+#[test]
+fn a_directory_first_reached_at_the_depth_limit_is_completed_when_named_as_an_input() {
+    let dir = tmpdir("check-depth-shallower");
+    // The 32nd folder is scanned from the top, but its child (and the workbook in it) is past the limit.
+    let mut limit = dir.clone();
+    for _ in 0..32 {
+        limit.push("d");
+    }
+    let child = limit.join("c");
+    std::fs::create_dir_all(&child).unwrap();
+    ok(&["run", "--out", limit.join("at_limit.xlsx").to_str().unwrap(), "--quiet"]);
+    ok(&["run", "--out", child.join("below.xlsx").to_str().unwrap(), "--quiet"]);
+    let out = run(&["check", dir.to_str().unwrap(), limit.to_str().unwrap(), "--json"]);
+    let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    // Named from depth 0 the subtree is read: the workbook below the limit is checked, the one at it only once.
+    assert_eq!(report["totals"]["opened"], 2, "{report}");
+    let _ = std::fs::remove_dir_all(dir);
+}
+
 /// A script line whose parameters are under a key `run` does not read used to run the command
 /// with no parameters at all, and the run still exited 0: three mistaken lines made Sheet2,
 /// Sheet3 and Sheet4 instead of the named sheets, and nothing said so (#2256).