Conversation
`PhpCode::is_string` listed `Php::String2` (kind_id 25), which `bca dump` shows only ever as the sole child of a `primitive_type` wrapper: it is the `string` *type* keyword, not a literal. Since `bca find -t string` and `bca count string` route that filter word through `is_string_with_code`, every PHP type annotation was reported as a string. On the repo corpus that was 12 of 24 hits on `php/strings.php` and 19 of 27 on `php/classes.php`. Both reasons #288 gave for the arm have since been retired. #1293 moved `String2` into `get_op_type`'s `primitive_type`-suppression arm, so the claimed Halstead parity now argues the other way, and #1261 removed the identical keyword aliases from TypeScript and TSX. PHP was the only offender: all 29 `is_string` invocations were read, and Java, C#, Kotlin and Groovy spell their string types as `type_identifier` / `predefined_type`, which none of them lists. The JS-family `String2` aliases are string literals, not keywords. `String3` stays as the defensive hidden-supertype arm, and the `Alterator` keeps `String2`: that arm is a dump-shape contract rather than an `is_string` mirror, and dropping it would be grammar-dispatch section 6's narrow-by-deletion. No metric consumes `is_string`, so no snapshot moves. Fixes #1474
Withdrawing `Php::String2` from `Checker::is_string` changes what `bca find --type string` and `bca count --type string` report for every PHP file carrying a type annotation — 24 hits to 12 on the integration corpus's `strings.php`, 27 to 8 on `classes.php` — and nothing in `[Unreleased]` said so. #1261, the TypeScript and TSX analogue, documented exactly this change in its own release section, so the omission is a gap rather than a deliberate silence. Refs #1474
The type-keyword exclusion test guarded its fixture with a boolean `ast_has_kind_id`, which is true for one `string` keyword exactly as for five. Measured: stripping four of the five spellings (property type, parameter, `?string`, `string|int` union) left every assertion passing, so the comment's claim to cover every position could decay silently. The `(string)` cast the comment calls a deliberate negative control was equally unpinned -- deleting it failed nothing. Pin both by count: five `Php::String2` nodes and one `Php::CastType`. The keyword count subsumes the presence guard the boolean gave. Also correct the rationale. `is_string` is a `kind_id` `matches!`, so no narrowing can miss one spelling and catch another; the five positions earn their place against a future ancestor-sensitive `is_string_with_code` override of the kind Tcl already carries. Refs #1474
The C# grammar's `_parameter_type_with_modifiers` aliases the bare tokens `this scoped ref out in readonly` to `$.modifier`, so in parameter position each is a childless `modifier` node rather than its own kind. No arm classified it, so an extension-method receiver, a `ref` / `out` / `in` parameter and a `scoped` or `readonly` one each contributed to neither Halstead half. `Modifier` cannot simply join an arm: every other `modifier` wraps a keyword leaf the getter already classifies, so listing the wrapper would double-count `public` / `static` / `async`. Gate on child-presence instead (grammar-dispatch section 6) and read the token's text, which needs `get_op_type_with_code` — the spelling the walk calls. The role follows the text, never the position: `this` is an operand as in #1380, the other five are operators as their bare kinds already are. Operators are keyed by `kind_id` unless `is_primitive` routes them to the lexeme-keyed map, so classifying the alias alone would either split one `ref` across the two maps and count it twice in `n1`, or collapse all five spellings into one operator that `bca ops` renders as the word `modifier`. `CsharpCode::is_primitive` therefore also lists the five bare kinds; each renders exactly one text, so no existing count moves and no corpus snapshot shifts. Fixes #1418
`get_op_type_with_code` sliced the node span by hand where the getter module already has `node_text` for exactly that, with a documented `None` for a span past the buffer or for non-UTF-8 bytes. Twelve sibling getters call it; here the byte-string arms become string arms and the fallthrough stays the same `Unknown`. Also record why `is_primitive` answers `true` for the `this` receiver, which `get_op_type_with_code` bills as an operand. The two share one predicate and cannot disagree: `compute_halstead` consults `is_primitive` only under its `Operator` arm, so the answer is never read for a receiver. Refs #1418
`csharp_is_aliased_parameter_modifier` tested "childless `modifier`"
and its doc read that as "aliased in parameter position", asserting
that every other `modifier` is the rule itself — a wrapper around a
keyword leaf. The pinned grammar says otherwise:
`_lambda_expression_init` and `anonymous_method_expression` also alias
a bare token onto `$.modifier`, so `static (int x) => x` and `async
delegate { }` produce childless nodes outside any parameter.
Nothing miscounts today. Those nodes fall to the getter's
fallthrough and stay unclassified, exactly as before #1418, and
`is_primitive` is only consulted under `compute_halstead`'s
`Operator` arm, so its widened answer is never read for them. What
was wrong was the reasoning three comments rest on, including one
calling that fallthrough "defensive: the alias set is closed at the
pinned grammar" when it is live.
Rename the predicate to `csharp_is_aliased_modifier`, state both
alias sites in its doc, correct the `is_primitive` and npa/shared
comments, and pin the second population in the shape test: a lambda
fixture whose two aliases must be `static`/`async` and must classify
as `Unknown`. The first walk now matches the six parameter spellings
by name rather than treating "not `this`" as an operator, since that
is false of the kind at large. Verified by perturbation — billing a
lambda's `static` fails only the new assertion.
Refs #1418
The `ref`/`out` "one operator across both kind spellings" assertion ran against CSHARP_PARAMETER_MODIFIERS, which spells `ref`, `in`, `scoped` and `readonly` in parameter position only. For those the count could only ever be one, so the assertion was dead and four of the five bare kinds `CsharpCode::is_primitive` lists had no input: dropping `Csharp::Ref`, `In`, `Readonly` or `Scoped` failed no test in the workspace. Add a fixture pairing each keyword's aliased `modifier` with its own token — a ref local, a field `readonly`, a `foreach` separator, an out argument, a scoped ref local — assert the one-operator count over all five, and guard the pairing itself so the fixture cannot decay back into a vacuous count. Refs #1418
`Checker::is_call` counts call sites only; ABC's Fitzpatrick branch rule also counts object creation and constructor delegation. That was the workspace convention already — pinned since #430 by one Groovy test — but the trait doc was a single line, neither C# site referenced the other, and the book never defined the `call` filter at all. Expand the trait doc into the contract plus the per-language survey: thirteen languages where `branches` exceeds `is_call`, seven where the two coincide, and Tcl / iRules / Elixir, which diverge in both directions. Cross-reference `CsharpCode::is_call` and `csharp_count_token_branch` at both sites. Also record where a primary constructor's superclass call lands. The call sits in the class header — C#'s `base_list`, Kotlin's `delegation_specifiers` — which is a sibling of the primary constructor's parameter list, not a child, so no function space encloses it and the branch is the class's. The classic `: base(x)` / `: super(x)` spelling puts the same call inside a constructor declaration, which does open one. File sums agree; per-function `branches_max` and a per-space `abc` threshold do not. New `csharp_is_call_excludes_constructors` mirrors the Groovy test. Probing every invocation spelling with `bca dump` showed only `InvocationExpression3` is emitted at the 0.23.5 pin, so the other two aliases get grammar-dispatch section 2 drift markers rather than positive assertions. The Kotlin primary-constructor test now asserts per-space attribution instead of only the file sum. No metric or predicate behaviour changes. Fixes #1456
The maintainer half of the new `Checker::is_call` doc — the parallel `matches!()` drift note and the two test names pinning it — named private test items from rustdoc, which a consumer of the published crate cannot see. Demote it to a `//` comment; the contract and the per-language survey stay in `///`. `csharp_is_call_excludes_constructors` also re-imported `CsharpCode` and `CsharpParser`, which `use super::*` already supplies through the module's `use crate::*` — the sibling Groovy test names them unqualified. rustc does not flag that redundancy. Refs #1456
Two of the per-language claims in the new `Checker::is_call` doc do not hold when measured against the code and `bca`: - `set` was named as a Tcl / iRules mutator command that `is_call` counts. It is neither. `set x 1` parses to a `Tcl::Set` / `Irules::Set` production rather than a `command`, so `is_call` never matches it, and the mutator list the ABC walkers test against is `incr` / `append` / `lappend`. - Perl was listed among the languages whose two sets coincide. Its ABC branch arm skips a bareword call whose parent is one of the argument-bearing call wrappers, so `print shift;` measures four `is_call` matches against three branches. Refs #1456
The new semantic-filter list said `-t error` and `-t ERROR` match the same nodes. They do not: the filter dispatches to `Checker::is_error`, which is `Node::has_error` — "is, or contains, an ERROR node" — so it also matches every ancestor up to the root. Measured on a two-line Rust file with one syntax error: `-t error` finds 5 nodes, `-t ERROR` finds 1. Refs #1456
The #1456 tests survived every fixture-decay and production perturbation, but two claims their comments make do not hold as written. The Kotlin test said `class Plain : Marker` was what makes a bare `DelegationSpecifier` arm fail, and that without it only `kotlin_constructor_delegation_is_a_branch` would catch the broadening. Measured: with `Plain` deleted the broadening still fails here at `branches_sum` 4 against 3, because `class Classic : Base` — added by the same change — is an argument-less delegation specifier too. `Plain` still earns its place as the stated negative case that a per-space assertion anchors; it is no longer the sole discriminator. The C# test named three other `argument_list` nodes for the parent assertion to distinguish `Sub`'s from. The fixture parses five in all: the record's `: Base(x)` carries one under `primary_constructor_base_type`. Refs #1456
`bca check --baseline` warns per violation, so an entry whose offender stopped breaching its threshold produces nothing to warn about and only a full regeneration finds it. `baseline-freshness.yml` runs that regeneration quarterly — on the 22nd, a week after the last of the three existing out-of-band crons — and diffs the result against the committed file with `bca diff-baseline`, which pairs entries on `(path, qualified, metric)` and so does not read a moved `start_line` as staleness the way `git diff` would. Unlike the three precedents it deduplicates: a stale baseline survives until someone commits a refresh, so a later run comments on the open `self-scan` issue rather than opening a second one. A `make` or tool failure ends the step before the output the filing step keys on, so an infrastructure error files nothing. The `self-scan` and `mutation-testing` labels were created in the tracker rather than in this commit; `mutation-test.yml` already passed the latter, so its filing step would have failed on an unknown label. Fixes #1473
A GitHub issue body caps at 65 536 characters and the filing step pasted `diff.md` whole. A diff that pairs nothing — what an `.bcaignore` edit or a baseline-anchor change produces — renders one row per entry on each side: 28 446 bytes for today's 240 entries, roughly twice that for a remove + add, and the file only grows. Over the cap `gh issue create` fails 422, `set -e` ends the step, and the finding is lost on a scheduled run nobody is watching — the one outcome the job exists to prevent. The three sibling jobs already bound their excerpts; this one now caps at 200 lines and points at the artifact, closing the `diff-baseline` fence first when the cut lands inside one. Also record why the diff step exits 1 on staleness. Nothing in the tree is wrong there, so "make it green" is a plausible later edit, and it would silently stop the `failure()`-gated filing step from ever running. The book said the job regenerates "at the tier the committed file records". It does not read `[provenance]` — it pins `self-scan-write-baseline-headroom`, which the committed file happens to match. Say that, since the page offers the shape to copy. Refs #1473
Record the policy #1395 asked for: punctuation and delimiters are Halstead vocabulary and classify as operators whatever grammatical role they serve, so a decision metric gating the same token by role is a deliberate disagreement rather than drift. ABC's exclusion of a non-comparison `<` / `>` (#1274, #1275, #1280, #1297) is a decision-metric concern only. The one getter-side suppression the policy admits is a literal's own delimiter, where the enclosing literal node is itself the operand — the five arms behind #1256, #1312, #1314, #1360 and Groovy's slashy string. Written up on `Getter::get_op_type` and in the book's Halstead section, with a clause on the ABC deviations row so the exclusion cannot read as global. Applying the policy closes one real gap. `LTSLASH` (`</`) and `SLASHGT` (`/>`) were in neither arm of `impl_js_family_get_op_type!`, so a JSX element billed its opening `<` and its `>`s and nothing for its closers: `<br />` reported one bracket operator where the source spells two. They join the operator set as per-language extras for JavaScript, MozJS and TSX — TypeScript is the one grammar of the four with no JSX and so has no such variants. `Perl::GT2` turned out not to be a gap. tree-sitter-perl closes `<FH>` with `token.immediate('>')`, which gets its own symbol, but the generated `public_symbol_map` collapses it onto `GT` before `kind_id()` — as tree-sitter-cpp does with the `GT2` in its own operator arm. Listing it would be inert today and wrong the day it became live, since `compute_halstead` keys a non-primitive operator on `kind_id()` and two live kinds rendering `">"` would split one operator across two `n1` entries. Pinned as unreachable instead. `tests/parity/abc_halstead_bracket_parity.rs` holds both sides together over seven rows. Verified by perturbation: dropping `LT` from the Lua getter, `GT` from the Perl getter, the JSX extras from the JavaScript call site, or either language's ABC `BinaryExpression` gate fails it, and the unperturbed tree passes. Fixes #1395
The delimiter policy #1395 recorded on `Getter::get_op_type` rests on two statements about the existing impls, and both were wider than the tree. "Every impl here already bills `(`, `{`, `[`, `,`, `;`, `.` and `:`" is false for two: `getter/python.rs` classifies only `,` and `.` of that set — no `(`, `[`, `{`, `;` or `:` appears in it — and `getter/rust.rs` bills `COLONCOLON` but no `COLON`. Offered as evidence that the reading is already the status quo, the universal invites a later pass to "finish" Python and move every Python Halstead score. "Five arms" suppress a literal's own delimiter; nine do. The enumeration missed Ruby's regex `/` (a separate arm from the subshell backtick it does name, #1312), the raw-string `(` of `getter/cpp.rs` and `getter/mozcpp.rs` (#1314), and `getter/objc.rs`'s `@` before an `NSString` literal. Each is the same parent-guarded shape and cites grammar-dispatch section 5, so an audit counting five would read three of them as policy violations. Refs #1395
The Tsx, Javascript and Mozjs rows of `fixture` carried three byte- identical copies of the source, the operator vocabulary and `N1`, differing only in the cosmetic extension. The Mozjs row's comment claims a delta between the fork and upstream JavaScript would be a fork divergence — true only while the three rows feed the same input, which three literals leave a convention rather than a property. An edit to one row would break it in silence, since each row's expectations are self-consistent. Name the three values once as `const`s and have the rows reference them. They sit inside `fixture` so they inherit its `cfg`: at module scope `check-test-lang-gates` derives the same 23-feature union for each, which would be three more copies of the block the file already spells twice. No assertion changes; the suite is unaffected. Refs #1395
The ABC deviations row listed TypeScript among the grammars whose JSX tag delimiters the `<` / `>` gate excludes, but the `.ts` dialect of tree-sitter-typescript has no JSX production at all: its enum carries only `JsxText`, and `<div>…</div>` in a `.ts` file parses as nested `type_assertion` / `binary_expression` nodes. Nothing there is ever excluded as a JSX delimiter. The contradiction became visible in this range: the new policy paragraph on `Getter::get_op_type` says TypeScript is the one grammar of the four with no JSX, which is why `LTSLASH` / `SLASHGT` are per-language `op_extras` rather than members of the shared arm. The row now names the three grammars that do have JSX and says TypeScript carries the same gate through the shared macro with nothing to exclude. Refs #1395
`Getter::braced_word_op_type` asked `is_value_braced_word` alone, so a
braced word filling a *value* slot of a script-taking command still
reported a `{}` operator for a block the source does not contain —
`after`'s millisecond count, `time`'s iteration count, a braced `proc`
name, `namespace export`'s patterns, `namespace ensemble create -map`'s
dictionary, a `trap` or `on` handler's error code and variable list.
`Checker::is_string_with_code` and `Alterator::keeps_children` have
called those bytes a literal since #1381, so the three classifiers
disagreed on one node (grammar-dispatch §7) and `after {100} {puts hi}`
scored N1 2 against bare `after 100 {puts hi}`'s 1 — the score moving
with the author's choice of delimiter.
That call site now asks `is_braced_script_word`, the predicate the other
two ask. Tcl and iRules `halstead.*` move for the constructs above; no
operand column moves anywhere, which is the #1318 invariant.
The issue's other half — a braced value scoring one operand under `set`
and one per word under `lappend` — is closed as the contract rather than
fixed. Reading the contents was measured and rejected by #1318, and a
value-taking command allowlist needs an open set where the script-taking
one is closed. Both are recorded on `braced_word_op_type` and in the
book.
Fixes #1382
`is_braced_script_word` states that it is documented where it is "rather than in each of the five call sites" so the string, dump and Halstead classifiers cannot drift apart. #1382 then re-narrated the same history on `braced_word_op_type`: the same #1318-to-#1382 exception, the same `after {100} {puts hi}` example with the same N1 2-against-1 numbers, plus a list of constructs that `is_braced_literal_slot` and `lang_helpers::tcl_family` already carry. Keep the sentence that says what this call site asks and why, and point at the owner for the rest. Refs #1382
Tcl models neither `on` nor `trap` outside a `try`, so a top-level
`try {…} trap {p} {v} {b} finally {c}` parses the whole tail as one
generic `trap` command with five arguments. `ScriptSlots::Last` read
"the script is the last argument" off that, naming the `finally` body
and calling the handler body a value — so #1382 withdrew the `{}` of a
block the source does contain (N1 6 to 3), `bca find --type string`
reported it, and the `Ast` dump flattened it away. That last is the
#1381 defect, reopened for one shape.
`LastOf(n)` carries the clause's documented arity. An argument count
the signature does not admit means the grammar grouped something else
in, so no position in the list is trustworthy and every argument keeps
the construct-wide script answer — the same fallback
`argument_slots_are_readable` gives for an ERROR token holding a slot.
`an_error_occupying_a_slot_withdraws_the_layout` loses one argument so
that guard stays the only one its fixture trips; both new rows were
verified by reverting the arity check, which fails them and nothing
else.
Refs #1382
#1382 moves `halstead.n1` / `N1` for every Tcl and iRules braced value slot, and the CHANGELOG's own stability note requires each metric drift to be called out in the entry that introduces it. Both siblings (#1318, #1381) have one; this one had none. Three corrections travel with it. The book attributed every value slot to `lang_helpers/tcl_family.rs`, but the braced `proc` name and the defaulted parameter are positions the grammar names and are read off the tree. `on` and `trap` were missing from the book's list of script-taking commands although the paragraph below it names their value slots. And #1381's entry said a multi-line `try … trap` clause always lands inside an error node, which holds only inside a `proc` or `namespace eval` body — at the top level it parses cleanly, which is how the `finally` defect went unseen. Refs #1382
`an_error_occupying_a_slot_withdraws_the_layout` asserted only that
the error fixture spells no value slot, with nothing pinning that the
clause spells one when well-formed. Renaming `on` out of
`SCRIPT_TAKING_COMMANDS` empties both fixtures, so the test passed
vacuously; it now fails on the added well-formed row, verified by that
perturbation. Its two siblings already anchor this way.
Two comment claims in `BRACED_WORD_VALUE_CASES` did not survive
measurement. The `time` pair's discriminating row is the *bare*
spelling, not the braced one: giving `time` `after`'s slot rule
("argument 0 is the value") leaves `time {puts hi} {3}` at
[1, 1, 4, 4] — the `{}` withdrawn from `{puts hi}` is the one `{3}`
gains — while `time {puts hi} 3` drops to n1 0 / N1 0. And the eight
trailing rows are not all pairs; only the first four are.
Measured alongside, no change needed: both `switch` guard rows do
reach `is_switch_arm` through `is_braced_literal_slot` (N1 2 -> 1 and
6 -> 5 with it removed there alone); every new row's operand
vocabulary moves under the #1318-rejected rule that collapses a value
word's contents; and the arity and error guards fail disjoint tests.
Refs #1382
The #1395 policy paragraph said the only suppression its reading admits is a literal's own delimiter "where the enclosing literal node is itself the operand", and counted nine arms. Tcl and iRules have a tenth, `braced_word_op_type`, and it does not fit that qualifier: a braced value's operands are the words inside it, not the braced word, so `puts {c d}` reports no operator at all while its three operands come from the interior. #1382 widened that arm on this same branch, which makes the omission larger rather than smaller. Restate the rule as one suppression with two shapes and name both, in the trait doc and in the CHANGELOG entry that repeats it. Refs #1395
The entry said `n1` and `N1` both fall by one per braced value slot.
Only `N1` does: `n1` counts the vocabulary, so it moves only when the
withdrawn `{` was the space's sole `{}`. The branch's own fixture
table shows both outcomes — `after {100} {puts hi}` holds n1 at 1
while `namespace export {a b}` goes 2 to 1.
Refs #1382
"The other out-of-band gate" dates from when there were two; with fuzzing the third and baseline-freshness the fourth, benchmarking is the second. The `string` semantic filter's exclusion example now names PHP beside TypeScript, since #1474 put its `: string` keyword in the same set. Refs #1473, #1474
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1488 +/- ##
========================================
Coverage 98.02% 98.02%
========================================
Files 359 359
Lines 95916 96216 +300
Branches 95485 95785 +300
========================================
+ Hits 94022 94318 +296
- Misses 1213 1214 +1
- Partials 681 684 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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.
Summary
Six issues fixed on one integration branch, each through its own fix →
simplify-rust→rust-optimize→review→audit-testspipeline,then a whole-branch review.
make pre-commitreportsBCA_GATE: pass.PhpCode::is_stringno longer matchesString2, thestringtype keyword;bca find/count -t stringreport literals only (PHP was the sole outlier across all 29 impls)this scoped ref out in readonly, which the grammar emits as a childlessmodifier— are now billed by Halstead:thisas an operand (#1380), the rest as operators, keyed so both spellings of one keyword count onceChecker::is_callcontract documented (call sites only; construction and delegation are ABC branches — the convention in every language) with a C# pinning test; primary-constructor attribution to the class space documented and pinned for Kotlin; book gains a Semantic filters sectionbaseline-freshnessworkflow (cron on the 22nd): regenerates.bca-baseline.toml, verdict frombca diff-baseline --exit-code, files or comments on oneself-scanissue, body capped under GitHub's limit</and/>now billed in JavaScript / MozJS / TSX;Perl::GT2pinned enum-only;tests/parity/abc_halstead_bracket_parity.rsfails if either side is later "made consistent"braced_word_op_typeasks the sharedis_braced_script_word, so Tcl / iRules value slots of script-taking commands (after {100} …,proc {my proc},namespace export {…}) stop billing a{}; an arity guard keeps a top-leveltry … trap … finallyread as scripts; the operand asymmetry is documented as the contractMetric drift
halstead.*rises for C# sources with parameter modifiers (fix(getter/csharp): an extension-methodthisis neither operator nor operand #1418).halstead.n1/N1rise for JSX input (chore(halstead): decide whether non-comparison < / > are operators #1395).halstead.N1falls by one per Tcl / iRules braced value slot;n1only where that brace was the space's sole
{}(fix(getter): Tcl braced values score by which command takes them #1382).No integration snapshot moved: no corpus contains Tcl, iRules, JSX or a
C# parameter modifier.
Verification
BCA_GATE: pass (gate=pre-commit)on the final tree.make check-test-lang-gates-compare COMPARE_REF=mainOK.assertion-failure arms inside one new test, so every production line
is covered.
Follow-ups filed
#1482 (lambda
static/asyncunbilled), #1483 (JsxTextunbilled),#1484, #1485, #1486, #1487.
Fixes #1474
Fixes #1418
Fixes #1456
Fixes #1473
Fixes #1395
Fixes #1382