Skip to content

Commit 9288e17

Browse files
hyperpolymathclaude
andcommitted
fix(analyzer): clear 4 -Wunused-variable warnings + latent bug
Zero-warnings-policy violations surfaced by the last release build: analyzer.rs:928 unwrap_threshold (declared, never compared) analyzer.rs:948 unbounded_vec_patterns (declared, never used) analyzer.rs:949 unbounded_box_patterns (declared, never used) analyzer.rs:950 unbounded_string_patterns (declared, never used) Drops the three `unbounded_*_patterns` locals — they duplicated the allocation-site count without feeding into any finding (the actual unbounded-pattern *classification* happens in the `has_unbounded_allocations` keyword block below). Fixes the latent `unwrap_threshold` bug: the test-file suppression branch declared panic_threshold and unwrap_threshold but only ever compared panic_sites > panic_threshold, then returned `unwrap_calls` unmodified on the hot branch and `(0, 0)` on the cold branch. Silently dropped any excessive-unwrap signal in test files. New behaviour: panic and unwrap axes are evaluated independently via saturating_sub, so a test file with 30 panics + 5 unwraps now reports (10, 0) instead of (10, 5) / (0, 0). Pre-existing test failure surfaced by this fix: readiness.rs's `readiness_c_assail_detects_unwrap` used a fixture named `unwrap_test.rs`, which matches the `ends_with("_test.rs")` is_test_file heuristic. Test was supposed to exercise production-code PanicPath detection; fixture rename to `unwrap_fixture.rs` makes the intent match the implementation. Full test suite: 18/18 readiness, all other targets green, 0 warnings, 0 Clippy pedantic on changed hunk. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b8d177d commit 9288e17

3 files changed

Lines changed: 32 additions & 22 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/assail/analyzer.rs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -920,18 +920,24 @@ impl Analyzer {
920920
let panic_sites = code_only.matches("panic!(").count() + code_only.matches("unreachable!(").count();
921921
let unwrap_calls = code_only.matches(".unwrap()").count() + code_only.matches(".expect(").count();
922922

923-
// Apply test file suppression
923+
// Apply test file suppression. In test files, normal
924+
// assert-macro use pushes panic/unwrap counts high with no
925+
// production-code signal, so we suppress the counts unless they
926+
// exceed a "clearly excessive" threshold — only the delta above
927+
// the threshold is attributed to the file's statistics.
928+
//
929+
// Panic and unwrap thresholds are evaluated independently: a
930+
// test file with 30 panics + 5 unwraps should report 10 panics
931+
// (30 − 20) and 0 unwraps, not suppress everything because the
932+
// unwrap count is normal. The previous version declared both
933+
// thresholds but only compared panics, silently dropping any
934+
// excessive unwrap signal.
924935
let (effective_panic_sites, effective_unwrap_calls) = if is_test_file {
925-
// In test files, only count excessive usage that might indicate real issues
926-
// Suppress normal test patterns but flag extreme cases
927-
let panic_threshold = 20; // More than 20 panics might indicate poor test design
928-
let unwrap_threshold = 10; // More than 10 unwraps might indicate poor test design
929-
930-
if panic_sites > panic_threshold {
931-
(panic_sites - panic_threshold, unwrap_calls)
932-
} else {
933-
(0, 0) // Suppress normal test patterns
934-
}
936+
let panic_threshold = 20;
937+
let unwrap_threshold = 10;
938+
let eff_panics = panic_sites.saturating_sub(panic_threshold);
939+
let eff_unwraps = unwrap_calls.saturating_sub(unwrap_threshold);
940+
(eff_panics, eff_unwraps)
935941
} else {
936942
(panic_sites, unwrap_calls) // Production code: count all
937943
};
@@ -943,13 +949,13 @@ impl Analyzer {
943949
let vec_new_count = content.matches("Vec::new()").count();
944950
let box_new_count = content.matches("Box::new(").count();
945951
let string_new_count = content.matches("String::new()").count();
946-
947-
// Detect potential unbounded allocations
948-
let unbounded_vec_patterns = content.matches("Vec::with_capacity(").count();
949-
let unbounded_box_patterns = content.matches("Box::new(").count(); // Box can be unbounded
950-
let unbounded_string_patterns = content.matches("String::with_capacity(").count();
951-
952-
// Count allocations but flag unbounded patterns separately
952+
953+
// Count allocations; unbounded-pattern *classification* (tiny
954+
// with_capacity, unlimited-read keywords, etc.) happens in the
955+
// `has_unbounded_allocations` block below — not as a per-site
956+
// counter. The previous `unbounded_*_patterns` locals duplicated
957+
// the count without feeding into any finding and have been
958+
// removed to clear dead-code warnings.
953959
stats.allocation_sites += vec_new_count + box_new_count + string_new_count;
954960

955961
// Flag unbounded allocation patterns as high-risk. `code_only`

tests/readiness.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ fn readiness_c_assail_detects_unsafe() {
105105
#[test]
106106
fn readiness_c_assail_detects_unwrap() {
107107
let dir = TempDir::new().unwrap();
108-
// Need >5 unwrap/expect calls to exceed the reporting threshold
108+
// Fixture name must NOT match the `is_test_file` heuristic
109+
// (`_test.rs` / `_tests.rs` / `/tests/` / `_spec.` / …) — the
110+
// analyser suppresses normal test-file panic/unwrap counts so the
111+
// PanicPath category only surfaces on production code. This
112+
// fixture exercises the production-code path.
109113
let code = r#"
110114
fn main() {
111115
let _a = Some(1).unwrap();
@@ -117,7 +121,7 @@ fn main() {
117121
let _g = Some(7).expect("seven");
118122
}
119123
"#;
120-
let src = dir.path().join("unwrap_test.rs");
124+
let src = dir.path().join("unwrap_fixture.rs");
121125
fs::write(&src, code).unwrap();
122126
let report = assail::analyze(&src).expect("assail should succeed");
123127
assert!(

0 commit comments

Comments
 (0)