fix(metrics): six ABC and cyclomatic fixes - #1459
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1459 +/- ##
==========================================
+ Coverage 97.97% 98.00% +0.03%
==========================================
Files 359 359
Lines 93425 94399 +974
Branches 92994 93968 +974
==========================================
+ Hits 91533 92520 +987
+ Misses 1224 1212 -12
+ Partials 668 667 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
dekobon
added a commit
that referenced
this pull request
Sep 14, 2026
The `cargo-deny` job has been red on `main` since 78a11ec, and on PR #1459, for one reason: `advisories FAILED` against `rustls 0.23.43`. Every other job in both runs passed, so the failure is not attributable to either change — the advisory was published against a version the lockfile already carried, which is why a dependency-free commit turned CI red. RUSTSEC-2026-0285: rustls accepted TLS 1.3 handshake messages sent at the wrong encryption level when they followed a key-changing message in the same record — a plaintext `EncryptedExtensions` packed into the `ServerHello` record, for example — where RFC 8446 section 5.1 requires the connection be terminated with an `unexpected_message` alert. The handshake transcript remains authenticated, so a network-position attacker can neither alter nor complete a handshake; the practical effect is that a peer may send in plaintext handshake messages that should have been encrypted without rustls rejecting the connection. Functionally the same bug as Go's GO-2026-4340 (CVE-2025-61730). `rustls` reaches this workspace only as a dev-only transitive dependency (`jsonschema` -> `reqwest` -> `hyper-rustls` / `tokio-rustls` / `rustls-platform-verifier`), so no shipped code path negotiates TLS through it and no published crate's dependency graph changes. The fix is therefore a lockfile bump to the advisory's minimum fixed release, 0.23.45. The diff is deliberately two lines. `cargo update -p rustls` also re-resolved `tempfile`'s `getrandom` edge from `0.4.3` to `0.3.4` — incidental churn, since `tempfile 3.27.0` requires `>=0.3.0, <0.5` and both versions remain in the lock through other dependents. That hunk is reverted; `cargo metadata --locked` accepts the result without re-resolving, so the minimal lockfile is stable rather than something the next cargo invocation would churn back. No pinning test accompanies this, unlike `h2`/RUSTSEC-2026-0258 in `big-code-analysis-web/src/lockfile_tests.rs`. That test exists because krates filters `h2 0.3.27` out of the graph before cargo-deny's checks run, leaving the advisory gate blind. `rustls` is in the graph and the gate saw it — a second guard over the same lockfile line would be redundant with a check that already works. Verified: `cargo deny --log-level warn --manifest-path ./Cargo.toml --all-features check advisories bans licenses sources` (CI's exact invocation) exits 0 with `advisories ok, bans ok, licenses ok, sources ok`; `make pre-commit` reports `BCA_GATE: pass (gate=pre-commit)`.
dekobon
commented
Sep 14, 2026
dekobon
left a comment
dekobon
left a comment
Owner
Author
There was a problem hiding this comment.
Code Review: fix(metrics): six ABC and cyclomatic fixes
Verdict: REQUEST CHANGES
| Severity | Count |
|---|---|
| BLOCKER | 0 |
| CRITICAL | 0 |
| HIGH | 2 |
| MEDIUM | 0 |
Findings
| # | Severity | File:Line | Title | Category |
|---|---|---|---|---|
| 1 | HIGH | src/metrics/abc/kotlin.rs:238 |
Preserve the subject-less when arm decision for non-terminal predicates |
correctness |
| 2 | HIGH | src/metrics/abc/csharp.rs:534 |
Count the guard slot when its expression already has operator structure | correctness |
Review passes executed
- Correctness and logic bugs
- Grammar-dispatch and cross-language parity
- Regression-test discrimination
- Performance and scalability
- Metric/documentation consistency
- Semgrep static analysis (0 findings)
Files reviewed: 12
All GitHub CI checks are green. This review is submitted as a comment because GitHub does not allow an author to request changes on their own pull request.
dekobon
added a commit
to dekobon/big-code-analysis-output
that referenced
this pull request
Sep 14, 2026
C# ABC now counts the null-coalescing `??`, which C# cyclomatic already counted (dekobon/big-code-analysis#1459, review finding on #1422). `linq.cs` is the only C# corpus file containing one. Only abc keys move; no cyclomatic, halstead, cognitive, loc or mi line changes. SafeCustomer (order?.Customer ?? "anonymous") conditions 1 -> 2 HighestTotal (max ?? fallback) conditions 1 -> 2 class roll-up and file root conditions 10 -> 12
This was referenced Sep 14, 2026
dekobon
added a commit
that referenced
this pull request
Sep 14, 2026
Both fixes close a condition slot that silently contributed zero for a valid boolean expression outside its allowlist. Found by the code review of PR #1459, which caught them as regressions of #1421 and a gap #1422 left open. Kotlin's peel understood only a prefix `!` and parentheses. `unary_expression` is one kind for both unary spellings, so the slot routed a postfix `a!!` in as handled while the peel's positional read found the `!!` token and stopped; an `as` cast was not a wrapper at all. #1421 moved payment from the `when` entry to this slot, so both regressed from 1 to 0. The peel now reads its operand by grammar field and `kotlin_count_condition` derives its wrapper set from the peel rather than restating it — those two lists disagreeing is the defect, so there is now one source of truth. The safe cast `as?` stays excluded: its `AsQMARK` token is already a condition, and peeling it too scores `a as? T` twice. C# `??` was a cyclomatic decision and no ABC condition, so a `when b ?? false` guard scored zero where every other spelling of the same guard scored one. Counting the token levels the spellings without touching the slot, which keeps a compound guard's sub-structure. The review asked for an unconditional slot increment instead; measured, that takes `when x > 2` from 3 to 4 — the double count #1422 removed — and suppression inside the guard flattens `when a > 1 && b < 2` from 4 to 3, which #1422 preserves deliberately. The JS family has always counted `??`, so this closes a divergence rather than opening one. The `??` fix narrows the ABC-versus-cyclomatic gap on a guard from two to one without closing it: cyclomatic counts `??` in addition to the guard clause while ABC's condition substitutes for the slot. Away from a guard, parity is exact. `??=` is unchanged and remains an assignment, as in the JS family. Costs one corpus snapshot: `linq.cs` is the only C# corpus file with a `??`, and its file-level conditions go 10 to 12. Only abc keys move. Kotlin has no corpus.
A C# comparison-operator overload declares a name; it applies nothing. #1297 established that for `<` and `>`, but C# overloads six operators and spells the other four with distinct tokens — `<=` `>=` `==` `!=` — which reached a different, ungated arm of `csharp_count_token_condition`. Each still scored one spurious ABC condition per declaration: measured 1 apiece against `<` / `>`'s 0, on a class where `cyclomatic()` is 1 for all six members. The four tokens are merged onto the `binary_expression` parent allowlist `<` / `>` already carried, which subsumes #1383's `relational_pattern` denial for `<=` / `>=` and fails closed on a grammar bump (§1). `Else`, `Case`, `Try` and `Catch` are split out of the old shared arm and stay ungated: each comes from one production, so there is nothing to gate on. Two claims the previous comments made are corrected rather than inherited, both by measurement. `BinaryExpression2` was described as listed defensively and unreachable, on the grounds that C#'s preprocessor admits no bare `<` / `>`. That is true of `<` / `>` and irrelevant to `==` / `!=`, which the preprocessor does admit. The reason the alias entry is not load-bearing is different: `#if A == B` parses as `BinaryExpression` (369), so the preprocessor spelling counts through the first entry. `csharp_preproc_equality_counts_through_the_binary_expression_alias` pins both halves, since the gate is now the only thing keeping `#if` equality a condition. `Case` comes from two productions, not one — `switch_section` and `goto_statement` — so `goto case 2;` scores a condition without being an arm. It is left counting deliberately: C# cyclomatic counts the same token, so gating it here alone would break the §8 parity `conditions == cyclomatic() - 1` that a two-arm-plus-`goto case` fixture currently satisfies. Filed separately rather than changed here. The new test gives each token its own member. A partial fix then names the spelling it missed, where the file total cannot: that total reads 4 before and 0 after, so any two-of-four fix halves it and still looks like movement. Method `n` is the §11 control — the same four tokens applied, inside `binary_expression` parents, which must keep scoring one each. No corpus snapshot moves: neither C# corpus declares a comparison-operator overload. Fixes #1420
A C# 12 primary constructor passes arguments to its base in the declaration header — `class Sub(int x) : Base(x)` — which invokes the base constructor exactly as the `: base(x)` initializer #1279 added does. It scored zero. Measured before: `class Sub1(int x) : Base(x)`, `record R1(int x) : Base(x);` and `record class R2(int x) : Base(x);` all 0 branches, against 1 for the `: base(x)` spelling beside them. tree-sitter-c-sharp 0.23.5 spells the two declaration families differently, so there is no single node to match — verified by `bca dump`, not inferred. A record nests the arguments under a `primary_constructor_base_type`; a class hangs them straight off the `base_list`. Matching only the former covers records and misses every class, so the two are independent paths (§11) and both are listed. Both kinds are gated on a `base_list` parent. The gate is load-bearing on `ArgumentList`, which is otherwise the argument list of every call in the file: ungated, 19 existing C# tests fail and each call bills two branches instead of one. This is §5's container-plus-containable shape — a `primary_constructor_base_type` holds an `argument_list` — and the gate is what makes it safe rather than a double count: in the record spelling the inner list's parent is the wrapper, not the base list, so the two are never both matched. `BaseList` and `BaseList2` both render to "base_list" and only 252 is observed; both are listed per §1, with a zero-count drift marker pinning that 246 is unemitted. An argument-less base type still costs nothing — `struct S(int x) : IBase`, `record R(int x) : Base`, `enum E : byte`, `interface I : IBase` emit neither node. Attribute, indexer and type argument lists are distinct kind ids and never occur under a base list. Matching `ArgumentList` makes `compute` return before `csharp_walk_for_conditions`, so a base list's argument list no longer reaches that function's `ArgumentList` arm. Measured harmless: the arm is dead for every argument list, because an `argument_list`'s children are `argument` wrappers `csharp_inspect_container` rejects on the first iteration — `Helper(!b)` and `Helper((b))` each score zero conditions before this change as well as after. Repairing it means revisiting that exclusion, not this arm. Three grammar-reachable shapes are not valid C# and now score 1 (`interface I : IBase(x)`, `enum E : Base(x)`, a base call on a class with no primary constructor). No valid program distinguishes the behaviours, so per §6 the gap is documented in the arm and left untested rather than pinned. Java and Groovy were swept and need no sibling fix: neither language has a primary constructor, their `superclass` node carries no argument list, and a base call is an `explicit_constructor_invocation` / `method_invocation` already counted. Kotlin was fixed in #1384. No corpus snapshot moves — no C# corpus file uses the construct. Fixes #1406
A C# `when` guard is a branch and neither metric modelled it. Cyclomatic had no arm for either spelling — `when_clause`, shared by `switch_expression_arm` and `switch_section`, and `catch_filter_clause` on `catch (E e) when (…)` — so a guarded arm contributed one decision where it fails two ways: the pattern does not match, or it matches and the guard is false. ABC counted whatever operator token happened to sit inside the guard, so three semantically identical guards produced two numbers: `when x % 2 == 0` and `when x > 2` scored one condition via the comparison-token arm, `when IsEven(x)` scored none. Cyclomatic now counts both clause kinds toward the standard and modified tiers, matching Rust, whose match guard already counts. ABC models the guard as a condition slot rather than suppressing its operator: every spelling contributes exactly one, and a compound guard (`when a > 1 && b < 2`) keeps its sub-structure instead of collapsing. Suppression would have reached the same internal agreement one count below C#'s own decision count. Two grammar details the slot turns on. The clause nodes, not the `when` keyword they share: `When` is also a `_reserved_identifier` at this pin, so `int when = 1;` emits it under an `identifier` and the token would have scored a decision per mention of the variable. And every named child, not the first: tree-sitter extras are named nodes, so a leading comment (`when /*c*/ g`) would otherwise hand the slot a `comment` and silently restore the spelling-dependence this removes. Costs one corpus snapshot. `control_flow.cs` gains a cyclomatic decision on `Bucket` (switch guard) and `SafeDivide` (catch filter), rippling into `mi`, `wmc` and the file aggregates; ABC is unchanged there because both corpus guards are operator-shaped. Fixes #1422
A subject-less `when` arm's condition is an ordinary boolean expression,
compiled exactly as an `if` predicate, so the comparison operator inside
it is already an ABC condition through the token arms. The `WhenEntry`
arm added a blanket one on top, and
`when { x > 5 -> 1; x < 0 -> 2; else -> 0 }` reported 4 conditions
against a cyclomatic decision count of 2. All six comparison spellings
were affected, not the two `<` / `>` the issue names: `>=` and `==`
reach the count through a different arm and scored 2 against 1.
A subject-less entry now goes through the same condition slot `if` uses,
and a subject-ful one keeps its per-entry count — its arm lists a
pattern, not an independent boolean expression, so the implicit
`subject == pattern` is a decision nothing in the source spells and the
entry must pay for it. The operator is not suppressed instead, which
would have collapsed a compound condition `when { a > 1 && b < 2 -> … }`
to one count, a decision below Kotlin's own.
Deferring to the slot exposed a second gap it would otherwise have
regressed: `is_expression` and `in_expression` are the two relational
forms the grammar spells as their own production rather than as a
`binary_expression`, so no comparison-token arm sees them and nothing
counted them at all. Listing both in `kotlin_bool_terminal_kinds!()`
reaches the predicate slot, the `&&` / `||` walker and the paren / `!`
unwrapper together, and fixes `if (a is String)` and `if (a in 1..2)`,
which scored zero against a decision count of one. `WhenEntry` joins the
walker's boolean-context set so a parenthesised condition counts; a
negated one already did.
Two divergences from `conditions == cyclomatic() - 1` remain and are
correct. A comparison nested inside a comparison is two ABC conditions
and one branch, so `when (x) { y > 5 -> … }` reads 2 against 1 exactly
as `if (x == (y > 5))` always has. And a multi-alternative entry,
`when { x > 5, y < 0 -> … }`, reads 2 against 1 because cyclomatic
scores the whole entry as one decision; it was 3 before, and the residue
is cyclomatic's model rather than a double count here.
There is no Kotlin corpus — one unsnapshotted `.kt` file repo-wide — so
every number above was measured on hand fixtures against a live build,
before and after, and each changed path was verified by perturbation.
Fixes #1421
An enum constant carrying constructor arguments — `A(1)` — is an object construction under Fitzpatrick's "function invocation or object construction" rule, but scored zero in every JVM-family language. #1279 and #1384 settled the two sibling delegation forms (`super(…)` / `this(…)` and `class Sub : Base(1, 2)`) and left this one behind, because both looked at superclass delegation only. Adds one arm per language, gated on the entry's argument-list child so the childless spelling does not regress (grammar-dispatch §6): Kotlin `enum_entry` + `value_arguments`, Java `enum_constant` + `argument_list`, Groovy `enum_constant` + `argument_list`. Java's gate names `argument_list` specifically, so an annotated constant cannot satisfy it via the distinct `annotation_argument_list` production. The rationale, including the counter-argument that an enum constant is a declaration rather than a call site a reader navigates to, is recorded on each arm. Java and Groovy's branch functions move from `if matches!` to the `match` + `is_branch` shape #1406 established in the C# sibling; neither needed widening to take `Ancestors`, since the gate reads a child. Cost is one branch per argument-carrying constant, and +2 cyclomatic on `KotlinCode::compute` (18 -> 20), whose baseline entry is refreshed here. The baseline regeneration also lowers `src/spaces/compute.rs:: metrics_inner`'s `halstead.effort` from 119147.75 to 116715.61. That entry is stale on `main` and is not this change's doing: it is a decrease, so it never made the gate red and no earlier commit was forced to refresh it. It rides along because the baseline is regenerated wholesale rather than edited by hand. Measured with `bca dump` / `bca metrics` against a live build: Kotlin `enum class E(val v: Int) { A(1), B(2), C(3) }` 0 -> 3, Java `enum E { A(1), B, C(2); }` 0 -> 2, Groovy `enum E { A(1), B, C(2) }` 0 -> 2; every bare and annotated-bare spelling stays at 0, and `A(f())` scores 2 rather than 1. No corpus or snapshot movement. Fixes #1407
`src/spaces/compute.rs::metrics_inner` recorded a `halstead.effort` of 119147.75 against a live measurement of 116715.61. Nothing on this branch touches `src/spaces/`; the entry went stale on `main`, in `d4479cdf` (PR #1446's review-findings commit), which changed `compute.rs` after the last baseline refresh. It went unnoticed because the value *fell*. The baseline filter suppresses a violation only while the live measurement stays at or below the recorded value, so a decrease never reddens the gate and nothing forces a refresh — the recorded number simply stops describing the tree, and the gate runs looser than intended until someone regenerates for an unrelated reason. Here that reason was #1407 refreshing a Kotlin entry, since the write target regenerates the file wholesale. Recorded separately from that fix so the metric commits carry only their own baseline movement. The staleness mechanism is filed as #1465.
PHP, Groovy and the name-keyed C / C++ / Mozcpp / Objective-C terminal-bool sets named no numeric literal kind, so a truthy number in a boolean operand slot scored no condition. `$a && 1` scored one against `$a && $b`'s two, and in the C family the gap was visible *within* one language: `if (true)` scored one condition and `if (1)` none, because `"true"` was in the set and `number_literal` was not. The operand kind fell through the terminal gate, the walker recursed one level, and the recursion then failed the list-kind gate. Nothing warned. Adds `Php::Integer` / `Php::Float`, `Groovy::NumberLiteral`, and the name-keyed `"number_literal"` / `"char_literal"` arms. `char_literal` is included because a character literal has integral type (`int` in C, `char` in C++) and is truthy for the same reason a number is; omitting it would reproduce the same asymmetry one kind narrower. `Php::Float2` is deliberately excluded: it renders to the same `"float"` string but is the type keyword, not a value. C++'s `user_defined_literal` and Objective-C's `version_number` are excluded for the reasons recorded on the set. Every spelling was re-derived with `bca dump` per language rather than by an alias sweep, which is what #1379's first cut got wrong: PHP folds every radix, separator and exponent into `integer` / `float`; Groovy and all four C-family grammars emit one `number_literal` each. #1379 deferred these six on the stated grounds that all three sets had integration-corpus files. That held only for the C family. PHP's six corpus files carry no truthy numeric operand, and no corpus carries a `.groovy` file at all — the 16 `.gradle` files in DeepSpeech sit outside every test glob. Both stale rationales are corrected rather than dropped. Costs seven DeepSpeech C/C++ snapshots, all `abc.conditions` and its derived `magnitude` / `value` / per-space min-max-average and no other metric, every move traceable to a `while (1)` or `do { … } while (0)`. Fixes #1410
Six ABC and cyclomatic fixes landed on this branch with their changelog entries held back to avoid conflicting edits to one file. Each records its metric drift, since all six move published numbers.
Both were invisible per-fix and visible only reading the branch as one
diff: each fix looked correct alone while contradicting another.
C# listed `IsPatternExpression` (392) and not `IsExpression` (391) in
`csharp_bool_terminal_kinds!()`. They are distinct kinds, not aliases —
the grammar emits the second only once a pattern is involved
(`x is int y`, `x is null`) and the first for the bare type test — so
two spellings of one test disagreed: `if (x is int)` scored zero
conditions against a cyclomatic decision of one while `if (x is int y)`
scored one. That also falsified the `when` guard fix's own claim to
score one however spelled, measured `when x is int` 1 against
`when IsEven(x)` 2, and it is the C# half of the gap the Kotlin
subject-less `when` fix closed by adding `IsExpression | InExpression`
there. Neither fix swept toward the other.
Kotlin spells boolean `and` / `or` / `xor` as infix *functions*, so
`a and b` parses as `infix_expression` — not a `binary_expression`, and
matched by no token arm. The blanket per-entry count the subject-less
`when` fix removed had been covering it, so `when { a and b -> … }`
fell from one condition to zero while the `if (a and b)` it is supposed
to agree with still scored one. That contradiction is the whole premise
of that fix inverted, which is what made it diagnosable.
`InfixExpression` joins `kotlin_bool_terminal_kinds!()` for the same
reason `CallExpression` is there: `a and b` is `a.and(b)`.
Neither arm double-counts (§5): no arm counts the `is` token, and the
infix operator is an identifier the walker never reaches. Removing
either entry fails exactly its own test.
The book's C# row said a relational pattern in a `when` guard "scores 0,
one below the equivalent `x > 0`". The guard became a condition slot
earlier on this branch, which falsified it; corrected here, along with
two missing rows in the ABC per-language deviations table.
No corpus snapshot moves — no C# corpus file uses a bare `is` type test,
and there is no Kotlin corpus.
A boolean test the grammar gives its own production, rather than
spelling as a `binary_expression`, reaches no comparison-token arm and
was absent from its `<lang>_bool_terminal_kinds!()` set, so it scored
zero conditions where a bare identifier in the same slot scores one.
Both walker paths were short: the `if` predicate and the operands of a
`&&` chain each measured one below their identifier control at equal
cyclomatic.
Eight constructs across three languages, each taking exactly one of the
two routes so nothing is counted twice:
- Groovy `a in l` / `a !in l` joins the terminal set. A token arm
cannot serve it: `in` is shared with `for_in_statement`, so an
ungated arm would score every `for (x in l)` header, and `!in` emits
no operator token at all — `token(seq("!in", …))` makes it invisible
to tree-sitter, so no gate could reach the negated spelling.
- Groovy `===` / `!==` / `=~` / `==~` become condition tokens, so they
also score outside a boolean slot as `==` already did, closing a
within-language asymmetry between two equality operators. A
grammar.json sweep gives each token exactly one producing rule, so
no gate is needed.
- Perl `/^#/` and `m{^#}` — sibling rules, not aliases — join the set.
A bound `$x =~ /^#/` still scores one, not two.
- Ruby `a in Integer` joins the set; `match_pattern` stays out, since
`expr => pat` raises rather than yielding a bool.
Rust's `matches!(…)` measures short the same way and is deliberately
left out. Its fix is `macro_invocation`, which also catches `cfg!` /
`dbg!` / `todo!`, and the one corpus snapshot it moves is a `cfg!`
rather than a `matches!` — so the arm's first effect here is precisely
the breadth #1461 item 2 exists to decide. Settling that by inclusion
would decide the issue by accident.
Groovy, Perl and Ruby have no snapshotted corpus, so nothing moves.
`own_production_bool_constructs` pins both slots per language against an
identifier control plus the absolute conditions/cyclomatic pair, and
pins the two Perl spellings as distinct grammar kinds so neither
terminal-set entry can go dead unnoticed. Each arm was verified by
removal, confirming the module fails naming its own language.
Fixes #1449
Both findings from the code review of this PR. Same root cause: a condition slot delegates to a per-language helper whose third outcome is silence, so a valid boolean expression outside its terminal / paren / unary allowlist contributes nothing. The blanket counts the `when`-arm and guard fixes removed had been covering those shapes. Kotlin's peel understood only a prefix `!` and parentheses. `unary_expression` is one kind for both unary spellings, so the slot routed a postfix `a!!` in as handled while the peel's positional read found the `!!` token and stopped; an `as` cast was not a wrapper at all. Both regressed from 1 to 0 when the subject-less `when` fix moved payment from the entry to this slot. The peel now reads its operand by grammar field, and `kotlin_count_condition` derives its wrapper set from the peel rather than restating it — those two lists disagreeing is the defect, so there is one source of truth. The safe cast `as?` stays excluded: its `AsQMARK` token is already a condition, and peeling it too scores `a as? T` twice. C# `??` was a cyclomatic decision and no ABC condition, so a `when b ?? false` guard scored zero where every other spelling scored one. Counting the token levels the spellings without touching the slot, which keeps a compound guard's sub-structure. The review asked for an unconditional slot increment instead; measured, that takes `when x > 2` from 3 to 4 — the double count the guard fix removed — and suppression inside the guard flattens `when a > 1 && b < 2` from 4 to 3, which that fix preserves deliberately. The JS family has always counted `??`, so this closes a divergence rather than opening one. It narrows the ABC-versus-cyclomatic gap on a guard from two to one without closing it, since cyclomatic counts `??` in addition to the guard clause while ABC's condition substitutes for the slot; away from a guard, parity is exact. Also here, from the whole-branch review of the same change: the Groovy terminal set's comment claimed the grammar has no `array_access` analogue, which is false — `subscript_expression`, `safe_navigation_expression` and three siblings all exist, are legal in a boolean slot, and are unlisted, so `if (l[0])` and `if (a?.b)` score zero. Filed separately; the sentence is corrected because a comment asserting the analogue does not exist is what stops the next reader looking. `kotlin_chain_operands_peel_null_assertions_and_casts` gained a kind census per row — the bare `a && b` control scores the same 2, so trimming the `!!` out of a fixture had left it green with its subject removed. And `assert_fixture_spells` now refuses an empty kind list, which had made every following assertion in a caller vacuous. Costs one corpus snapshot: `linq.cs` is the only C# corpus file with a `??`, and its file-level conditions go 10 to 12. Only abc keys move.
dekobon
force-pushed
the
fix/batch-2026-09-13
branch
from
September 14, 2026 20:56
839b672 to
6721256
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.
Fixes six ABC / cyclomatic issues, plus two defects found reviewing the
batch as one diff.
f04a0075268b9760eb4468cbwhenguard is a decision in cyclomatic and a condition slot in ABC605c771ewhenarms stop double-counting their comparisonef20b2ca39e01116All six move published metric values; each CHANGELOG entry carries a
Metric drift line.
Verification
Each commit passed a full
make pre-commitindependently, with the verdictread from the
BCA_GATE:line. Patch coverage is 100.00% (740/740 addedinstrumented lines) against a project 95.58% regions / 96.96% lines. Every new
match arm was verified by perturbation — exact inverse edits, never a
file-level restore — with the harness asserted non-vacuous first.
Two submodule bumps are pushed and recorded in the same parent commit as their
fix:
cf1ef381(one C# snapshot, #1422) andee1bd859(seven DeepSpeechsnapshots, #1410). Across all seven of the latter the only sections that move
are
abc; across the former, onlycyclomaticand its derived metrics.Two fixes the whole-branch review added
Per-issue review and six green gates missed both; only reading the commits
together exposed them.
d08eb8e9—csharp_bool_terminal_kinds!()listedIsPatternExpression(392) but not
IsExpression(391). They are distinct kinds: the grammaremits the second only once a pattern is involved. So
if (x is int)scored0 conditions against a cyclomatic decision of 1 while
if (x is int y)scored 1 — and fix(csharp): a
whenguard on a switch arm is a branch neither metric counts #1422's guard was still spelling-dependent in exactly thecase its own comment claimed to have fixed. This is the C# half of the gap
fix(abc/kotlin): subject-less
whenarm double-counts its comparison operator #1421 closed for Kotlin four commits earlier; neither fix swept toward theother.
ac59a1a3— a regression fix(abc/kotlin): subject-lesswhenarm double-counts its comparison operator #1421 introduced. Kotlin spells booleanand/or/xoras infix functions, soa and bis aninfix_expressionmatched by no token arm, and the blanket per-entry count that fix removed had
been covering it.
when { a and b -> … }fell from 1 condition to 0 whilethe
if (a and b)it is supposed to agree with still scored 1 — thatcontradiction is what made it diagnosable. Third shape the blanket count
turned out to be propping up, after
is/inand bare parens.Issue bodies that were wrong
Worth flagging for anyone reading the issues alongside this branch — each was
re-verified against a live build before any code was written:
whenarm double-counts its comparison operator #1421 proposed a fix that cannot be written (WhenConditionmaps to"_when_condition", a hidden rule the parser never emits), and listed Rubyas swept clean when Ruby has the defect verbatim (fix(abc/ruby): subject-less
casedouble-counts its comparison #1453).a
.groovyfile — and its alias warning pointed at a trap that is not presentwhile missing the one that is (PHP's
Float2is a type keyword rendering tothe same string). Both stale comments are corrected in the diff rather than
deleted.
whenguard on a switch arm is a branch neither metric counts #1422 described only half the defect;catch (E e) when (…)is a separategrammar node with the same problem, fixed here too.
recorded on each arm.
Two things to look at before merging
whenarm double-counts its comparison operator #1421 required a scope expansion. Fixing it meant addingis/in/infix calls to Kotlin's terminal set, so
if (a is String)goes 0 → 1condition. Without it the fix regresses four shapes. Documented in the
commit and CHANGELOG.
.bca-baseline.tomlcarries one unrelated line —metrics_inner'shalstead effort, stale on
main, a decrease that rode along with awholesale regeneration. Named in
ef20b2ca's message.Follow-ups filed
#1451, #1453, #1454, #1455, #1456, #1457 — sibling gaps this batch did not
close, each with measurements.
#1458 is the meta-issue: five of six fixes were correct in their target
language and unswept to a sibling with the same defect, and every issue body
had a sibling-sweep section that was absent, incomplete, or twice actively
wrong.