From e40f9841e0ce9fcc77b3197a41342e2b9cfed9a5 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 15 Oct 2026 12:22:57 -0700 Subject: [PATCH 1/3] Check every public routine for all inputs with no_panic The no_panic driver called each routine with particular values. After inlining, the compiler specialized each routine to those values, so a panic that depends on the input could be optimized away and missed: a panic in write_f32_le on negative values passed the check, because the driver only passed non-negative floats. Take the address of each routine instead. That forces a standalone copy that has to handle any arguments, and that copy is what the linker checks. The panic above is now detected. Replace the text-matching coverage test with one that parses the source with syn, so it is not affected by formatting or attribute order and matches routines by type. --- CHANGELOG.md | 2 +- Cargo.toml | 1 + README.md | 13 ++- tests/no_panic.rs | 196 ++++++++++++++++++++----------------- tests/no_panic_coverage.rs | 154 ++++++++++++++++++----------- 5 files changed, 211 insertions(+), 155 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3dff8a..aed1a23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ### 0.5.1 ### * :bug: `ReadCursor::transaction` closures can now return data borrowed from the input. See [#12](https://github.com/stepfunc/scursor/issues/12). * :bug: `WriteCursor::at_pos` now accepts `FnOnce` closures. See [#13](https://github.com/stepfunc/scursor/issues/13). -* :wrench: Replaced the Kani proofs with link-time `no_panic` checks on every public routine, run in CI. +* :wrench: Replaced the Kani proofs with link-time `no_panic` checks, run in CI, covering every public routine for all inputs. A test fails if a new routine is left out of the check. * :wrench: Added round-trip property tests and a CI workflow. * Declared the minimum supported Rust version (1.77). diff --git a/Cargo.toml b/Cargo.toml index 3d10071..19df5db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ no-panic = { version = "0.1", optional = true } [dev-dependencies] proptest = "1" +syn = { version = "2", features = ["full", "visit"] } [features] # CI-only: fails to link if any public routine can panic. Requires an optimized diff --git a/README.md b/README.md index 4dc134d..3e59e51 100644 --- a/README.md +++ b/README.md @@ -71,13 +71,20 @@ cargo test --profile no-panic --features no-panic --test no_panic What the check does and doesn't tell you: -* It covers every input, not a sample: it checks that no panic path survives in the compiled code. +* Every public method is checked, for every input. `tests/no_panic.rs` takes the address of each + one, which forces the compiler to keep a standalone copy that has to handle any arguments. That + copy is what the linker checks. `tests/no_panic_coverage.rs` runs with the normal tests and fails + if a public method is missing the annotation or isn't listed in `tests/no_panic.rs`. * It applies to the configuration that was checked: an optimized build (`opt-level = 3`) with `overflow-checks = true` and `panic = "unwind"`. Debug builds and size-optimized builds keep some unreachable panic paths, so the check reports false failures there. Those builds are still panic-free; they just can't be checked this way. -* The linker only examines code that is called, so `tests/no_panic.rs` calls every public routine. - `tests/no_panic_coverage.rs` fails if a routine is missing the annotation or the call. +* Outside the check: + * Callbacks passed to `transaction` and `at_pos`. Those routines add no panics of their own, + but a panic in your callback still propagates. + * Derived trait impls (`Clone`, `Copy`, `Debug`, `PartialEq`, `Eq`), which can't be annotated. + Derived `Debug` writes to the formatter you supply. + * `read_array` is checked for a few sizes (0, 3 and 32). Every size runs the same code. * The `no-panic` feature exists for this crate's CI. Don't enable it in your own builds. ## License diff --git a/tests/no_panic.rs b/tests/no_panic.rs index 8629cff..b60393c 100644 --- a/tests/no_panic.rs +++ b/tests/no_panic.rs @@ -1,9 +1,13 @@ //! Link-time panic check for every public routine. //! //! Every public function is annotated with `#[no_panic]` when the `no-panic` -//! feature is enabled. The check happens when this test binary is linked, so -//! each routine must be called here. `tests/no_panic_coverage.rs` verifies that -//! this file stays in sync with the public API. +//! feature is enabled. The check happens when this test binary is linked, and +//! the linker only examines code that is linked, so this file takes the address +//! of every public routine. Passing the address through `black_box` forces a +//! standalone copy of each routine that knows nothing about its arguments, so +//! the check covers every input rather than particular values. +//! +//! `tests/no_panic_coverage.rs` verifies that every public routine is listed. //! //! Run with: //! @@ -11,106 +15,114 @@ //! cargo test --profile no-panic --features no-panic --test no_panic //! ``` -use scursor::{ReadCursor, WriteCursor}; +use scursor::{ReadCursor, ReadError, WriteCursor, WriteError}; use std::hint::black_box; -#[inline(never)] -fn exercise_read_cursor(input: &[u8], count: usize) { - let mut cursor = ReadCursor::new(input); - - black_box(cursor.read_u8().ok()); - black_box(cursor.read_bytes(count).ok()); - black_box(cursor.read_array::<0>().ok()); - black_box(cursor.read_array::<3>().ok()); - black_box(cursor.read_array::<32>().ok()); +/// Callback for the `transaction` and `at_pos` instances. Those routines are +/// panic-free only if the caller's callback is, so they are checked with a +/// callback that is. +fn read_callback(cursor: &mut ReadCursor<'_>) -> Result { + cursor.read_u8() +} - black_box(cursor.read_u16_le().ok()); - black_box(cursor.read_i16_le().ok()); - black_box(cursor.read_u32_le().ok()); - black_box(cursor.read_i32_le().ok()); - black_box(cursor.read_u48_le().ok()); - black_box(cursor.read_u64_le().ok()); - black_box(cursor.read_i64_le().ok()); - black_box(cursor.read_u128_le().ok()); - black_box(cursor.read_i128_le().ok()); - black_box(cursor.read_f32_le().ok()); - black_box(cursor.read_f64_le().ok()); +fn write_callback(cursor: &mut WriteCursor<'_>) -> Result<(), WriteError> { + cursor.write_u8(0) +} - black_box(cursor.read_u16_be().ok()); - black_box(cursor.read_i16_be().ok()); - black_box(cursor.read_u32_be().ok()); - black_box(cursor.read_i32_be().ok()); - black_box(cursor.read_u48_be().ok()); - black_box(cursor.read_u64_be().ok()); - black_box(cursor.read_i64_be().ok()); - black_box(cursor.read_u128_be().ok()); - black_box(cursor.read_i128_be().ok()); - black_box(cursor.read_f32_be().ok()); - black_box(cursor.read_f64_be().ok()); +fn read_transaction<'a, T>(_: T) -> fn(&mut ReadCursor<'a>, T) -> Result +where + T: FnOnce(&mut ReadCursor<'a>) -> Result, +{ + ReadCursor::transaction +} - black_box(cursor.transaction(|cur| cur.read_bytes(count)).ok()); - black_box(cursor.expect_empty().ok()); - black_box(cursor.remaining()); - black_box(cursor.position()); - black_box(cursor.is_empty()); - black_box(cursor.read_all()); +fn write_transaction<'a, T>(_: T) -> fn(&mut WriteCursor<'a>, T) -> Result<(), WriteError> +where + T: FnOnce(&mut WriteCursor<'_>) -> Result<(), WriteError>, +{ + WriteCursor::transaction } -#[inline(never)] -fn exercise_write_cursor(dest: &mut [u8], value: u128, pos: usize) { - let mut cursor = WriteCursor::new(dest); +fn write_at_pos<'a, T>(_: T) -> fn(&mut WriteCursor<'a>, usize, T) -> Result<(), WriteError> +where + T: FnOnce(&mut WriteCursor<'_>) -> Result<(), WriteError>, +{ + WriteCursor::at_pos +} - black_box(cursor.write_u8(value as u8).ok()); - black_box(cursor.write_bytes(black_box(&[0xCA, 0xFE])).ok()); - black_box(cursor.skip(pos).ok()); - black_box(cursor.seek_to(pos).ok()); +#[test] +fn public_api_links_without_panic_paths() { + black_box:: _>(ReadCursor::new); + black_box:: _>(ReadCursor::read_u8); + black_box:: _>(ReadCursor::expect_empty); + black_box:: _>(ReadCursor::remaining); + black_box:: _>(ReadCursor::position); + black_box(read_transaction(read_callback)); + black_box:: _>(ReadCursor::is_empty); + black_box:: _>(ReadCursor::read_all); + black_box:: _>(ReadCursor::read_bytes); + black_box:: _>(ReadCursor::read_array::<0>); + black_box:: _>(ReadCursor::read_array::<3>); + black_box:: _>(ReadCursor::read_array::<32>); - black_box(cursor.write_u16_le(value as u16).ok()); - black_box(cursor.write_i16_le(value as i16).ok()); - black_box(cursor.write_u32_le(value as u32).ok()); - black_box(cursor.write_i32_le(value as i32).ok()); - black_box(cursor.write_u48_le(value as u64).ok()); - black_box(cursor.write_u64_le(value as u64).ok()); - black_box(cursor.write_i64_le(value as i64).ok()); - black_box(cursor.write_u128_le(value).ok()); - black_box(cursor.write_i128_le(value as i128).ok()); - black_box(cursor.write_f32_le(value as f32).ok()); - black_box(cursor.write_f64_le(value as f64).ok()); + black_box:: _>(ReadCursor::read_u16_le); + black_box:: _>(ReadCursor::read_i16_le); + black_box:: _>(ReadCursor::read_u32_le); + black_box:: _>(ReadCursor::read_i32_le); + black_box:: _>(ReadCursor::read_u48_le); + black_box:: _>(ReadCursor::read_u64_le); + black_box:: _>(ReadCursor::read_i64_le); + black_box:: _>(ReadCursor::read_u128_le); + black_box:: _>(ReadCursor::read_i128_le); + black_box:: _>(ReadCursor::read_f32_le); + black_box:: _>(ReadCursor::read_f64_le); - black_box(cursor.write_u16_be(value as u16).ok()); - black_box(cursor.write_i16_be(value as i16).ok()); - black_box(cursor.write_u32_be(value as u32).ok()); - black_box(cursor.write_i32_be(value as i32).ok()); - black_box(cursor.write_u48_be(value as u64).ok()); - black_box(cursor.write_u64_be(value as u64).ok()); - black_box(cursor.write_i64_be(value as i64).ok()); - black_box(cursor.write_u128_be(value).ok()); - black_box(cursor.write_i128_be(value as i128).ok()); - black_box(cursor.write_f32_be(value as f32).ok()); - black_box(cursor.write_f64_be(value as f64).ok()); + black_box:: _>(ReadCursor::read_u16_be); + black_box:: _>(ReadCursor::read_i16_be); + black_box:: _>(ReadCursor::read_u32_be); + black_box:: _>(ReadCursor::read_i32_be); + black_box:: _>(ReadCursor::read_u48_be); + black_box:: _>(ReadCursor::read_u64_be); + black_box:: _>(ReadCursor::read_i64_be); + black_box:: _>(ReadCursor::read_u128_be); + black_box:: _>(ReadCursor::read_i128_be); + black_box:: _>(ReadCursor::read_f32_be); + black_box:: _>(ReadCursor::read_f64_be); - black_box( - cursor - .transaction(|cur| cur.write_u16_le(value as u16)) - .ok(), - ); - black_box( - cursor - .at_pos(pos, |cur| cur.write_u32_be(value as u32)) - .ok(), - ); - black_box(cursor.written_since(pos).map(<[u8]>::len).ok()); - black_box(cursor.get(pos..pos.wrapping_add(4)).map(<[u8]>::len)); - black_box(cursor.written().len()); - black_box(cursor.remaining()); - black_box(cursor.position()); -} + black_box:: _>(WriteCursor::new); + black_box:: _>(WriteCursor::position); + black_box:: _>(WriteCursor::get); + black_box:: _>(WriteCursor::skip); + black_box:: _>(WriteCursor::seek_to); + black_box(write_transaction(write_callback)); + black_box(write_at_pos(write_callback)); + black_box:: _>(WriteCursor::written); + black_box:: _>(WriteCursor::written_since); + black_box:: _>(WriteCursor::remaining); + black_box:: _>(WriteCursor::write_bytes); + black_box:: _>(WriteCursor::write_u8); -#[test] -fn public_api_links_without_panic_paths() { - let input: Vec = (0..=255).collect(); - exercise_read_cursor(black_box(&input), black_box(7)); + black_box:: _>(WriteCursor::write_u16_le); + black_box:: _>(WriteCursor::write_i16_le); + black_box:: _>(WriteCursor::write_u32_le); + black_box:: _>(WriteCursor::write_i32_le); + black_box:: _>(WriteCursor::write_u48_le); + black_box:: _>(WriteCursor::write_u64_le); + black_box:: _>(WriteCursor::write_i64_le); + black_box:: _>(WriteCursor::write_f32_le); + black_box:: _>(WriteCursor::write_f64_le); + black_box:: _>(WriteCursor::write_u128_le); + black_box:: _>(WriteCursor::write_i128_le); - let mut dest = [0u8; 256]; - exercise_write_cursor(black_box(&mut dest), black_box(u128::MAX), black_box(5)); + black_box:: _>(WriteCursor::write_u16_be); + black_box:: _>(WriteCursor::write_i16_be); + black_box:: _>(WriteCursor::write_u32_be); + black_box:: _>(WriteCursor::write_i32_be); + black_box:: _>(WriteCursor::write_u48_be); + black_box:: _>(WriteCursor::write_u64_be); + black_box:: _>(WriteCursor::write_i64_be); + black_box:: _>(WriteCursor::write_u128_be); + black_box:: _>(WriteCursor::write_i128_be); + black_box:: _>(WriteCursor::write_f32_be); + black_box:: _>(WriteCursor::write_f64_be); } diff --git a/tests/no_panic_coverage.rs b/tests/no_panic_coverage.rs index f82b9e2..8925b92 100644 --- a/tests/no_panic_coverage.rs +++ b/tests/no_panic_coverage.rs @@ -1,73 +1,109 @@ -//! Ensures every public routine is annotated with `#[no_panic]` and exercised by -//! `tests/no_panic.rs`. Functions that are never called are not checked by the -//! linker, so a missing call would silently skip the check. +//! Checks that every public method in `src/` is annotated with `no_panic` and +//! listed in `tests/no_panic.rs`, which only checks the routines it lists. -const DRIVER: &str = include_str!("no_panic.rs"); -const ATTRIBUTE: &str = "#[cfg_attr(feature = \"no-panic\", no_panic)]"; +use std::collections::BTreeSet; +use std::path::Path; -/// Names of all `pub fn` items in `source`, each paired with whether it is -/// immediately preceded by the no_panic attribute. -fn public_fns(source: &str) -> Vec<(String, bool)> { - let lines: Vec<&str> = source.lines().map(str::trim).collect(); - lines - .iter() - .enumerate() - .filter_map(|(i, line)| { - let rest = line.strip_prefix("pub fn ")?; - let name: String = rest - .chars() - .take_while(|c| c.is_alphanumeric() || *c == '_') - .collect(); - let annotated = i > 0 && lines[i - 1] == ATTRIBUTE; - Some((name, annotated)) - }) - .collect() -} +use syn::visit::{self, Visit}; +use syn::{Attribute, ImplItem, Meta, Type, Visibility}; + +/// Public methods found in the library source, with whether each is annotated. +#[derive(Default)] +struct PublicMethods(Vec<(String, bool)>); + +impl<'ast> Visit<'ast> for PublicMethods { + fn visit_item_mod(&mut self, item: &'ast syn::ItemMod) { + if !has_attribute(&item.attrs, "cfg", "test") { + visit::visit_item_mod(self, item); + } + } -/// The body of the driver function `name`, up to the next driver function. -fn driver_body(name: &str) -> &'static str { - let start = DRIVER - .find(&format!("fn {name}(")) - .unwrap_or_else(|| panic!("driver function {name} not found")); - let body = &DRIVER[start..]; - let end = body[1..].find("\nfn ").map_or(body.len(), |i| i + 1); - &body[..end] + fn visit_item_impl(&mut self, item: &'ast syn::ItemImpl) { + let Type::Path(owner) = &*item.self_ty else { + return; + }; + let owner = &owner.path.segments.last().unwrap().ident; + for item in &item.items { + if let ImplItem::Fn(f) = item { + if matches!(f.vis, Visibility::Public(_)) { + let annotated = + has_attribute(&f.attrs, "cfg_attr", r#"feature="no-panic",no_panic"#); + self.0 + .push((format!("{owner}::{}", f.sig.ident), annotated)); + } + } + } + } } -fn check(source: &str, driver_fn: &str, type_name: &str) { - let body = driver_body(driver_fn); - let fns = public_fns(source); - assert!(!fns.is_empty(), "no public functions found for {type_name}"); +/// `Type::routine` paths referenced by the driver. +#[derive(Default)] +struct ListedRoutines(BTreeSet); - for (name, annotated) in fns { - assert!( - annotated, - "{type_name}::{name} is missing the no_panic attribute" - ); - let called = body.contains(&format!(".{name}(")) - || body.contains(&format!(".{name}::<")) - || body.contains(&format!("{type_name}::{name}(")); - assert!( - called, - "{type_name}::{name} is not exercised by {driver_fn} in tests/no_panic.rs" - ); +impl<'ast> Visit<'ast> for ListedRoutines { + fn visit_expr_path(&mut self, expr: &'ast syn::ExprPath) { + let segments: Vec<_> = expr + .path + .segments + .iter() + .map(|s| s.ident.to_string()) + .collect(); + if let [.., owner, routine] = segments.as_slice() { + self.0.insert(format!("{owner}::{routine}")); + } + visit::visit_expr_path(self, expr); } } -#[test] -fn every_read_cursor_routine_is_checked() { - check( - include_str!("../src/read.rs"), - "exercise_read_cursor", - "ReadCursor", - ); +/// True if an attribute is exactly `#[name(args)]`, ignoring whitespace. +fn has_attribute(attrs: &[Attribute], name: &str, args: &str) -> bool { + attrs.iter().any(|attr| match &attr.meta { + Meta::List(list) => { + list.path.is_ident(name) && list.tokens.to_string().replace(' ', "") == args + } + _ => false, + }) +} + +fn parse(path: &Path) -> syn::File { + syn::parse_file(&std::fs::read_to_string(path).unwrap()).unwrap() } #[test] -fn every_write_cursor_routine_is_checked() { - check( - include_str!("../src/write.rs"), - "exercise_write_cursor", - "WriteCursor", +fn every_public_routine_is_annotated_and_listed() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + + let mut methods = PublicMethods::default(); + for entry in std::fs::read_dir(root.join("src")).unwrap() { + let path = entry.unwrap().path(); + if path.extension().is_some_and(|e| e == "rs") { + methods.visit_file(&parse(&path)); + } + } + assert!(!methods.0.is_empty(), "no public methods found"); + + let mut listed = ListedRoutines::default(); + listed.visit_file(&parse(&root.join("tests/no_panic.rs"))); + + let unannotated: Vec<_> = methods + .0 + .iter() + .filter(|(_, annotated)| !annotated) + .map(|(name, _)| name) + .collect(); + assert!( + unannotated.is_empty(), + "missing the no_panic attribute: {unannotated:?}" + ); + + let unlisted: Vec<_> = methods + .0 + .iter() + .map(|(name, _)| name) + .filter(|name| !listed.0.contains(*name)) + .collect(); + assert!( + unlisted.is_empty(), + "not listed in tests/no_panic.rs: {unlisted:?}" ); } From a5ba043f1d2a6345c264f58e43eb13adf8ee450b Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 15 Oct 2026 12:30:43 -0700 Subject: [PATCH 2/3] Add a canary that the no_panic check must reject no_panic relies on a linker trick. If a toolchain change stopped it from detecting panics, every check would pass silently. The canary is a function that can panic; CI requires its link to fail with the expected no_panic error. --- .github/workflows/ci.yml | 13 +++++++++++++ CHANGELOG.md | 2 +- Cargo.toml | 5 +++++ README.md | 3 +++ tests/no_panic_canary.rs | 23 +++++++++++++++++++++++ 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/no_panic_canary.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4eb2bb..41e8fc5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,6 +27,19 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - run: cargo test --profile no-panic --features no-panic --test no_panic + # the check is only meaningful if it still detects a known panic + - name: canary must fail to link + run: | + log=$(mktemp) + if cargo test --profile no-panic --features no-panic --test no_panic_canary > "$log" 2>&1; then + echo "::error::the no_panic canary linked, so the check is no longer detecting panics" + exit 1 + fi + if ! grep -q 'detected panic in function `canary`' "$log"; then + cat "$log" + echo "::error::the no_panic canary failed for an unexpected reason" + exit 1 + fi msrv: name: MSRV (1.77) diff --git a/CHANGELOG.md b/CHANGELOG.md index aed1a23..2f67831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ### 0.5.1 ### * :bug: `ReadCursor::transaction` closures can now return data borrowed from the input. See [#12](https://github.com/stepfunc/scursor/issues/12). * :bug: `WriteCursor::at_pos` now accepts `FnOnce` closures. See [#13](https://github.com/stepfunc/scursor/issues/13). -* :wrench: Replaced the Kani proofs with link-time `no_panic` checks, run in CI, covering every public routine for all inputs. A test fails if a new routine is left out of the check. +* :wrench: Replaced the Kani proofs with link-time `no_panic` checks, run in CI, covering every public routine for all inputs. A test fails if a new routine is left out of the check. A canary test confirms the check still detects panics. * :wrench: Added round-trip property tests and a CI workflow. * Declared the minimum supported Rust version (1.77). diff --git a/Cargo.toml b/Cargo.toml index 19df5db..6beb3e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,11 @@ no-panic = ["dep:no-panic"] name = "no_panic" required-features = ["no-panic"] +# must fail to link; CI checks that it does +[[test]] +name = "no_panic_canary" +required-features = ["no-panic"] + [profile.no-panic] inherits = "release" overflow-checks = true diff --git a/README.md b/README.md index 3e59e51..56bec18 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,9 @@ What the check does and doesn't tell you: one, which forces the compiler to keep a standalone copy that has to handle any arguments. That copy is what the linker checks. `tests/no_panic_coverage.rs` runs with the normal tests and fails if a public method is missing the annotation or isn't listed in `tests/no_panic.rs`. +* CI also links `tests/no_panic_canary.rs`, which contains a function that can panic, and + requires that link to fail. If a toolchain change ever stopped `no_panic` from detecting + panics, CI would fail rather than pass silently. * It applies to the configuration that was checked: an optimized build (`opt-level = 3`) with `overflow-checks = true` and `panic = "unwind"`. Debug builds and size-optimized builds keep some unreachable panic paths, so the check reports false failures there. Those builds are diff --git a/tests/no_panic_canary.rs b/tests/no_panic_canary.rs new file mode 100644 index 0000000..714cc1a --- /dev/null +++ b/tests/no_panic_canary.rs @@ -0,0 +1,23 @@ +//! Canary for the link-time panic check: this test must fail to link. +//! +//! `canary` panics when its argument is zero. If the `no_panic` check is +//! working, linking this test fails with "detected panic in function `canary`", +//! and CI requires that failure. If this test ever links, the check has stopped +//! detecting panics, and a passing `tests/no_panic.rs` would mean nothing. +//! +//! ```text +//! cargo test --profile no-panic --features no-panic --test no_panic_canary +//! ``` + +use no_panic::no_panic; +use std::hint::black_box; + +#[no_panic] +fn canary(divisor: u8) -> u8 { + 100 / divisor +} + +#[test] +fn canary_must_not_link() { + black_box:: _>(canary); +} From 4689a208e4926bb965c234cb6ef31e057720b2f0 Mon Sep 17 00:00:00 2001 From: jadamcrain Date: Thu, 15 Oct 2026 12:39:08 -0700 Subject: [PATCH 3/3] Scan nested module directories in the no_panic coverage test The scan only read the top level of src/, so a public method in a nested module directory (e.g. src/foo/mod.rs) escaped the check. --- tests/no_panic_coverage.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/no_panic_coverage.rs b/tests/no_panic_coverage.rs index 8925b92..d79b39d 100644 --- a/tests/no_panic_coverage.rs +++ b/tests/no_panic_coverage.rs @@ -69,17 +69,24 @@ fn parse(path: &Path) -> syn::File { syn::parse_file(&std::fs::read_to_string(path).unwrap()).unwrap() } +/// Visit every `.rs` file under `dir`, including nested module directories. +fn visit_dir(dir: &Path, methods: &mut PublicMethods) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + visit_dir(&path, methods); + } else if path.extension().is_some_and(|e| e == "rs") { + methods.visit_file(&parse(&path)); + } + } +} + #[test] fn every_public_routine_is_annotated_and_listed() { let root = Path::new(env!("CARGO_MANIFEST_DIR")); let mut methods = PublicMethods::default(); - for entry in std::fs::read_dir(root.join("src")).unwrap() { - let path = entry.unwrap().path(); - if path.extension().is_some_and(|e| e == "rs") { - methods.visit_file(&parse(&path)); - } - } + visit_dir(&root.join("src"), &mut methods); assert!(!methods.0.is_empty(), "no public methods found"); let mut listed = ListedRoutines::default();