fix: ten quick-fix issues (ABC gates, VCS saturation, enums codegen) - #1311
Merged
Conversation
The superclass clause of a class definition spells its `<` with the same LT token as a comparison, so every subclass declaration scored a phantom ABC condition. Count LT/GT only under a `binary` parent — the positive polarity the other guarded languages use, and forward-proof against future non-comparison `<` uses. It also stops counting the `<` that names an operator method. Fixes #1280
super(...)/this(...) in Java, : base(...)/: this(...) in C#, and : super(...) in Kotlin each parse as their own production rather than the call kind those branch counters matched, so a delegation scored zero where Groovy scored one for identical source. None wraps a call node for the delegation itself, so no double count arises. Fixes #1279
var x = 5 and x := 5 are the same binding spelled two ways, but only the latter matched an assignment arm. Count a var_spec with a value field, which covers both var x = 5 and var y int = 6 without matching an uninitialized var z int. Matching the spec rather than the declaration counts each initialized line of a grouped var block. Fixes #1278
Bash's only ternary form scored zero cyclomatic and zero cognitive while every sibling with a ternary counts one. Add it to both cyclomatic tiers and to the cognitive nesting arm, matching the C-family ConditionalExpression treatment. A drift-marker test pins the defensively-listed TernaryExpression2 alias as unreachable at the pinned grammar. Fixes #1268
build_cached and BlameWalk::new computed their window boundaries with a plain subtraction while four sibling sites saturated, each carrying its own copy of the hazard comment. Both operands are attacker- adjacent, so the plain form panics in a debug build and wraps to a future-dated boundary in release. Route all six through one window_boundary helper that documents the invariant once and is directly unit-testable at the i64 extremes. days_between carried the same hazard under a .max(0) clamp a wrapped delta defeats. Fixes #1271
DEFAULT_BOT_PATTERN's doc has promised case-insensitive matching since the option shipped, but BotFilter::new compiled a plain Regex and push_if_human matches raw signature bytes, so a capitalised bot identity was counted as a human author. Compile with RegexBuilder::case_insensitive, leaving (?-i) as the opt-out. Bump CACHE_SCHEMA_VERSION: the fingerprint hashes only the pattern string, so a cached event log would otherwise replay under the old semantics. Fixes #1265
The two exhaustiveness-claiming tests were plain Vecs asking to be kept in step, and #956 showed they are not. #1245 fixed the client-input half by generating its list from production, but left the environment half hand-checked with nothing able to enumerate it — the file's own comment said so. Add a test-only wildcard-free variant_index match: adding a variant now fails to compile the file that must gain the new cases. Both lists route through a coverage bitmap over it, so an unlisted variant of either kind fails. VARIANT_COUNT is guarded in both directions. Fixes #1269
format_regressed_tag computed its percentage in the higher-is-worse direction and prefixed a literal +, so a lower-is-worse mi.* metric — where the classifier emits Regressed on a drop — rendered the double-signed [regr +-29%]. Measure the magnitude in the metric's own direction so the tag keeps one greppable shape, and correct the two comments and two book passages that described the old behaviour. Fixes #1242
No production caller passed escape=true: #862 established the JSON generator was the last one and was double-escaping by mistake. Removing the flag deletes the dead double-backslash branch, its test, the JSON_TOKEN_ESCAPE constant that existed only to document why the answer must be false, and a boolean from every call site — making the #862 bug unrepresentable rather than one flipped boolean away. Codegen output is byte-identical. Fixes #1241
get_token_names minted a suffixed name (Foo -> Foo2) and returned it without registering it, so a mint could silently duplicate a node kind whose own sanitized name is literally base + digits. tree-sitter-php is one aliased rule away: it already emits cast_type_token1 through cast_type_token12, so a single new id carrying cast_type_token1 would mint a second CastTypeToken12. The generator would exit 0 and the duplicate would surface as a rustc error in the parent crate during a grammar bump. Route both mint sites through a claim_name helper that probes upward past a squatter and registers what it hands out, and widen the uniqueness test from Rust alone to every linked grammar. Codegen output is byte-identical. Fixes #1237
Four movements, all consequences of this branch: the Go ABC var_spec arm raises GoCode::compute's cyclomatic and halstead.effort, the direction-aware regression tag adds eight ploc to thresholds.rs, and build_cached got cheaper now that its boundary arithmetic is a helper call. The fifth is an identity shift, not a regression: removing four comment lines from history.rs renamed the anonymous space diff_collect::<anon@L217> to <anon@L213> at an unchanged nexits of 6.
Code review found the #1280 polarity applies in two more places. Bash spells an I/O redirection with the same < and > tokens as a comparison, parented under file_redirect, so every redirect in a script scored a condition — the Bash instance of the bug #1280 fixed for Ruby, missed by that issue's cross-language sweep. Both tokens now require a binary_expression parent. Ruby's gate covered only LT/GT, but every comparison and equality token is equally definable as an operator method, so def ==(other) kept the phantom condition def <(other) lost. The gate now covers all eight. Bash ABC also scored zero for the arithmetic ternary while #1268 had already brought it into cyclomatic and cognitive; ABC now matches the C-family, so Bash and C agree at 2 for the same expression.
replace_symbol_body dropped days_between's /// contract (whole days, clamped at zero so a clock-skewed commit reads as today), leaving only the implementation note about saturation. The C# branch counter lost its lead line the same way, so an arm-specific note read as the function's description while Java's sibling kept its own.
Four merges, no new entries and no renumbering: the polarity of a grammar-role gate (#1274, #1280) into 89, a report's site list not being a census (#1271) into 59, a false claim about a sibling language holding a wrong expected value (#1278) into 84, and an alias that is unreachable without being hidden (#1268) into 34.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1311 +/- ##
========================================
Coverage 98.38% 98.39%
========================================
Files 278 278
Lines 72352 72567 +215
Branches 71922 72137 +215
========================================
+ Hits 71181 71399 +218
+ Misses 762 761 -1
+ Partials 409 407 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Codecov flagged the PR's only patch gap as a partial branch in cyclomatic.rs: the drift-marker helper paired node.child(i) with an `if let Some(..)` whose None arm is unreachable for i < child_count(). Three modules each carried a byte-identical copy of that walk, so the same unreachable arm was measured three times. Replace all three with one test_support helper over Node::preorder, which has no Option to discriminate. Net -43 lines and the partial is gone by construction. Behaviour is unchanged: preorder visits the same nodes, and `any` short-circuits exactly where the early `return true` did.
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 ten
quick-fixissues, plus the follow-on findings from a code-review pass.Issues
<of a superclass clause as a comparison condition.LT/GTnow require abinaryparent — the positive polarity the other guarded languages use.var x = 5. An initializedvar_specis now one assignment, matching Rust, Java, C, C++, C#, JS, Lua and PHP.-. All six now route through onewindow_boundaryhelper.vcs::Errorexhaustiveness tests could not see a new environment variant. A compile-forced witness closes it.CACHE_SCHEMA_VERSIONbump.bca check --baselinerendered a malformed[regr +-29%]for everymi.*regression.escape=truebranch from theenumsgenerator.enumsminted collision-breaking names without registering them;tree-sitter-phpwas one aliased rule from a duplicate variant.Where the issues were wrong
Issue bodies were treated as evidence, not specification.
BlameWalk::newused a plain-for both boundaries, anddays_betweencarried the same hazard under a.max(0)clamp that a wrapped positive delta defeats.var y = 1is not counted "matching the Rust/Java rule" — both languages do the opposite. The expectation moved 6 → 7.Code-review follow-ups
A review pass found the #1280 polarity applies in two more places, both included here:
>and<spell a redirect as well as a comparison. This was missed by fix(abc/ruby): superclass < counts as a comparison condition #1280's own cross-language sweep — including mine, which checked Bash's module and dismissed it for having "no<type syntax".LT/GT. Every comparison and equality token is equally definable as an operator method, sodef ==(other)kept the phantom conditiondef <(other)lost.Metric drift
Behaviour changes, each called out in
CHANGELOG.md:abcdrops one per subclass declaration and per comparison-operator method definition.abcdrops one per redirection, rises one per arithmetic ternary;cyclomatic/cognitive/mirise for arithmetic ternaries.abcrise one per delegating constructor.abcrises one per initializedvarspec.CACHE_SCHEMA_VERSION1 → 2 forces one benign cold VCS walk..bca-baseline.tomlis refreshed in the same branch. No integration-snapshot drift — the corpora contain no Ruby, Go, Bash, Java or C# sources reaching these paths.Verification
make pre-commit→BCA_GATE: pass.enumscrate is workspace-excluded and outside the coverage run; its changes carry three new tests plus a greenenums-codegen-driftproving byte-identical output.#1237and#1241produce zero codegen drift by construction.Deliberately out of scope
</>with a denylist rather than the positive form. Correct for the roles someone thought of; recorded in lesson 89.Fixes #1280
Fixes #1279
Fixes #1278
Fixes #1271
Fixes #1269
Fixes #1268
Fixes #1265
Fixes #1242
Fixes #1241
Fixes #1237