fix: ten metric and tooling bugs (batch 2026-09-10)

.bca-baseline.toml

@@ -15,7 +15,7 @@ headroom = 0.95
 path = "big-code-analysis-ast/src/alterator.rs"
 qualified = "<file>"
 metric = "loc.ploc"
-value = 545.0
+value = 561.0
 
 [[entry]]
 path = "big-code-analysis-ast/src/alterator.rs"
@@ -27,7 +27,7 @@ value = 5.0
 path = "big-code-analysis-ast/src/alterator.rs"
 qualified = "Alterator::get_ast_node"
 metric = "nargs"
-value = 6.0
+value = 7.0
 
 [[entry]]
 path = "big-code-analysis-ast/src/alterator.rs"
@@ -189,7 +189,7 @@ value = 6.0
 path = "big-code-analysis-ast/src/ast.rs"
 qualified = "build"
 metric = "halstead.effort"
-value = 72581.41181251501
+value = 87299.1671815905
 
 [[entry]]
 path = "big-code-analysis-ast/src/c_macro.rs"
@@ -267,13 +267,13 @@ value = 120805.48245794396
 path = "big-code-analysis-ast/src/node.rs"
 qualified = "Node<'a>"
 metric = "nom"
-value = 33.0
+value = 35.0
 
 [[entry]]
 path = "big-code-analysis-ast/src/parser.rs"
 qualified = "Parser<T>::filters"
 metric = "halstead.effort"
-value = 58860.61554860244
+value = 85508.52631578948
 
 [[entry]]
 path = "big-code-analysis-ast/src/preproc.rs"
@@ -915,7 +915,7 @@ value = 15.0
 path = "src/metrics/abc/kotlin.rs"
 qualified = "KotlinCode::compute"
 metric = "cyclomatic"
-value = 16.0
+value = 18.0
 
 [[entry]]
 path = "src/metrics/abc/objc.rs"
@@ -1257,7 +1257,7 @@ value = 6.0
 path = "src/vcs/git/cached.rs"
 qualified = "build_cached"
 metric = "halstead.effort"
-value = 73179.83359417088
+value = 74812.5424082547
 
 [[entry]]
 path = "src/vcs/git/cached.rs"

.rustfmt-bail-baseline.txt

@@ -48,6 +48,13 @@
 #   big-code-analysis-ast/src/getter/ruby.rs
 #                              3 -> 4, the `BQUOTE` subshell guard
 #                              (#1360) in `get_op_type`
+#   big-code-analysis-ast/src/getter/csharp.rs
+#                              5 -> 8, and
+#   big-code-analysis-ast/src/getter/java.rs
+#                              2 -> 5: the self- and super-reference arms
+#                              (#1380) in `get_op_type` — each a nested
+#                              two-arm `match` on the parent kind, so
+#                              three stuck arms apiece
 #   src/vcs/error.rs           `classify_error_variants!` (#1245): its
 #                              matcher, plus the 11 `$pat => $sample`
 #                              entries at the invocation, which are
@@ -119,12 +126,12 @@ big-code-analysis-ast/src/getter.rs 3
 big-code-analysis-ast/src/getter/bash.rs 3
 big-code-analysis-ast/src/getter/c.rs 3
 big-code-analysis-ast/src/getter/cpp.rs 5
-big-code-analysis-ast/src/getter/csharp.rs 5
+big-code-analysis-ast/src/getter/csharp.rs 8
 big-code-analysis-ast/src/getter/elixir.rs 3
 big-code-analysis-ast/src/getter/go.rs 1
 big-code-analysis-ast/src/getter/groovy.rs 5
 big-code-analysis-ast/src/getter/irules.rs 6
-big-code-analysis-ast/src/getter/java.rs 2
+big-code-analysis-ast/src/getter/java.rs 5
 big-code-analysis-ast/src/getter/kotlin.rs 4
 big-code-analysis-ast/src/getter/lua.rs 3
 big-code-analysis-ast/src/getter/mozcpp.rs 5

AGENTS.md

@@ -468,8 +468,9 @@ exact form is `Node::parent`'s `O(depth)` per node and made every
 debug-build walk quadratic (#1122). The approximation misses a chain
 that is short by exactly one, so run this around any change to a walk's
 truncate/push bookkeeping — `src/spaces/compute.rs`, `src/ops.rs`,
-`src/suppression.rs`, and in `big-code-analysis-ast`, `comment_rm.rs` and
-`Search::act_on_node`. It is
+`src/suppression.rs`, and in `big-code-analysis-ast`, `comment_rm.rs`,
+`Search::act_on_node` (which `find` and `count` walk through) and the
+dump walk's `build` in `ast.rs`. It is
 not part of `make pre-commit`; the `chain-audit` CI job runs it per PR.
 See [Benchmarking](docs/development/benchmarking.md#chain-audit).
 

CHANGELOG.md

@@ -121,6 +121,213 @@ for historical reference.
 
 ### Fixed
 
+- **A `::`-qualified Tcl or iRules command now resolves to the core
+  command it names** in every metric, not just the ones reading the
+  braced-word slot table. `::switch` *is* `switch` — a leading `::` names
+  the global namespace, and inside a `namespace eval` body it is the
+  spelling that guarantees the core command over a shadowing proc — and
+  `Getter::command_leading_word` stripped it, so Halstead, `bca find
+  --type string` and the AST dump read the qualified form correctly. The
+  four metrics that resolve a leading word *without* that table did not:
+  `::switch` and `::for` contributed nothing to `cognitive` or
+  `cyclomatic`, `::incr` / `::append` / `::lappend` counted as an ABC
+  branch rather than an assignment, and `::return` / `::error` /
+  `::throw` / `::exit` were not counted as exits. The two halves
+  therefore disagreed on identical bytes. The strip now lives in one
+  shared helper both sides call. iRules resolves its exit and mutator
+  names in its own walkers rather than through Tcl's, so those were
+  swept in the same change. Only the *leading* qualifier is stripped:
+  `ns::eval` is a different command living in `ns` and is still not
+  promoted. **Metric drift:** on Tcl and iRules sources that spell a core
+  command with a leading `::`, `cognitive`, `cyclomatic` and `nexits`
+  rise, and ABC moves one count per mutator command from `branches` to
+  `assignments`. No integration snapshot moves — the corpora contain no
+  Tcl.
+
+- **A grammar span reaching past end-of-input no longer counts as a line
+  of code** (#1398), so `loc.ploc` and `loc.cloc` can no longer exceed a
+  space's own row span. A childless zero-width recovery token placed one
+  row past the last physical row was inserted into PLOC verbatim, making
+  the arithmetic identity `blank = sloc − ploc − cloc` read as satisfied
+  (`blank` saturates at 0) while `ploc / sloc` exceeded 1. A sweep of 322
+  truncated fixtures across 23 languages found the shape in **ten** —
+  Bash, C, C++, Mozcpp, Objective-C, Elixir, Groovy, Lua, Perl and Ruby
+  — over four unrelated recovery shapes (heredoc, unterminated string
+  delimiter, unclosed bracket, dangling line continuation), so each
+  space's line sets are now clamped to its own span at finalization
+  rather than per arm. A Perl POD block that runs to end-of-file without
+  `=cut` — valid Perl that parses cleanly — reached `cloc > sloc` by a
+  different route: the node ends at column 0 of the row past the last
+  one, and the comment arms counted that raw end row. The same clamp
+  removes it. A clean parse is not evidence of an in-span tree in the
+  other direction either: Ruby's `x = <<~DOC` with an unterminated body
+  parses without an error node and still emits the phantom row.
+  **Metric drift:** `loc.ploc` / `loc.cloc` fall and `loc.blank` rises
+  on files whose tree reaches past their last row — mostly files a
+  grammar cannot fully parse, including at least one ordinary shell
+  script in the DeepSpeech corpus, and also Perl files whose last POD
+  block has no `=cut` (`cloc` −1, `blank` +1).
+
+- **ABC counts a bare numeric literal operand in Ruby, Elixir and Perl**
+  (#1379). All three are truthy-valued, so a number used as a bare `&&`
+  / `||` operand is a Fitzpatrick Rule 9 unary condition — but their
+  terminal-operand sets named `integer` alone (Ruby, Elixir) or no
+  numeric kind at all (Perl). Ruby scored `a && 1.0`, `a && 1r`,
+  `a && 2i` and `a && 1ri` one condition against `a && 1`'s two; Perl
+  scored even `$a && 1` one against `$a && $b`'s two, and `if (1)` zero
+  against Python's one for `if 1:`. Each set now names every member of
+  its grammar's numeric family — five sibling rules for Perl
+  (`integer`, `floating_point`, `scientific_notation`, `hexadecimal`,
+  `octal`), and `char` for Elixir, since `?a` is the codepoint 97.
+  Ruby's `rational` / `complex` wrap the numeral and the walker cannot
+  descend into a wrapper, so the wrapper is classified and `integer`
+  stays listed alongside it for the bare `1`. Lua, Tcl, iRules and the
+  JS family were measured and have no gap. PHP, Groovy and the C family
+  (C, C++, Mozcpp, Objective-C) carry the same defect and are tracked in
+  #1410. **Metric drift:** `abc.conditions` and `abc.magnitude` rise for
+  Perl files using any numeric literal, and Ruby or Elixir files using a
+  non-integer one (`1.0`, `1r`, `2i`, `?a`), as a bare `&&` / `||`
+  operand or (Perl, Ruby) an `if` predicate.
+
+- **A relational pattern no longer double-counts its operator against
+  the arm that owns it** in C# ABC (#1383). `x switch { > 5 => …, < 0
+  => … }` scored one condition per arm *plus* each arm's `>` / `<`,
+  giving 4 where the equivalent constant-pattern switch gives 2 and
+  where `cyclomatic() - 1` is 2; `if (x is > 0)` gave 2 against 1. The
+  enclosing switch arm and `is` condition slot already pay for the
+  decision, so the pattern's own operator is now excluded — matching how
+  a constant pattern is treated. The gate is on the operator's parent,
+  so a relational pattern no arm or condition slot owns scores 0 too —
+  `bool b = x is > 5;`, `return x is > 5;`, a lambda or expression body,
+  a `when` guard or `catch` filter — in step with `x is 5`, `x is int`
+  and cyclomatic but one below the equivalent `x > 5` (guards: #1422).
+  This covers `>=` and `<=` as well as
+  `>` and `<`: those are distinct token ids reaching a separate arm, so
+  the allowlist named in #1297 never saw them. **Metric drift:**
+  `abc.conditions` and `abc.magnitude` fall for every C# file using
+  relational patterns; `abc` is a gated threshold metric. Kotlin shares
+  the defect in `when { x > 5 -> }` and is tracked in #1421.
+
+- **Kotlin ABC counts a primary-constructor superclass call** (#1384).
+  `class Sub : Base(1, 2)` parses as a `constructor_invocation` under a
+  `delegation_specifier` — a third production, distinct from both
+  `call_expression` and the `constructor_delegation_call` #1279 added
+  for the secondary form — so the spelling most Kotlin actually uses
+  contributed nothing while `constructor(x) : super(x)` beside it scored
+  one. It is now one branch, as is an object expression's superclass
+  call (`object : Base(1) { }`). The arm is gated on its parent: the
+  grammar reuses `constructor_invocation` for an annotation's argument
+  list (`@Suppress("x")`, `@file:Suppress("x")`), which is not a
+  run-time call and stays at zero. A supertype with no argument list
+  (`class Sub : Marker`) is unaffected. **Metric drift:** Kotlin `abc`
+  rises by one per class or object expression passing arguments to its
+  supertype.
+
+- **A self-reference and a super-reference are Halstead operands in
+  Java, C# and Kotlin** (#1380), matching the eleven other languages
+  that classify one. A member access is `<receiver> <op> <field>`, so
+  billing the receiver as an operator scored `this.x` as a binary
+  operator with one operand where `p.x` is one operator with two. Two
+  declarator uses of the same token kinds stay operators behind a parent
+  gate: C#'s `indexer_declaration`, which names the member with `this`,
+  and Java's `? super T` wildcard bound, the mirror of `? extends T`.
+  Kotlin's label-qualified `this@` / `super@` were previously classified
+  as *nothing* and are now operands. PHP's `self::` / `parent::` are
+  unchanged — those are scope-resolution class references, not instance
+  references. **Metric drift:** `n1` / `N1` fall by one occurrence per
+  self/super reference and `n2` / `N2` rise by the same, carrying
+  `volume`, `difficulty`, `effort`, `time`, `bugs` and all three
+  maintainability-index variants with them.
+
+- **PHP heredoc, nowdoc and backtick literals credit every row they
+  span to `ploc`** (#1396), as the languages #778 and #1260 routed
+  already did; rows empty inside the literal had been counted as
+  `blank`. #778 recorded PHP as already correct; it was not, for the one
+  shape that release never measured. Nowdoc and backtick also lost
+  non-empty rows, each for its own reason: a nowdoc body is one
+  `nowdoc_string` per line, each starting at the end of the row before,
+  so its last body row was credited to nothing; a multi-row backtick
+  command is a single node, so every interior row was. The wrapper is
+  routed rather than the body, because a heredoc whose body is a single
+  empty row emits no body node at all. The sweep stopped at PHP:
+  measured on this branch, Ruby's backtick subshell — the exact twin of
+  the PHP shape fixed here — and `%w[…]`, Tcl and iRules braced values
+  (`set x {a\n\nb}`), and C# interpolated strings all still lose such
+  rows, each reporting `ploc 2, blank 1` of `sloc 3`.
+  **Metric drift:** `loc.ploc` rises and `loc.blank` falls for PHP files
+  containing these literals — by one for every nowdoc, empty rows or
+  not.
+
+- **Ruby `npm` no longer counts `initialize`, `initialize_copy`,
+  `initialize_dup`, `initialize_clone` or `respond_to_missing?` as
+  public methods** (#1400). Ruby privatises all five at definition, so
+  any class with a constructor reported one public method too many.
+  `nm` is unchanged — they are still methods; only the public split
+  moves. An explicit `public :initialize` or `public def initialize`
+  still counts as public, and a `class << self` singleton `initialize`
+  stays public, because the rule is instance-only. **Metric drift:**
+  `npm` falls by one for most Ruby classes.
+
+- **`bca find --type string` and `bca count --type string` no longer
+  report a Tcl or iRules script body as a string literal** (#1381) — a
+  `proc` or `if` body, or an iRules `when` handler. The withdrawal
+  covers every braced word the grammar hangs off a *modelled* construct
+  rather than off a generic command's argument list, so a braced
+  **condition** goes with it: `if {$x > 1}`, `while {$y}`,
+  `expr {$a + $b}` and `catch {…}` are no longer reported either. A
+  Tcl condition is an `expr` script, so that is the same call as the
+  body, but it is a wider change than "bodies". A braced *value* is
+  still reported: `lappend x {a b}`, a braced `proc` name, the arguments
+  of a `namespace` subcommand other than `eval` / `inscope` / `code`, and
+  the pattern and variable list of an `on` / `trap` handler clause the
+  grammar leaves as a plain command. `Checker::is_string`
+  could not tell the two apart because it received neither the source
+  bytes nor the ancestor chain; it gains an `is_string_with_code`
+  sibling that routes both dialects through the role predicate #1318
+  built. The `Ast::dump` API and the REST `/ast` endpoint likewise keep
+  a script body's children instead of flattening it to a single leaf,
+  which had been dropping entire `proc` and `when` bodies. (The `bca
+  dump` subcommand prints the raw tree-sitter tree and never consulted
+  the alterator, so it is unaffected.) Recognition is a leading-word
+  heuristic, so Tcl's subcommand-dispatched script takers — `dict for`,
+  `interp eval`, `apply` — are still reported; iRules models its
+  handlers structurally and has no such gap. The reverse misses remain
+  too: a braced value that `after cancel` or the separate-argument form
+  of `switch` takes, or that sits in a multi-line `try … trap` clause
+  the Tcl grammar leaves inside an error node, is still treated as a
+  script.
+
+- **A `.mailmap` edit invalidates the persistent VCS history cache**
+  (#1262). Author identities are canonicalised through the repository
+  mailmap at walk time and stored in the cached event log as digests,
+  but neither the entry key (`head_sha`) nor the options fingerprint
+  observed the mailmap — so `bca vcs` served stale `authors_long`,
+  `ownership_top_share`, bus-factor and `risk_score` values after any
+  mailmap change. Worse, the incremental splice re-persisted the
+  pre-edit digests under each new head, so the divergence survived
+  `HEAD` moving and only `--clear-cache` cleared it. A digest of the
+  repository's effective mailmap now feeds `cache::fingerprint`,
+  covering the pure hit, the splice's ancestor selection and the
+  persisted entry at once. No `CACHE_SCHEMA_VERSION` bump is needed:
+  pre-fix entries simply fingerprint differently and cost one cold
+  walk. The digest hashes gix's *merged* mailmap snapshot rather than
+  re-deriving the four conditional sources `open_mailmap` consults, so
+  a source cannot be missed.
+
+- **`bca.to_sarif` emits findings in the same order as `bca check -O
+  sarif`** (#1402), so the two documents can be compared positionally
+  and not just as sets — which is what the binding's own parity claim
+  had been promising. The binding now sorts its findings as `bca check`
+  does after its walk — by path, then start line, then metric name —
+  with ties kept in depth-first source order. It used to emit them in
+  walk order, and its walk pushed each sibling set onto a LIFO stack in
+  source order, so every sibling set came out reversed; a space
+  breaching several metrics followed the caller's `thresholds` dict
+  order; and files followed the input iterable rather than the CLI's
+  path order. The comparison is against `bca check --no-suppress`:
+  `to_sarif` still applies no in-source suppression markers, baseline
+  or `[check] exclude` globs.
+
 - **`bca preproc` documents are byte-identical across runs** (#1304).
   `PreprocResults.files` and `PreprocFile`'s three `HashSet<String>`
   fields serialized straight off hash order, so an unchanged tree
@@ -142,8 +349,11 @@ for historical reference.
   declared name, a Kotlin qualified super call (`super<A>.g()`), and
   Perl's `<FH>` / `<$fh>` readlines each scored phantom conditions.
   Perl was listed as immune by the original survey and was not. C#
-  additionally keeps counting a relational pattern's operator
-  (`x is > 0`), a genuine comparison outside `binary_expression`.
+  initially kept counting a relational pattern's operator
+  (`x is > 0`) on the grounds that it is a genuine comparison outside
+  `binary_expression`; #1383 above reverses that, because the switch
+  arm or `is` condition owning the pattern already pays for the
+  decision.
   Elixir was swept the same way in the same release: an operator
   *named* rather than applied — the capture `&</2`, the qualified call
   `Kernel.<(a, b)` — puts a bare `<` under an `operator_identifier`,

Cargo.toml

@@ -121,8 +121,8 @@ tempfile = "^3.0"
 missing_docs = "warn"
 # `chain_audit` turns `Ancestors::checked` back into the exact
 # parent-identity assertion it was before #1122. Off by default because
-# that assertion costs `Node::parent`'s `O(depth)` per node on all five
-# checked walks; `make chain-audit` and the CI lane of the same name set
+# that assertion costs `Node::parent`'s `O(depth)` per node on every
+# checked walk; `make chain-audit` and the CI lane of the same name set
 # it. Declared here so `-D warnings` does not reject the `#[cfg]`.
 unexpected_cfgs = { level = "warn", check-cfg = ['cfg(chain_audit)'] }
 

Makefile

@@ -322,10 +322,11 @@ test-doc:
 # triples the lib suite's wall time. Runs in the `chain-audit` CI lane;
 # run it locally around any change to a walk's truncate/push bookkeeping.
 #
-# Library-scoped, matching that lane: the five walks that thread a chain
-# live in the root crate (`spaces::compute`, `ops`, `suppression`) and
-# in `big-code-analysis-ast` (`comment_rm`, `Search::act_on_node`), so
-# the CLI / web / integration tiers would re-pay the quadratic cost
+# Library-scoped, matching that lane: the walks that thread a chain live
+# in the root crate (`spaces::compute`, `ops`, `suppression`) and in
+# `big-code-analysis-ast` (`comment_rm`, `Search::act_on_node` — which
+# `find` and `count` also walk through — and the `Ast` dump's `build`),
+# so the CLI / web / integration tiers would re-pay the quadratic cost
 # without reaching an assertion the lib tests do not already reach.
 #
 # RUSTFLAGS rather than a Cargo feature on purpose: `make test` passes