fix: ten metric and tooling bugs (batch 2026-09-10) - #1424
Merged
Merged
Conversation
This was referenced Sep 11, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1424 +/- ##
==========================================
+ Coverage 97.96% 97.97% +0.01%
==========================================
Files 358 359 +1
Lines 91516 92971 +1455
Branches 91085 92540 +1455
==========================================
+ Hits 89650 91090 +1440
- Misses 1201 1214 +13
- Partials 665 667 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
dekobon
force-pushed
the
fix/batch-2026-09-10
branch
2 times, most recently
from
September 11, 2026 20:02
24ece9f to
9977d97
Compare
dekobon
commented
Sep 12, 2026
| kinds: &BracedWordKinds, | ||
| ) -> bool { | ||
| node.kind_id() == kinds.script | ||
| && !Self::is_value_braced_word(node, code, ancestors, kinds) |
Owner
Author
There was a problem hiding this comment.
Location: big-code-analysis-ast/src/getter.rs:803
MEDIUM -- Do not classify every braced argument of a script-taking command as script
is_value_braced_word decides this by command name alone, so reusing its negation here makes every braced_word passed to a name in SCRIPT_TAKING_COMMANDS a script. Several documented signatures mix value and script slots. For example, Tcl 8.6 defines time script ?count? and after ms / after ms ?script ...?, but at this head:
$ bca find --no-config -t string time.tcl # time {set x 1} {5}
<no matches>
$ bca count --no-config -t string time.tcl
Found nodes: 0
{5} is the literal count value. Likewise after {100} returns zero strings even though its sole argument is the millisecond value, and bca dump expands {100} into a nested command instead of preserving the literal leaf. The same reproduces with .irule inputs. switch {foo} {...} also loses its subject, and namespace eval {foo bar} {...} loses the namespace name because the namespace helper classifies all arguments of eval/inscope/code together.
Before this PR, those values were still returned as strings and flattened in the dump. The new Checker::is_string_with_code and Alterator::keeps_children call sites expose this coarse Halstead heuristic as public find/count and AST behavior. This predicate needs argument-position handling for each mixed signature (at minimum after, time, switch, uplevel, and the namespace forms), with value/script tests for both Tcl and iRules.
Owner
Author
There was a problem hiding this comment.
Fixed in 21230d4a (plus test coverage in c09c5650). All four repros you listed now return the value as a string and keep it a flat leaf in Ast::dump, in both dialects.
is_braced_literal_slot now reads one per-command argument-slot table instead of answering on/trap and namespace ad hoc. Each SCRIPT_TAKING_COMMANDS entry pairs a Tcl 8.6 signature with a variant saying which arguments the interpreter evaluates (Every / NoneOfThem / EveryButFirst / Only(n) / Last / EveryButLeadingLevel / SwitchArms), with the man-page citation beside each row. The rule lives in a new lang_helpers::tcl_family module, next to the two dialects' kind tables — getter.rs was at 1140 loc.sloc against a 1300 limit and could not absorb 240 lines.
Four things turned up that your comment and my initial reading both had wrong:
namespace inscope ns script ?arg …?— only the second argument is the script. Trailing arguments are list elements appended to it, so they are values. Encoded asOnly(1), not "name is a value, rest are scripts".switchoption arity — "first argument that is not a leading-option" stops on the varName of-matchvar varName/-indexvar varNameand mistakes it for the subject. The scan steps over those two options' operands.switch's flat spelling has the same bug as its subject —switch $v {^a} {puts 1} {^b} {puts 2}pairs by index with no marker, so patterns at odd offsets from the subject are values too. The braced arm-list form is unaffected and its interior is still owned byis_switch_arm.switchis Tcl-only here. The iRules grammar rejects a braced subject:switch {foo} {…}putsswitchunder anERRORand the subject becomes the recovered command's name, which the pre-existing "braced word directly under acommand" rule already reports as a string. Verified by dump.
One accepted imprecision, documented at the table: after cancel {…} / after info {…} identify a pending script by its text, so EveryButFirst calls them scripts. Distinguishing them needs the subcommand dispatch the table's own rationale rules out, and the text is real code either way.
On the bca dump half of your report — that one is misattributed. bca dump is the raw tree walk and flattens nothing; lappend x {a b}, a value that has always been flattened, shows nested children there too. The alterator flattening is Ast::dump and the /ast endpoint, which is what the fix restores and what the new alterator_string_flattening rows cover.
Halstead is deliberately unchanged: braced_word_op_type still keys on the command name alone, so bca ops on time {set x 1} {5} still bills a {}. Moving the rule into the shared predicate changes bca metrics for Tcl and wants its own measured change.
Performance. The subject scan stops at the first argument that is not a plain option word, and a braced word never is one, so its length is the leading-option count rather than the argument count. On one-line switches of 500–4000 arms (release, bca count --type string): 3.2 → 3.7 ms at 500 and 15.0 → 19.3 ms at 4000, linear both sides. A deliberately quadratic variant was verified to trip the same ladder at 40 / 149 / 589 / 2359 ms, so the probe is sized to catch a regression.
Tests. A verdict test asserting exact kept / withdrawn vectors — count and content, both polarities, every line carrying a value and a script — 13 Tcl lines and 7 iRules, plus 9 dump rows, all gated on the feature union with a non-vacuity guard. 27 semantics perturbations plus 11 coverage perturbations, each caught by exactly the intended test.
Owner
Author
Code Review: fix: ten metric and tooling bugs (batch 2026-09-10)Verdict: REQUEST CHANGES
Findings
Review passes executed
Files reviewed: 70. Focused validation included Tcl/iRules GitHub does not allow the PR author to submit a formal request-changes review on their own PR, so this summary records the verdict as a regular comment; all three findings are posted as inline code-review comments. |
dekobon
commented
Sep 12, 2026
| // Grammar accidents, as Groovy and Objective-C are: `self` is a | ||
| // plain `identifier` in both grammars. | ||
| LANG::Python => ( | ||
| "class A:\n def f(self):\n return self.x\n", |
Owner
Author
There was a problem hiding this comment.
Location: tests/parity/self_reference_operand_parity.rs:150
MEDIUM -- Isolate the value-form self that this parity test claims to guard
The Python fixture contains self twice: once as the parameter and once as the self.x receiver. The assertion only checks whether "self" appears anywhere in the deduplicated operand vocabulary, so replacing return self.x with return 0 leaves every_language_bills_a_self_reference_as_an_operand passing. That means the test cannot detect the grammar-drift scenario described above this row: if receiver self becomes a dedicated, unclassified kind while the parameter remains an identifier, the parameter still satisfies the test.
The Rust row at line 119 has the same masking shape (&self plus self.x) and also passes after removing the value-form occurrence. Please make the receiver occurrence independently observable, for example by using a syntactically valid fixture whose declaration does not contribute the same operand spelling where possible, or by adding a structural/count assertion that distinguishes the receiver token from the declaration. As written, this new cross-language guard overstates the coverage it provides.
Owner
Author
There was a problem hiding this comment.
Fixed in e4236699. You were right on both rows, and I confirmed the masking before changing anything.
Ops::operands is a deduplicated vocabulary, so in Python the def f(self) parameter and in Rust the &self receiver-declaration each satisfy the assertion on their own. Measured: replacing return self.x with return 0 leaves Python's n2 at 4 with self still in the vocabulary, and the Rust row behaves the same way. The grammar-drift scenario the rows exist to catch — a receiver promoted to a dedicated unclassified kind while the declaration stays an identifier — would not have failed either.
N2 is the axis that can see the second occurrence, so each masked row now carries a receiver-stripped variant (return self.x → return x; self.x → x, both of which parse clean) and the test asserts the receiver contributes exactly one operand occurrence:
assert_eq!(with, without + 1, "…the receiver must contribute exactly one operand occurrence…");
Verified by deleting the receiver from each fixture in turn — the new assertion is the only failure, and it names the numbers:
- Python:
N2 was 4 with it and 4 without - Rust:
N2 was 6 with it and 6 without
Both passed silently before this change.
I kept the existing membership and "not an operator" assertions, since those are what pin the #1380 contract; the delta assertion is additive. The other rows spell the keyword only in receiver position (Groovy, Ruby, C++, Objective-C, PHP, Lua, and the JS/Java/C#/Kotlin rows), so receiver_stripped returns None for them and they are unaffected — that reasoning is recorded on the function.
dekobon
commented
Sep 12, 2026
| //! this binding compares raw metric values, so it applies none of the | ||
| //! in-source suppression markers `bca check` honours by default (a marked | ||
| //! space keeps its `suppressed` key for a caller that wants to filter), | ||
| //! no baseline, and no `[check] exclude` globs. |
Owner
Author
There was a problem hiding this comment.
Location: big-code-analysis-py/src/sarif.rs:103
LOW -- Qualify positional CLI parity for duplicate input paths
The new parity contract omits one input-shape difference: the CLI deduplicates repeated path seeds through SeedSet::seen (big-code-analysis-cli/src/walk.rs), while analyze_batch deliberately preserves one result per input and collect_offenders_from_iter appends every result dict. With one violating file, bca check -p a.py -p a.py ... -O sarif emits one finding, but to_sarif(analyze_batch([a, a]), ...) emits two identical findings.
Deduplicating here would be unsafe because two distinct analyze_source results may intentionally share the same name; the appropriate fix is to state that entry-for-entry parity requires a unique file set, alongside the suppression/baseline/exclude caveats. The same qualification should be applied to the new _native.pyi and book wording.
Owner
Author
There was a problem hiding this comment.
Fixed in e4236699 — documented rather than deduplicated, for the reason you gave.
Confirmed in the code first: the CLI folds repeated seeds in SeedSet::seen (big-code-analysis-cli/src/walk.rs), where push_explicit returns false on a duplicate and push_walked only inserts when new, while collect_offenders_from_iter appends every entry it is handed with no dedup. So bca check -p a.py -p a.py -O sarif emits one finding and to_sarif(analyze_batch([a, a]), …) emits two.
Deduplicating in the binding would be wrong exactly as you say — analyze_source takes the caller's name, so two distinct results may legitimately share one and the binding cannot tell a repeated path from two analyses of the same name. The contract now states that entry-for-entry parity assumes a unique file set, alongside the existing suppression / baseline / exclude caveats, in all three places that claim the parity:
- the
sarif.rsmodule doc, next to the--no-suppressparagraph - the
to_sarifdocstring in_native.pyi big-code-analysis-book/src/python/sarif.md
Each states the asymmetry, gives the [a, a] example, and says why deduplicating here is not the fix.
dekobon
added a commit
that referenced
this pull request
Sep 12, 2026
Two review findings on PR #1424. The self-reference parity test asserted membership in `Ops::operands`, which is a *deduplicated* vocabulary. Python's `def f(self)` parameter and Rust's `&self` spell the keyword in the declaration, so those two rows passed on the declaration alone: deleting `return self.x` left both green, and the grammar drift the rows exist to catch -- a receiver promoted to its own unclassified kind -- would not have failed them. N2 is the axis that can see the second occurrence. Each masked row now carries a receiver-stripped variant and asserts the receiver contributes exactly one operand occurrence. Verified by deleting the receiver from each fixture in turn: Python fails with N2 4 and 4, Rust with 6 and 6. Also documents a qualification the SARIF parity contract omitted. The CLI folds repeated path seeds together in `SeedSet::seen`, while `analyze_batch` returns one result per input and `collect_offenders_from_iter` renders every one, so `to_sarif(analyze_batch([a, a]))` emits each finding twice where `bca check -p a.py -p a.py` emits it once. Deduplicating in the binding would be wrong, because two distinct results may legitimately share a name, so the contract now states that entry-for-entry parity assumes a unique file set. Recorded in all three places that claim the parity: the `sarif.rs` module doc, the `to_sarif` stub, and the book.
dekobon
added a commit
that referenced
this pull request
Sep 12, 2026
Two review findings on PR #1424. The self-reference parity test asserted membership in `Ops::operands`, which is a *deduplicated* vocabulary. Python's `def f(self)` parameter and Rust's `&self` spell the keyword in the declaration, so those two rows passed on the declaration alone: deleting `return self.x` left both green, and the grammar drift the rows exist to catch -- a receiver promoted to its own unclassified kind -- would not have failed them. N2 is the axis that can see the second occurrence. Each masked row now carries a receiver-stripped variant and asserts the receiver contributes exactly one operand occurrence. Verified by deleting the receiver from each fixture in turn: Python fails with N2 4 and 4, Rust with 6 and 6. Also documents a qualification the SARIF parity contract omitted. The CLI folds repeated path seeds together in `SeedSet::seen`, while `analyze_batch` returns one result per input and `collect_offenders_from_iter` renders every one, so `to_sarif(analyze_batch([a, a]))` emits each finding twice where `bca check -p a.py -p a.py` emits it once. Deduplicating in the binding would be wrong, because two distinct results may legitimately share a name, so the contract now states that entry-for-entry parity assumes a unique file set. Recorded in all three places that claim the parity: the `sarif.rs` module doc, the `to_sarif` stub, and the book.
dekobon
force-pushed
the
fix/batch-2026-09-10
branch
from
September 12, 2026 14:57
c09c565 to
1d6df06
Compare
fix(py): emit SARIF findings in the CLI's walk order `push_child_spaces` pushed a space's children onto the walk stack in source order while `collect_offenders` pops from the end, so every sibling set was visited in reverse, at every level. `bca.to_sarif` and `bca check -O sarif` therefore produced the same finding set in different sequences, against the byte-equivalence the module doc, the `_native.pyi` docstring and the book page all claim. Record the stack length before pushing and reverse only the tail this call appended, the way the CLI's `evaluate_with_policy` pushes `spaces.iter().rev()`. Reversing the appended tail rather than the input keeps the walk working for any Python iterable — `spaces` need not be a sequence — and allocates nothing extra. Emission order is now stated as part of the contract in all three places that claim parity, and pinned by two tests: a positional, unsorted comparison of the two documents on a fixture with three nested sibling pairs, and a hand-built-dict test covering the shapes the child walk tolerates but `analyze` never produces (a skipped non-dict child, a missing `spaces` key, an empty `spaces`). The existing parity helpers keep their sorts — they serve callers whose claim is set membership — but `_sarif_rows` is now a wrapper over a new unsorted extractor so the SARIF location traversal keeps one spelling. Fixes #1402 fix(py): order one space's SARIF findings by metric name Review of the sibling-order fix turned up the second half of the same divergence. The CLI builds its threshold entries by iterating a `BTreeMap`, so a space breaching several metrics reports them alphabetically by canonical name; the binding iterated the `thresholds` `PyDict` and so reported them in the caller's insertion order. `{"nargs": 1, "cyclomatic": 1}` came out `nargs` first against the CLI's `cyclomatic` first, and the same call spelled the other way round changed the document. Sort the resolved thresholds by name, which reproduces the CLI's order and drops the dependency on how the caller spelled the dict. Also qualify the parity claim where it was overstated: order *between* files is the caller's, since `to_sarif` follows the iterable it is handed while `bca check` follows its own resolved walk list. `_cli_check_sarif` now accepts a sequence of `--threshold` specs so the multi-metric shape is reachable from the harness at all. Fixes #1402 test(py): make the SARIF ordering tests able to fail An audit pass found three ways the new ordering tests could decay into passing against the unfixed binding, and one dead assertion. `test_to_sarif_orders_one_spaces_metrics_alphabetically` rested on its limits dict being spelled reverse-alphabetically, which nothing asserted — spelled the other way round it passes with the sort deleted. It also could not tell "alphabetical by metric name" from "emitted in `METRIC_FIELDS` declaration order", since `cyclomatic`/`nargs` sits the same way round under both; a binding iterating `METRIC_FIELDS` passed. It now loops over two pairs, adding `abc`/`cognitive`, which separates the two rules, and asserts the non-alphabetical spelling instead of describing it in a docstring. `test_to_sarif_child_order_survives_skipped_and_childless_spaces` named two tolerated shapes that are invisible in its asserted rows, so trimming the non-dict children or giving the childless space an empty `spaces` list left it green with nothing left to skip. Both are now asserted present. The `assert py_rows == cli_rows` in `test_to_sarif_emits_results_in_cli_walk_order` could not fail: the two lines above it already compare both operands against the same literal. Fixes #1402 Squashed from 9dc5ec6, e7f8357, 4035673.
fix(vcs/cache): invalidate the history cache on mailmap change Author identities are canonicalised through the repository `.mailmap` at walk time and stored in the event log as digests, but neither the entry key (`head_sha`) nor `cache::fingerprint` observed the mailmap. An edit therefore served stale author statistics — and the incremental splice re-persisted the pre-edit digests under each new head, so the divergence survived HEAD moving until `--clear-cache`. Fold a digest of the repository's effective mailmap into `cache::fingerprint`. That covers all three cache paths at once (the pure hit, `load_compatible`'s ancestor selection for the splice, and the persisted entry) and needs no `CACHE_SCHEMA_VERSION` bump: every pre-fix entry simply fingerprints differently and costs one cold walk. `repo::mailmap_digest` hashes gix's *merged snapshot* rather than the raw source bytes. Re-deriving the four conditional sources `open_mailmap` consults would be a coverage claim nothing checks, and a source missed — or added by a future gix release, the dependency being caret-ranged — silently reproduces this bug for that source. Also document the sibling blind spot the issue flags: the fingerprint hashes an option's *value*, never the behaviour it selects, so a change to how `BotFilter` matches its pattern belongs to `CACHE_SCHEMA_VERSION` instead (as #1265 already did). Fixes #1262 test(vcs/cache): pin the mailmap digest's design claims Review and test-audit findings on the #1262 fix. The regression tests covered the bug; these cover the reasoning the fix rests on. - `mailmap.file` invalidation. The digest hashes gix's merged snapshot so a source cannot be missed, but every test wrote the working-tree `.mailmap` — the one source a naive byte digest also covers. Verified: re-implementing the digest as `fs::read` of the work-tree file left the whole suite green before this test, and fails it now. - A comment-only `.mailmap` edit still hits. This is the sole behavioural difference from a byte digest, and was asserted nowhere. - The splice test's "the persisted entry replays correctly" step could not tell a hit from a second cold walk — writing a mismatched fingerprint in `persist` left it green. It now empties the entries and proves the replay was served. - Cross-process fingerprint stability had no guard at all: the CLI's two-process test compared two runs' stdout, which matches whether or not the second hits. Mixing the pid into `fingerprint` now fails it. Pre-existing, but the mailmap term is a new way to break it. Also record the residual window the review found (#1409): the digest and the walk open the mailmap separately, so an edit landing between them mis-stamps an entry. Self-healing unless the mailmap is reverted before the next run; closing it means threading one snapshot through the walk, which is a larger change than this fix. Squashed from 702b5ac, 206cb83.
Kotlin ABC counted a secondary constructor's `: super(x)` delegation (`constructor_delegation_call`, #1279) but not the primary-constructor form `class Sub : Base(1, 2)`, which the grammar spells as a `constructor_invocation` under a `delegation_specifier`. Both invoke the superclass constructor at run time, so both are branches. The arm is gated on that parent. tree-sitter-kotlin-ng gives `constructor_invocation` exactly three parents, and two of them are annotations -- `@Suppress("x")` is `annotation > constructor_invocation` and `@file:Suppress("x")` is `file_annotation > constructor_invocation` -- so an ungated arm would bill every argument-carrying annotation in a Kotlin file as a branch. Gating positively on `delegation_specifier` rather than denying the two annotation kinds also keeps any future annotation-shaped parent at zero. `kotlin_super_type_argument_is_not_a_condition` used `class B : A()` as scaffolding and anchored on `branches_sum() == 1`; its comment already recorded that the `A()` header contributed none and named this issue. The anchor moves to 2 and the comment with it. Siblings checked, no change needed: Kotlin `nom`, `wmc` and cyclomatic reference neither constructor production, correctly -- a superclass call is not a method declaration nor a decision point. `is_call` stays `CallExpression` only, matching Java's `MethodInvocation` and C#'s invocation kinds. Fixes #1384
chore(self-scan): refresh baseline after wave 1 Two entries moved, both from fixes merged in this wave: - KotlinCode::compute cyclomatic 16 -> 18, from the gated ConstructorInvocation arm (#1384). - build_cached halstead.effort 73179.83 -> 74812.54, from the mailmap digest call (#1262). Refreshed with the headroom variant so the soft tier does not re-fire on untouched files. chore(self-scan): refresh baseline after wave 3 Four entries grew from the #1381 seam; no new offenders (241 -> 241). - alterator.rs loc.ploc 545 -> 561 - Alterator::get_ast_node nargs 6 -> 7 (the ancestor chain; branching at the call site instead measured +53% halstead on the dump walk) - ast.rs build halstead.effort 72581 -> 87299 - parser.rs Parser<T>::filters halstead.effort 58860 -> 85508 Squashed from aa5632e, 8a51243.
The terminal-bool operand sets that ABC's Fitzpatrick Rule 9 walker
consults named `integer` alone for Ruby and Elixir and no numeric kind
at all for Perl, so a bare numeric operand scored no condition:
Ruby `a && 1` 2, `a && 1.0` / `1r` / `2i` / `1ri` 1
Elixir `a && 1` 2, `a && 1.0` 1, `a && ?a` 1
Perl `$a && $b` 2, `$a && 1` 1, `1.0` / `1.5e10` / `0xff` 1,
`if (1)` 0
All three are truthy-valued languages, and Python has counted both
`Integer` and `Float` since #772, so the three disagreed with the
control for no recorded reason. The count drops rather than errors
because the walker recurses one level past an unlisted kind and the
recursion then fails the parent list-kind gate.
The unit to sweep is the grammar's numeric *supertype*, not the alias
list. None of these kinds is aliased, so an alias sweep comes back
clean and proves nothing — the misses were sibling rules under a shared
hidden choice. Perl's `_numeric_literals` has five members and Elixir's
`char` is an integer codepoint, both easy to miss by checking only for
numeric suffixes. Ruby's `rational` / `complex` wrap the numeral (`1ri`
is `complex(rational(integer))`) and the walker cannot descend into a
wrapper, so the wrapper is what must be listed; `Integer` stays
alongside for the bare `1`.
The sweep also measured Lua, Tcl, iRules and the JS family (one
`Number` kind covering every spelling, no gap) and found PHP and Groovy
carrying the identical defect. Both have integration-corpus files, so
their fix moves snapshots and is deferred to #1410 with a FIXME anchor
at each set.
Fixes #1379
#778 routed PHP's quoted literals through add_multiline_string_ploc and excluded the heredoc, recording that its body "already reaches PLOC through its inner statement nodes". Half true: tree-sitter-php emits a body child only for a row that has text, so a row empty inside the literal held no node and blank = sloc - ploc - cloc claimed it. Nowdoc was worse than the report measured. On the issue's six-row fixture heredoc gave ploc 5, blank 1 while nowdoc gave ploc 4, blank 2, because its body is not one nowdoc_string per row: the grammar emits one for the first line and a single multi-row node for the rest, whose interior rows the catch-all's start-row insertion all lost. Route the heredoc / nowdoc wrapper rather than heredoc_body / nowdoc_body: a body of one empty row emits no body node at all, so the wrapper is the node present for every spelling. Review found the backtick shell_command_expression carrying the nowdoc shape exactly, so it is routed too - the arm now agrees with PhpCode::is_string on every kind that grammar can span rows with. Fixes #1396
Ruby privatises `initialize`, `initialize_copy`, `initialize_dup`, `initialize_clone` and `respond_to_missing?` at definition, so essentially every Ruby class with a constructor reported `npm` one higher than `instance_methods(false)`. The rule is applied in `RubyClassBody::declare`, where the name is already read, rather than at the tally. #1255's retroactive refile pass runs afterwards over the recorded declarations, so an explicit `public :initialize` republishes the method with no extra code. Four edges of the rule were measured against ruby 3.0.2 rather than inferred, and each is pinned by a test: - it outranks the body-wide flag, so an explicit `public` marker above `def initialize` still yields a private method; - it loses to a keyword naming the declaration directly, so `public def initialize` is public; - it is *not* overridden by a keyword naming the other method family: `public_class_method def initialize` leaves the instance method private; - it is instance-only, so `def self.initialize` and a `def initialize` inside `class << self` both stay public. The last needs the enclosing body's kind, since that declaration is an ordinary `Method` node. `nm` is unchanged throughout — only the public/private split moves. `RubyVisibilityCall::governs` names the "does this keyword decide the visibility of that declaration" test that `ruby_wrapped_is_public` already applied, so `Npm` can ask it without restating the rule. Fixes #1400
`braced_word` is both the literal of `lappend x {a b}` and the script of
a `proc` body or an iRules `when` handler, and `Checker::is_string` is a
kind table that cannot separate them. So `bca find --type string`
reported every script body in the file as a string literal, and
`Alterator::alterate` flattened one into a single leaf, dropping the
whole body from the AST dump and the REST `/ast` endpoint.
#1318 built the predicate that settles this but could not apply it:
`is_string` takes neither `code` nor `ancestors`.
Add `Checker::is_string_with_code`, a `_with_code` sibling with a
forwarding default, and an `Alterator::keeps_children` veto for the
dump. Both resolve the role through one shared
`Getter::is_braced_script_word` over one kinds table per dialect,
hoisted into `lang_helpers` where the three classifiers that ask it can
share it (grammar-dispatch §7).
Both questions are about an enclosing command, so both need an ancestor
chain, and `Node::parent` is `O(depth)` — asking it per node made the
walks quadratic in nesting depth. `find`, `count` and the dump walk now
thread the chain they already had the information to build, which is
what `parser.rs` had recorded as the fix since #1162. On 8 KB of nested
braces, measured against an unknown chain: `count --type string`
7.4 ms against 434 ms, `Ast::dump` 10 ms against 418 ms, and both
growth curves linear rather than quadratic.
`Filter::any` and `Alterator::get_ast_node` take the chain as a new
argument. Neither is covered by the stability contract — STABILITY.md
places the whole `big-code-analysis-ast` crate outside it — and the
root crate re-exports neither.
Fixes #1381
Java, C# and Kotlin swept `this` / `super` / `base` into their operator arm under a `// Operator: … keywords` heading, while the eleven other languages that classify a self-reference call it an operand. A member access is `<receiver> <op> <field>`, so billing the receiver as an operator made `this.x` a binary operator with one operand where `p.x` is one operator with two, and scored the same source differently on each side of a translation between two of these languages. Two uses of the same token kinds are not references and keep the operator classification, each behind a parent gate: C#'s `indexer_declaration` names the member with the `this` keyword, and Java's `? super String` wildcard bound mirrors `? extends String`, whose `extends` this same match already bills as an operator. C#'s extension-method receiver needs no gate — it is a childless `modifier` node, not the `this` kind at all, and remains unclassified as it was. Java's own declarator use goes the other way: the explicit receiver parameter `void m(T T.this)` is a parameter *name*, and parameter names are operands here. Each site now cross-references the other so the two opposite calls read as one decision. Kotlin's labelled spellings were not a reclassification at all. `ThisAT` / `SuperAT` were in neither arm before, so `this@Outer` and `super@Inner` contributed nothing — the silent-drop shape of #1361 rather than an operator-to-operand move — and their `n2` / `N2` rise with no offsetting `n1` / `N1` fall. The leaves are the keepers rather than the `this_expression` / `super_expression` wrappers, because a `constructor_delegation_call` emits a bare leaf with no wrapper at all. PHP's `Zelf` / `Parent` are untouched: `self::` / `parent::` are class references in scope-resolution position, a different construct from `$this`, which is a `variable_name` and already an operand. Fixes #1380
A grammar hands back spans it cannot honour: an unterminated Bash heredoc gets a zero-width `heredoc_end` at (3, 1) of a two-row file, an unterminated Elixir, Lua or Groovy string gets its closing delimiter the same way. Such a token is childless, so it reaches the leaf branch of its language's catch-all arm, which inserts a raw start row into PLOC -- and there that start row is itself the phantom. `Node::end_line` already encodes "a node whose end column is 0 does not occupy the row it ends on"; nothing encoded the same rule for a node that begins past the span. The result is `ploc > sloc`, a contract violation rather than a rounding artifact: `Stats::blank` saturates at 0, so the clamp there hides it while a consumer computing `ploc / sloc` gets a ratio above 1. A sweep of 322 truncated fixtures across all twenty-three languages found ten reproducing it -- Bash, C, C++, Mozcpp, Objective-C, Elixir, Groovy, Lua, Perl and Ruby -- over four unrelated recovery shapes, plus a matching `cloc > sloc` on an unterminated Perl POD block. Ten per-arm skips would have been ten edits that still could not cover a grammar whose recovery shape nobody sampled, so the rule lives once, in `Stats::clamp_line_sets_to_span`, keyed on the span every language already reports and run per space at finalization. Per space rather than per unit: an unterminated Elixir `do` block violated the contract on its own `defmodule` space as well as on the file. A `debug_assert!` on the same path pins the invariant for every space of every walk -- 821 workspace tests reach it -- rather than only for the fixtures the regression tests name. It is deliberately stated against the row span and not against `sloc()`: `Sloc::exclude_span` counts a pruned span whole, including a row a retained sibling shares, so `sloc()` can still fall below `ploc` under `--exclude-tests`. That is a separate defect with a separate cause, filed as #1417. Two things the review turned up that the fix's first draft got wrong, both now recorded where they will be read: - "no parsed space records a row outside its span, measured over the corpora" was a load-bearing claim in `line_set.rs`, and it was false. `DeepSpeech/parse_valgrind_suppressions.sh` leaves a MISSING `}` at (58, 1) of a 57-row file; its `ploc` drops 36 -> 35 here. No snapshot covers it -- `snapshots/` holds only the C-family files -- so a green integration run is not evidence this change has no effect on real trees. - A clean parse is not evidence of an in-span tree. Ruby's `x = <<~DOC\na\n` parses with no error node at all and still yields the phantom row, so the regression table anchors each fixture on a node past the last row rather than on `has_error()`. `make bench-scaling`: all 27 probes within bound, the five `loc/*` at exponents 0.64-1.10 against 1.50. `make chain-audit` clean. Fixes #1398
A C# `relational_pattern`'s comparison operator scored an ABC condition
of its own, on top of the `switch_expression_arm` or `if` condition slot
that owns it. A relational arm therefore scored twice what the equivalent
constant arm scores, and twice C#'s own cyclomatic decision count.
Both spellings are fixed, because the pattern can use any of the four
relational tokens and they reach two different arms: `<` / `>` lose their
`RelationalPattern` entry in the parent allowlist, and `<=` / `>=` gain a
`RelationalPattern` denial they never had. Fixing only the first would
have left `x switch { >= 10 => … }` double-counted while
`x switch { > 10 => … }` was not.
This changes a published metric for any C# file using pattern matching.
Over the two C# corpora (27 files, 291 spaces) the aggregate falls from
369 to 348 conditions; the DeepSpeech corpus predates C# patterns and is
unchanged, so all of it lands on one snapshot fixture, whose `Bucket`
goes 13 -> 6.
The gate is on the operator's parent, not on what encloses the pattern,
so a relational pattern outside any decision slot (`bool b = x is > 5;`)
drops from 1 to 0 as well. That puts it in agreement with the plain type
test `x is int` and with cyclomatic, where it previously disagreed with
both; the price is that the equivalent `x > 5` still scores 1 in the same
slot. Pinned by `csharp_relational_pattern_outside_a_decision_slot_scores_zero`.
Two adjacent divergences are measured and deliberately left alone. An
operator inside a `when` guard keeps counting, so ABC can still exceed
`cyclomatic() - 1` on a guarded arm — but not because either metric
models guards: neither does, and a call-shaped guard (`when IsEven(x)`)
is at parity, so the count follows the guard's spelling (#1422). And the
`<=` / `>=` / `==` / `!=` operator overloads still score a spurious
condition, which is residue of #1297 rather than of this fix (#1420),
marked with a FIXME at the arm.
The book's C# row said the opposite of what the code now does, so it is
corrected here rather than left to drift; the `## [Unreleased]` #1297
entry needs the same correction and is left to the batch's changelog
commit.
Fixes #1383
`Parser::filters` falls back to a match-all predicate when no arm pushed one; a Filter holding no predicates would make Filter::any return false for every node, so a bare `bca find` / `bca count` would silently report zero. Nothing exercised it. Deleting the fallback failed no test in the crate before this one, and fails only this one after. Asserting against the `all` keyword rather than a hand-counted total is what gives the test teeth: both paths push the identical closure, so dropping the fallback sends the empty request to 0 while `all` stays put.
Six items, none a behaviour change. Each was a statement in a comment, a changelog entry, or a test that did not describe what the code does. kind_sets.rs: the note added with #1379 said C and C++ name no numeric bool-terminal kind because "a bare number in a boolean slot is a compile error there". They are integer-truthy -- `if (1)` and `a && 1` are idiomatic -- and measure 1 condition where Python measures 4 for the same constructs. Worse, `if (true)` scores 1 and `if (1)` scores 0 inside one language. The set is shared by C, C++, Mozcpp and Objective-C; all four are now named on #1410 alongside PHP and Groovy, deferred for snapshot churn rather than because the rule does not apply. CHANGELOG: #1381 credited "the AST dump", but `bca dump` prints the raw tree-sitter tree and never consulted the alterator -- only `Ast::dump` and the REST `/ast` changed. The same entry read as if the Tcl script-body class were closed; recognition is a leading-word heuristic, so `dict for`, `interp eval` and `apply` are still reported. npm.rs: the comment said no fixture-decay anchor was available for the four tests whose expected answer is "the rule does not apply", so a rename of `initialize` would be silent. No *count* can anchor them, but the space tree can -- the Ruby walk names a space per `def`. `ruby_public_keyword_wrapping_initialize_wins` now carries that anchor, since it is the sole guard on the `named_by_keyword` disjunct; verified by renaming the fixture, which now fails only that test. halstead.rs: `csharp_indexer_declaration_keyword_is_not_a_self_reference` derived the exact role sequence in its comment and asserted two `contains` calls, leaving the length unpinned so a space could appear or vanish unnoticed. Asserts the sequence now. cache_tests.rs: dropped a comparison of a pure fixed-seed function to itself, keeping the note about where cross-process stability is actually covered. getter/groovy.rs: records that the `Super` operator arm never fires -- the grammar emits a plain identifier -- so Groovy agrees with #1380 by accident and would flip on a grammar bump. Tracked in #1419.
A max-effort review of the branch. Three findings change behaviour; the rest correct comments, docs and changelog entries that described the code wrongly. Behaviour: - Tcl literals (#1381 follow-up). is_braced_script_word inherited #1318's script-by-default rule, so braced literals -- a braced proc name, namespace subcommand arguments, on/trap patterns and variable lists -- stopped being strings and were expanded in Ast::dump as invented commands. On main they were strings and flat leaves. Those slots are literals again. Halstead still bills their braces, as it has since #1318; moving the rule into the shared predicate changes `bca metrics` for Tcl and wants its own measured change. - SARIF order (#1402 follow-up). `bca check` stable-sorts findings by (path, start_line, metric) after its walk, while to_sarif emitted walk order, so the entry-for-entry parity #1402 promised was false. to_sarif now sorts with the CLI's own comparator. - Linear Tcl switch. is_switch_arm_body located a word with an O(position) sibling scan, and #1381 routed find, count and dump through it, making all three quadratic in the width of a one-line switch. It now uses tree-sitter's O(log n) goto_first_child_for_byte. find and count now call Search::act_on_node instead of a hand copy of its ancestor-chain bookkeeping, so `make chain-audit` covers them. Corrections: - getter/groovy.rs: the note 4c5af94 added said the Super operator arm never fires. It fires for every `? super T` wildcard bound, which is right -- it mirrors Java's gated wildcard decision. Pinned by a new groovy_wildcard_super_bound_stays_an_operator test. - abc/csharp.rs: the #1383 parent gate also zeroes a relational pattern inside a `when` guard or `catch` filter, where no slot pays for it. Docs corrected and a FIXME(#1422) added; the behaviour is left to #1422. - vcs: the cache contract is not complete once the mailmap is fingerprinted -- git's diff config also decides recorded churn and is not fingerprinted -- and gix's open_mailmap swallows read errors, so a transient read does not heal. Docs corrected; the code gaps are tracked separately. - loc.rs: the clamp docs claimed parseable input is untouched; Perl POD to end-of-file and Ruby `<<~` parse cleanly and still move. - CHANGELOG, fuzz docs, benchmarking.md, AGENTS.md, Makefile, Cargo.toml and several test comments carried claims the #1381 chain threading made stale. - A vcs test helper matched paths via to_string_lossy, which AGENTS.md bans for identifiers.
Two review findings on PR #1424. The self-reference parity test asserted membership in `Ops::operands`, which is a *deduplicated* vocabulary. Python's `def f(self)` parameter and Rust's `&self` spell the keyword in the declaration, so those two rows passed on the declaration alone: deleting `return self.x` left both green, and the grammar drift the rows exist to catch -- a receiver promoted to its own unclassified kind -- would not have failed them. N2 is the axis that can see the second occurrence. Each masked row now carries a receiver-stripped variant and asserts the receiver contributes exactly one operand occurrence. Verified by deleting the receiver from each fixture in turn: Python fails with N2 4 and 4, Rust with 6 and 6. Also documents a qualification the SARIF parity contract omitted. The CLI folds repeated path seeds together in `SeedSet::seen`, while `analyze_batch` returns one result per input and `collect_offenders_from_iter` renders every one, so `to_sarif(analyze_batch([a, a]))` emits each finding twice where `bca check -p a.py -p a.py` emits it once. Deduplicating in the binding would be wrong, because two distinct results may legitimately share a name, so the contract now states that entry-for-entry parity assumes a unique file set. Recorded in all three places that claim the parity: the `sarif.rs` module doc, the `to_sarif` stub, and the book.
fix(ast): key Tcl braced-word roles on argument position
`is_value_braced_word` decides "script or value" by command name
alone, which is all Halstead's `{}` operator needs. Six of the eight
names it recognises have a mixed signature, so reusing that answer for
`Checker::is_string_with_code` and `Alterator::keeps_children` made a
value a script: `time {set x 1} {5}` lost the count, `after {100}` the
delay, `switch {foo} {…}` the subject, `uplevel {1} {…}` the level and
`namespace eval {foo bar} {…}` the namespace name. Each was a string
and a flat dump leaf before the braced-word split.
`is_braced_literal_slot` now reads a per-command slot table instead of
answering `on`/`trap` and `namespace` ad hoc. Each entry pairs a Tcl
8.6 signature with which of its arguments the interpreter evaluates,
so the whole policy is one table: `after` all but the first, `time`
argument 0, `on`/`trap` the last, `uplevel` all but a leading level
specifier, `switch` the arm bodies or the braced arm list, `namespace
eval` all but the name, `namespace inscope` the second, `eval` / `for`
/ `namespace code` everything.
`switch` also gets its subject scan: the first argument that is
neither a leading option nor a `-matchvar` / `-indexvar` operand, with
`--` ending the options. The scan stops at the first non-option word
and a braced word never is one, so its length is the leading-option
count rather than the argument count — the index itself still comes
from the `O(log n)` cursor lookup, so a wide one-line arm list stays
linear.
The rule moves to `lang_helpers::tcl_family`, beside the two dialects'
kind tables, for the reason that module already gives: three
classifiers consult it and must agree on the same bytes.
Halstead is unchanged by design. `braced_word_op_type` still asks
`is_value_braced_word` alone, so a value slot's braces keep billing a
`{}` operator; moving that boundary changes `bca metrics` for Tcl and
wants its own measured change.
test(ast): cover the Tcl argument-slot fallbacks
Eleven of the twelve early returns the slot rule added were uncovered.
Four turn out to be reachable from ordinary input and now have
fixtures; six are unreachable from a walk but are the contract of a
`pub` trait method or a `pub(crate)` helper, and now have direct tests
asserting the documented answer; one pair collapsed into a single exit.
Reachable, and asserted through the existing exact-vector verdict
harness in both dialects where both can reach them:
- `namespace $sub {…}` — a subcommand that is not a plain word
resolves no layout at all.
- `switch -matchvar {m} -indexvar {i}` — both options consume the word
after them, so the subject scan runs off the end. Two pairs, because
with one the invented subject 0 gives the same answers.
- `switch bar {p} {puts c} {q} {puts d}` — a bare-word subject, the one
spelling the braced and `$v` fixtures miss. Two arms, for the same
reason.
Unreachable from a walk, tested against the method's own contract:
- the two ancestor-chain exits of `is_braced_literal_slot`, asserted
over every node at depth 0 and 1 of a real tree — a value slot is an
argument, so nothing that shallow can be one.
- `is_switch_arm_body` on an argument-less command, paired with its
positive answer on a real arm list.
- the two placement guards in `fills_script_slot`, using a word nested
inside a sibling slot rather than one from another command: a word
outside the list's byte range makes the cursor answer `None` and
never reaches the id check.
`reads_as_uplevel_level` now takes one exit for text that is not a
readable `{…}` instead of two. Unreadable bytes and unbraced text are
both "not a level" and no caller can tell them apart, so the split
bought nothing.
The remaining uncovered line is the third-owner arm of
`is_braced_literal_slot`. Both pinned grammars give a `word_list`
exactly two owners, verified against their `node-types.json`, so no
input reaches it; a new assertion walks both dialects and fails if a
third owner ever appears, which is the event the arm exists for.
Squashed from 15bb946, ae1b8ed.
`is_braced_literal_slot` gated on `Node::has_error()`, which is
transitive, so a parse error anywhere inside one argument withdrew the
value-slot rescue from every sibling. `trap {pat} {v} {puts ]}` reported
zero strings where main reported them, and via `keeps_children` the dump
rendered `{100}` as a fabricated `command` named `100` — the defect the
slot table was added to remove. `has_error` stays as the O(1)
pre-filter; the argument list's own direct children are then scanned for
ERROR/MISSING, since an error *inside* an argument moves no sibling's
index while one occupying a slot moves every index after it.
Deleting the old guard failed none of the 3361 tests, so both new tests
are verified by revert: the old spelling fails exactly the new test, and
removing the guard entirely fails exactly the other.
Also from the same review pass:
- `ScriptSlots::Last` used a raw `usize` subtraction that a `Last` row
added to `NAMESPACE_SCRIPT_SUBCOMMANDS` would underflow; now
`checked_sub`, matching the arm two lines above it.
- The `crate::Tcl` import in `braced_slot_tests` was gated wider than
its uses, so an `irules`-without-`tcl` build warned and ci.yml's
workspace-wide `-D warnings` would have failed that leg.
- `switch_subject_index` takes the caller's cursor rather than
allocating a second one per braced word.
- `Node::is_error` / `Node::is_missing` added; `has_error` now documents
the transitivity trap, and `goto_first_child_for_byte` documents that
it cannot return a zero-width child, which its two call sites resolve
in opposite directions.
- `FIXME(#1410)` anchors the C-family bool-terminal gap in source rather
than only in the tracker; the Halstead/string divergence cites #1382.
- CHANGELOG: #1381 also withdraws braced *conditions*, not only bodies,
and #1396's list of siblings that still lose literal rows was missing
Ruby.
- Baseline: `Node<'a>` nom 33 -> 35, via
`make self-scan-write-baseline-headroom`.
`::switch` is `switch` — a leading `::` names the global namespace, and inside a `namespace eval` body it is the spelling that guarantees the core command rather than a local proc shadowing it. `Getter::command_leading_word` stripped it, so the braced-word slot table, Halstead, `bca find --type string` and the AST dump all read the qualified form correctly. The four metrics that resolve a leading word *without* that table read it raw, and so scored the same bytes differently: - `::switch` / `::for` contributed nothing to cognitive or cyclomatic — Tcl models neither construct, so the leading word is their only seam. - `::incr` / `::append` / `::lappend` counted as an ABC branch instead of an assignment. - `::return` / `::error` / `::throw` / `::exit` were not exits. The strip now lives in one `strip_global_qualifier`, which both `command_leading_word` and `tcl_command_name` call, so the two halves cannot drift again. It stays leading-only: `ns::eval` is a different command living in `ns`, the direction `BRACED_WORD_VALUE_CASES` already pins for Halstead. iRules resolves its exit and mutator names in its own walkers rather than through `tcl_command_name`, so both were swept in the same change per grammar-dispatch.md. `irules_command_is_assignment` moves from a raw byte compare to the same resolve-then-match shape as its Tcl sibling; its leading word is still addressed by index rather than by the `name` field, which is left alone as a separate behaviour change. Twelve tests, six qualified and six namespaced controls, across both dialects. Verified by revert: perturbing the helper to the identity fails exactly the six qualified tests plus the two pre-existing `::eval` Halstead rows, and leaves all six controls passing. No integration snapshot moves — the corpora contain no Tcl.
`clamp_line_sets_to_span`'s `span == 0` arm passes `(1, 0)` to `retain_range`, whose inverted-range path calls `words.clear()` — so every PLOC and CLOC row the space holds is discarded, and the parent never recovers it, because a child is clamped before `Ploc::merge` lifts it. The two assertions at the end of the function cannot see that: they compare against `span`, which the clear forces to `0 <= 0`. The one branch that destroys data was the one branch nothing observed. Assert the precondition before the clear instead, where the question can still be answered. No behaviour change in either profile — the branch still clears — but a walk arriving with a row now fails loudly in debug rather than losing it silently. The review that raised this called it a live bug. It does not reproduce: `span == 0` needs a space opened on a genuinely zero-width node at column 0, every language's `is_func_space` matches compound productions rather than a raw token or a recovery node, and a MISSING node is always a single terminal. The assertion now backs that with the whole corpus rather than with reasoning — it holds across every lib and integration target, none of which trips it. The justification comment is softened to match. It claimed a corpus measurement, and `line_set`'s own header retracts a claim of exactly that shape, in this same release, as having been wrong *and* load-bearing (#1398). `a_zero_span_keeps_no_row` becomes `a_zero_span_holding_rows_trips_the _guard`: seeding a populated set at zero span is now precisely the state the guard rejects. The clearing behaviour it used to assert is covered a layer down by `retain_range_inverted_empties_the_set`, which seeds `[0, 1, 400]` and asserts row 0 specifically is gone — the same discrimination, at the layer that owns it. A companion test pins the empty zero-span case every real walk takes.
`MAX_AST_SERIALIZE_DEPTH` bounds the `AstNode` tree *after* the alterator, and its doc quoted one number: the deepest AST across the ~8 000-file corpus, 188 levels. That is a count of AstNode levels, not a conversion rate from source nesting, and since #1381 the two came apart for the Tcl family — a script body keeps its children, so one brace spans `braced_word` -> `command` -> `word_list` before the next. Measured rather than asserted: 196 levels over 64 nested `eval` braces, three per brace, putting the effective ceiling near 170 nested braces rather than 512. `a_tcl_brace_level_costs_three_ast_levels` pins both figures, so the doc cannot drift from the grammar again. The first draft of this commit said four levels and 128 braces; the test is what caught it. Nothing here changes behaviour. The bound is not raised: it exists because serde's recursion overflows the native stack into `SIGABRT` rather than a catchable panic (#1056), and the constant is published from two crates. Coverage for what the bound does when reached: - `server_tests.rs` had no Tcl or iRules fixture at all. One now pins that a script body renders its children rather than collapsing to a verbatim leaf, and a second pins that a 250-brace input fails the *request* — 500, `serialize_failed` — rather than the process. The depth message never reaches the client, because `Format::encode` collapses the serializer error, so the token is the contract. - `fuzz/src/nested.rs` gains `NestLang::Tcl`, the first entry whose constructs nest scripts rather than expressions, with a seed pair. Appending it last keeps every committed seed decoding unchanged: each selects a language with `byte % N` and all eight use a byte below 4. `dump_error` and `space_error` now take the `Nesting` rather than its rendered bytes, so the language parsed is always the language rendered — handing Tcl source to the Rust grammar yields a shallow `ERROR` tree that reports no depth failure, which is the same "looks like coverage" shape the seed corpus already had once.
dekobon
force-pushed
the
fix/batch-2026-09-10
branch
from
September 12, 2026 15:25
1d6df06 to
a304125
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ten bug fixes from one batch, each taken through
simplify-rust,rust-optimize,reviewandaudit-tests, followed by a fresh-context whole-branch review and a max-effort/code-review.Fixes
.mailmapedit now invalidates the persistent VCS history cachethis/super/baseare Halstead operands in Java, C# and Kotlinbca find --type stringno longer reports a Tcl/iRules script body as a stringplocnpmtreatsinitializeand its siblings as privateto_sarifemits findings in the same order asbca check -O sarif#1397 was already fixed on
mainby0c987134and was closed separately.Metric drift
Metric values move for these fixes. Each move is described in its
CHANGELOG.mdentry:abcfor C#, Kotlin, Ruby, Elixir and Perl (fix(abc/csharp): relational pattern operator double-counts against its arm #1383, fix(abc/kotlin): primary-constructor superclass call is not a branch #1384, fix(abc): float and suffixed numeric operands score no condition in Ruby and Elixir #1379).abcis a gated threshold metric.locon PHP files with these literals (fix(metrics/loc): PHP heredoc rows that are empty inside the literal read as blank #1396), and on files a grammar cannot fully parse (fix(metrics/loc): an unterminated Bash heredoc makes ploc exceed sloc #1398). The fix(metrics/loc): an unterminated Bash heredoc makes ploc exceed sloc #1398 change reaches ten languages and at least one ordinary shell script in the DeepSpeech corpus.npmfor most Ruby classes (fix(npm/ruby): initialize is counted as a public method #1400).Integration snapshots move only for
csharp/control_flow.cs(#1383) andphp/strings.php(#1396). Both diffs contain only metric values. They are recorded as submodule commitc03ffa97, which is already pushed tobig-code-analysis-output.Before merging
Please squash-merge, or rewrite history before a rebase-merge. Six intermediate commits fail on their own. The submodule bump and the self-scan baseline refreshes landed as later follow-up commits instead of alongside the fixes that needed them, which AGENTS.md disallows. That was a consequence of consolidating the submodule work across parallel worktrees. Commits
194ffb72through1b92c1e0record the old snapshot SHA and fail the corpus tests. The #1384, #1262 and #1381 commits fail self-scan until their baseline refresh commits land. The branch tip passes. A rebase-merge would put the failing intermediate commits ontomain. The clean alternative is to fold each baseline refresh into its fix, and to splitc03ffa97into a PHP part and a C# part fixed up into194ffb72and1b92c1e0.The tip commit
24ece9fdis unsigned, because the GPG agent couldn't be unlocked on the build host. The other 24 commits are signed. It will be re-signed and force-pushed.Verification
make pre-commit:BCA_GATE: passat the tip.make chain-audit,make fuzz-checkandmake bench-scaling: all pass.bench-scalingis clean for theloc/*probes.Follow-ups filed, not fixed here
#1406, #1407, #1408, #1410 (now also covers C, C++, Mozcpp and Objective-C), #1411–#1423, #1425.
Fixes #1262
Fixes #1379
Fixes #1380
Fixes #1381
Fixes #1383
Fixes #1384
Fixes #1396
Fixes #1398
Fixes #1400
Fixes #1402