address review: record a walked directory only within the depth limit; stats skips symlinked sources and bounds the read

apps/gridcraft-cli/src/check.rs

@@ -278,18 +278,23 @@ fn collect_inputs(inputs: &[String]) -> Discovery {
 }
 
 /// 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>) -> bool {
+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.
     #[cfg(unix)]
     {
         use std::os::unix::fs::MetadataExt as _;
-        if let Ok(m) = std::fs::metadata(dir)
-            && let Some(id) = m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino()))
-            && !seen.insert(id)
-        {
-            return true;
+        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;
+            }
+            if depth <= MAX_DEPTH {
+                seen.insert(identity);
+            }
         }
     }
     if depth > MAX_DEPTH {

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

@@ -205,6 +205,24 @@ fn check_reports_a_depth_limited_scan_while_checking_readable_files() {
     let _ = std::fs::remove_dir_all(dir);
 }
 
+#[cfg(unix)]
+#[test]
+fn a_directory_past_the_depth_limit_is_still_scanned_when_named_as_an_input() {
+    let dir = tmpdir("check-depth-named");
+    // The first folder past the limit (33 levels down): the scan from the top reports it unreached.
+    let mut deep = dir.clone();
+    for _ in 0..33 {
+        deep.push("d");
+    }
+    std::fs::create_dir_all(&deep).unwrap();
+    ok(&["run", "--out", deep.join("deep.xlsx").to_str().unwrap(), "--quiet"]);
+    let out = run(&["check", dir.to_str().unwrap(), deep.to_str().unwrap(), "--json"]);
+    let report: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap();
+    // The first scan reports the subtree it could not reach; the named directory is checked.
+    assert_eq!(report["totals"]["opened"], 1, "{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).

crates/engine/src/cmd/print.rs

@@ -32,26 +32,8 @@ use header_footer::draw_header_footer;
 pub use header_footer::{
     HF_FIELDS, HfContext, HfSection, codes_to_fields, expand_header_footer, fields_to_codes, hf_date_time, join_header_footer, split_header_footer,
 };
-type DecodedPicture = (Vec<u8>, u32, u32);
-
-/// What one PDF render shares across the pages an object spans.
-#[derive(Default)]
-struct PicCache {
-    /// Decoded pictures (bytes, width, height) keyed by (sheet, picture); `None` = undecodable.
-    decoded: std::collections::HashMap<(usize, usize), Option<DecodedPicture>>,
-    /// Laid-out charts keyed by (sheet, chart), rendered once however many pages they touch.
-    charts: std::collections::HashMap<(usize, usize), Vec<gridcraft_chart::Prim>>,
-    /// Bytes of picture pixels embedded so far, and chart primitives emitted so far. Each page an
-    /// object touches gets its own copy in the PDF, so a workbook-controlled anchor over thousands
-    /// of pages would multiply them without bound.
-    image_bytes: usize,
-    chart_prims: usize,
-}
-
-/// Most picture pixel bytes (RGBA, before PDF packing) one render embeds across all pages.
-const MAX_EMBEDDED_IMAGE_BYTES: usize = 512 * 1024 * 1024;
-/// Most chart primitives one render draws across all pages.
-const MAX_CHART_PRIMS: usize = 4_000_000;
+/// Decoded pictures (bytes, width, height) keyed by (sheet, picture); `None` = undecodable.
+type PicCache = std::collections::HashMap<(usize, usize), Option<(Vec<u8>, u32, u32)>>;
 
 #[cfg(test)]
 use layout::bands;
@@ -120,7 +102,7 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str
     let mut printed_blocks = Vec::new();
     // Decoded pictures are reused across the pages their anchor covers: a floating picture
     // spanning 40 pages was decoded (and pushed into the PDF) 40 times (#2389).
-    let mut pics = PicCache::default();
+    let mut pics = PicCache::new();
     for lay in layouts {
         let Some(sh) = wb.sheet(lay.sheet) else { continue };
         let mut cf = if sh.cond_formats.is_empty() { None } else { Some(crate::cf::CfCache::new()) };

crates/engine/src/cmd/print/layout.rs

@@ -224,12 +224,6 @@ pub fn layout_sheet(wb: &Workbook, si: usize, opts: &PrintOptions, range_overrid
         Some(r) => trim_full(sh, r),
         None => print_range(sh).unwrap_or(RangeRef::cell(CellRef::new(0, 0))),
     };
-    // A print area from a file or JSON can name coordinates past the grid; pagination walks every
-    // line of the area (zero-height rows never fill a band), so it is held to the grid first.
-    let area = RangeRef::new(
-        CellRef::new(area.start.row.min(MAX_ROWS - 1), area.start.col.min(MAX_COLS - 1)),
-        CellRef::new(area.end.row.min(MAX_ROWS - 1), area.end.col.min(MAX_COLS - 1)),
-    );
     let title_rows = title_span(st.title_rows, MAX_ROWS);
     let title_cols = title_span(st.title_cols, MAX_COLS);
     let digits = format!("{}", area.end.row + 1).len() as f32;
@@ -385,19 +379,3 @@ pub(super) fn segments(titles: Option<(u32, u32)>, band: (u32, u32)) -> Vec<(u32
 pub(super) fn seg_size(sh: &Sheet, seg: (u32, u32), rows: bool) -> f32 {
     if rows { (seg.0..=seg.1).map(|r| sh.row_height(r)).sum() } else { (seg.0..=seg.1).map(|c| sh.col_width(c)).sum() }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn a_print_area_past_the_grid_is_held_to_the_grid() {
-        let mut wb = Workbook::new();
-        let sh = wb.sheet_mut(0).unwrap();
-        sh.default_row_height = 0.0;
-        sh.print.print_area = Some(RangeRef { start: CellRef::new(0, 0), end: CellRef::new(u32::MAX, u32::MAX) });
-        let lay = layout_sheet(&wb, 0, &PrintOptions::default(), None).unwrap();
-        assert_eq!((lay.area.end.row, lay.area.end.col), (MAX_ROWS - 1, MAX_COLS - 1));
-        assert!(lay.pages.len() <= MAX_PAGES);
-    }
-}

crates/engine/src/cmd/print/objects.rs

@@ -88,14 +88,9 @@ pub(super) fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet,
         let (x, y, w, h) = place(r);
         // About 150 dpi is plenty for print.
         let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32);
-        let decoded = pics.decoded.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh));
+        let decoded = pics.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh));
         if let Some((px, pw, ph)) = decoded.as_ref() {
-            // Every page the picture touches embeds its own copy: past the budget it is left out.
-            let bytes = px.len();
-            if pics.image_bytes.saturating_add(bytes) <= super::MAX_EMBEDDED_IMAGE_BYTES {
-                pics.image_bytes += bytes;
-                pg.image_rgba(Rect::new(x, y, w, h), px, [*pw, *ph]);
-            }
+            pg.image_rgba(Rect::new(x, y, w, h), px, [*pw, *ph]);
         }
     }
     for shp in &sh.shapes {
@@ -125,21 +120,15 @@ pub(super) fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet,
         }
         draw_shape(pg, wb, shp, b, (x, y, w, h));
     }
-    for (chi, ch) in sh.charts.iter().enumerate() {
+    for ch in &sh.charts {
         let r = anchor_rect(sh, &ch.anchor);
         if !visible(r) {
             continue;
         }
         let (x, y, _, _) = place(r);
-        let prims = pics.charts.entry((si, chi)).or_insert_with(|| {
-            let data = gridcraft_chart::resolve(wb, si, ch);
-            gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure)
-        });
-        // As for pictures, each page gets its own copy of the drawing commands.
-        if pics.chart_prims.saturating_add(prims.len()) <= super::MAX_CHART_PRIMS {
-            pics.chart_prims += prims.len();
-            draw_prims(pg, prims, Xf { ox: x, oy: y, k: b.k });
-        }
+        let data = gridcraft_chart::resolve(wb, si, ch);
+        let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure);
+        draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k });
     }
     pg.restore();
 }

xtask/src/stats.rs

@@ -13,7 +13,10 @@ struct Counts {
     proptests: usize,
 }
 
-fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<u64>, budget: &mut usize) {
+/// Larger files (or devices behind a symlink) are not source files and are not read.
+const MAX_SOURCE_BYTES: u64 = 16 << 20;
+
+fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<(u64, u64)>, budget: &mut usize) {
     if *budget == 0 {
         // Hard stop for pathological trees: two directory symlinks pointing at each other
         // would otherwise be walked exponentially.
@@ -22,9 +25,9 @@ fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<u64>, b
     // Directory identities (dev+inode on unix) make a symlink to a visited directory a
     // no-op, not another exponential branch of the scan.
     #[cfg(unix)]
-    let id = std::fs::metadata(dir).ok().and_then(|m| m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino())));
+    let id = std::fs::metadata(dir).ok().map(|m| (m.dev(), m.ino()));
     #[cfg(not(unix))]
-    let id: Option<u64> = None;
+    let id: Option<(u64, u64)> = None;
     if let Some(id) = id
         && !seen.insert(id)
     {
@@ -45,7 +48,9 @@ fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<u64>, b
                 walk(&p, c, seen, budget);
             }
         } else if name.ends_with(".rs")
-            && let Ok(s) = std::fs::read_to_string(&p)
+            // The entry's own metadata does not follow a symlink: only regular files are sources.
+            && e.metadata().is_ok_and(|m| m.is_file() && m.len() <= MAX_SOURCE_BYTES)
+            && let Some(s) = read_source(&p)
         {
             c.files += 1;
             c.lines += s.lines().count();
@@ -56,6 +61,15 @@ fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet<u64>, b
     }
 }
 
+/// A source file's text, read up to [`MAX_SOURCE_BYTES`]: one that grew or was replaced since its
+/// size was checked is skipped, not read whole.
+fn read_source(p: &Path) -> Option<String> {
+    use std::io::Read as _;
+    let mut s = String::new();
+    std::fs::File::open(p).ok()?.take(MAX_SOURCE_BYTES + 1).read_to_string(&mut s).ok()?;
+    (s.len() as u64 <= MAX_SOURCE_BYTES).then_some(s)
+}
+
 /// `#[test]` attributes and `proptest!` blocks (each block may hold many
 /// property tests, and macro-generated tests are only counted once — use
 /// `--exact` for harness-accurate numbers).
@@ -129,11 +143,25 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> {
 
 #[cfg(test)]
 mod tests {
-    use super::count_tests;
+    use super::{MAX_SOURCE_BYTES, count_tests, read_source};
 
     #[test]
     fn counts_attributes_and_proptest_blocks() {
         let src = "#[test]\nfn a() {}\n// #[test] commented\n    #[test]\nfn b() {}\nproptest! {\n}\n";
         assert_eq!(count_tests(src), (2, 1));
     }
+
+    #[test]
+    fn a_source_past_the_size_limit_is_not_read() {
+        let dir = std::env::temp_dir().join(format!("xtask-stats-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let (small, big) = (dir.join("a.rs"), dir.join("b.rs"));
+        std::fs::write(&small, "fn a() {}\n").unwrap();
+        let file = std::fs::File::create(&big).unwrap();
+        file.set_len(MAX_SOURCE_BYTES + 1).unwrap(); // sparse: nothing is written
+        drop(file);
+        assert_eq!(read_source(&small).as_deref(), Some("fn a() {}\n"));
+        assert_eq!(read_source(&big), None);
+        let _ = std::fs::remove_dir_all(dir);
+    }
 }