tests(suppression): mixed valid+unknown marker voiding contract untested at lib level

[dekobon]

Summary

The library-level suppression tests never exercise a mixed marker that lists a valid metric and an unknown metric (e.g. suppress(cyclomatic, no_such_metric)), so the "unknown identifier voids the entire marker" contract is untested at the library boundary — a regression that silently honored only the valid part would pass the whole suite.

Location

• tests/suppression_test.rs:257-279 (unknown_metric_in_marker_has_no_effect)
• companion gap in the unit test src/suppression.rs:666 (native_unknown_metric_errors)

Evidence

parse_metric_list voids the whole marker on the first unknown token via ?:

// src/suppression.rs:459-461
let metric: Metric = name
    .parse()
    .map_err(|_| SuppressionError::UnknownMetric(name.to_owned()))?;

So suppress(cyclomatic, no_such_metric) returns Err(UnknownMetric) for the entire marker — the valid cyclomatic is dropped too. This is the documented hard contract (big-code-analysis-book/src/commands/suppression.md: an unknown identifier "warns and voids the entire marker").

Every existing test uses a marker whose only metric is unknown:

• unknown_metric_in_marker_has_no_effect (lib integration, line 269): # bca: suppress(no_such_metric) → asserts fine.suppressed.is_empty().
• native_unknown_metric_errors (unit, line 667): // bca: suppress(no_such_metric) → asserts the error variant.

With a single unknown token, is_empty() (or Err) holds under both the current "void whole marker" behavior and a hypothetical "skip unknown token, keep the rest" behavior. The two are indistinguishable until a marker mixes a valid metric with an unknown one — which no test does (rg 'suppress\(' ... confirms no mixed case anywhere in tests/, src/suppression.rs, or big-code-analysis-cli/tests/).

This is the library-side twin of #896 (which is scoped to the CLI stderr + exit-code surface through bca check); #896 even cites these two tests as covering only is_empty() / the error variant. The mixed-marker voiding gap is not addressed by #896.

Expected Behavior

A test should pin that suppress(cyclomatic, no_such_metric) produces an empty scope on the enclosing function — i.e. the valid cyclomatic is not honored — proving the unknown token voids the whole marker rather than being dropped in isolation.

Actual Behavior

No test distinguishes voiding from partial-honor. Swapping ? for continue in parse_metric_list (skip unknown, keep valid) would silently widen suppression on a typo and pass the entire test suite — the most dangerous failure mode, since a misspelled metric would still silence the correctly-spelled one beside it.

Impact

The void-on-typo guarantee is the safety property that prevents a typo from silently silencing a real violation. The data-structure half of that guarantee is unverified at the library level; a regression ships unnoticed.

[dekobon]

General Plan for Fix

Root Cause

The voiding contract — "an unknown metric identifier voids the entire marker, including any valid metrics listed beside it" — is enforced by the ? in parse_metric_list (src/suppression.rs:459-461). Every existing test (tests/suppression_test.rs:257, src/suppression.rs:666) feeds a marker whose only metric is unknown, so is_empty() / Err holds equally under the real "void whole marker" behavior and a hypothetical "skip the unknown token, keep the rest" behavior. The two are indistinguishable without a mixed valid+unknown marker, which no test uses.

Implementation Steps

1. Add a library-level integration test in tests/suppression_test.rs (e.g. mixed_valid_and_unknown_metric_voids_whole_marker) that analyzes a function carrying # bca: suppress(cyclomatic, no_such_metric) and asserts the enclosing function's scope is empty — specifically assert!(!fine.suppressed.covers(Metric::Cyclomatic)) and assert!(fine.suppressed.is_empty()). The covers assertion is the load-bearing one: it fails under a partial-honor regression that the is_empty() check on the single-unknown test cannot.
2. (Optional, defense in depth) Add a unit test beside native_unknown_metric_errors (src/suppression.rs) asserting parse_marker("// bca: suppress(cyclomatic, no_such_metric)") returns Err(UnknownMetric("no_such_metric")) — proving the parser rejects the whole list, not just that a downstream consumer drops it.

Tests to Add/Update

• New tests/suppression_test.rs::mixed_valid_and_unknown_metric_voids_whole_marker
• Optional new unit test in src/suppression.rs mod tests for parse_metric_list mixed-list rejection

Verification

• cargo test -p big-code-analysis passes
• Test-via-revert: temporarily change parse_metric_list's ? to continue (skip unknown token) and confirm the new mixed-marker test FAILS (it should now report cyclomatic as covered); restore the ? and confirm it passes again. This proves the test exercises the voiding line, per .claude/rules/testing.md.
• cargo clippy -p big-code-analysis clean