fix: Objc block nargs, diagnostic-prefix lexer, and 2.1.0 delta-review polish

.claude/rules/testing.md

@@ -46,6 +46,28 @@ work, do one of:
 - when a partial revert would not build, patch the production line to
   a no-op in place instead (the approach used in the #615 fix session).
 
+**A sweep of several perturbations needs a script, not a shell loop.**
+The inverse-edit advice above does not scale past one or two: a loop
+that perturbs, tests, and restores is where `git checkout -- <file>`
+gets reached for, and it destroyed an entire uncommitted rewrite a third
+time during #1219. Read the file once into a variable, write each
+perturbation from that string, and restore from it in a `finally` — the
+backup then lives in memory and cannot be defeated by the working tree
+changing underneath. Put the driver in a file rather than nesting quotes
+through zsh into `python3 -c`, which is how those edits came to be
+applied inconsistently in the first place.
+
+**A broken harness reports a plausible failure count, not an error.**
+In that same sweep the restore silently reverted the gate to its `main`
+version, so every subsequent perturbation ran the *new* test suite
+against the *old* implementation and reported an identical 34 — six
+failures and twenty-eight errors, reproducible today by checking out
+either side alone. Three perturbations in a row returning the same
+number reads as "well covered"; it meant the file under test was no
+longer the file being edited. Before believing any perturbation result,
+confirm the subject still contains the change: `rg -c <new symbol>` on
+the file, or a `git diff --stat` that shows what you expect.
+
 After restoring, `git status` / `git diff --stat` must show exactly
 the edits you intend — nothing extra, nothing missing.
 
@@ -291,3 +313,40 @@ executable name, a path separator, a line ending — in one OS's spelling.
 `.exe` on Windows: green on Linux and macOS, failing only on the Windows
 leg. Mirror the production code's platform logic in the fixture
 (`bca{EXE}`) rather than hardcoding one OS's form.
+
+## Gate a feature-gated fixture table on the union of its rows
+
+A test whose case list is built from `#[cfg(feature = …)]` rows has two
+failure modes, and fixing one reintroduces the other. Left ungated, a
+feature set that enables none of the rows leaves an empty list, a loop
+of zero iterations, and a test that passes having asserted nothing —
+which is why `assert_fixtures_present` (`src/test_support.rs`) exists to
+make that state loud. But loudness alone turns the same build into a
+spurious *failure* that reads as a defect in whatever was being changed.
+
+Both halves are required:
+
+- `#[cfg(any(feature = "a", feature = "b", …))]` on the **`fn`**, naming
+  the union of the features its rows use, so the test is *absent* rather
+  than failing when none is enabled.
+- The non-vacuity assertion (`assert_fixtures_present`, or a `ran > 0`
+  counter for a hand-rolled loop) inside it, covering the residual case
+  where a runtime `is_enabled()` check stops agreeing with the feature
+  it compiled under.
+
+This bit twice in one batch (#1220, PR #1221).
+`the_1184_constructs_open_quiet_function_spaces` had the assertion and
+no gate, so `--no-default-features --features rust` failed with "at
+least one language feature must be enabled for this test to mean
+anything"; its two siblings had been gated in an earlier fix and it was
+missed. Then the fix for #1218
+rewrote a Tcl/iRules parity test as a loop, added a `ran > 0` guard, and
+reproduced the identical false failure in a second file.
+
+Verify against a subset that enables **none** of the named features.
+`rust,typescript` — the canonical minimal-langs configuration — is not
+such a subset for any table containing a TypeScript or TSX row, so a
+single non-listed language (`--features go`) is the reproducer.
+
+Note the union is over *features*, not languages: `LANG::Tsx` rides
+`feature = "typescript"`, so a seven-row table can need only six.

CHANGELOG.md

@@ -897,14 +897,21 @@ for historical reference.
 - C's `(void)` marker is no longer counted as a parameter. `int f(void)`
   declares nothing, but the grammar emits a real `parameter_declaration`
   for the `void` and every `nargs` filter counted it, so `f(void)` and
-  `f(int)` both reported 1. Fixed for C, C++, Mozcpp and Objective-C.
+  `f(int)` both reported 1. Fixed for C, C++, Mozcpp and Objective-C,
+  in both the function and the closure channel — an Objective-C block
+  literal `^(void){ … }` counted the marker too, because its arm
+  matched parameter kinds positively instead of routing through the
+  shared `count_args` helper, and so never consulted the hook (#1218).
   The distinction needs the source bytes rather than the tree — an
   unnamed parameter is the same shape and really is one argument — so
   `Checker` gained an `is_empty_param_marker` hook that defaults to
   `false` and reads them.
 
   **Metric drift.** 24 recorded values fall to 0 in the `DeepSpeech`
-  corpus, all in `pywrapfst.cc`, its only `(void)` definitions.
+  corpus, all in `pywrapfst.cc`, its only `(void)` definitions. The
+  block-literal half moves nothing recorded: `^(void)` appears in two
+  corpus files, both `.mm`, which route to C++ — where blocks are not a
+  construct.
 
 - A comment written inside a parameter list is no longer counted as a
   parameter (#1201). tree-sitter attaches such a comment as a direct
@@ -916,8 +923,12 @@ for historical reference.
   TypeScript, TSX, Python, Rust, Java, C#, PHP, Ruby, Groovy, Elixir and
   Kotlin lambdas in one shared predicate. Go, Lua, Tcl, iRules,
   Objective-C methods and blocks, Kotlin functions and Groovy closures
-  were already correct and are unchanged; Perl had carried the exclusion
-  privately since its signature support landed.
+  already reported the right count here and needed no fix; Perl had
+  carried the exclusion privately since its signature support landed.
+  Objective-C blocks were correct only incidentally — their arm listed
+  the parameter kinds it wanted rather than excluding comments, and
+  nothing asserted it — so #1218 routed them through the shared
+  predicate and added the fixture.
 
   **Metric drift.** Serialized `nargs` falls wherever a signature
   carries a comment — across the `DeepSpeech` corpus, 854 recorded
@@ -1062,6 +1073,36 @@ for historical reference.
   the 126 files under the per-language subdirectories were never checked.
   Latent — no live count changed.
 
+- `utils/check-diagnostic-prefix.py` decides string and comment state
+  with a lexer rather than a per-line regex (#1219). Three inputs made
+  it read a raw-string open where there was none — a plain string whose
+  closing quote follows `r` (`"dir/r"`), and an unterminated `r"` in a
+  trailing `//` or a `/* … */` comment — after which every line to the
+  next quote was skipped and any severity literal in between was a
+  false clean. Neither cheap fix works: no lookbehind can express the
+  first, since what distinguishes it is that the quote *closes* a
+  literal, and stripping comments by regex would truncate `"http://x"`
+  mid-literal and open a phantom span of its own. The walk is ported
+  from `check-snapshot-anchors.py`, which needed the identical machine;
+  both sides now name the other. One deliberate widening: a severity
+  quoted inside any comment is skipped, where before only a whole-line
+  comment was. Latent — no live count changed, verified by diffing both
+  scanners over all 559 tracked Rust files.
+
+- The book and [`STABILITY.md`](./STABILITY.md) scope the
+  object-oriented emission rule to `npm` and `npa` (#1220). Both said
+  all three blocks follow the space's kind with no grammar deviating in
+  either direction; that holds for `npm` / `npa`, which are gated
+  centrally by kind, and not for `wmc`, which is decided per language.
+  Go emits no `wmc` block on any space including the file root while its
+  `npa` / `npm` do appear there, and a `namespace` space — a C++ or
+  Mozcpp `namespace`, or a Ruby `module` — carries `npm` / `npa` but no
+  `wmc` because its member functions are free functions rather than
+  methods of a class. Both narrowings are now asserted in
+  `container_scope_tests.rs`, which previously tracked only `npm` and
+  `npa` — nothing pinned `wmc`'s scope, which is how one rule came to
+  describe three blocks.
+
 - **Metric values move.** A Java record's compact constructor
   (`record R(int a) { R { … } }`) now opens its own function space
   instead of charging its body to the enclosing class (#1160).

STABILITY.md

@@ -1076,9 +1076,12 @@ dicts, byte-identical to the CLI output), so it only narrows the
 static type. Every metric block is `NotRequired` because a `metrics=`
 selection can elide blocks, and because the object-oriented blocks are
 scope-gated: `wmc`, `npm` and `npa` are emitted on container spaces and
-on the file root, and never on a function space (#1197, #1203). Since
-the latter, the space's own kind is the sole input for every language,
-so no grammar deviates from that rule in either direction. A language
+on the file root, and never on a function space (#1197, #1203). For
+`npm` and `npa` the space's own kind has been the sole input for every
+language since the latter, with no grammar deviating in either
+direction. `wmc` is narrower in two language-level ways: Go emits no
+`wmc` block on any space, including the file root, and a `namespace`
+space carries `npm` and `npa` but no `wmc`. A language
 with no class-like construct emits them nowhere. Which spaces carry
 which block is not part of the shape contract — the `wire` struct
 definitions are. The VCS *report* dicts are now single-sourced and typed too

big-code-analysis-book/src/metrics.md

@@ -950,10 +950,12 @@ counts as "attribute" rather than "method".
 
 ### Which spaces carry NPA, NPM and WMC {#oop-emission-scope}
 
-The three object-oriented blocks are emitted on **container spaces** —
-`class`, `struct`, `trait`, `impl`, `namespace`, `interface` — and on
-the whole-file `unit` root, which carries the roll-up across every
-container in the file. A **function space does not carry them**: a
+NPA and NPM are emitted on **container spaces** — `class`, `struct`,
+`trait`, `impl`, `namespace`, `interface` — and on the whole-file `unit`
+root, which carries the roll-up across every container in the file. WMC
+follows the same rule wherever it is computed at all, which is narrower
+in two ways set out below. A **function space does not carry any of
+them**: a
 method owns no methods or attributes of its own, so the block would be
 all zeros. Before big-code-analysis 2.1.0, NPA and NPM did emit that
 all-zero block on function spaces in C#, JavaScript, MozJS, TypeScript,
@@ -971,12 +973,28 @@ roll up through every enclosing space regardless. So a type declared
 inside a function body is reported by the nearest enclosing container,
 or by the file root when there is none.
 
-Two things read differently. A Go file's NPA and NPM live on the `unit`
+Four things read differently. A Go file's NPA and NPM live on the `unit`
 root and nowhere else, because Go is the one language that emits them
 without having a container kind in its space tree — `type … struct` and
 `type … interface` open no space of their own. (Bash, C, Lua, Perl,
 Tcl and iRules have no container kind either, but they emit neither
-block anywhere, so the question does not arise.) And the CSV projection
+block anywhere, so the question does not arise.)
+
+Neither WMC narrowing moves NPA or NPM, which is why the rule above
+still holds as stated for those two. The first is language-level: **Go
+emits no `wmc` block at all**, on any space including the `unit` root,
+because its flat space model cannot attribute a method to a receiver
+class — so a Go file carries `npa` and `npm` at the root and `wmc`
+nowhere. The second is space-kind-level: a **`namespace` space carries
+`npa` and `npm` but no `wmc`**, because a namespace's member functions
+are free functions rather than methods of a class, so there is no
+per-class complexity to weight. That covers every construct mapping to
+`SpaceKind::Namespace` — a C++ or Mozcpp `namespace`, and a Ruby
+`module` — not just the C++ spelling. Objective-C has no namespace
+construct of its own, so the case does not arise there. The class
+*inside* the namespace carries all three, and so does the file root.
+
+Finally, the CSV projection
 is a fixed-column format: it writes the `npa.*` / `npm.*` columns on
 **every** row regardless of space kind, carrying the real accessor
 values rather than eliding them.

docs/development/lessons_learned.md

@@ -132,6 +132,7 @@ number and the higher number stays as a redirect.
 | [86](#86-a-test-helper-that-normalizes-the-value-under-test-blinds-every-caller-at-once) | A helper normalising the observation blinds every caller |
 | [87](#87-an-assertion-can-be-correct-and-still-be-about-the-wrong-rows) | An assertion can be about the wrong rows |
 | [88](#88-a-text-scan-that-does-not-lex-the-language-measures-noise) | A text scan that does not lex the language measures noise |
+| [89](#89-a-positive-enumeration-and-a-negative-filter-differ-on-what-neither-names) | A positive enumeration and a negative filter differ on what neither names |
 
 ---
 
@@ -3202,7 +3203,10 @@ model the language's lexical structure — strings, raw strings, comments,
 not report a wrong number occasionally; it reports a confident, uniform,
 plausible one every run, which is then quoted as fact. Give the scanner
 its own tests, and pin **both** directions: the under-lex that misses
-constructs and the over-lex that swallows them.
+constructs and the over-lex that swallows them. **Do not try to buy the
+distinction back with a sharper pattern.** Whether a quote opens or
+closes a literal is *state*, and a regex sees only the characters around
+it, so no lookbehind can separate the two cases.
 
 The remedy for a wrong claim is normally "run the measurement" (lesson
 84). That does not help here, because the measurement *was* run. When
@@ -3238,4 +3242,68 @@ because no file under `src/metrics/` spells one. Lifting the fix's
 `'a` has no closing quote and treating it as a literal swallows the file
 the other way — closes both.
 
+**The third gate in the family, and the proof no pattern would have
+done** (#1219, PR #1221). `utils/check-diagnostic-prefix.py` matched
+raw-string opens with a regex and skipped to the next quote, so three
+shapes opened a span that hid every severity literal until it closed:
+`let p = "dir/r";`, where the closing quote of an ordinary string
+follows an `r`; and an unterminated `r"` inside a trailing `//` or a
+`/* … */` comment, neither of which a scanner that only skips
+*whole-line* comments can see. The first is the one that settles the
+approach — `/` is a legitimate raw-open context, so the character before
+`r` carries no information; what differs is that the `"` **closes** a
+literal. Stripping comments first fails the same way, because
+`"http://x"` truncates mid-literal and opens a phantom span of its own.
+Finding where a comment starts already requires knowing whether you are
+inside a string, which is the lexer. All three were latent: replaying
+both scanners over all 559 tracked Rust files gave identical output.
+
+**Porting a lexer without porting the shape that makes its tests
+discriminate** (#1219). The port's lifetime fixture used an *even* number
+of lifetimes, so a greedy `char_literal_end` pairs them off, every bogus
+span closes before the offender, and the test passes against the exact
+bug it names. The donor's suite had recorded that trap — "Three
+lifetimes, not two, and a real char literal after the call" — and the
+copy dropped it. When you lift a scanner, lift its fixtures' arithmetic,
+not just its assertions.
+
+---
+
+## 89. A positive enumeration and a negative filter differ on what neither names
+
+**Lesson:** Replacing a bespoke `matches!(A | B)` with a shared
+`!is_x && !is_y` predicate does not just move the rule — it changes the
+set. The two agree on every kind either one names and disagree on
+everything else, so the inputs that move are exactly the ones no one
+enumerated and no test covers. Before consolidating, enumerate the node
+kinds the container can actually hold, and decide the leftovers
+deliberately: adopting the shared answer is usually right, but it is a
+decision, not a refactor. (cf. lesson 59 for why you are consolidating,
+and lesson 65 for the structural inverse.)
+
+The direction of the change is what hides it. A positive filter is
+closed — a grammar that starts emitting a new kind silently scores zero,
+which is lesson 19. A negative filter is open, so the same grammar
+change silently scores *one*, and neither shows up as a diff in the
+snapshot suite unless a fixture happens to hold the construct. Both
+forms read as "the obvious thing" at the call site.
+
+**The Objective-C block arm inherited three exclusions and two
+inclusions** (#1218, PR #1221). The `Objc::BlockLiteral` `nargs` arm
+counted `matches!(ParameterDeclaration | VariadicParameter)`, which
+bypassed the shared `count_args` and therefore
+`Checker::is_empty_param_marker`, so `^(void){ … }` reported one
+parameter where zero belongs. Routing it through `count_args` fixed
+that and put the comment and punctuation rules on the shared footing
+too — those had *happened* to be right, since a `comment` child is not
+a `ParameterDeclaration` either, but for a reason no test asserted. It
+also changed two shapes nobody had considered: on invalid source
+`^(int a,,)` went 1 → 2 as an `ERROR` child began counting, and
+`^({ int x; })` went 0 → 1 for a `compound_statement` child. Both were
+kept, because both are what the function channel already reported
+through the same helper — `int f(int a,,)` gives 2 and
+`int g({ int x; })` gives 1 — so the block arm had been the one caller
+answering differently. Recording that in the arm's comment is what
+stops the next reader from filing it as a regression.
+
 ---

src/c_declarator.rs

@@ -234,6 +234,24 @@ pub(crate) fn declarator_name<'tree, T: Checker>(node: &Node<'tree>) -> Option<N
     // `function_declarator` has no `declarator` field", which the
     // grammars declare required and only an `ERROR` could violate — two
     // arms no test can reach, and coverage counts them.
+    //
+    // The two forms are not identical on that unreachable input, which is
+    // worth stating rather than leaving for the next reader to rediscover
+    // (#1220). On an `ERROR` tree where a `function_declarator` lacks its
+    // `declarator` field, the loop returned `None` and this chain yields
+    // that `function_declarator` itself — the whole declarator span in
+    // place of a name.
+    //
+    // Nothing observes the difference, and the reason is external to this
+    // module: every caller gates the result on its own grammar's
+    // identifier kinds (`TypeIdentifier | Identifier | FieldIdentifier`,
+    // plus the C++ name forms in `getter/cpp.rs` and `getter/mozcpp.rs`),
+    // and `function_declarator` is in none of those lists. The `matches!`
+    // falls through and `get_func_space_name` returns `None` — the same
+    // answer the loop gave. That dependency is the thing to preserve: a
+    // getter that widened its gate to accept `function_declarator` would
+    // start naming functions after their whole declarator span on
+    // malformed input, and this comment is the only place that says so.
     std::iter::successors(
         innermost_declarator::<T>(node)?.child_by_field_name("declarator"),
         |link| {