fix(code-unwrap): cfg(not(test)) polarity; add missing negative-case tests

src/audits/source/rust/unwrap.rs

@@ -120,11 +120,21 @@ fn walk<'a>(
     for child in node.children() {
         let kind = child.kind();
         if kind.as_ref() == "attribute_item" || kind.as_ref() == "inner_attribute_item" {
-            // An inner attribute (`#![cfg(test)]` at the head of a mod body)
-            // gates the enclosing item — i.e. *every* sibling of the inner
-            // attribute inherits the gate. Conservatively, treat
-            // `inner_attribute_item` as if it gated all following siblings
-            // here, which covers the canonical position at the top of a mod.
+            // Both outer (`#[cfg(test)]`) and inner (`#![cfg(test)]`)
+            // attribute kinds are handled uniformly: when the attribute
+            // resolves to `cfg(test)`, set the one-shot `next_is_cfg_test`
+            // flag so the very next sibling (and only that one) inherits
+            // the gate.
+            //
+            // This is *not* canonical Rust semantics for inner attributes —
+            // a real `#![cfg(test)]` at the head of a mod body gates every
+            // sibling in that mod body — but the asymmetry is intentional
+            // here. Fixing it would require hoisting the gate to the
+            // enclosing `mod_item` or making the flag sticky across
+            // siblings; both are out of scope for this walker. The
+            // resulting behavior (only the next sibling is gated by an
+            // inner attribute) is pinned by
+            // `inner_attribute_one_shot_consumes_on_use_not_fn`.
             if attribute_text_is_cfg_test(child.text().as_ref()) {
                 next_is_cfg_test = true;
             }
@@ -195,15 +205,24 @@ fn attribute_text_is_cfg_test(attr: &str) -> bool {
     let Some(args) = rest.strip_prefix('(').and_then(|s| s.strip_suffix(')')) else {
         return false;
     };
-    cfg_args_contain_test(args)
+    cfg_args_contain_test(args, false)
 }
 
 /// Walk the textual argument list of a `cfg(...)` attribute looking for a bare
 /// `test` predicate. Skips string literals (so `feature = "test"` is rejected)
 /// and only matches full identifiers (so `testing_only` / `test_helper` /
 /// `cfg(unix)` are rejected). Handles arbitrary nesting under `any(...)`,
 /// `all(...)`, and `not(...)`.
-fn cfg_args_contain_test(args: &str) -> bool {
+///
+/// `negated` tracks the polarity inherited from enclosing `not(...)` wrappers:
+/// at even parity (`negated = false`) a bare `test` means the item is test-gated;
+/// at odd parity (`negated = true`) it means the item is *non*-test-gated
+/// (production-only). Only `not(...)` flips polarity on recursion; `any(...)`
+/// and `all(...)` preserve it. This is conservative: returning `true` from any
+/// recursion claims "this attribute resolves to test-gated"; sibling predicates
+/// inside `any(...)` / `all(...)` cannot revoke that claim, so a production
+/// item with `cfg(any(not(test), unix))` correctly returns `false`.
+fn cfg_args_contain_test(args: &str, negated: bool) -> bool {
     let bytes = args.as_bytes();
     let mut i = 0;
     while i < bytes.len() {
@@ -237,18 +256,25 @@ fn cfg_args_contain_test(args: &str) -> bool {
                 j += 1;
             }
             let next = bytes.get(j).copied();
-            if ident == "test" {
-                // Bare `test` predicate: must be followed by a separator
-                // (`,`, `)`, or end-of-args), not `=` (would make it a key)
-                // and not `(` (would make it a nested call, e.g. `test(...)`).
-                if next.is_none() || matches!(next, Some(b',') | Some(b')')) {
+            if ident == "test" && (next.is_none() || matches!(next, Some(b',') | Some(b')'))) {
+                // Bare `test` predicate, followed by a separator (`,`, `)`,
+                // or end-of-args). At even parity (`!negated`) the item is
+                // test-gated. At odd parity (under a `not(...)`) the
+                // predicate means production code; keep scanning sibling
+                // predicates in case another one resolves to test-gated.
+                if !negated {
                     return true;
                 }
+                continue;
             } else if (ident == "any" || ident == "all" || ident == "not")
                 && next == Some(b'(')
                 && let Some(inner) = balanced_parens(&args[j..])
             {
-                if cfg_args_contain_test(inner) {
+                let recurse_negated = match ident {
+                    "not" => !negated,
+                    _ => negated, // `any` and `all` preserve polarity.
+                };
+                if cfg_args_contain_test(inner, recurse_negated) {
                     return true;
                 }
                 i = j + inner.len() + 2; // skip past matching ')'
@@ -553,6 +579,182 @@ impl S {
         assert_eq!(status, AuditStatus::Pass);
     }
 
+    #[test]
+    fn cfg_not_test_does_not_exempt_production_unwrap() {
+        // Regression guard for the PR #77 polarity bug: `cfg(not(test))`
+        // gates *production-only* code (compiled when NOT testing) and must
+        // NOT be treated as cfg(test). The unwrap below is production and
+        // must flag.
+        let source = r#"
+#[cfg(not(test))]
+fn production_only() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn cfg_any_not_test_unix_does_not_exempt() {
+        // `any(not(test), unix)` compiles in (production AND any platform)
+        // OR (any compilation AND unix). Either way the item compiles in
+        // production builds — the unwrap is production code.
+        let source = r#"
+#[cfg(any(not(test), unix))]
+fn x() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn cfg_all_not_test_feature_does_not_exempt() {
+        // `all(not(test), feature = "x")` is production-only under feature
+        // `x`. The unwrap is production code.
+        let source = r#"
+#[cfg(all(not(test), feature = "x"))]
+fn x() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn cfg_any_test_or_not_feature_still_exempts() {
+        // Polarity fix must not break the positive case: `any(test, ...)`
+        // still has a bare `test` predicate at even parity. The unwrap is
+        // gated.
+        let source = r#"
+#[cfg(any(test, not(feature = "real")))]
+mod tests {
+    fn t() { foo().unwrap(); }
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        assert_eq!(status, AuditStatus::Pass);
+    }
+
+    #[test]
+    fn cfg_attr_test_does_not_exempt() {
+        // `cfg_attr(test, ...)` applies the inner attributes conditionally;
+        // the item itself compiles unconditionally. The walk must NOT treat
+        // `cfg_attr` as a gate. The unwrap is production code.
+        let source = r#"
+#[cfg_attr(test, allow(unused))]
+fn x() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn inner_attribute_one_shot_consumes_on_use_not_fn() {
+        // Pins the intentional one-shot semantics for inner attributes:
+        // `#![cfg(test)]` followed by `use foo;` and then `fn production`
+        // gates only the `use` (which is not item-like) — the `fn` slips
+        // through. This is *not* canonical Rust semantics; the walker is
+        // deliberately one-shot for both outer and inner attributes and
+        // accepts the asymmetry. See the comment block at the top of
+        // `walk` for the rationale.
+        let source = r#"
+#![cfg(test)]
+use foo;
+
+fn production() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn mod_tests_without_gate_does_not_exempt() {
+        // A module *named* `tests` without a `#[cfg(test)]` gate is
+        // production code; the walker uses the gate, not the name. The
+        // unwrap inside must flag.
+        let source = r#"
+mod tests {
+    fn helper() {
+        foo().unwrap();
+    }
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
+    #[test]
+    fn cfg_not_not_test_does_exempt() {
+        // `not(not(test))` is semantically equivalent to `test` (even
+        // parity restored). The polarity tracker must recognize this. The
+        // unwrap inside is test code and must NOT flag.
+        let source = r#"
+#[cfg(not(not(test)))]
+mod tests {
+    fn t() { foo().unwrap(); }
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        assert_eq!(status, AuditStatus::Pass);
+    }
+
+    #[test]
+    fn cfg_not_any_test_does_not_exempt() {
+        // `not(any(test))` is semantically equivalent to `not(test)` —
+        // production-only. The unwrap inside is production code.
+        let source = r#"
+#[cfg(not(any(test)))]
+fn x() {
+    foo().unwrap();
+}
+"#;
+        let status = audit_unwrap_with(source, "src/lib.rs", false);
+        if let AuditStatus::Fail(evidence) = &status {
+            assert_eq!(evidence.lines().count(), 1);
+            assert!(evidence.contains("foo().unwrap()"));
+        } else {
+            panic!("expected Fail, got {status:?}");
+        }
+    }
+
     #[test]
     fn include_tests_flag_disables_exemption() {
         // Regression guard for U1's contract: every cfg(test)-gated unwrap

tests/fixtures/cfg-test-edge-cases/Cargo.toml

@@ -0,0 +1,4 @@
+[package]
+name = "cfg-test-edge-cases"
+version = "0.1.0"
+edition = "2024"

tests/fixtures/cfg-test-edge-cases/src/lib.rs

@@ -0,0 +1,41 @@
+// Fixture for the code-unwrap audit's cfg-gate exemption logic. Exercises the
+// full parse -> walk -> AuditResult -> scorecard pipeline for the four cfg
+// shapes that matter:
+//
+// 1. Production code (no gate): MUST be flagged.
+// 2. `#[cfg(not(test))]` production-only items: MUST be flagged. This is the
+//    regression guard for the PR #80 polarity fix.
+// 3. `#[cfg(test)]`-gated functions: MUST be exempt.
+// 4. `#[cfg(test)] mod`-gated items: MUST be exempt.
+//
+// The integration test in tests/integration.rs asserts exactly which lines
+// surface in evidence; do not reorder items or add unrelated unwraps without
+// updating the expected line numbers there.
+
+fn maybe() -> Result<u32, ()> {
+    Ok(42)
+}
+
+pub fn production_path() {
+    let _ = maybe().unwrap();
+}
+
+#[cfg(not(test))]
+pub fn production_only_path() {
+    let _ = maybe().unwrap();
+}
+
+#[cfg(test)]
+pub fn test_only_helper() {
+    let _ = maybe().unwrap();
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn unit() {
+        let _ = maybe().unwrap();
+    }
+}

tests/integration.rs

@@ -349,6 +349,78 @@ fn test_perfect_fixture() {
     assert_eq!(error_count, 0, "perfect-rust fixture should have 0 errors");
 }
 
+#[test]
+fn test_cfg_test_edge_cases_fixture_flags_only_production_unwraps() {
+    // End-to-end coverage of the code-unwrap audit's cfg-gate exemption logic
+    // through the full parse -> walk -> AuditResult -> scorecard pipeline.
+    // Unit tests in src/audits/source/rust/unwrap.rs cover the parser in
+    // isolation; this test wires the parser to a real project on disk and
+    // checks that the scorecard reflects the polarity-correct behavior fixed
+    // in PR #80.
+    //
+    // Expected evidence (pinned to the fixture's current layout — see
+    // tests/fixtures/cfg-test-edge-cases/src/lib.rs):
+    //   - line 20: production_path()              (no cfg gate)
+    //   - line 25: production_only_path()         (#[cfg(not(test))])
+    // Exempted (must NOT appear in evidence):
+    //   - line 30: test_only_helper()             (#[cfg(test)])
+    //   - line 37: tests::unit()                  (inside #[cfg(test)] mod)
+    let path = fixture_path("cfg-test-edge-cases");
+
+    let assert = cmd().args(["audit", &path, "--output", "json"]).assert();
+    let output = assert.get_output().stdout.clone();
+    let json_str = String::from_utf8(output).expect("stdout should be valid UTF-8");
+    let parsed: serde_json::Value =
+        serde_json::from_str(&json_str).expect("output should be valid JSON");
+
+    let code_unwrap = parsed["results"]
+        .as_array()
+        .expect("results should be an array")
+        .iter()
+        .find(|r| r["id"].as_str() == Some("code-unwrap"))
+        .expect("results should include a code-unwrap row");
+
+    assert_eq!(
+        code_unwrap["status"].as_str(),
+        Some("fail"),
+        "code-unwrap must fail on the fixture (two production unwraps): {code_unwrap}",
+    );
+
+    let evidence = code_unwrap["evidence"]
+        .as_str()
+        .expect("code-unwrap fail must carry an evidence string");
+    let lines: Vec<&str> = evidence.lines().collect();
+    assert_eq!(
+        lines.len(),
+        2,
+        "evidence must list exactly two unwraps (production_path + cfg(not(test)) production_only_path), got: {evidence}",
+    );
+
+    let has_production_path = lines
+        .iter()
+        .any(|l| l.contains("/lib.rs:20:") && l.contains("maybe().unwrap()"));
+    let has_production_only_path = lines
+        .iter()
+        .any(|l| l.contains("/lib.rs:25:") && l.contains("maybe().unwrap()"));
+    assert!(
+        has_production_path,
+        "evidence must flag production_path at lib.rs:20: {evidence}",
+    );
+    assert!(
+        has_production_only_path,
+        "evidence must flag the cfg(not(test)) production_only_path at lib.rs:25 (PR #80 regression guard): {evidence}",
+    );
+
+    // The two cfg(test) cases live at lib.rs:30 and lib.rs:37 in the fixture;
+    // neither must appear in evidence.
+    for forbidden in [":30:", ":37:"] {
+        assert!(
+            !evidence.contains(forbidden),
+            "cfg(test)-exempt line {forbidden} leaked into evidence: {evidence}",
+        );
+    }
+}
+
 // ── Bare invocation test ──────────────────────────────────────────
 
 #[test]