Merge PR #1726: Fail CLI checks on incomplete or empty input discovery

apps/gridcraft-cli/src/check.rs

@@ -139,7 +139,7 @@ impl RoundTrip {
 #[derive(Clone, Debug, Default, PartialEq)]
 pub struct FileReport {
     pub path: String,
-    /// GridCraft couldn't open the file: its message.
+    /// Opening the file or discovering the requested inputs failed: its message.
     pub error: Option<String>,
     /// Import warnings, in the order the importer produced them.
     pub warnings: Vec<String>,
@@ -205,67 +205,112 @@ pub fn main(args: &[String]) -> ExitCode {
             Err(_) => return usage(format!("--max-report needs a whole number, not `{n}`")),
         }
     }
-    let files = collect_inputs(&a.positional);
-    let reports: Vec<FileReport> = files
-        .iter()
-        .map(|p| {
-            let path = p.to_string_lossy().into_owned();
-            match gridcraft_engine::io::read_file(&path) {
-                Ok(bytes) => check_bytes(&path, &bytes, &opts),
-                Err(e) => FileReport { path, error: Some(e.to_string()), ..Default::default() },
-            }
-        })
-        .collect();
+    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) {
+            Ok(bytes) => check_bytes(&path, &bytes, &opts),
+            Err(e) => FileReport { path, error: Some(e.to_string()), ..Default::default() },
+        }
+    }));
     if a.has("json") {
         outln!("{}", pretty(&report_json(&reports, &opts)));
     } else {
         print_human(&reports, &opts);
     }
-    if reports.iter().all(FileReport::is_clean) { ExitCode::SUCCESS } else { ExitCode::FAILURE }
+    if !files.is_empty() && reports.iter().all(FileReport::is_clean) { ExitCode::SUCCESS } else { ExitCode::FAILURE }
+}
+
+#[derive(Default)]
+struct Discovery {
+    files: Vec<PathBuf>,
+    failures: Vec<FileReport>,
+}
+
+impl Discovery {
+    fn fail(&mut self, path: &Path, error: String) {
+        self.failures.push(FileReport { path: path.to_string_lossy().into_owned(), error: Some(error), ..Default::default() });
+    }
+
+    /// False means the file budget is exhausted: the remaining scan is incomplete.
+    fn add_file(&mut self, path: PathBuf) -> bool {
+        if self.files.len() >= MAX_FILES {
+            self.fail(&path, format!("file limit of {MAX_FILES} reached; this and remaining inputs were not checked"));
+            return false;
+        }
+        self.files.push(path);
+        true
+    }
 }
 
 /// Files named on the command line are checked whatever their extension; directories contribute
 /// their `.xlsx`/`.xlsm` files, recursively, in name order. Hidden files and Excel lock files
 /// (`~$…`) are skipped.
-fn collect_inputs(inputs: &[String]) -> Vec<PathBuf> {
-    let mut out = Vec::new();
+fn collect_inputs(inputs: &[String]) -> Discovery {
+    let mut out = Discovery::default();
     for i in inputs {
         let p = PathBuf::from(i);
         if p.is_dir() {
-            walk(&p, 0, &mut out);
-        } else {
-            // A missing path is reported as a file that failed to open.
-            out.push(p);
-        }
-        if out.len() >= MAX_FILES {
-            out.truncate(MAX_FILES);
+            if !walk(&p, 0, &mut out) {
+                break;
+            }
+        } else if !out.add_file(p) {
             break;
         }
     }
+    if out.files.is_empty()
+        && out.failures.is_empty()
+        && let Some(input) = inputs.first()
+    {
+        out.fail(Path::new(input), "no workbook files were found in the requested inputs".into());
+    }
     out
 }
 
-fn walk(dir: &Path, depth: usize, out: &mut Vec<PathBuf>) {
-    if depth > MAX_DEPTH || out.len() >= MAX_FILES {
-        return;
+/// Returns false only when the global file budget stops traversal.
+fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool {
+    if depth > MAX_DEPTH {
+        out.fail(dir, format!("directory depth limit of {MAX_DEPTH} exceeded; this subtree was not checked"));
+        return true;
+    }
+    let rd = match std::fs::read_dir(dir) {
+        Ok(rd) => rd,
+        Err(e) => {
+            out.fail(dir, format!("cannot read directory: {e}"));
+            return true;
+        }
+    };
+    let mut entries = Vec::new();
+    for entry in rd {
+        match entry {
+            Ok(entry) => entries.push(entry.path()),
+            Err(e) => out.fail(dir, format!("cannot read directory entry: {e}")),
+        }
     }
-    let Ok(rd) = std::fs::read_dir(dir) else { return };
-    let mut entries: Vec<PathBuf> = rd.flatten().map(|e| e.path()).collect();
     entries.sort();
     for p in entries {
         let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default();
         if name.starts_with('.') || name.starts_with("~$") {
             continue;
         }
-        if p.is_dir() {
-            walk(&p, depth + 1, out);
-        } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) {
-            out.push(p);
-        }
-        if out.len() >= MAX_FILES {
-            return;
+        let metadata = match p.metadata() {
+            Ok(metadata) => metadata,
+            Err(e) => {
+                out.fail(&p, format!("cannot inspect input: {e}"));
+                continue;
+            }
+        };
+        if metadata.is_dir() {
+            if !walk(&p, depth + 1, out) {
+                return false;
+            }
+        } else if p.extension().and_then(|e| e.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
 }
 
 /// Checks one file's bytes. Never fails: problems are recorded in the report. A panic escaping
@@ -1271,6 +1316,30 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    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();
+        assert!(walk(&path, 0, &mut discovery));
+        assert!(discovery.files.is_empty());
+        assert_eq!(discovery.failures.len(), 1);
+        assert_eq!(discovery.failures[0].path, path.to_string_lossy());
+        assert!(discovery.failures[0].error.as_ref().unwrap().contains("cannot read directory"));
+        let _ = std::fs::remove_file(path);
+    }
+
+    #[test]
+    fn discovery_reports_omissions_only_after_the_file_limit() {
+        let mut discovery = Discovery { files: vec![PathBuf::from("book.xlsx"); MAX_FILES - 1], ..Default::default() };
+        assert!(discovery.add_file(PathBuf::from("last.xlsx")));
+        assert!(discovery.failures.is_empty());
+        assert!(!discovery.add_file(PathBuf::from("omitted.xlsx")));
+        assert_eq!(discovery.files.len(), MAX_FILES);
+        assert_eq!(discovery.failures[0].path, "omitted.xlsx");
+        assert!(discovery.failures[0].error.as_ref().unwrap().contains("file limit"));
+    }
+
     fn at(a1: &str) -> CellRef {
         CellRef::parse(a1).unwrap()
     }

apps/gridcraft-cli/tests/cli.rs

@@ -174,3 +174,35 @@ fn check_exit_codes_and_json() {
     assert_eq!(run(&["check", book_s, "--tolerance", "x"]).status.code(), Some(2));
     assert_eq!(run(&["check", book_s, "--bogus"]).status.code(), Some(2));
 }
+
+#[test]
+fn check_rejects_an_empty_scan() {
+    let dir = tmpdir("check-empty");
+    std::fs::write(dir.join("notes.txt"), "not a workbook").unwrap();
+    std::fs::write(dir.join(".hidden.xlsx"), "ignored").unwrap();
+    std::fs::write(dir.join("~$locked.xlsx"), "ignored").unwrap();
+    let out = run(&["check", dir.to_str().unwrap(), "--json"]);
+    assert_eq!(out.status.code(), Some(1));
+    let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    assert!(report["files"][0]["error"].as_str().unwrap().contains("no workbook files"));
+    let _ = std::fs::remove_dir_all(dir);
+}
+
+#[test]
+fn check_reports_a_depth_limited_scan_while_checking_readable_files() {
+    let dir = tmpdir("check-depth");
+    let book = dir.join("readable.xlsx");
+    ok(&["run", "--out", book.to_str().unwrap(), "--quiet"]);
+    let mut deep = dir.clone();
+    for _ in 0..34 {
+        deep.push("d");
+    }
+    std::fs::create_dir_all(&deep).unwrap();
+    std::fs::write(deep.join("skipped.xlsx"), "not reached").unwrap();
+    let out = run(&["check", dir.to_str().unwrap(), "--json"]);
+    assert_eq!(out.status.code(), Some(1));
+    let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    assert_eq!(report["totals"]["opened"], 1);
+    assert!(report["files"].as_array().unwrap().iter().any(|r| r["error"].as_str().is_some_and(|e| e.contains("depth limit"))));
+    let _ = std::fs::remove_dir_all(dir);
+}

docs/cli.md

@@ -102,7 +102,11 @@ sheet's rules in priority order, priority numbers aside), `validations`, `charts
 `hyperlinks`, `pivots`. `#SPILL!`/`#CALC!` compare equal to `#VALUE!` and `#CIRC!` to 0, as
 that's what the file format stores.
 
-Files that fail to open are reported with GridCraft's error and don't stop the run. Exit code: 0 when every file
+Files that fail to open are reported with GridCraft's error and don't stop the run. Unreadable directories,
+directory-entry/metadata errors, and subtrees or files omitted by the depth (32) and file (100,000) limits appear
+as failed path reports too, in both human and JSON output. Intentional hidden/lock-file exclusions remain ignored.
+JSON `totals.files` includes failed discovery paths; `totals.opened` counts only workbooks that opened successfully.
+An empty scan fails with a diagnostic. Exit code: 0 when at least one file was checked, discovery was complete, and every file
 opened with no mismatch and no round-trip difference, 1 otherwise, 2 on a usage error. `--max-report` caps the
 examples listed per file and category (counts are always complete).