From edcc76d0a85d3af238727a32cc211ec81ef6172a Mon Sep 17 00:00:00 2001 From: TheRenegadeDev Date: Fri, 16 Oct 2026 20:39:34 -0500 Subject: [PATCH] Fail CLI checks on incomplete or empty input discovery (#1051) --- apps/gridcraft-cli/src/check.rs | 137 ++++++++++++++++++++++++-------- apps/gridcraft-cli/tests/cli.rs | 32 ++++++++ docs/cli.md | 6 +- 3 files changed, 140 insertions(+), 35 deletions(-) diff --git a/apps/gridcraft-cli/src/check.rs b/apps/gridcraft-cli/src/check.rs index 2b8529876..6341bb068 100644 --- a/apps/gridcraft-cli/src/check.rs +++ b/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, pub warnings: usize, pub calc: CalcReport, @@ -204,67 +204,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 = 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, + failures: Vec, +} + +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 { - 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) { - 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 = 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 @@ -1270,6 +1315,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() } diff --git a/apps/gridcraft-cli/tests/cli.rs b/apps/gridcraft-cli/tests/cli.rs index fb25b6320..3105819ec 100644 --- a/apps/gridcraft-cli/tests/cli.rs +++ b/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); +} diff --git a/docs/cli.md b/docs/cli.md index ec9578d60..321c6827d 100644 --- a/docs/cli.md +++ b/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).