diff --git a/.bca-baseline.toml b/.bca-baseline.toml index 86835f9e6..8e184ad8e 100644 --- a/.bca-baseline.toml +++ b/.bca-baseline.toml @@ -915,7 +915,7 @@ value = 15.0 path = "src/metrics/abc/kotlin.rs" qualified = "KotlinCode::compute" metric = "cyclomatic" -value = 18.0 +value = 20.0 [[entry]] path = "src/metrics/abc/objc.rs" @@ -1203,7 +1203,7 @@ value = 7.0 path = "src/spaces/compute.rs" qualified = "metrics_inner" metric = "halstead.effort" -value = 119147.7514530567 +value = 116715.6053686016 [[entry]] path = "src/spaces/compute.rs" diff --git a/CHANGELOG.md b/CHANGELOG.md index 9da18865e..f99adf468 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,150 @@ for historical reference. ### Fixed +- **ABC now counts a boolean test that the grammar gives its own + production** (#1449). A construct spelled as a dedicated node rather + than a `binary_expression` reaches no comparison-token arm, so Groovy's + `in` / `!in` / `===` / `!==` / `=~` / `==~`, Perl's bare `//` and `m{}` + matches, and Ruby's `a in Integer` each scored zero conditions in an + `if` predicate or a `&&` operand where a bare identifier scores one. + Groovy's `===` / `!==` / `=~` / `==~` are counted as comparison + operator tokens rather than terminal kinds, so they now also score + outside a boolean slot, as `==` already did; `in` / `!in` cannot take + that route, because `in` is shared with `for_in_statement` and `!in` + emits no operator token at all. **Metric drift:** `abc.conditions` + rises by one per occurrence of these constructs in a boolean slot, and + by one per `===` / `!==` / `=~` / `==~` anywhere in Groovy. + +- **Kotlin ABC counts a null-asserted or cast condition, and C# ABC + counts `??`** (#1459). Both languages model a condition slot that + delegates to a per-language helper, and both helpers contributed + nothing for a valid boolean expression outside their allowlist. In + Kotlin, `when { a!! -> … }` and `when { a as Boolean -> … }` scored + zero against a decision count of one: `unary_expression` is a single + kind for both the prefix `!x` and the postfix `x!!`, so the slot routed + the null assertion in as handled while the peel looked for the operand + on the wrong side of the token, and an `as` cast was not recognised at + all. The peel now reads its operand through the grammar's fields and + unwraps parentheses, `!`, `!!` and `as` in any combination; the safe + cast `as?` is deliberately left to the `AsQMARK` token that already + counts it, so the two cast spellings come out level rather than the + safe one scoring twice. In C#, `??` was a cyclomatic decision that no + ABC arm counted, so `when b ?? false` scored one fewer condition than + `when x > 2`, `when E(x)` or `when x is int` — the spelling-dependence + #1422 exists to remove. It joins the condition tokens, matching the + JS/TS family, which has always counted it. `??=` is unchanged and + remains an assignment, as in the JS family. **Metric drift:** Kotlin + `abc.conditions` rises by one per `!!` or `as` cast standing as a + predicate or a `&&` / `||` operand; C# `abc.conditions` rises by one + per `??` anywhere. + +- **C# comparison-operator overloads no longer score a spurious + condition** (#1420). C# overloads six comparison operators and #1297 + gated only `<` and `>`; the other four are distinct tokens that + reached a different arm, so `operator <=`, `operator >=`, + `operator ==` and `operator !=` each scored one ABC condition on their + *declaration*, where the token names the operator being defined rather + than applying it. All six now share one arm gated on a + `binary_expression` parent, which also subsumes #1383's + `relational_pattern` denial and fails closed on a grammar bump. + **Metric drift:** C# `abc.conditions` falls by one for each + `operator <=` / `>=` / `==` / `!=` declaration. `operator <` and + `operator >` already scored zero under #1297's allowlist, so a type + overloading all six — which C# requires to be declared in pairs — + falls by four, not six. + +- **C# ABC counts a primary-constructor base call** (#1406). C# 12 lets a + class, struct or record declare its constructor in the header and pass + arguments to its base there — `class Sub(int x) : Base(x)` — which + invokes the base constructor exactly as the `: base(x)` initializer + added in #1279 does, but scored nothing. It is now one branch, in both + spellings tree-sitter-c-sharp uses: the record form nests the arguments + under a `primary_constructor_base_type`, the class form hangs them + straight off the `base_list`, so one node would have covered only half + the declarations. The arm is gated on that `base_list` parent, since an + `argument_list` is otherwise the argument list of every call in the + file. A base type passing no arguments (`struct S(int x) : IBase`) is + unchanged at zero. This is the C# sibling of Kotlin's #1384. + **Metric drift:** C# `abc.branches` rises by one per type passing + arguments to its base from a primary constructor. + +- **A C# `when` guard now counts as a decision in both cyclomatic + complexity and ABC** (#1422). Cyclomatic had no arm for either guard + spelling — `when_clause` on a switch arm or `case` section, and + `catch_filter_clause` on `catch (E e) when (…)` — so a guarded arm + scored one decision where it has two ways to fail: the pattern does not + match, or it matches and the guard is false. ABC counted whatever + operator happened to sit inside the guard, so `when x > 2` scored one + condition while the equivalent `when IsEven(x)` scored none. The guard + is now a condition slot like an `if` condition, so every spelling + scores exactly one and a compound guard keeps its sub-structure. The + same change adds the bare type test `x is int` to C#'s terminal-bool + operand set: it is `is_expression`, a distinct kind from the + `is_pattern_expression` of `x is int y`, and only the latter was + listed — so the two spellings of one test disagreed, `if (x is int)` + scoring zero conditions against a cyclomatic decision of one. + **Metric drift:** C# `cyclomatic` (standard and modified) gains one per + guard; `abc.conditions` gains one for any guard not already + operator-shaped, and one for every bare `is` type test in a boolean + slot. `wmc` and `mi` are derived from cyclomatic and move with it — the + C# corpus snapshot records `class_wmc_sum` 27 → 29 and a matching fall + in all three `mi` variants — so a `wmc` or `mi` threshold can newly + fire on an unedited C# file carrying guarded arms. + +- **Kotlin no longer double-counts a subject-less `when` arm's comparison + operator** (#1421). `when { x > 5 -> 1; x < 0 -> 2; else -> 0 }` + reported 4 conditions against a cyclomatic decision count of 2, because + the arm added a blanket one on top of the comparison the token arms + already scored; all six comparison spellings were affected, not only + `<` and `>`. A subject-less arm now scores its condition through the + same slot an `if` predicate uses, while a subject-ful arm keeps its + per-entry count — its pattern is not an independent boolean expression, + so the implicit `subject == pattern` is a decision nothing in the + source spells. As part of the same fix, Kotlin's `is` and `in` tests + count as conditions wherever a boolean is evaluated: `if (a is String)` + and `if (a in 1..2)` previously scored zero against a decision count of + one, as did `a and b` — Kotlin spells boolean `and` / `or` / `xor` as + infix *functions*, so they parse as `infix_expression` rather than as + a binary expression and no token arm ever saw them either. + **Metric drift:** Kotlin `abc.conditions` falls for subject-less + `when` arms carrying a comparison, and rises for any `is` / `in` test + and any infix boolean call in a boolean slot. + +- **An enum constant carrying constructor arguments is counted as a + branch** (#1407). `A(1)` in `enum E { A(1), B, C(2); }` invokes the + enum's constructor, so it is an object construction under + Fitzpatrick's rule — the same as the `super(…)` / `this(…)` and + `class Sub : Base(1, 2)` forms counted since #1279 and #1384 — and + scored zero in Kotlin, Java and Groovy alike. Each arm is gated on the + entry's argument-list child, so a constant with no arguments (`B`), and + every constant of an enum with no constructor, stays at zero; Java's + gate names `argument_list` specifically, so an annotated constant + cannot satisfy it through the distinct `annotation_argument_list` + production. **Metric drift:** `abc.branches` rises by one per + argument-carrying enum constant in Kotlin, Java and Groovy. + +- **A truthy numeric literal in a boolean operand slot now scores a unary + condition in PHP, Groovy and the C family** (#1410). `abc.conditions` + keys on a per-language terminal-bool kind set, and six of them named no + numeric kind — the last deferrals from #1379. `$a && 1` scored one + condition against `$a && $b`'s two; `def f(a) { a && 1 }` the same in + Groovy; and in C, C++, Mozcpp and Objective-C the omission showed + *within* one language, `if (true)` scoring one condition and `if (1)` + none, because `"true"` was in the set and `number_literal` was not. + Each set now names every numeric kind its grammar emits — `integer` / + `float` for PHP, one consolidated `number_literal` for Groovy and for + each C-family grammar — verified by `bca dump` per language rather than + by an alias sweep, which is the check #1379's first cut skipped. The + C-family set also gains `char_literal`: `'c'` has integral type and is + truthy exactly as a number is. PHP's `float` *type* keyword, C++'s + `user_defined_literal` and Objective-C's `version_number` are + deliberately excluded — none is a truthy value literal. **Metric + drift:** `abc.conditions` and `abc.magnitude` rise for PHP, Groovy, C, + C++, Mozcpp and Objective-C code using a numeric (or, in the C family, + character) literal as a bare `&&` / `||` operand, an `if` / `while` / + `do`-`while` / `for` condition, or a ternary condition — `while (1)` + and `do { … } while (0)` being the common idioms. + - **Single-language feature subsets build their tests again, and CI gates them** (#1426). Five of the twenty-two single-language subsets of `-p big-code-analysis --all-targets` failed, all in diff --git a/big-code-analysis-ast/src/macros/kind_sets.rs b/big-code-analysis-ast/src/macros/kind_sets.rs index 187fc95c5..09493e1d4 100644 --- a/big-code-analysis-ast/src/macros/kind_sets.rs +++ b/big-code-analysis-ast/src/macros/kind_sets.rs @@ -53,7 +53,7 @@ macro_rules! csharp_prefix_unary_expr_kinds { // the C# grammar. Anything in this set, when it appears in a known- // boolean context (if / while / do / for / ternary / binary), counts // as one condition. The set bundles `csharp_invocation_expr_kinds!()` -// with the bare `Identifier` / `BooleanLiteral` leaves *and* the five +// with the bare `Identifier` / `BooleanLiteral` leaves *and* the six // expression kinds whose evaluated value is implicitly boolean in any // idiomatic codebase: // @@ -61,11 +61,25 @@ macro_rules! csharp_prefix_unary_expr_kinds { // - `AwaitExpression` — `await CheckAsync()` // - `CastExpression` — `(bool)v`, `(IDisposable)x is not null` // - `IsPatternExpression` — `x is null`, `x is not Foo f` +// - `IsExpression` — `x is int`, the bare type test // - `ElementAccessExpression` — `flags[0]`, `dict["key"]` // // Before #372 only the first three (invocation / identifier / // boolean) were recognised, so all five kinds above silently scored // zero conditions in `if` / `while` / `do` / ternary contexts. +// +// `IsExpression` (391) and `IsPatternExpression` (392) are distinct +// kinds, not aliases: the grammar emits the first for a bare type test +// (`x is int`) and the second only once a pattern is involved +// (`x is int y`, `x is null`, `x is not Foo`). Listing only the second +// scored `if (x is int)` zero conditions against a cyclomatic decision +// of one, while `if (x is int y)` scored one — an asymmetry between two +// spellings of the same test, and the C# half of the gap #1421 closed +// for Kotlin by adding `IsExpression | InExpression` there. It also +// left #1422's guard slot spelling-dependent in the one case that fix +// claims to have fixed: `when x is int` scored 1 where +// `when IsEven(x)` scored 2. No arm counts the `is` token itself, so +// there is nothing to double-count (§5). #[macro_export] #[doc(hidden)] macro_rules! csharp_bool_terminal_kinds { @@ -79,6 +93,7 @@ macro_rules! csharp_bool_terminal_kinds { | $crate::Csharp::AwaitExpression | $crate::Csharp::CastExpression | $crate::Csharp::IsPatternExpression + | $crate::Csharp::IsExpression | $crate::Csharp::ElementAccessExpression }; } @@ -139,13 +154,71 @@ macro_rules! java_bool_terminal_kinds { // `cast_expression` inside `parenthesized_expression`). The set bundles // the bool-evaluating terminals added by #372 (`FieldAccess`, // `CastExpression`, `ParenthesizedTypeCast`, `InstanceofExpression`); -// the dekobon Groovy grammar has no `await` or `array_access` -// analogues, so those collapse out of the C# set. +// the dekobon Groovy grammar has no `await` analogue, so that one +// collapses out of the C# set. +// +// It DOES have an indexing analogue, and four navigation kinds beside +// it — `subscript_expression`, `safe_subscript_expression`, +// `safe_navigation_expression`, `safe_chain_dot_expression` and +// `direct_field_access_expression`, all alternatives of `_expression` +// and all legal in a boolean slot. None is listed, so `if (l[0])`, +// `if (a?.b)` and `if (a.@b)` score zero where `if (a)` scores one, +// while C# scores `l[0]` through `ElementAccessExpression` and Kotlin +// scores both through `IndexExpression` / `NavigationExpression`. +// `a?.b` is the worst of them: Groovy cyclomatic counts `?.` as a +// decision, so ABC sits two below its own decision count on an +// idiomatic predicate. Tracked separately — an earlier revision of +// this comment claimed the analogue did not exist, which is the sort +// of claim that stops the next reader looking. +// +// Groovy truth makes every non-zero number truthy, so `NumberLiteral` +// is a unary condition here for the same reason Python's `Integer` / +// `Float` are. Without it `a && 1` scored one condition against +// `a && b`'s two (#1410). +// +// One kind covers every spelling: the dekobon grammar consolidated the +// per-radix rules the prior one split (`getter/groovy.rs`), so `0x1f`, +// `0b101`, `017`, `1_000`, `1e3` and every type suffix (`1L`, `1.5f`, +// `1G`, `1I`, `1.5d`, `1.2g`) all lex as `number_literal` — verified by +// `bca dump`, not read off the grammar, because #1379's misses were +// sibling rules under a hidden choice rather than aliases of one rule. +// +// #1379 deferred this on the stated grounds that "the integration +// corpora carry Groovy files"; they do not. No corpus carries a +// `.groovy` file at all, and DeepSpeech's 16 `.gradle` files are +// outside every test glob (`tests/corpus/deepspeech_test.rs` globs +// `*.cc` / `*.cpp` / `*.h` / `*.hh`), so this fix moves no snapshot. +// +// `membership_expression` (`a in l`, `a !in l`) is the Groovy spelling +// of Kotlin's `in_expression`, and joins the set for the same reason +// #1421 added that one: the grammar gives membership its own +// production rather than a `binary_expression`, so no comparison-token +// arm ever sees it and `if (a in l)` scored zero conditions against a +// cyclomatic decision of one. // -// FIXME(#1410): Groovy truth makes every non-zero number truthy, so this -// set is missing the numeric literal kinds — `a && 1` scores one -// condition where `a && b` scores two. Deferred out of #1379 because the -// integration corpora carry Groovy files and the fix moves snapshots. +// It is the one Groovy relational form that has to come through the +// terminal set rather than through `groovy_count_token_condition`'s +// token arm, and a `grammar.json` sweep of dekobon-tree-sitter-groovy +// 0.2.2 says why on both halves: the `in` token is shared with +// `for_in_statement`, so an ungated token arm would score every +// `for (x in list)` header, and the `!in` spelling emits **no operator +// token at all** — `bca dump` shows `membership_expression` with two +// `identifier` children and nothing between them — so a token arm +// could not reach the negated form however it were gated. Listing the +// wrapper covers both spellings at once and double-counts neither, +// since neither token is counted anywhere (§5). +// +// The sibling relational productions `identity_expression` (`===`, +// `!==`) and `regex_find_expression` / `regex_match_expression` (`=~`, +// `==~`) are deliberately **absent** here: each emits an operator +// token that the same sweep finds in that one production and nowhere +// else, so they are counted as plain comparison tokens beside `==` / +// `!=` in `groovy_count_token_condition` — the spelling every other +// language in the workspace uses for those operators (Kotlin, JS, +// PHP, Elixir for `===`; Perl, Ruby, Bash for `=~`), and the one that +// also scores them outside a boolean slot, where `def r = (a == b)` +// already scores and `def r = (a === b)` did not. Listing them in +// both places would score each twice (§5). #[macro_export] #[doc(hidden)] macro_rules! groovy_bool_terminal_kinds { @@ -154,10 +227,12 @@ macro_rules! groovy_bool_terminal_kinds { | $crate::Groovy::CommandChain | $crate::Groovy::Identifier | $crate::Groovy::BooleanLiteral + | $crate::Groovy::NumberLiteral | $crate::Groovy::FieldAccess | $crate::Groovy::CastExpression | $crate::Groovy::ParenthesizedTypeCast | $crate::Groovy::InstanceofExpression + | $crate::Groovy::MembershipExpression }; } @@ -178,6 +253,7 @@ macro_rules! rust_bool_terminal_kinds { // the C# fix in #372 (lesson 19), which closed the same gap // for `CastExpression`, `MemberAccessExpression`, and // `AwaitExpression` on the C# side. + // () => { $crate::Rust::Identifier | $crate::Rust::BooleanLiteral @@ -210,13 +286,33 @@ macro_rules! go_bool_terminal_kinds { }; } -// FIXME(#1410): C and C++ are integer-truthy, so this set is missing the -// numeric literal kinds — `if (1)` scores no condition where `if (true)` -// scores one, within the same language. Name-keyed, so C, C++, Mozcpp and -// Objective-C are all affected, which makes this the largest of the three -// sets #1379 left behind (see `perl_bool_terminal_kinds!` below for the -// measurement). Deferred out of #1379 because the DeepSpeech corpus is -// C/C++ and the fix moves snapshots. +// The C family is integer-truthy — `if (1)`, `while (1)`, +// `do { … } while (0)` and `a && 1` are all legal and idiomatic — so +// `number_literal` and `char_literal` belong here for the same reason +// `"true"` does. Omitting them made the gap visible *within* one +// language: `if (true)` scored one condition and `if (1)` none (#1410). +// +// One `number_literal` arm covers every spelling in all four grammars. +// `0x1f`, `017`, `0b101`, `1u`, `1L`, `1ULL`, `1.5f`, `1e3` and C++'s +// `1'000` digit separator all lex to it, verified by `bca dump` per +// language rather than read off the grammar — #1379's misses were +// sibling rules under a hidden choice, which an alias sweep cannot see. +// +// `char_literal` is here because a character literal has integral type +// (`int` in C, `char` in C++) and is contextually convertible to bool +// exactly as a number is; leaving it out would reproduce the same +// within-language asymmetry one kind narrower. It is also the correct +// half of the wrapper/leaf pair (grammar-dispatch §5): an operand slot +// always holds the `char_literal`, never its inner `character` / +// `escape_sequence` child, so the wrapper is the only node reachable +// here and there is nothing to double-count. +// +// Two neighbouring kinds are deliberately absent. C++'s +// `user_defined_literal` (`1.0_km`) wraps a `number_literal` but +// evaluates to whatever `operator""` returns, which need not be +// numeric or contextually boolean. Objective-C's `version_number` is +// the `@available(iOS 13.0, *)` token, not a literal, and never +// occupies an operand slot. #[macro_export] #[doc(hidden)] macro_rules! cpp_bool_terminal_kinds { @@ -240,6 +336,8 @@ macro_rules! cpp_bool_terminal_kinds { "identifier" | "true" | "false" + | "number_literal" + | "char_literal" | "call_expression" | "message_expression" | "field_expression" @@ -249,10 +347,23 @@ macro_rules! cpp_bool_terminal_kinds { }; } -// FIXME(#1410): PHP treats every non-zero number as truthy, so this set -// is missing the numeric literal kinds — `$a && 1` scores one condition -// where `$a && $b` scores two. Deferred out of #1379 because the -// integration corpora carry PHP files and the fix moves snapshots. +// PHP treats every non-zero number as truthy, so `Integer` and `Float` +// are unary conditions here for the same reason Python's `Integer` / +// `Float` are. Naming neither scored `$a && 1` one condition against +// `$a && $b`'s two, and `if ($a && 1.0)` one against two (#1410). +// +// The unit to check is the supertype, not the alias list (#1379), and +// PHP has neither to miss: every radix prefix, `_` separator and +// exponent folds into one of those two kinds — `0x1f`, `0b101`, `017`, +// `0o17` and `1_000` all lex as `integer`, and `1e3`, `1.5e10`, `.5` +// and `1.` as `float` (verified by `bca dump`). +// +// `Float2` (52) is **not** a third numeric kind despite rendering to +// the same `"float"` string: it is the `float` *type* keyword of a +// parameter type or a `(float)` cast, which `Getter::get_op_type` +// groups with `Int` / `Bool` / `String2` rather than with the `Integer` +// / `Float` value operands (`getter/php.rs`). Listing it would be the +// `Number2` mistake `typescript_bool_terminal_kinds!` records below. #[macro_export] #[doc(hidden)] macro_rules! php_bool_terminal_kinds { @@ -271,6 +382,8 @@ macro_rules! php_bool_terminal_kinds { | $crate::Php::Name2 | $crate::Php::VariableName | $crate::Php::Boolean + | $crate::Php::Integer + | $crate::Php::Float | $crate::Php::FunctionCallExpression | $crate::Php::MemberCallExpression | $crate::Php::ScopedCallExpression @@ -348,20 +461,41 @@ macro_rules! python_bool_terminal_kinds { // those five, so there is nothing to count. // // That rationale does **not** extend to the C family, which an earlier -// revision of this comment wrongly grouped with them. C and C++ are -// integer-truthy — `if (1)`, `while (1)`, `do { … } while (0)` and -// `a && 1` are all legal and idiomatic — so they carry the same gap PHP -// and Groovy do. `cpp_bool_terminal_kinds!` is name-keyed and shared by -// C, C++, Mozcpp and Objective-C, so all four are affected, and the -// omission is visible *within* one language: `if (true)` scores one -// condition and `if (1)` scores none, because `"true"` is in the set -// and `number_literal` is not. -// -// PHP, Groovy and the four C-family languages are all tracked in #1410. -// They are deferred rather than deliberate, and for a scheduling reason -// only: each has integration-corpus files (the DeepSpeech `native_client` -// snapshots are C/C++), so fixing them moves snapshots and wants its own -// measurement pass. +// revision of this comment wrongly grouped with them: C and C++ are +// integer-truthy, so they carried the same gap PHP and Groovy did. +// #1410 closed all six — PHP, Groovy and the name-keyed C, C++, Mozcpp +// and Objective-C set — and each carries its own rationale above. +// +// #1379 deferred those six on the grounds that all three sets had +// integration-corpus files. That held only for the C family (the +// DeepSpeech `native_client` tree, seven snapshots): PHP's corpus files +// carry no truthy numeric operand, and no corpus carries a `.groovy` +// file at all. +// +// `pattern_matcher` (`/^#/`) and `pattern_matcher_m` (`m{^#}`) are the +// two spellings of a match against the implicit `$_`. They are +// **sibling rules, not aliases** — ids 337 and 336, the distinction +// this file's own numeric-literal note warns is invisible to an alias +// sweep — so both have to be listed or the `m{}` half stays at zero. +// A bound match (`$x =~ /^#/`) is a `binary_expression` whose `=~` +// token the dispatcher already counts; the bare form carries no +// operator token at all, which is why `if (/^#/)` scored zero +// conditions against `if ($x)`'s one. +// +// Listing them double-counts nothing (§5). In `$x =~ /^#/` the +// pattern is a child of the `binary_expression`, and every walker that +// consumes this set either breaks on that `binary_expression` +// (`perl_inspect_container`, `perl_count_condition`) or requires the +// list node itself to be the `&&`-chain parent +// (`perl_count_unary_conditions`), so the pattern node is never +// reached alongside its own `=~`. +// +// Three further rules in the same grammar family are deliberately +// absent because whether they are boolean *tests* is a judgement call, +// not a dispatch gap: `substitution_pattern_s` (`s///`) and +// `transliteration_tr_or_y` (`tr///`) each evaluate to a count rather +// than a bool, and `regex_pattern_qr` (`qr//`) to a compiled-pattern +// object that is always true. All three measure zero conditions today. #[macro_export] #[doc(hidden)] macro_rules! perl_bool_terminal_kinds { @@ -388,6 +522,8 @@ macro_rules! perl_bool_terminal_kinds { | $crate::Perl::CallExpressionRecursive | $crate::Perl::CallExpressionWithBareword | $crate::Perl::MethodInvocation + | $crate::Perl::PatternMatcher + | $crate::Perl::PatternMatcherM }; } @@ -574,6 +710,28 @@ macro_rules! tsx_bool_terminal_kinds { // (`arr[0]`), and `this_expression`. Comparison operands (`x > 0`) are // themselves `binary_expression` nodes, so they are absent from this set // and contribute nothing — matching the paper's "only unary conditions". +// +// `infix_expression` is a call to an infix function, and Kotlin spells +// boolean `and` / `or` / `xor` that way — `a and b` is `a.and(b)`. It +// belongs here for the same reason `call_expression` does, and its +// absence was a regression of #1421 rather than a pre-existing gap: the +// blanket per-entry count that 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. The set +// does not discriminate on return type — `f()` counts in a boolean slot +// whatever it returns — so a non-boolean infix call in a boolean slot is +// out of scope here for the same reason. +// +// `is_expression` (`a is String`, `a !is String`) and `in_expression` +// (`a in 1..2`, `a !in 1..2`) are the two relational forms the grammar +// spells as their own production rather than as a `binary_expression`, +// so the comparison-token arms never see them and nothing else in the +// Kotlin impl counts them. They are terminal for every consumer of this +// set — neither carries a nested chain link, and neither's own operator +// token (`is` / `!is` / `in` / `!in`) is counted anywhere — so listing +// them here scores each exactly once, as Fitzpatrick Rule 5 scores any +// other relational operator. Before #1421 `if (a is String)` scored +// zero conditions against a cyclomatic decision of one. #[macro_export] #[doc(hidden)] macro_rules! kotlin_bool_terminal_kinds { @@ -583,6 +741,9 @@ macro_rules! kotlin_bool_terminal_kinds { | $crate::Kotlin::NavigationExpression | $crate::Kotlin::IndexExpression | $crate::Kotlin::ThisExpression + | $crate::Kotlin::IsExpression + | $crate::Kotlin::InExpression + | $crate::Kotlin::InfixExpression }; } @@ -622,6 +783,19 @@ macro_rules! kotlin_bool_terminal_kinds { // None of the four kinds has a numeric-suffix alias in tree-sitter-ruby // 0.23.1; `_int_or_float` (`Ruby::IntOrFloat`) is a hidden supertype the // parser never emits (grammar-dispatch §2). +// +// `test_pattern` is Ruby 3.0's one-line pattern test (`a in Integer`), +// which evaluates to a boolean. The grammar gives it its own +// production, so the comparison-token arm in `metrics/abc/ruby.rs` +// never sees it — that arm is gated on a `binary` parent and lists no +// `in` token — and `if a in Integer` scored zero conditions against a +// cyclomatic decision of one. Nothing counts the `in` token itself, so +// listing the wrapper scores it exactly once (§5). +// +// Its neighbour `match_pattern` (`expr => pat`, id 252) is **not** +// here and must not be added: that spelling raises `NoMatchingPattern` +// on failure rather than yielding a boolean, so it is a destructuring +// assignment, not a condition. #[macro_export] #[doc(hidden)] macro_rules! ruby_bool_terminal_kinds { @@ -643,6 +817,7 @@ macro_rules! ruby_bool_terminal_kinds { | $crate::Ruby::Float | $crate::Ruby::Rational | $crate::Ruby::Complex + | $crate::Ruby::TestPattern }; } diff --git a/big-code-analysis-book/src/metrics.md b/big-code-analysis-book/src/metrics.md index f1c88a92c..a0c866ea4 100644 --- a/big-code-analysis-book/src/metrics.md +++ b/big-code-analysis-book/src/metrics.md @@ -145,13 +145,15 @@ application would over-count. | All languages | `default` / `_` wildcard arm excluded from the condition set | Fitzpatrick's Figure 2 lists `default`, but it falls through unconditionally — counting it would inflate `C` on every `switch` / `match` regardless of body. big-code-analysis omits it for every language (the Rust `_ =>` and Java `default:` arms included). | | Tcl | Chain-operand, bare-truthy and ternary slots wired; argument and `return` slots are not | Each operand of a `&&` / `\|\|` chain inside `expr {…}` counts as one condition, so `if {$a && $b}` reports two. `if` / `elseif` / `while` route their `expr {…}` predicate, so a bare-truthy `if {$a}` reports one and `if {!$a}` likewise — matching C's `if (a)` (#1180). A predicate written as a command substitution is a truthy test of that command's result, so `if {[somecmd]}` also reports one, and the redundant `if {[expr {$a \|\| $b}]}` idiom reports three (the chain's two operands plus the substitution). The argument and `return` slots remain unrouted: a negation reached only through a standalone `expr {…}` command still reports zero. | | iRules | Chain-operand, bare-truthy and ternary slots wired; argument and `return` slots are not | Each operand of a `&&` / `\|\|` / `and` / `or` chain counts as one condition (Rule 9), so `if {!$a && !$b}` reports two. iRules also recognises the word-form string-match comparators (`contains`, `starts_with`, `ends_with`, `equals`, `matches`, …) that Tcl lacks (Tcl's `eq` / `ne` / `in` / `ni` are shared). Bare-truthy and ternary slot routing matches its Tcl sibling exactly (#1180) — see that row for the shared detail, including the argument and `return` slots that remain unrouted. | -| All Phase 2 languages (Java, Groovy, C#, Rust, Go, JavaScript, TypeScript, TSX, Mozjs, PHP, C, C++, Objective-C, Mozcpp, Python, Perl, Lua) | `if (true) {}`, `m(!a, !b)`, `return !x` count their operand(s) | Phase 2B routes `if` / `while` / `do-while` / argument-list / `return` slots through the same walker, so the rule applies uniformly across decision-bearing positions. A bare `return x` continues to report zero — Fitzpatrick treats an identifier in a return slot as a value, not a unary conditional. | +| All Phase 2 languages (Java, Groovy, C#, Rust, Go, JavaScript, TypeScript, TSX, Mozjs, PHP, C, C++, Objective-C, Mozcpp, Python, Perl, Lua) | `if (true) {}`, `m(!a, !b)`, `return !x` count their operand(s) | Phase 2B routes `if` / `while` / `do-while` / argument-list / `return` slots through the same walker, so the rule applies uniformly across decision-bearing positions. A bare `return x` continues to report zero — Fitzpatrick treats an identifier in a return slot as a value, not a unary conditional. C# is the one exception to the argument-list half of the rule: its grammar wraps each argument in an `argument` node that the walker does not descend, so `M(!a, !b)` reports zero where every other language in this row reports two. `if (true)` and `return !x` behave as stated. | | Ternary slots: Java, Groovy, C#, C, C++, Objective-C, Mozcpp, JavaScript, TypeScript, TSX, Mozjs, PHP, Perl, Ruby, Python, Tcl, iRules | `a ? !b : !c` counts its condition and both branch operands | The same walker also runs over a ternary's three operand slots, so `a ? !b : !c` reports 4 (the `?` plus three unary conditions) rather than 1. Tcl and iRules reach the same 4 without grammar fields: their `ternary_expr` exposes none, and `_expr` inlines `( … )`, so the slots are located relative to the `?` and `:` tokens instead of by index (#1180). Python arrives at the same total by a different route: `not` operands are counted by the `NotOperator` rule wherever they appear, so only the condition slot is routed through the walker and `(not b) if a else (not c)` likewise reports 4. Languages with no ternary (Rust, Go, Kotlin, Lua, Elixir) are unaffected. | | `for` header condition slot: Java, Groovy, C#, C, C++, Objective-C, Mozcpp, JavaScript, TypeScript, TSX, Mozjs, PHP, Go, Perl | `for (; a; )` counts one condition; `for (;;)` counts none | The loop header's condition is a decision-bearing slot like an `if` predicate, so `for (; a; )`, `for (; !a; )` and `for (; (a); )` each report one. A comparison-shaped condition (`i < n`) is already counted by its own operator arm and is not double-counted. Every one of these grammars except Go exposes a `condition` field on `for_statement` (Perl's sits on its C-style `for_statement_1`), so the slot is read by field name rather than by child index; Go's field sits on the nested `for_clause`, and its outer header slot is located structurally (first named child that is neither the body nor a comment). An **empty** condition counts zero in every language: Fitzpatrick's rules count conditional operators and unary conditions that are present, and an omitted test is not a decision. Java and Groovy previously scored `for (;;)` as one vacuously-true condition and no longer do. Tcl, iRules and Bash are command-dispatched grammars whose loop headers carry no boolean-expression slot to route (grammar-dispatch §9) (#1276). | | Ruby | Bare-predicate `if` / `unless` / `while` / `until` (block and modifier forms) count one condition | Idiomatic Ruby favours bare predicates (`if flag`, `x if flag`); counting the condition slot keeps ABC conditions at or above Ruby's cyclomatic decision count (the alignment enforced across the other languages). A comparison (`if a == b`) or `&&` / `\|\|` chain in the predicate is counted by its own operator / walker arm and is not double-counted. | | Bash | `if` / `elif` / `while` and each non-wildcard `case` arm count one condition | A Bash predicate is a *command*, so the branch keyword itself — not an embedded boolean expression — is the condition signal. Each matches a Bash cyclomatic decision; the bare `*)` case arm (the analogue of `default:`) is excluded, mirroring the cyclomatic standard count. The arithmetic ternary `$(( a ? b : c ))` therefore contributes nothing: it carries no branch keyword, so it falls outside the rule set rather than through a gap in it. | | Kotlin | `try` counts a condition alongside `catch` | Fitzpatrick counts both keywords, and Java / C# / C++ / Groovy already count both; Kotlin previously counted only the catch block. | -| C++, Objective-C, Mozcpp, Rust, Go, Java, Groovy, C#, Kotlin, TypeScript, TSX, JavaScript, Mozjs, Lua, Perl, Ruby, Bash, Elixir | A `<` or `>` that is not a comparison is not a condition | Every one of these grammars spells at least one non-comparison construct with the same bare `<` / `>` token a comparison uses, so the comparison rule is gated on the token's parent. What that excludes, per family: template and generic brackets in C++, Objective-C, Mozcpp, Rust, Go, Java, Groovy, C#, Kotlin, TypeScript and TSX (#1274); JSX tag delimiters in TypeScript, TSX, JavaScript and Mozjs; Lua 5.4 variable attributes (`local x = 1`); a C# comparison-operator overload's declared name (`operator <`); Kotlin's qualified super call (`super.g()`) (#1297); Perl's filehandle and lexical-handle readlines (``, `<$fh>` — but not ``, which the grammar lexes as one token); Ruby's superclass clause (`class Foo < Bar`) and comparison-operator method names (`def <(other)`), and Bash I/O redirection (`cmd > out`) (#1280); Elixir's sigil delimiters (`~s`) (#1256). C# additionally excludes the operator of a relational pattern (`x is > 0`, and the `> 5 =>` arm of a switch expression): the arm or `if` condition slot that owns the pattern already scores the decision, so counting the operator too charged a relational arm twice what the constant arm `5 => 1` scores (#1383). Its `>=` / `<=` spelling is excluded by the same rule through a separate token. The exclusion holds wherever the pattern sits, so a pattern no arm or condition slot owns (`bool b = x is > 0;`, `return x is > 0;`, a `when` guard) scores 0, one below the equivalent `x > 0`. PHP, Python, Tcl and iRules emit a bare `<` / `>` from no non-comparison production; C carries the same gate as its C-family siblings although, having no templates, it has nothing to exclude. The gate is a claim about the grammar's productions, not about every parse: where a grammar resolves a generic *call* into nested `binary_expression` nodes, as tree-sitter-kotlin-ng does for `id(a)`, no polarity can exclude it (#1394). | +| C# | A `when` guard is a condition slot, scoring one however it is spelled | A guard on a switch arm (`when_clause`, shared by `switch_expression_arm` and `switch_section`) or on a `catch` (`catch_filter_clause`) is a branch: the pattern can match while the guard fails. Neither metric modelled it, and ABC scored whatever operator happened to sit inside, so `when x > 2` counted one and the equivalent `when IsEven(x)` counted none. The guard's expression is now scored exactly as an `if` condition is — one for a call, `is` test or bare identifier, one via the operator arm for a comparison — so every spelling agrees, and a compound guard (`when a > 1 && b < 2`) keeps its sub-structure rather than collapsing to one (#1422). | +| Kotlin | A subject-less `when` arm scores through the predicate slot; a subject-ful arm scores per entry | A subject-less arm's condition (`when { x > 5 -> … }`) is an ordinary boolean expression, compiled as an `if` predicate, so the comparison inside it is already counted by the operator arms and a blanket per-entry increment double-counted it. A subject-ful arm (`when (x) { in 1..2 -> … }`) lists a pattern rather than an independent boolean expression, so the implicit `subject == pattern` is a decision nothing in the source spells and the entry itself pays for it (#1421). | +| C++, Objective-C, Mozcpp, Rust, Go, Java, Groovy, C#, Kotlin, TypeScript, TSX, JavaScript, Mozjs, Lua, Perl, Ruby, Bash, Elixir | A `<` or `>` that is not a comparison is not a condition | Every one of these grammars spells at least one non-comparison construct with the same bare `<` / `>` token a comparison uses, so the comparison rule is gated on the token's parent. What that excludes, per family: template and generic brackets in C++, Objective-C, Mozcpp, Rust, Go, Java, Groovy, C#, Kotlin, TypeScript and TSX (#1274); JSX tag delimiters in TypeScript, TSX, JavaScript and Mozjs; Lua 5.4 variable attributes (`local x = 1`); a C# comparison-operator overload's declared name (`operator <`); Kotlin's qualified super call (`super.g()`) (#1297); Perl's filehandle and lexical-handle readlines (``, `<$fh>` — but not ``, which the grammar lexes as one token); Ruby's superclass clause (`class Foo < Bar`) and comparison-operator method names (`def <(other)`), and Bash I/O redirection (`cmd > out`) (#1280); Elixir's sigil delimiters (`~s`) (#1256). C# additionally excludes the operator of a relational pattern (`x is > 0`, and the `> 5 =>` arm of a switch expression): the arm or `if` condition slot that owns the pattern already scores the decision, so counting the operator too charged a relational arm twice what the constant arm `5 => 1` scores (#1383). Its `>=` / `<=` spelling is excluded by the same rule through a separate token. The declared name of an operator overload is excluded on every spelling too: C# overloads six comparison operators and gives each a distinct token, so `operator <=`, `operator >=`, `operator ==` and `operator !=` join `operator <` and `operator >`, which alone were excluded before #1420. The exclusion holds wherever the pattern sits, so a pattern no arm or condition slot owns (`bool b = x is > 0;`, `return x is > 0;`) scores 0, one below the equivalent `x > 0`. A `when` guard is no longer such a place: since #1422 the guard is itself a condition slot, so `when n is > 5` reads level with `when n > 5` rather than one below it. PHP, Python, Tcl and iRules emit a bare `<` / `>` from no non-comparison production; C carries the same gate as its C-family siblings although, having no templates, it has nothing to exclude. The gate is a claim about the grammar's productions, not about every parse: where a grammar resolves a generic *call* into nested `binary_expression` nodes, as tree-sitter-kotlin-ng does for `id(a)`, no polarity can exclude it (#1394). | | Java, Groovy, C#, TypeScript, TSX | A `?` used as type syntax is not a ternary | In each of these grammars the ternary `?` and the type-syntax `?` are the *same* anonymous token, so the ternary rule above is gated on the token's parent. Java and Groovy exclude the wildcard bound `List` (#1274); C# excludes the nullable type `int? x` and the constraint `where T : class?`; TypeScript and TSX exclude optional parameters, properties, methods, class fields and tuple elements, and conditional types (`T extends U ? X : Y`, which the type checker resolves and erases before runtime, so it is no more a branch than the `<` / `>` already excluded) (#1275). Safe navigation is untouched: C#'s `a?.b` shares the same token and still counts, while the other languages spell theirs as a distinct one. | #### Worked example diff --git a/src/metrics/abc.rs b/src/metrics/abc.rs index fbdcd6118..936edd449 100644 --- a/src/metrics/abc.rs +++ b/src/metrics/abc.rs @@ -455,7 +455,8 @@ implement_metric_trait!(Abc, PreprocCode, CcommentCode); )] mod tests { use crate::test_support::{ - ast_has_kind_id, check_func_space_only_shim, check_metrics_only_shim, metrics_verbatim, + ast_has_kind_id, check_func_space_only_shim, check_metrics_only_shim, child_space, + metrics_verbatim, }; use crate::traits::ParserTrait; @@ -529,6 +530,40 @@ mod tests { } } + // The second sibling, for a fixture whose point is that a construct + // moves a member *off* the value its unguarded control scores. + // `assert_every_member_scores` cannot serve those: it takes one + // expected value for the whole container, and every #1422 guard + // fixture deliberately pairs guarded members with an unguarded + // control one lower — which is the comparison the test exists to + // make. Both tiers per member, because a guard is a decision in each + // and a fix that moved only one of them would satisfy neither claim + // on its own, and because a member may legitimately sit *above* its + // decision count — an `else` arm, or a comparison nested inside + // another comparison, is an ABC condition with no cyclomatic + // decision behind it (#1421). + #[track_caller] + fn assert_members_score(container: &crate::FuncSpace, expected: &[(&str, u64, u64)]) { + assert_eq!( + container.spaces.len(), + expected.len(), + "member count changed — the fixture moved, not the metric" + ); + for (name, conditions, cyclomatic) in expected { + let member = child_space(container, name); + assert_eq!( + member.metrics.abc.conditions(), + *conditions, + "{name}: abc.conditions" + ); + assert_eq!( + member.metrics.cyclomatic.cyclomatic(), + *cyclomatic, + "{name}: cyclomatic" + ); + } + } + // #1383's fix makes a relational pattern's operator score *zero*, so // its tests have no second axis to anchor on: trimming `> 5` down to // `5` leaves every assertion satisfied and the construct under test @@ -544,12 +579,12 @@ mod tests { // of the control. Measured — with presence-only anchoring, rewriting // `if (x is > 0)` to `if (x > 0)` in one method of five failed // nothing. - fn assert_csharp_fixture_spells(src: &str, kinds: &[(u16, usize, &str)]) { - let parser = CsharpParser::new( - src.as_bytes().to_vec(), - std::path::Path::new("foo.cs"), - None, - ); + #[track_caller] + fn assert_fixture_spells(src: &str, path: &str, kinds: &[(u16, usize, &str)]) { + // An empty list would make every following assertion vacuous, + // which is the anchor's own failure mode rather than a caller's. + assert!(!kinds.is_empty(), "anchor asserted nothing"); + let parser = P::new(src.as_bytes().to_vec(), std::path::Path::new(path), None); for (kind, want, spelling) in kinds { let found = parser .root() @@ -563,6 +598,14 @@ mod tests { } } + // The C# binding of the anchor above. Fourteen callers pass a `foo.cs` + // fixture, so the parser and path are fixed here rather than repeated + // at each one. + #[track_caller] + fn assert_csharp_fixture_spells(src: &str, kinds: &[(u16, usize, &str)]) { + assert_fixture_spells::(src, "foo.cs", kinds); + } + /// Regression for #227: a `Stats::default()` that never sees an /// observation must not leak the `f64::MAX` sentinel for /// `assignments_min`, `branches_min`, or `conditions_min`. All @@ -1765,6 +1808,180 @@ mod tests { ); } + #[test] + fn csharp_primary_constructor_base_call_is_a_branch() { + // The C# 12 primary-constructor superclass call invokes the base + // constructor exactly as the `: base(…)` above it does, and scored + // zero until #1406 — the C# sibling of Kotlin's #1384. + // + // tree-sitter-c-sharp 0.23.5 spells the `record` and `class` + // families differently, so per `.claude/rules/grammar-dispatch.md` + // §11 they are two independent paths and each needs its own + // fixture: the record form nests the arguments under a + // `primary_constructor_base_type`, the class form hangs them + // straight off the `base_list`. A record-only fixture says nothing + // about the class arm, and vice versa. + // + // `S` and `R3` are the negatives: a base type with no argument + // list is not a call, and neither spelling emits an + // `argument_list` for it. + let src = "class Sub(int x) : Base(x) { } + record R1(int x) : Base(x); + record class R2(int x) : Base(x); + class G(int x) : GBase(x) where T : class { } + class Multi(int x) : Base(x), IBase { } + struct S(int x) : IBase { } + record R3(int x) : Base;"; + // Anchors every spelling against a fixture edit: with presence + // checks only, deleting `(x)` from the class form leaves the + // record form satisfying the arm and this test green. The last + // two counts anchor the declarations that would otherwise decay + // into duplicates of `Sub` — `Multi` owns the fixture's only + // comma (`Base(x), IBase`) and `G` its only type argument list + // (`GBase`), so trimming either back fails here by name. + assert_csharp_fixture_spells( + src, + &[ + ( + Csharp::PrimaryConstructorBaseType as u16, + 2, + "`record` base calls", + ), + (Csharp::ArgumentList as u16, 5, "base-call argument lists"), + (Csharp::BaseList2 as u16, 7, "base lists"), + // The §2 drift marker for the arm's other half. Both ids + // render to `"base_list"`; at this pin every declaration + // family above emits 252 and none emits 246, so + // `BaseList` is defensive and this zero says so. A + // grammar bump that starts emitting it fails here rather + // than silently changing which arm does the work. + (Csharp::BaseList as u16, 0, "unaliased `base_list` nodes"), + (Csharp::COMMA as u16, 1, "`Multi`'s trailing interface"), + (Csharp::TypeArgumentList as u16, 1, "`G`'s generic base"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + for name in ["Sub", "R1", "R2", "G", "Multi"] { + assert_eq!( + child_space(&space, name).metrics.abc.branches(), + 1, + "{name}: the primary-constructor base call is one branch" + ); + } + for name in ["S", "R3"] { + assert_eq!( + child_space(&space, name).metrics.abc.branches(), + 0, + "{name}: a base type with no argument list is not a call" + ); + } + }); + } + + #[test] + fn csharp_base_list_gate_excludes_other_argument_lists() { + // The `base_list` parent gate on the `ArgumentList` arm is + // load-bearing, not decoration: ungated it bills a branch for the + // argument list of every call and allocation in the file, on top + // of the `InvocationExpression` / `ObjectCreationExpression` the + // arm already counts — doubling both (#1406). + // + // The fixture pairs one real base call with the four other + // argument-carrying productions C# spells nearby: an attribute + // (`attribute_argument_list`), an invocation, an object creation, + // and an indexer (`bracketed_argument_list`) alongside a generic + // (`type_argument_list`). Those last three are distinct kind ids + // rather than `argument_list`, so they pin the §1 neighbour set + // as much as the gate. + let src = "[System.Obsolete(\"why\")] + class A(int x) : Base(x) { + void M(int y) { Helper(y); } + object N() { return new Foo(1); } + int P(System.Collections.Generic.List l, int i) { return l[i]; } + }"; + assert_csharp_fixture_spells( + src, + &[ + ( + Csharp::AttributeArgumentList as u16, + 1, + "attribute argument lists", + ), + ( + Csharp::BracketedArgumentList as u16, + 1, + "bracketed argument lists", + ), + (Csharp::TypeArgumentList as u16, 1, "type argument lists"), + (Csharp::ArgumentList as u16, 3, "argument lists"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + let class = child_space(&space, "A"); + assert_eq!( + class.metrics.abc.branches(), + 1, + "`: Base(x)` alone — the attribute's arguments are not a call" + ); + // The controls. 2 apiece would mean the gate let the + // argument list through beside the call that owns it. + assert_eq!( + child_space(class, "M").metrics.abc.branches(), + 1, + "`Helper(y)` is one branch, not one per argument list" + ); + assert_eq!( + child_space(class, "N").metrics.abc.branches(), + 1, + "`new Foo(1)` is one branch, not one per argument list" + ); + // `P` carries the indexer and the generic deliberately, but + // gets no assertion: `bracketed_argument_list` and + // `type_argument_list` are distinct kind ids, so `P` reads 0 + // with the gate, without it, and without this change + // altogether. The census above is what actually pins them — + // an assertion here would read as coverage it cannot provide. + }); + } + + #[test] + fn csharp_base_call_and_constructor_initializer_do_not_double_count() { + // A class can spell *both* delegations at once: the primary + // constructor's `: Base(x)` and a secondary constructor's + // `: this(a)`. They are separate nodes in separate spaces, so the + // file scores 2 — one each. 3 would mean the #1406 arm also fired + // on something `ConstructorInitializer` already owned; 1 would + // mean one of the two stopped counting. + // + // Java has the equivalent guard + // (`java_constructor_delegation_does_not_double_count_arguments`); + // C# now has two delegation spellings and this is the pairing + // test for them. + let src = "class Both(int x) : Base(x) { + public Both(int a, int b) : this(a) { } + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::ConstructorInitializer as u16, 1, "`: this(a)`"), + (Csharp::BaseList2 as u16, 1, "the primary-constructor base"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + let class = child_space(&space, "Both"); + assert_eq!( + class.metrics.abc.branches(), + 1, + "the class space owns `: Base(x)` and nothing else" + ); + assert_eq!( + child_space(class, "Both").metrics.abc.branches(), + 1, + "the secondary constructor owns `: this(a)` and nothing else" + ); + }); + } + #[test] fn kotlin_constructor_delegation_is_a_branch() { // Kotlin's secondary-constructor delegation is a @@ -1882,6 +2099,133 @@ mod tests { ); } + // The three tests below are one fix seen from three grammars (#1407): + // an enum constant carrying constructor arguments is an object + // construction and scored zero in every JVM-family language, which + // #1279 and #1384 left behind because both looked at superclass + // delegation only. + // + // Each fixture pairs an argument-carrying constant with a bare one in + // the *same* enum, because that is the only shape that shows the gate + // working: a fixture of only `A(1)` scores the same whether the arm is + // gated on the argument list or matches every constant + // (`.claude/rules/grammar-dispatch.md` §6). Each also carries a + // constant whose argument is itself a call, which must score 2 rather + // than 1 (§5). + // + // `branches_sum()` alone cannot notice the bare constant being trimmed + // out — it contributes to no axis — so every fixture is anchored on a + // node census that fails by name when a spelling is edited away + // (`.claude/rules/testing.md`, "Perturb the fixture as well as the + // production line"). The two are coupled in one helper for the same + // reason `assert_kotlin_class_members` couples its pair: a branch + // total added without the census reads as coverage and is not. + + fn assert_enum_branches( + src: &str, + path: &str, + kinds: &[(u16, usize, &str)], + branches: u64, + ) { + assert_fixture_spells::

(src, path, kinds); + check_func_space::(src, path, |space| { + assert_eq!(space.metrics.abc.branches_sum(), branches); + }); + } + + #[test] + fn java_enum_constant_with_arguments_is_a_branch() { + // `@SuppressWarnings("x") B` is the discriminating case for the + // gate's *kind*, not just its presence: the annotation carries an + // argument list of its own, so an arm gated on "has some argument + // list anywhere below" would score it. It cannot satisfy this one + // twice over — `annotation_argument_list` is a distinct kind id + // from `argument_list`, and it hangs off the constant's `modifiers` + // child rather than the constant itself. Both verified with `bca + // dump`. + // + // expected: 4 branches — `A(1)`, `K(2)`, `D(…)`, and the `f()` + // inside `D`'s argument list. `B` scores 0. + let src = "class Outer { + static int f() { return 1; } + enum E { + A(1), + @SuppressWarnings(\"x\") B, + K(2), + D(f()); + E(int v) {} + } + }"; + assert_enum_branches::( + src, + "foo.java", + &[ + (Java::EnumConstant as u16, 4, "enum constants"), + (Java::ArgumentList as u16, 4, "argument lists"), + ( + Java::AnnotationArgumentList as u16, + 1, + "`@SuppressWarnings`'s argument list", + ), + (Java::MethodInvocation as u16, 1, "`D`'s nested `f()` call"), + ], + 4, + ); + } + + #[test] + fn kotlin_enum_entry_with_arguments_is_a_branch() { + // Kotlin needs a defaulted primary-constructor parameter to spell + // both cases in one enum: `enum class E(val v: Int)` obliges every + // entry to pass an argument, so `= 0` is what lets the bare `B` + // sit beside `A(1)`. `enum class P` covers the other shape the gate + // has to leave alone — an enum with no constructor at all, whose + // entries can never carry an argument list. + // + // expected: 3 branches — `A(1)`, `C(…)`, and the `g()` inside `C`'s + // argument list. `B`, `X` and `Y` score 0. + let src = "fun g(): Int = 1 + enum class E(val v: Int = 0) { A(1), B, C(g()) } + enum class P { X, Y }"; + assert_enum_branches::( + src, + "foo.kt", + &[ + (Kotlin::EnumEntry as u16, 5, "enum entries"), + (Kotlin::ValueArguments as u16, 3, "argument lists"), + (Kotlin::CallExpression as u16, 1, "`C`'s nested `g()` call"), + ], + 3, + ); + } + + #[test] + fn groovy_enum_constant_with_arguments_is_a_branch() { + // Groovy's enum has no annotated-constant case to cover: the + // dekobon grammar cannot parse `@Deprecated A(1)` inside an enum + // body and recovers into `ERROR` nodes, emitting no `enum_constant` + // at all. The remaining two cases are the ones that matter. + // + // expected: 3 branches — `A(1)`, `D(…)`, and the `Outer.f()` inside + // `D`'s argument list. `B` scores 0. + let src = "class Outer { static int f() { 1 } } + enum E { A(1), B, D(Outer.f()) }"; + assert_enum_branches::( + src, + "foo.groovy", + &[ + (Groovy::EnumConstant as u16, 3, "enum constants"), + (Groovy::ArgumentList as u16, 3, "argument lists"), + ( + Groovy::MethodInvocation as u16, + 1, + "`D`'s nested `Outer.f()` call", + ), + ], + 3, + ); + } + #[test] fn groovy_no_abc() { // Comment-only file has no executable code → all-zero ABC. @@ -3789,6 +4133,130 @@ mod tests { ); } + // #1420, the other four spellings of the test above. C# overloads six + // comparison operators and #1297 gated two: `<=` `>=` `==` `!=` are + // distinct tokens that reach a different arm of + // `csharp_count_token_condition` and each still scored one spurious + // condition on its declaration — measured 1 apiece against `<` / `>`'s + // 0, on a class where `cyclomatic()` is 1 for all six. + // + // Each token gets its own member so a partial fix names the spelling + // it missed: gating `EQEQ | BANGEQ` and forgetting `GTEQ | LTEQ` fails + // here by operator, where the file total cannot tell the two apart — + // it reads 4 before the fix and 0 after, so any two-of-four fix halves + // it and still looks like movement. + // + // `n` is the deliberate control (§11): the same four tokens *applied*, + // inside `binary_expression` parents, which must keep scoring one + // condition each. Without it the gate could be satisfied by refusing + // to count these tokens at all. + #[test] + fn csharp_comparison_operator_overloads_are_not_conditions() { + let src = "class V { + public static bool operator <=(V a, V b) { return true; } + public static bool operator >=(V a, V b) { return true; } + public static bool operator ==(V a, V b) { return true; } + public static bool operator !=(V a, V b) { return true; } + int n(int a, int b) { + if (a <= b) { return 1; } + if (a >= b) { return 2; } + if (a == b) { return 3; } + if (a != b) { return 4; } + return 0; + } + }"; + // Two of each token: one declaring, one applying. Trimming either + // half out collapses this test into the other half's control and + // is what this anchor exists to catch. + assert_csharp_fixture_spells( + src, + &[ + ( + Csharp::OperatorDeclaration as u16, + 4, + "operator declarations", + ), + (Csharp::LTEQ as u16, 2, "`<=`"), + (Csharp::GTEQ as u16, 2, "`>=`"), + (Csharp::EQEQ as u16, 2, "`==`"), + (Csharp::BANGEQ as u16, 2, "`!=`"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + let class = &space.spaces[0]; + assert_eq!(class.spaces.len(), 5, "four overloads plus `n`"); + for (member, spelling) in class.spaces.iter().zip(["<=", ">=", "==", "!="]) { + assert_eq!( + member.metrics.abc.conditions(), + 0, + "`operator {spelling}` declares an operator, it does not apply one" + ); + assert_eq!( + member.metrics.abc.conditions(), + member.metrics.cyclomatic.cyclomatic() - 1, + "`operator {spelling}`: §8 parity with the cyclomatic decision count" + ); + } + // The control, found by name so a reordered fixture cannot + // silently re-point it at an overload. 0 here would mean the + // gate swallowed the applying form along with the declaring + // one. + let control = child_space(class, "n"); + assert_eq!( + control.metrics.abc.conditions(), + 4, + "`n` applies all four tokens: one condition each" + ); + }); + } + + // The allowlist #1420 installed is the only thing keeping `#if A == B` + // a condition, and it keeps it for a reason no arm states: the grammar + // aliases `preproc_binary_expression` onto `binary_expression`, so the + // preprocessor's `==` arrives with kind id `BinaryExpression` (369) + // and not with one of its own. Before #1420 the token was ungated and + // no fact about its parent mattered; now a grammar bump that gave the + // alias a distinct id would stop counting every `#if` equality with + // nothing red. This is that pin. + // + // `BinaryExpression2` (469) is the pre-alias symbol the arm also + // lists, defensively, per `.claude/rules/grammar-dispatch.md` §1. §2 + // asks that such an entry be kept *and* have its unreachability + // asserted, so that a pin promoting the symbol changes behaviour + // loudly rather than invisibly. + #[test] + fn csharp_preproc_equality_counts_through_the_binary_expression_alias() { + let src = "class P { + #if A == B + int x; + #endif + #if C != D + int y; + #endif + }"; + let parser = CsharpParser::new( + src.as_bytes().to_vec(), + std::path::Path::new("foo.cs"), + None, + ); + assert!( + ast_has_kind_id(&parser, Csharp::BinaryExpression as u16), + "the `#if` expressions must parse as `binary_expression` — \ + the alias is what the arm's allowlist matches" + ); + assert!( + !ast_has_kind_id(&parser, Csharp::BinaryExpression2 as u16), + "`BinaryExpression2` is the pre-alias symbol and must stay \ + unreachable; if the grammar starts emitting it, re-derive the \ + allowlist rather than trusting this arm" + ); + // 2, one per `#if` expression. The two directives carry no + // declarations the walk would score, so nothing else contributes. + check_metrics::(src, "foo.cs", |metric| { + assert_eq!(metric.abc.conditions_sum(), 2); + }); + } + // #1383: a `relational_pattern`'s operator is not a condition of // its own — the `switch_expression_arm` that owns it already scores // the decision, exactly as it does for the constant arm `5 => 1`. @@ -3802,21 +4270,23 @@ mod tests { // `>=` / `<=` in the file, so a readmitted `RelationalPattern` // parent is the only thing that can lift the count. // - // Both spellings are covered because they reach two different arms: - // `n`'s `>` / `<` are gated by the `GT | LT` parent allowlist, - // `g`'s `>=` / `<=` by their own `RelationalPattern` denial. Fixing - // one arm and not the other leaves the *other method* at 4, which - // is why this asserts per member and not through a total: + // Both spellings are covered, and asserted per member rather than + // through a total, because they reached two different arms when this + // was written: `n`'s `>` / `<` were gated by a `GT | LT` parent + // allowlist and `g`'s `>=` / `<=` by a separate `RelationalPattern` + // denial, so fixing one and not the other left the *other method* at + // 4. #1420 merged all six comparison tokens onto the one allowlist, + // which removes that particular way to get it half right — but the + // per-member shape still earns its keep, since a total of 6 is what + // both `{2, 2, 2}` and a regressed `{4, 2, 0}` report: // // | state | `n` | `g` | `c` | // |---|---|---|---| - // | both halves gated (shipped) | 2 | 2 | 2 | - // | only `GT \| LT` gated | 2 | 4 | 2 | - // | only `GTEQ \| LTEQ` gated | 4 | 2 | 2 | - // | neither | 4 | 4 | 2 | + // | `relational_pattern` excluded (shipped) | 2 | 2 | 2 | + // | readmitted to the allowlist | 4 | 4 | 2 | // - // `c` is the constant-pattern control and reads 2 in every column: - // it is what the relational methods are supposed to agree with. + // `c` is the constant-pattern control and reads 2 in both rows: it is + // what the relational methods are supposed to agree with. #[test] fn csharp_relational_pattern_does_not_double_count_its_arm() { let src = "class A { @@ -3950,71 +4420,432 @@ mod tests { // guard too. // // A relational *pattern* in the guard (`g`) is the other side of that - // line: it scores nothing, though no slot pays for it, because the - // gate is on the operator's parent and a guard is a decision slot - // neither metric models. It is the outside-slot trade - // `csharp_relational_pattern_outside_a_decision_slot_scores_zero` - // records, and it leaves `when n is > 5` one below `when n > 5`; - // modelling the guard as a slot would score the `is` test once and - // close the gap (#1422). + // line: the operator itself still scores nothing, because the gate is + // on the operator's parent — but since #1422 the guard is a condition + // slot, and an `is` test is one of `csharp_bool_terminal_kinds!()`, so + // the slot pays for it. That is what closed the gap this test used to + // carry a `FIXME(#1422)` for: `when n is > 5` now reads level with + // `when n > 5` instead of one below it. // - // It is also where §8 does not hold, and the reason is worth - // stating precisely, because the obvious reading is wrong. Neither - // metric models the guard as a branch — C# cyclomatic has no - // `when_clause` arm, and ABC has no guard rule either. ABC's extra - // count is simply the `==` *token*, which happens to sit inside the - // guard. Measured: + // §8 holds throughout, which it did not before #1422. The old table + // here recorded the defect: neither metric modelled the guard, so + // ABC's count was whatever operator token happened to sit inside it + // and cyclomatic's was nothing at all. Both tiers measured, before + // and after: // // | guard | `conditions()` | `cyclomatic() - 1` | // |---|---|---| - // | `when x % 2 == 0` (this fixture) | 3 | 2 | - // | `when x > 2` | 3 | 2 | - // | `when IsEven(x)` | 2 | 2 | - // - // So the gap is not "ABC models guards better"; it is that a - // call-shaped guard restores parity while an operator-shaped one - // does not. That inconsistency is real, predates #1383, and is - // filed rather than changed here — see #1422. + // | `when x % 2 == 0` (this fixture) | 3 → 3 | 2 → 3 | + // | `when x > 2` | 3 → 3 | 2 → 3 | + // | `when IsEven(x)` | 2 → 3 | 2 → 3 | // // The `==` is the guard's own operator and the only `==` in the - // file, so trimming the `when` clause out of the fixture drops the - // count to 2 rather than leaving the assertion satisfied by - // something else. + // file, so trimming the `when` clause out of `w` drops its count to + // 2 rather than leaving the assertion satisfied by something else; + // `assert_csharp_fixture_spells` pins both guards by kind against + // the same decay. #[test] fn csharp_switch_arm_guard_operator_still_counts() { - check_func_space::( - "class A { + let src = "class A { int w(int x) => x switch { > 0 when x % 2 == 0 => 1, > 0 => 2, _ => 3 }; int g(int x) => x switch { int n when n is > 5 => 1, _ => 0 }; - }", - "foo.cs", - |space| { - let m = &space.spaces[0].spaces[0]; - assert_eq!(m.name.as_deref(), Some("w")); - // 4 if the `when_clause` itself started counting, 2 if - // the guard's `==` were suppressed along with the - // pattern operators — both are live regressions. - assert_eq!( - m.metrics.abc.conditions(), - 3, - "two arms plus the guard's `==`" - ); - assert_eq!(m.metrics.cyclomatic.cyclomatic(), 3); - - let g = &space.spaces[0].spaces[1]; - assert_eq!(g.name.as_deref(), Some("g")); - // FIXME(#1422): the one non-discard arm alone. The guard's - // pattern `>` scores nothing where `when n > 5` would add - // one; a guard modelled as a condition slot would score the - // `is` test instead, taking this to 2. - assert_eq!( - g.metrics.abc.conditions(), + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::WhenClause as u16, 2, "`when` guards"), + (Csharp::EQEQ as u16, 1, "the guard's `==`"), + (Csharp::RelationalPattern as u16, 3, "relational patterns"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + let m = &space.spaces[0].spaces[0]; + assert_eq!(m.name.as_deref(), Some("w")); + // 4 if the guard slot started counting a `binary_expression` + // on top of its `==`, 2 if the `==` were suppressed along + // with the pattern operators — both are live regressions. + assert_eq!( + m.metrics.abc.conditions(), + 3, + "two arms plus the guard's `==`" + ); + assert_eq!(m.metrics.cyclomatic.cyclomatic(), 4); + + let g = &space.spaces[0].spaces[1]; + assert_eq!(g.name.as_deref(), Some("g")); + assert_eq!( + g.metrics.abc.conditions(), + 2, + "one arm plus the guard slot, which scores the `is` test" + ); + assert_eq!(g.metrics.cyclomatic.cyclomatic(), 3); + }); + } + + // #1422's headline: four spellings of one guard, previously worth + // two different ABC numbers because nothing modelled the guard and + // ABC counted whatever operator token happened to sit inside it — + // `when x % 2 == 0` and `when x > 2` scored 1 through the + // comparison-token arm, `when IsEven(x)` scored 0. Modelled as a + // condition slot each contributes exactly 1, and the cyclomatic + // `WhenClause` arm lifts the decision count to match. + // + // `none` is the unguarded control and the anchor: a two-arm switch + // over the same relational patterns carrying no guard at all, so + // every member here is exactly one guard away from it. Without that + // row a fix that scored the *arms* differently would satisfy the + // four guarded rows alike. Its arms run `> 2` before `> 0` because + // the reverse order is a C# compile error (CS8510, the later arm + // subsumed) — the guarded members escape that only because a guard + // can fail, which is what makes their repeated `> 0` legal. + // + // `paren` is not a fourth spelling for its own sake — it is the only + // member reaching the `WhenClause` seed added to + // `csharp_inspect_container`, since `when_clause` wraps a + // parenthesised guard in a real `parenthesized_expression` rather + // than the anonymous parens `catch_filter_clause` uses. + // + // `nullc` is #1459's row and the counter-example that falsified the + // claim above when it was first written. A `??` guard is a + // `binary_expression`, which the slot declines — it leaves an + // operator guard to the arm that already counts the operator — and + // no ABC arm counted `??`, so this one spelling read 2 where the + // other four read 3. Counting the token levels it without touching + // the slot; an unconditional `+1` in the slot instead would have + // taken `cmp` to 4, re-creating the double count #1422 removed. + // + // It is the one guarded member whose cyclomatic is not 4, and that + // is not a discrepancy to fix: C# cyclomatic counts `??` as a + // decision *in addition to* the guard clause, so the arm below + // narrows the ABC-versus-cyclomatic gap from two to one rather than + // closing it. The remaining one is the slot's standing policy of + // leaving a `binary_expression` to its operators, and `cmp` pays it + // too — it just happens to break even there. + #[test] + fn csharp_switch_arm_guard_scores_one_condition_however_spelled() { + let src = "class A { + static bool IsEven(int x) { return true; } + int tok(int x) => x switch { > 0 when x % 2 == 0 => 1, > 0 => 2, _ => 3 }; + int cmp(int x) => x switch { > 0 when x > 2 => 1, > 0 => 2, _ => 3 }; + int call(int x) => x switch { > 0 when IsEven(x) => 1, > 0 => 2, _ => 3 }; + int paren(int x) => x switch { > 0 when (IsEven(x)) => 1, > 0 => 2, _ => 3 }; + int nullc(int x, bool? b) => x switch { > 0 when b ?? false => 1, > 0 => 2, _ => 3 }; + int none(int x) => x switch { > 2 => 1, > 0 => 2, _ => 3 }; + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::WhenClause as u16, 5, "`when` guards"), + (Csharp::EQEQ as u16, 1, "`tok`'s `==`"), + (Csharp::QMARKQMARK as u16, 1, "`nullc`'s `??`"), + ( + Csharp::ParenthesizedExpression as u16, 1, - "one arm; the guard's relational pattern scores nothing" - ); - assert_eq!(g.metrics.cyclomatic.cyclomatic(), 2); - }, + "`paren`'s parenthesised guard", + ), + // By the call's *argument list*, which carries no + // numeric-suffix alias, rather than by + // `invocation_expression`, which has three + // (`InvocationExpression` / `2` / `3`) and would pin this + // anchor to whichever one the pinned grammar happens to + // emit. `call` and `paren` hold the only two argument + // lists in the fixture — `IsEven(int x)` is a + // `parameter_list`. + (Csharp::ArgumentList as u16, 2, "the `IsEven` calls"), + ], ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[ + ("IsEven", 0, 1), + ("tok", 3, 4), + ("cmp", 3, 4), + ("call", 3, 4), + ("paren", 3, 4), + // Was 2 — the guard spelling nothing counted. + // Cyclomatic 5 rather than 4 because `??` is a + // decision there on top of the guard clause. + ("nullc", 3, 5), + ("none", 2, 3), + ], + ); + }); + } + + // The bare type test `x is int` is `is_expression` (391); only once + // a pattern is involved (`x is int y`, `x is null`) does the grammar + // emit `is_pattern_expression` (392). They are distinct kinds, and + // `csharp_bool_terminal_kinds!()` listed only the second, so the 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. + // + // Found by the whole-branch review of this batch, and it falsified + // #1422's own claim in the arm above that a guard "scores one + // however it is spelled" — `when x is int` read 1 where + // `when IsEven(x)` read 2, which is the spelling-dependence that + // fix exists to remove. It is the C# half of the gap #1421 closed + // for Kotlin by adding `IsExpression | InExpression` there. + // + // The plain `if` members are the control that keeps this honest: a + // guard-only fixture could be satisfied by a guard-specific rule, + // and the defect was never guard-specific. `q` and `iq` are the + // already-correct `is_pattern_expression` twins, so a regression + // that reintroduced the asymmetry fails on the pair rather than on + // an absolute number. + #[test] + fn csharp_bare_is_type_test_scores_one_condition() { + let src = "class A { + static bool IsEven(int x) { return true; } + int g(object x) => x switch { object o when x is int => 1, _ => 0 }; + int gq(object x) => x switch { object o when x is int y => 1, _ => 0 }; + int gc(object x) => x switch { object o when IsEven(1) => 1, _ => 0 }; + int p(object x) { if (x is int) { return 1; } return 0; } + int q(object x) { if (x is int y) { return 1; } return 0; } + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::IsExpression as u16, 2, "the bare `is` type tests"), + ( + Csharp::IsPatternExpression as u16, + 2, + "the designation-pattern twins", + ), + (Csharp::WhenClause as u16, 3, "`when` guards"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[ + ("IsEven", 0, 1), + // Level with `gq` and `gc`: every guard spelling + // scores one, which is what #1422 claims. + ("g", 2, 3), + ("gq", 2, 3), + ("gc", 2, 3), + // Level with `q`: the two spellings of one type + // test agree outside a guard too. + ("p", 1, 2), + ("q", 1, 2), + ], + ); + }); + } + + // The statement `switch` reaches the same `when_clause` kind through + // a different parent — `switch_section` rather than + // `switch_expression_arm` — which makes it an independent path in + // the sense of `.claude/rules/grammar-dispatch.md` §11: the arms + // themselves are counted by two different owners (a `Case` token + // here, the `SwitchExpressionArm` node there), so a guard rule + // written against one shape could be dead for the other and every + // expression-form fixture would still read correct. + #[test] + fn csharp_statement_switch_section_guard_counts() { + let src = "class A { + static bool IsEven(int x) { return true; } + int guarded(int x) { + switch (x) { + case > 0 when IsEven(x): return 1; + case > 0: return 2; + default: return 3; + } + } + int plain(int x) { + switch (x) { + case > 2: return 1; + case > 0: return 2; + default: return 3; + } + } + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::WhenClause as u16, 1, "the `case … when` guard"), + (Csharp::SwitchSection as u16, 6, "switch sections"), + ( + Csharp::SwitchExpressionArm as u16, + 0, + "expression arms (this fixture is the statement form)", + ), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[("IsEven", 0, 1), ("guarded", 3, 4), ("plain", 2, 3)], + ); + }); + } + + // The sibling #1422's issue body does not mention, measured to have + // the identical defect: `catch (E e) when (…)` is a second guard + // spelling on its own `catch_filter_clause` kind, and it scored + // conditions 3 / 2 / 2 for the comparison / call spellings and the + // unfiltered control while `cyclomatic()` sat at 2 for all three. + // + // ABC runs one above `cyclomatic() - 1` on every row, guarded and + // unguarded alike, and that offset is not this fix's: C# ABC counts + // the `Try` token as a condition (Fitzpatrick Rule 5, the same + // family as `else`) where cyclomatic counts only the `catch`. The + // guard moves both tiers by exactly one, which is what `none` + // anchors — the offset is a constant here, not something a guard + // introduces. + // + // `paren` carries the *double* parenthesis on purpose. The grammar + // spells `catch_filter_clause`'s own parentheses as anonymous tokens + // the way `if_statement` does, so `when (IsEven(x))` hands the slot a + // bare `invocation_expression` and never reaches the + // `CatchFilterClause` seed in `csharp_inspect_container`; only a + // second pair of parentheses produces a `parenthesized_expression` + // there. + #[test] + fn csharp_catch_filter_guard_scores_one_condition_however_spelled() { + let src = "class A { + static bool IsEven(int x) { return true; } + int tok(int x) { try { return 1; } catch (System.Exception e) when (x % 2 == 0) { return 2; } } + int cmp(int x) { try { return 1; } catch (System.Exception e) when (x > 0) { return 2; } } + int call(int x) { try { return 1; } catch (System.Exception e) when (IsEven(x)) { return 2; } } + int paren(int x) { try { return 1; } catch (System.Exception e) when ((IsEven(x))) { return 2; } } + int none(int x) { try { return 1; } catch (System.Exception e) { return 2; } } + }"; + assert_csharp_fixture_spells( + src, + &[ + ( + Csharp::CatchFilterClause as u16, + 4, + "`catch … when` filters", + ), + (Csharp::CatchClause as u16, 5, "catch clauses"), + (Csharp::WhenClause as u16, 0, "switch guards (none here)"), + ( + Csharp::ParenthesizedExpression as u16, + 1, + "`paren`'s inner parentheses", + ), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[ + ("IsEven", 0, 1), + ("tok", 3, 3), + ("cmp", 3, 3), + ("call", 3, 3), + ("paren", 3, 3), + ("none", 2, 2), + ], + ); + }); + } + + // A guarded discard is where the two rules meet. `_ when g =>` is + // already exempt from the `default:` exclusion — + // `csharp_switch_expression_arm_is_bare_discard` looks for a + // `when_clause` for exactly this reason — so after #1422 it scores + // twice: once as an arm that is no longer unconditional, once for + // the guard that makes it conditional. `bare` is the control that + // keeps the exclusion itself pinned: drop the guard and the arm goes + // back to costing nothing. + #[test] + fn csharp_guarded_discard_arm_scores_arm_and_guard() { + let src = "class A { + static bool IsEven(int x) { return true; } + int guarded(int x) => x switch { > 0 => 1, _ when IsEven(x) => 2, _ => 3 }; + int bare(int x) => x switch { > 0 => 1, _ => 3 }; + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::WhenClause as u16, 1, "the guarded discard"), + (Csharp::Discard as u16, 3, "discard patterns"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[("IsEven", 0, 1), ("guarded", 3, 4), ("bare", 1, 2)], + ); + }); + } + + // A comment inside the guard must not cost it its condition. + // tree-sitter `extra`s are *named* nodes, so `comment` can be the + // first named child of either guard clause — and a slot that read + // only the first named child scored `when /*c*/ IsEven(x)` as 1 + // where `when IsEven(x)` scores 2, silently reinstating the very + // spelling-dependence #1422 removes and breaking the §8 parity the + // fix establishes. Found in review of #1422 before it shipped. + // + // This is the C# instance of the class #1181 fixed for the ternary, + // where a comment between a token and its operand shifted every + // positional read. Each commented member is asserted against the + // same literal pair as its uncommented twin in the same table, so a + // regression that moved only the commented spelling fails here; the + // literals themselves are pinned by + // `csharp_switch_arm_guard_scores_one_condition_however_spelled` and + // its catch-filter sibling. + #[test] + fn csharp_guard_keeps_its_condition_across_a_comment() { + let src = "class A { + static bool IsEven(int x) => true; + int plain(int x) => x switch { > 0 when IsEven(x) => 1, _ => 0 }; + int cmt(int x) => x switch { > 0 when /*c*/ IsEven(x) => 1, _ => 0 }; + int cplain(int x) { try { return 1; } catch (System.Exception e) when (IsEven(x)) { return 2; } } + int ccmt(int x) { try { return 1; } catch (System.Exception e) when (/*c*/ IsEven(x)) { return 2; } } + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::Comment as u16, 2, "the in-guard comments"), + (Csharp::WhenClause as u16, 2, "switch guards"), + (Csharp::CatchFilterClause as u16, 2, "catch filters"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score( + &space.spaces[0], + &[ + ("IsEven", 0, 1), + ("plain", 2, 3), + ("cmt", 2, 3), + ("cplain", 3, 3), + ("ccmt", 3, 3), + ], + ); + }); + } + + // Why the guard is a condition *slot* and not a flat +1: a compound + // guard keeps its sub-structure. `when a > 1 && b < 2` scores the + // two comparisons through the token arm and nothing extra from the + // slot, because a `binary_expression` is not one of + // `csharp_bool_terminal_kinds!()` — so it reads one above `single`'s + // lone comparison rather than collapsing to the same number, and + // cyclomatic agrees because it counts the `&&`. + #[test] + fn csharp_compound_guard_keeps_its_sub_structure() { + let src = "class A { + int compound(int a, int b) => a switch { > 0 when a > 1 && b < 2 => 1, _ => 0 }; + int single(int a, int b) => a switch { > 0 when a > 1 => 1, _ => 0 }; + }"; + assert_csharp_fixture_spells( + src, + &[ + (Csharp::WhenClause as u16, 2, "`when` guards"), + (Csharp::AMPAMP as u16, 1, "`compound`'s `&&`"), + (Csharp::LT as u16, 1, "`compound`'s `<`"), + ], + ); + check_func_space::(src, "foo.cs", |space| { + assert_members_score(&space.spaces[0], &[("compound", 3, 4), ("single", 2, 3)]); + }); } // #1275: tree-sitter-c-sharp spells `int?`, `where T : class?`, the @@ -4056,24 +4887,55 @@ mod tests { // // This test is what stops a later "make all three languages // consistent" pass from flipping C# to a `ConditionalExpression` - // allowlist: that would silently drop both counts here to 0 while - // every other ABC test still passed. `??` is deliberately absent - // from the expected total — C# ABC does not list `QMARKQMARK` as a - // condition (it does in the TS family), which is pre-existing and - // out of scope for #1275. + // allowlist: that would silently drop both `?` counts here while + // every other ABC test still passed. The third condition is the + // `??`, which #1459 added to close the gap this comment used to + // record as out of scope for #1275 — so a flipped `QMARK` polarity + // now reads 1 rather than 0, and the discrimination is unchanged. #[test] fn csharp_conditional_access_still_counts_as_a_condition() { - check_metrics::( - "class A { + let src = "class A { object M(string s, int[] xs) { return s?.Length ?? xs?[0]; } - }", - "foo.cs", - |metric| { - assert_eq!(metric.abc.conditions_sum(), 2); - }, + }"; + // The total is now fed by two token arms rather than one, so it + // carries the per-source claim only alongside these counts: 3 is + // two bare `?` plus one `??`, and without them a `QMARK` arm + // that had stopped counting `a?[0]` while `??` gained a second + // count elsewhere would still read 3. + assert_csharp_fixture_spells( + src, + &[ + (Csharp::QMARK as u16, 2, "the `?.` and `?[` operators"), + (Csharp::QMARKQMARK as u16, 1, "the `??`"), + ], ); + check_metrics::(src, "foo.cs", |metric| { + assert_eq!(metric.abc.conditions_sum(), 3); + }); + } + + // The `QMARKQMARK` arm away from any guard, where nothing else can + // supply the count (#1459). `coalesce`'s body holds no comparison, + // no call and no control-flow keyword, so its one condition is the + // `??` and nothing else; it read 0 before the arm while C# + // cyclomatic scored the decision, which is the direction ABC is not + // allowed to sit on. + // + // `plain` is the control that keeps this from being an assertion + // about `return` or about the method shape: same body without the + // operator, 0 conditions and 1 decision. + #[test] + fn csharp_null_coalescing_is_a_condition() { + let src = "class A { + int coalesce(int? x) { return x ?? 0; } + int plain(int x) { return x; } + }"; + assert_csharp_fixture_spells(src, &[(Csharp::QMARKQMARK as u16, 1, "the `??`")]); + check_func_space::(src, "foo.cs", |space| { + assert_members_score(&space.spaces[0], &[("coalesce", 1, 2), ("plain", 0, 1)]); + }); } // The other direction of #1275's C# gate, isolated: narrowing @@ -4768,72 +5630,561 @@ function f(int $a, int $b): int { ); } - #[test] - fn kotlin_identity_equality_conditions() { - // `===` / `!==` are referential equality in Kotlin; they count too. - check_metrics::( - "fun m(a: Any, b: Any): Boolean { - return a === b || a !== b - }", - "foo.kt", - |metric| { - assert_eq!(metric.abc.conditions_sum(), 2); - insta::assert_json_snapshot!(metric.abc); - }, + #[test] + fn kotlin_identity_equality_conditions() { + // `===` / `!==` are referential equality in Kotlin; they count too. + check_metrics::( + "fun m(a: Any, b: Any): Boolean { + return a === b || a !== b + }", + "foo.kt", + |metric| { + assert_eq!(metric.abc.conditions_sum(), 2); + insta::assert_json_snapshot!(metric.abc); + }, + ); + } + + #[test] + fn kotlin_else_branch_counts() { + check_metrics::( + "fun m(x: Int): Int { + return if (x > 0) 1 else -1 + }", + "foo.kt", + |metric| { + // condition: > (1) + else (1) = 2 + assert_eq!(metric.abc.conditions_sum(), 2); + insta::assert_json_snapshot!(metric.abc); + }, + ); + } + + #[test] + fn kotlin_when_entries_count() { + check_metrics::( + "fun m(x: Int): Int { + return when (x) { + 1 -> 10 + 2 -> 20 + else -> 0 + } + }", + "foo.kt", + |metric| { + // Non-`else` WhenEntry arms count; the `else ->` fallback + // arm does not (issue #456). Two case arms + zero for the + // `else` arm = 2. + assert_eq!(metric.abc.conditions_sum(), 2); + insta::assert_json_snapshot!(metric.abc); + }, + ); + } + + // Pins the `else ->` exclusion directly: a `when` whose only fallback + // is `else ->` must not count that arm. Revert-verified — gating the + // `WhenEntry` arm on `!kotlin_when_entry_is_else` is what drops this + // from 3 to 2 (issue #456, lesson 11). Mirrors the cyclomatic gate. + #[test] + fn kotlin_when_else_not_a_condition() { + check_metrics::( + "fun m(x: Int): Int { + return when (x) { 1 -> 10; 2 -> 20; else -> 0 } + }", + "foo.kt", + |metric| { + // case `1 ->` (+1) + case `2 ->` (+1) + `else ->` (+0) = 2. + assert_eq!(metric.abc.conditions_sum(), 2); + }, + ); + } + + // The Kotlin binding of the two assertions every #1421 fixture needs, + // coupled on purpose. A per-member score table is worth only as much + // as the anchor proving the fixture still spells what it claims to + // (`.claude/rules/testing.md`, "Perturb the fixture as well as the + // production line"), and the `when_subject` count is the anchor no + // score can stand in for: a `when` that grew a subject scores its + // entries the pre-#1421 way with every other row still satisfied. + // Each fixture is one class, so the members are `spaces[0]`'s. + #[track_caller] + fn assert_kotlin_class_members( + src: &str, + kinds: &[(u16, usize, &str)], + expected: &[(&str, u64, u64)], + ) { + assert_fixture_spells::(src, "foo.kt", kinds); + check_func_space::(src, "foo.kt", |space| { + assert_members_score(&space.spaces[0], expected); + }); + } + + // #1421's headline. A subject-less `when` arm's condition is an + // ordinary boolean expression, so its comparison operator is already + // an ABC condition through the token arms; the `WhenEntry` arm added + // a second one on top, and `two` reported 4 against a decision count + // of 2. + // + // `bare` is the control that isolates the defect rather than merely + // observing it: same arm count, same entry shape, no comparison + // operator anywhere — so it scored the right 2 before the fix and + // after it. A fix that changed both members alike would be attacking + // the entry count, which was never wrong. + // + // `ge` and `eq` exist because the issue named `<` and `>` only. All + // six comparison spellings share the defect: `>=` and `==` reach + // `stats.conditions` through the `LTEQ | GTEQ | EQEQ | …` arm rather + // than the parent-gated `LT | GT` arm, and both scored 2 against a + // decision count of 1. + // + // `iff` is the member that legitimately sits *above* its decision + // count, and it is here so the table cannot be read as "§8 parity + // everywhere". ABC counts the `else` (Fitzpatrick Rule 5) and + // cyclomatic does not, so 2 conditions against 1 decision is correct + // and unchanged by this fix. + // + // The `when_subject` anchor is the load-bearing one: the whole test + // turns on these `when`s having no subject, and nothing in the + // measured numbers would notice a fixture that grew one — a + // subject-ful `when (x) { … }` scores its entries the old way and + // `two` would read 4 again with every other row still satisfied. + #[test] + fn kotlin_subjectless_when_arm_counts_its_condition_once() { + let src = "class K { + fun f(): Boolean = true + fun two(x: Int): Int = when { x > 5 -> 1; x < 0 -> 2; else -> 0 } + fun bare(x: Boolean, y: Boolean): Int = when { x -> 1; y -> 2; else -> 0 } + fun ge(x: Int): Int = when { x >= 5 -> 1; else -> 0 } + fun eq(x: Int): Int = when { x == 5 -> 1; else -> 0 } + fun andd(a: Int, b: Int): Int = when { a > 1 && b < 2 -> 1; else -> 0 } + fun call(): Int = when { f() -> 1; else -> 0 } + fun iff(x: Int): Int = if (x > 5) 1 else 0 + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::WhenSubject as u16, 0, "`when` subjects"), + (Kotlin::WhenEntry as u16, 14, "`when` entries"), + ( + Kotlin::BinaryExpression as u16, + 8, + "comparison / chain expressions", + ), + (Kotlin::AMPAMP as u16, 1, "`andd`'s `&&`"), + ], + &[ + // (member, abc.conditions, cyclomatic) + ("f", 0, 1), + // Was 4: two entries, two comparisons, counted twice. + ("two", 2, 3), + // The no-comparison control — 2 before and after. + ("bare", 2, 3), + // Was 2 each: the `GTEQ` / `EQEQ` spellings. + ("ge", 1, 2), + ("eq", 1, 2), + // Was 3. A compound condition keeps both of its + // comparisons — suppressing the operators instead + // would have collapsed this one to 1. + ("andd", 2, 3), + // The condition slot scores a call directly, so a + // spelling with no operator in it still reads 1. + ("call", 1, 2), + // Above its decision count on purpose: ABC counts + // the `else`, cyclomatic does not. + ("iff", 2, 2), + ], + ); + } + + // #1459, the regression #1421 left behind. Turning the `when` + // entry's blanket count into a condition slot moved the payment from + // the entry to the condition — so every shape the slot cannot + // classify went from 1 to 0, silently. Two shapes did: + // + // - `a!!`. `unary_expression` is one kind for *both* Kotlin unary + // spellings, and the slot routed it to the peel, so this reads as + // handled at every call site. The peel then looked for the operand + // at child(1) — right for the prefix `!x`, wrong for a postfix + // `x!!`, whose child(1) is the `!!` token — and stopped. + // - `a as Boolean`. `as_expression` was not a wrapper the peel knew + // at all, so the slot fell off the end of its `else if`. + // + // The controls are `bare` / `cmp` / `call`, one per surviving path + // (terminal, comparison token, invocation): a fix that moved the + // entry count back would take those to 2 and fail here, which is + // what distinguishes repairing the peel from reverting #1421. + // + // `safe` is the §5 row and the one measurement decided rather than + // taste. `as?` is already a condition *token* + // (`src/metrics/abc/kotlin.rs`), so a peel that treated `as` and + // `as?` alike would score it 2 — the double count that arm's own + // fix was avoiding. `kotlin_wrapper_operand` excludes `as?` for + // that reason, and this member is what fails if the exclusion goes: + // `(a as? Boolean)!!` is valid Kotlin whose peel reaches the cast + // through two other wrappers, so it needs no invalid fixture to + // discriminate. + // + // `iff` carries the same `a!!` through the `if` slot. That half is + // not #1421 fallout — `if (a!!)` has read 0 since the slot landed in + // #773 — but it is the same peel and the same edit, and a fix + // narrowed to `when` entries would leave it at 0. + // + // Every member is valid Kotlin: `a!!` on a `Boolean?`, `as` on an + // `Any`, and `(a as? Boolean)!!` back to a `Boolean`. The kind + // anchors are what stop a later edit from trimming a spelling out + // and turning its member into a silent copy of `bare`. + #[test] + fn kotlin_condition_slot_peels_null_assertions_and_casts() { + let src = "class K { + fun g(x: Boolean): Boolean = true + fun bare(a: Boolean): Int = when { a -> 1; else -> 0 } + fun cmp(a: Int): Int = when { a > 5 -> 1; else -> 0 } + fun call(a: Boolean): Int = when { g(a) -> 1; else -> 0 } + fun bang(a: Boolean?): Int = when { a!! -> 1; else -> 0 } + fun paren(a: Boolean?): Int = when { (a!!) -> 1; else -> 0 } + fun cast(a: Any): Int = when { a as Boolean -> 1; else -> 0 } + fun chain(a: Any?): Int = when { a!! as Boolean -> 1; else -> 0 } + fun safe(a: Any): Int = when { (a as? Boolean)!! -> 1; else -> 0 } + fun cmt(a: Boolean): Int = when { ! /*c*/ a -> 1; else -> 0 } + fun iff(a: Boolean?): Int { if (a!!) { return 1 }; return 0 } + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::WhenSubject as u16, 0, "`when` subjects"), + ( + Kotlin::BANGBANG as u16, + 5, + "the `!!` null assertions of `bang` / `paren` / `chain` / `safe` / `iff`", + ), + ( + Kotlin::AsExpression as u16, + 3, + "the casts of `cast` / `chain` / `safe`", + ), + (Kotlin::AsQMARK as u16, 1, "`safe`'s safe cast"), + ( + Kotlin::BlockComment as u16, + 1, + "`cmt`'s interposed comment, spaces and all", + ), + ( + Kotlin::ParenthesizedExpression as u16, + 2, + "the parenthesised operands of `paren` / `safe`", + ), + ], + &[ + // (member, abc.conditions, cyclomatic) + ("g", 0, 1), + // The three controls, unchanged by this fix. + ("bare", 1, 2), + ("cmp", 1, 2), + ("call", 1, 2), + // Each was 0: the peel stopped on the wrapper. + ("bang", 1, 2), + ("paren", 1, 2), + ("cast", 1, 2), + ("chain", 1, 2), + // 1, not 2 — the `as?` token owns this one, and the + // peel declines to charge for it a second time. + ("safe", 1, 2), + // The peel reads its operand through the `operator` / + // `argument` fields, so an `extra` between the two no + // longer displaces it. The spacing is load-bearing and + // must not be tidied away: `!/*c*/a` unspaced parses + // with the comment somewhere a `child(1)` read still + // skips, so only the spaced spelling can tell the field + // read from the positional one. Measured both ways. + ("cmt", 1, 2), + // The `if` slot, which has read 0 since #773. + ("iff", 1, 2), + ], + ); + } + + // The other half of #1421: a subject-ful entry must not move. Its + // condition is a *pattern* matched against the subject, not an + // independent boolean expression, so the implicit `subject == + // pattern` is the decision and the entry itself pays for it — + // `range_test`, `type_test` and a bare constant carry no token the + // comparison arms would count. + // + // `cmp` is the interesting row and the reason `nested` sits beside + // it. `when (x) { y > 5 -> … }` is legal when `x` is `Boolean`, and + // it reads 2 conditions against 1 decision — the entry's implicit + // equality plus the `>` inside the operand. That is not the #1421 + // double-count reappearing: `nested` spells the same thing as an + // `if` (`x == (y > 5)`) and ABC has always scored it 2 against the + // same 1 decision. A comparison nested inside a comparison is two + // comparisons; only one of them is a branch. + #[test] + fn kotlin_subjectful_when_arms_keep_the_per_entry_count() { + let src = "class K { + fun inn(a: Int): Int = when (a) { in 1..2 -> 1; else -> 0 } + fun notin(a: Int): Int = when (a) { !in 1..2 -> 1; else -> 0 } + fun iss(a: Any): Int = when (a) { is String -> 1; else -> 0 } + fun konst(a: Int): Int = when (a) { 1 -> 10; 2 -> 20; else -> 0 } + fun cmp(x: Boolean, y: Int): Int = when (x) { y > 5 -> 1; else -> 0 } + fun nested(x: Boolean, y: Int): Int { if (x == (y > 5)) { return 1 }; return 0 } + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::WhenSubject as u16, 5, "`when` subjects"), + (Kotlin::WhenEntry as u16, 11, "`when` entries"), + (Kotlin::RangeTest as u16, 2, "`in` / `!in` patterns"), + (Kotlin::TypeTest as u16, 1, "the `is` pattern"), + ( + Kotlin::BinaryExpression as u16, + 3, + "`cmp`'s and `nested`'s comparisons", + ), + ], + &[ + ("inn", 1, 2), + ("notin", 1, 2), + ("iss", 1, 2), + ("konst", 2, 3), + // Above its decision count, like `nested` below it + // and for the same reason. + ("cmp", 2, 2), + ("nested", 2, 2), + ], + ); + } + + // `kotlin_enclosing_when_has_subject` stops scanning at the `{`, so + // that a subject-less `when` does not walk all N arms once per arm. + // The bound has to clear the header first, and the header can carry + // an `extra`: tree-sitter hangs `when /*c*/ (x)`'s comment off the + // `when_expression` between the keyword and the subject, so a bound + // one child tighter — `child(1)`, or a stop at the first *named* + // child — reads `cmtSubject` as subject-less. It would then score + // its `1` arm through the slot, where a `number_literal` is not a + // Kotlin terminal, and report 0 conditions instead of 1. That is + // what discriminates here; `cmtNone` is the other direction, where + // the brace is all the bound has to stop on. + // + // The `block_comment` census is the fixture anchor: delete either + // comment and the members still read 1, because a plain `when (x)` + // and a plain `when {` both do. The count fails by name instead. + #[test] + fn kotlin_when_subject_scan_clears_a_comment_before_the_brace() { + let src = "class K { + fun cmtSubject(x: Int): Int = when /*c*/ (x) { 1 -> 1; else -> 0 } + fun cmtNone(a: Boolean): Int = when /*c*/ { a -> 1; else -> 0 } + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::BlockComment as u16, 2, "the two header comments"), + (Kotlin::WhenSubject as u16, 1, "`cmtSubject`'s subject"), + (Kotlin::WhenEntry as u16, 4, "`when` entries"), + ], + &[("cmtSubject", 1, 2), ("cmtNone", 1, 2)], + ); + } + + // A subject-less condition reaches the slot through the same + // `kotlin_inspect_container` unwrapping an `if` predicate does, which + // is what separates this design from suppressing the entry's + // operator. `notted` and `paren` unwrap to a bare terminal and score + // through the slot; `notCmp` and `parenCmp` unwrap to a + // `binary_expression`, score nothing from the slot, and take their + // one condition from the `>` the token arm already owned. Both pairs + // land on 1, which no blanket `+1` can produce for the second pair — + // they read 2 before the fix. + // + // `paren` is the member that reaches the `WhenEntry` seed added to + // `kotlin_inspect_container`'s boolean-context set; without it the + // parenthesised operand is not in a slot the walker calls boolean and + // `paren` drops to 0. `notted` cannot stand in for it — a `!` + // operator proves boolean content on its own. + #[test] + fn kotlin_subjectless_when_condition_wrappers_count_once() { + let src = "class K { + fun notted(a: Boolean): Int = when { !a -> 1; else -> 0 } + fun paren(a: Boolean): Int = when { (a) -> 1; else -> 0 } + fun notCmp(a: Int): Int = when { !(a > 1) -> 1; else -> 0 } + fun parenCmp(a: Int): Int = when { (a > 1) -> 1; else -> 0 } + fun nest(x: Int, y: Int): Int = + when { x > 1 -> when { y > 2 -> 1; else -> 2 }; else -> 0 } + fun blockBody(x: Int): Int { when { x > 5 -> { return 1 } }; return 0 } + }"; + assert_kotlin_class_members( + src, + &[ + // Paired, not bare: "no subjects" alone is satisfied by a + // fixture that lost its `when`s altogether. + (Kotlin::WhenSubject as u16, 0, "`when` subjects"), + (Kotlin::WhenEntry as u16, 13, "`when` entries"), + (Kotlin::UnaryExpression as u16, 2, "the two `!` operands"), + ( + Kotlin::ParenthesizedExpression as u16, + 3, + "the three parenthesised operands", + ), + (Kotlin::BinaryExpression as u16, 5, "comparisons"), + ], + &[ + ("notted", 1, 2), + ("paren", 1, 2), + // Both were 2: the entry's blanket count plus the `>`. + ("notCmp", 1, 2), + ("parenCmp", 1, 2), + // Was 4 — one entry and one comparison per `when`, + // each counted twice. + ("nest", 2, 3), + // The `when`-as-statement shape: a `block` body and no + // `else` arm at all. Reading the condition from the + // `condition` field rather than by position is what keeps + // the block out of the slot; it was 2 before the fix. + ("blockBody", 1, 2), + ], ); } - #[test] - fn kotlin_else_branch_counts() { - check_metrics::( - "fun m(x: Int): Int { - return if (x > 0) 1 else -1 - }", - "foo.kt", - |metric| { - // condition: > (1) + else (1) = 2 - assert_eq!(metric.abc.conditions_sum(), 2); - insta::assert_json_snapshot!(metric.abc); - }, + // The one place this design deliberately does not reach parity, put + // under test so the choice cannot drift unremarked. A `when` entry + // may list alternatives (`when { a, b -> … }`, an implicit `or`), and + // the `condition` field is `multiple` — but cyclomatic scores the + // whole entry as one decision, so `bareAlts` is at parity only + // because the slot reads the *first* alternative and stops. Routing + // every alternative through the slot would push it to 2. + // + // `alts` is the row that cannot be brought to parity from the ABC + // side at all: the comparison-token arms count `>` and `<` wherever + // they appear, so it reads 2 against 1 decision no matter what the + // entry slot does. It was 3 before #1421 and the remaining gap is + // cyclomatic's single count per multi-alternative entry, not a + // double-count here. + // + // `subjAlts` is the subject-ful control, unchanged and at parity: its + // alternatives are constants carrying no token to count. + #[test] + fn kotlin_subjectless_when_multi_alternative_entry() { + let src = "class K { + fun alts(x: Int, y: Int): Int = when { x > 5, y < 0 -> 1; else -> 0 } + fun bareAlts(x: Boolean, y: Boolean): Int = when { x, y -> 1; else -> 0 } + fun subjAlts(a: Int): Int = when (a) { 1, 2 -> 10; else -> 0 } + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::WhenSubject as u16, 1, "`subjAlts`'s subject"), + (Kotlin::WhenEntry as u16, 6, "`when` entries"), + ( + Kotlin::BinaryExpression as u16, + 2, + "`alts`'s two comparisons", + ), + ], + &[ + // Above its decision count: cyclomatic scores the entry + // once, the token arms score both comparisons. Was 3. + ("alts", 2, 2), + ("bareAlts", 1, 2), + ("subjAlts", 1, 2), + ], ); } - #[test] - fn kotlin_when_entries_count() { - check_metrics::( - "fun m(x: Int): Int { - return when (x) { - 1 -> 10 - 2 -> 20 - else -> 0 - } - }", - "foo.kt", - |metric| { - // Non-`else` WhenEntry arms count; the `else ->` fallback - // arm does not (issue #456). Two case arms + zero for the - // `else` arm = 2. - assert_eq!(metric.abc.conditions_sum(), 2); - insta::assert_json_snapshot!(metric.abc); - }, + // `is_expression` and `in_expression` are the two relational forms + // tree-sitter-kotlin-ng spells as their own production rather than as + // a `binary_expression`, so no comparison-token arm sees them and + // nothing counted them: `if (a is String)` scored 0 against a + // decision count of 1, and the same test in a subject-less `when` + // scored 1 only because the entry's blanket `+1` happened to cover + // it. Removing that blanket without listing these two in + // `kotlin_bool_terminal_kinds!()` would have regressed `whenIs` and + // `whenIn` to 0, and `isAnd` — where the `is` test is one operand of + // a `&&` chain and so reaches the walker rather than the slot — from + // 2 to 1. + // + // The `if` members are here because the fix is in the shared + // terminal-kind set, not in the `when` arm: they are the call sites + // that prove it reaches the `if` / `while` predicate slot too. + #[test] + fn kotlin_is_and_in_expressions_are_unary_conditions() { + let src = "class K { + fun ifIs(a: Any): Int { if (a is String) { return 1 }; return 0 } + fun ifIn(a: Int): Int { if (a in 1..2) { return 1 }; return 0 } + fun whenIs(a: Any): Int = when { a is String -> 1; else -> 0 } + fun whenIn(a: Int): Int = when { a in 1..2 -> 1; else -> 0 } + fun isAnd(a: Any, b: Boolean): Int = when { a is String && b -> 1; else -> 0 } + }"; + assert_kotlin_class_members( + src, + &[ + // Paired, not bare — see the wrappers test above. + (Kotlin::WhenSubject as u16, 0, "`when` subjects"), + (Kotlin::WhenEntry as u16, 6, "`when` entries"), + (Kotlin::IsExpression as u16, 3, "`is` tests"), + (Kotlin::InExpression as u16, 2, "`in` tests"), + (Kotlin::AMPAMP as u16, 1, "`isAnd`'s `&&`"), + ], + &[ + // Both were 0 — the predicate slot saw a kind it did + // not classify and the token arms saw no operator. + ("ifIs", 1, 2), + ("ifIn", 1, 2), + ("whenIs", 1, 2), + ("whenIn", 1, 2), + ("isAnd", 2, 3), + ], ); } - // Pins the `else ->` exclusion directly: a `when` whose only fallback - // is `else ->` must not count that arm. Revert-verified — gating the - // `WhenEntry` arm on `!kotlin_when_entry_is_else` is what drops this - // from 3 to 2 (issue #456, lesson 11). Mirrors the cyclomatic gate. - #[test] - fn kotlin_when_else_not_a_condition() { - check_metrics::( - "fun m(x: Int): Int { - return when (x) { 1 -> 10; 2 -> 20; else -> 0 } - }", - "foo.kt", - |metric| { - // case `1 ->` (+1) + case `2 ->` (+1) + `else ->` (+0) = 2. - assert_eq!(metric.abc.conditions_sum(), 2); - }, + // Kotlin spells boolean `and` / `or` / `xor` as infix *functions*, so + // `a and b` parses as `infix_expression` — not `binary_expression`, + // and not any token arm. It belongs in + // `kotlin_bool_terminal_kinds!()` for the same reason + // `call_expression` does: `a and b` is `a.and(b)`. + // + // This was a regression of #1421, not a pre-existing gap, and it is + // the third shape the blanket per-entry count turned out to be + // propping up — after `is` / `in` and bare parens, which that fix + // caught. Found by the whole-branch review. Measured on both sides: + // `when { a and b -> … }` scored 1 before and 0 after, while the + // `if (a and b)` it is supposed to agree with still scored 1. + // + // The `if` members are the anchor rather than a second `when`: the + // whole premise of #1421 is that a subject-less arm scores like an + // `if` predicate, so a fixture where the two disagree is the + // statement of the bug. `andAmp` holds the pair level against the + // `&&` spelling, which never regressed and would otherwise be the + // only form under test. + #[test] + fn kotlin_infix_boolean_functions_are_unary_conditions() { + let src = "class K { + fun whenAnd(a: Boolean, b: Boolean): Int = when { a and b -> 1; else -> 0 } + fun whenOr(a: Boolean, b: Boolean): Int = when { a or b -> 1; else -> 0 } + fun whenXor(a: Boolean, b: Boolean): Int = when { a xor b -> 1; else -> 0 } + fun whenAmp(a: Boolean, b: Boolean): Int = when { a && b -> 1; else -> 0 } + fun ifAnd(a: Boolean, b: Boolean): Int { if (a and b) { return 1 }; return 0 } + }"; + assert_kotlin_class_members( + src, + &[ + (Kotlin::WhenSubject as u16, 0, "`when` subjects"), + (Kotlin::InfixExpression as u16, 4, "the infix calls"), + (Kotlin::AMPAMP as u16, 1, "`whenAmp`'s `&&`"), + ], + &[ + // 0 before this fix: the predicate slot saw a kind it + // did not classify and no token arm matched. + ("whenAnd", 1, 2), + ("whenOr", 1, 2), + ("whenXor", 1, 2), + // Never regressed — `&&` is a `binary_expression` whose + // operator the token arm owns. Holds the others level. + ("whenAmp", 2, 3), + // The agreement #1421 exists to establish: a + // subject-less arm scores what the equivalent `if` + // predicate scores. + ("ifAnd", 1, 2), + ], ); } @@ -5090,6 +6441,62 @@ function f(int $a, int $b): int { ); } + // `kotlin_inspect_container` has two callers, and #1459's fixture + // members all reach it through the *slot* + // (`kotlin_count_condition`). This is the other one: the `&&` / `||` + // walker, which routes any named non-terminal operand through the + // same peel. The two wrappers #1459 taught it therefore have a + // second, structurally independent path into the count, and a + // fixture covering only the slot leaves it untested + // (`.claude/rules/grammar-dispatch.md` §11). + // + // Each chain is one identifier plus one wrapped operand, so 2 is + // "the walker peeled the wrapper" and 1 is "it gave up on it" — + // which is what both lines scored before #1459. The bare `a && b` + // control is `kotlin_unary_conditions_in_chain`'s shape at 2, so a + // regression cannot be read as the chain itself changing. + #[test] + fn kotlin_chain_operands_peel_null_assertions_and_casts() { + // Anchored per row, because the assertion alone cannot tell the + // wrapper from its operand: `a && b` scores the same 2 / 3 as + // every row here, so trimming the `!!` or the cast out of a + // fixture would leave this green with its subject gone + // (`.claude/rules/testing.md`, "Perturb the fixture as well as + // the production line"). + for (chain, spelling, anchors) in [ + ( + "a && b!!", + "null assertion", + &[(Kotlin::BANGBANG as u16, 1usize, "the `!!`")][..], + ), + ( + "a && b as Boolean", + "cast", + &[(Kotlin::AsExpression as u16, 1, "the `as` cast")][..], + ), + ( + "a && (b!!)", + "parenthesised null assertion", + &[ + (Kotlin::BANGBANG as u16, 1, "the `!!`"), + (Kotlin::ParenthesizedExpression as u16, 1, "the parens"), + ][..], + ), + ] { + let src = format!("fun f(a: Boolean, b: Any) {{ if ({chain}) {{ println(\"x\") }} }}"); + assert_fixture_spells::(&src, "foo.kt", anchors); + check_func_space::(&src, "foo.kt", |space| { + let f = child_space(&space, "f"); + assert_eq!( + f.metrics.abc.conditions(), + 2, + "`{chain}`: the identifier plus the {spelling} operand" + ); + assert_eq!(f.metrics.cyclomatic.cyclomatic(), 3, "`{chain}`: decisions"); + }); + } + } + #[test] fn kotlin_bare_if_predicate_is_one_condition() { // Issue #773: a bare-boolean `if` predicate (`if (flag)`) is one @@ -11881,6 +13288,12 @@ mod keyword_negation_parity { /// scored one. Python had both kinds since #772 and is the control the /// other three were brought level with. /// +/// #1410 closed the six sets #1379 deferred: PHP, Groovy, and the +/// name-keyed C / C++ / Mozcpp / Objective-C set, where the omission +/// 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 headline claim of each case is a *comparison*: a numeric operand /// must score exactly what an identifier operand scores in the same slot. /// That is what discriminates the defect — with the kind missing from the @@ -11917,7 +13330,13 @@ mod keyword_negation_parity { feature = "perl", feature = "python", feature = "lua", - feature = "javascript" + feature = "javascript", + feature = "php", + feature = "groovy", + feature = "c", + feature = "cpp", + feature = "mozcpp", + feature = "objc" ))] mod numeric_bool_operands { use crate::test_support::metrics_verbatim; @@ -11929,8 +13348,10 @@ mod numeric_bool_operands { type Slot = (&'static str, u64, u64); /// A language's two slots, its identifier baseline operand, the - /// numeric operands that must score the same, and how many of those - /// there should be. + /// truthy literal operands that must score the same, and how many of + /// those there should be. Every row but the C family's lists numeric + /// literals only; that one adds `char_literal`, which is a second + /// integral-literal kind rather than a second numeric spelling. /// /// The count is not bookkeeping. `for_each_case` counts *languages*, /// so trimming a row's operand list back to `&["1"]` — which is @@ -11974,6 +13395,20 @@ mod numeric_bool_operands { /// lexes as `integer`, and the `octal` kind is unreachable. /// - Elixir: `integer` / `float` / `char`. `?a` is the codepoint 97, /// a numeric literal wearing a sigil. + /// - PHP: `integer` / `float`. Every radix prefix, `_` separator and + /// exponent folds into those two, and the `float` *type* keyword + /// `Float2` is a third id rendering to the same name that must + /// stay out of the set (#1410). + /// - Groovy: one consolidated `number_literal`, type suffixes + /// (`1L`, `1.5f`, `1G`) included. + /// - C, C++, Mozcpp and Objective-C: one consolidated + /// `number_literal`, plus `char_literal` — not a numeric spelling + /// but a second integral-literal *kind*, since `'c'` has type + /// `int` in C and `char` in C++ and is truthy for the same reason. + /// All four share one name-keyed set and so run the same fixture, + /// which is the only coverage `Mozcpp` can have: it owns no file + /// extension, and `metrics_verbatim` is the one entry point that + /// reaches a `LANG` without going through one. /// /// Python, Lua and JavaScript were already correct and ride along as /// controls — they are what the first three were measured against, @@ -12036,6 +13471,33 @@ mod numeric_bool_operands { &["1", "1.0", "0xff"], 3, ), + LANG::Php => ( + [ + (" ( + [ + ("def f(a) {\n return a && {}\n}\n", 2, 3), + ("def f() {\n if ({}) { return 1 }\n}\n", 1, 3), + ], + "b", + &["1"], + 1, + ), + LANG::C | LANG::Cpp | LANG::Mozcpp | LANG::Objc => ( + [ + ("int f(int a) {\n return a && {};\n}\n", 2, 3), + ("int f() {\n if ({}) { return 1; }\n}\n", 1, 3), + ], + "b", + &["1", "'c'"], + 2, + ), _ => return None, }) } @@ -12136,3 +13598,314 @@ mod numeric_bool_operands { }); } } + +/// A relational construct with its own grammar production must score +/// like an identifier in the same boolean slot (#1449, #1461). +/// +/// Every language here spells at least one boolean test as a dedicated +/// node rather than as a `binary_expression`, so no comparison-token +/// arm ever sees it. A kind missing from `_bool_terminal_kinds!()` +/// scores **zero**, silently, and zero is indistinguishable from a +/// construct that legitimately scores zero — which is why the headline +/// claim of each case is a *comparison* against an identifier control +/// in the identical slot rather than an absolute number. +/// +/// The constructs, and the route each fix took: +/// +/// - **Groovy** `a in l` / `a !in l` (`membership_expression`) joins +/// the terminal set. The other four — `a === b` / `a !== b` +/// (`identity_expression`) and `s =~ /p/` / `s ==~ /p/` +/// (`regex_find_expression`, `regex_match_expression`) — are counted +/// as operator tokens in `groovy_count_token_condition` instead, so +/// they also score outside a boolean slot as `==` already did. Both +/// macro and arm carry the reasoning; the rule that matters here is +/// that a construct takes exactly one of the two routes, never both +/// (`.claude/rules/grammar-dispatch.md` §5). +/// - **Perl** `/^#/` (`pattern_matcher`) and `m{^#}` +/// (`pattern_matcher_m`), the two spellings of a match against the +/// implicit `$_`. They are sibling rules, not aliases, so both are +/// listed — a fixture carrying only the first would have left the +/// `m{}` half at zero and read as covered. +/// - **Ruby** `a in Integer` (`test_pattern`), the one-line pattern +/// test. Decided against `match_pattern` (`expr => pat`), which +/// raises rather than yielding a boolean. +/// +/// Rust's `matches!(…)` measures short in the same way, and is +/// deliberately **not** here: 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!`. That breadth is what +/// #1461 item 2 exists to decide, so it is left to that issue rather +/// than settled by inclusion here. +/// +/// Two slots per language, because the sets feed two independent walker +/// paths (§11) and every construct above measured short in **both**: the +/// operands of a `&&` chain, and the predicate of an `if`. A fixture of +/// only one leaves the other untested. +/// +/// Each row also carries the construct's negated spelling. That is not +/// redundancy: `_inspect_container` is a *third* consumer of every +/// terminal set, reached only after a `(…)` or `!…` wrapper is peeled, +/// and it is the consumer that decides whether the peeled operand sits +/// in boolean context at all. Without a negated row, Groovy's and +/// Rust's `inspect_container` arms were the site no fixture here +/// exercised. +/// +/// Even with it, no language exercises all three consumers through +/// these two slots, and which one is missed differs by language — +/// Perl's `perl_count_condition` (reached only from a ternary or a +/// C-style `for` header) and Ruby's `ruby_count_unary_conditions` +/// (unreachable here because `in` binds looser than `&&`, so the chain +/// slot must parenthesise and routes through `inspect_container` +/// instead). Both are covered elsewhere in this file; the gap is +/// recorded rather than papered over, because a reader comparing the +/// slot count to the consumer count will otherwise assume it is three. +/// +/// The `cyclomatic` half of `every_construct_scores_its_recorded_values` +/// is what rules out a regression that moved both metrics together: +/// cyclomatic must not move when only the operand spelling does, which +/// is what makes a `conditions` move unambiguously ABC's. The recorded +/// figures differ per slot (Rust's chain slot carries the extra `&&` +/// decision), so each slot pins its own pair rather than sharing one. +// Gated on the union of the three languages with rows, so a build +// enabling none of them drops the module instead of failing its +// guards (`.claude/rules/testing.md`, #1286 / #1411). +#[cfg(test)] +#[cfg(any(feature = "groovy", feature = "perl", feature = "ruby"))] +mod own_production_bool_constructs { + use crate::test_support::metrics_verbatim; + use crate::{LANG, MetricsOptions}; + + /// One fixture shape: a source template with a `{}` slot for the + /// whole boolean expression, and the `abc.conditions_sum` / + /// `cyclomatic_sum` every spelling in that slot must produce. + type Slot = (&'static str, u64, u64); + + /// A language's two slots, its identifier control, the constructs + /// that must score the same, and how many of those there should be. + /// + /// The count is not bookkeeping. `for_each_case` counts *languages*, + /// so trimming Groovy's list back to a single construct — or + /// dropping Perl's `m{}` half, which is a separate grammar rule from + /// its `//` half — leaves the module green with that coverage + /// deleted. Pinning the length makes such a trim a deliberate + /// two-line edit instead of a silent one. + type Case = ([Slot; 2], &'static str, &'static [&'static str], usize); + + fn conditions(lang: LANG, source: &str) -> u64 { + metrics_verbatim(lang, source.as_bytes(), MetricsOptions::default()) + .abc + .conditions_sum() + } + + fn cyclomatic_sum(lang: LANG, source: &str) -> u64 { + metrics_verbatim(lang, source.as_bytes(), MetricsOptions::default()) + .cyclomatic + .cyclomatic_sum() + } + + /// `([chain_slot, predicate_slot], identifier, constructs)` per + /// language. + /// + /// `{}` is the left-hand operand of a two-operand short-circuit + /// chain in the first slot and the whole `if` predicate in the + /// second, so every fixture differs from its own control in exactly + /// the construct under test. + /// + /// Ruby's chain slot parenthesises the slot because `in` binds + /// looser than `&&` there; the control is parenthesised identically, + /// so the parens cancel out of the comparison and + /// `ruby_inspect_container` unwraps them for both sides alike. + fn cases(lang: LANG) -> Option { + Some(match lang { + LANG::Groovy => ( + [ + ("def f(a, b, l, s) {\n return {} && b\n}\n", 2, 3), + ("def f(a, b, l, s) {\n if ({}) { return 1 }\n}\n", 1, 3), + ], + "b", + &[ + "a in l", + "a !in l", + "a === b", + "a !== b", + "s =~ /p/", + "s ==~ /p/", + "!(a in l)", + ], + 7, + ), + LANG::Perl => ( + [ + ("sub f {\n my $x = {} && $b;\n}\n", 2, 3), + ("sub f {\n if ({}) { 1; }\n}\n", 1, 3), + ], + "$a", + &["/^#/", "m{^#}", "!/^#/"], + 3, + ), + LANG::Ruby => ( + [ + ("def f(a)\n ({}) && b\nend\n", 2, 3), + ("def f(a)\n if {}\n 1\n end\nend\n", 1, 3), + ], + "a", + &["a in Integer", "!(a in Integer)"], + 2, + ), + _ => return None, + }) + } + + /// Runs `check` once per enabled language that has a case, having + /// first established that the case can still assert something. + /// + /// The three guards mirror `numeric_bool_operands::for_each_case`, + /// where each was added only after a measured perturbation of it + /// left that module green: `checked > 0` for the runtime half of + /// the feature gate, the recorded construct count so a trimmed row + /// cannot vanish silently, and the `{}` slot so `str::replace` does + /// not degenerate into comparing a string with itself. + fn for_each_case(check: impl Fn(LANG, Case)) { + let mut checked = 0; + for lang in LANG::into_enum_iter() { + if !lang.is_enabled() { + continue; + } + let Some(case @ (slots, _, constructs, expected_constructs)) = cases(lang) else { + continue; + }; + // Both halves are needed, and the second does not imply the + // first. The length check alone only asserts that the list + // and its recorded count *agree*, so emptying a row and + // setting its count to 0 — the shape a careless "the test + // failed, fix the number" edit takes — deleted that + // language's coverage with both tests still green when + // measured. `numeric_bool_operands` above has the same + // weakness and is left alone here, being out of this + // change's scope. + assert!( + !constructs.is_empty(), + "{lang:?}: the construct list is empty; this language asserted nothing" + ); + assert_eq!( + constructs.len(), + expected_constructs, + "{lang:?}: the construct list no longer covers every own-production \ + boolean spelling this language was fixed for" + ); + for (template, _, _) in slots { + assert!( + template.contains("{}"), + "{lang:?}: template lost its `{{}}` slot: {template}" + ); + } + check(lang, case); + checked += 1; + } + assert!( + checked > 0, + "no language with an own-production boolean construct enabled; \ + this test asserted nothing" + ); + } + + #[test] + fn an_own_production_construct_scores_like_an_identifier() { + for_each_case(|lang, (slots, identifier, constructs, _)| { + for (template, _, _) in slots { + let baseline = conditions(lang, &template.replace("{}", identifier)); + for construct in constructs { + let source = template.replace("{}", construct); + let scored = conditions(lang, &source); + assert_eq!( + scored, baseline, + "{lang:?}: `{construct}` scored {scored} conditions against \ + `{identifier}`'s {baseline}\n source: {source}" + ); + } + } + }); + } + + /// The two Perl spellings must remain two distinct grammar kinds. + /// + /// `perl_bool_terminal_kinds!()` lists `PatternMatcher` **and** + /// `PatternMatcherM` on the stated grounds that `/^#/` and `m{^#}` + /// are sibling rules rather than aliases of one. Nothing above pins + /// that: if a grammar bump collapsed `m{}` onto `pattern_matcher`, + /// both fixtures would keep scoring through the surviving entry, + /// the `PatternMatcherM` arm would become dead code, and every test + /// in this module would stay green — the silent-drift shape lesson + /// 34 and grammar-dispatch §2 exist to catch. + /// + /// Asserting the ids differ is not enough on its own, since two + /// distinct enum variants prove nothing about what the parser + /// emits. So each spelling is parsed and required to produce its + /// own kind and not the other's. + #[test] + #[cfg(feature = "perl")] + fn the_two_perl_match_spellings_are_distinct_kinds() { + use crate::{ParserTrait, Perl, PerlParser}; + use big_code_analysis_ast::test_support::ast_has_kind_id; + + assert_ne!( + Perl::PatternMatcher as u16, + Perl::PatternMatcherM as u16, + "the two spellings share one kind id; one terminal-set entry is dead" + ); + + for (src, present, absent) in [ + ( + "sub f { if (/^#/) { 1; } }", + ("pattern_matcher", Perl::PatternMatcher as u16), + ("pattern_matcher_m", Perl::PatternMatcherM as u16), + ), + ( + "sub f { if (m{^#}) { 1; } }", + ("pattern_matcher_m", Perl::PatternMatcherM as u16), + ("pattern_matcher", Perl::PatternMatcher as u16), + ), + ] { + let parser = PerlParser::new( + src.as_bytes().to_vec(), + &std::path::PathBuf::from("f.pl"), + None, + ); + let (present_name, present_id) = present; + let (absent_name, absent_id) = absent; + assert!( + ast_has_kind_id(&parser, present_id), + "`{src}` no longer emits `{present_name}`" + ); + assert!( + !ast_has_kind_id(&parser, absent_id), + "`{src}` now also emits `{absent_name}`; the two spellings have \ + collapsed and one `perl_bool_terminal_kinds!()` entry is dead" + ); + } + } + + /// The absolute anchor under the comparison above: every spelling + /// must produce the slot's recorded `conditions`, and must leave + /// `cyclomatic` alone. + #[test] + fn every_construct_scores_its_recorded_values() { + for_each_case(|lang, (slots, identifier, constructs, _)| { + for (template, expected_conditions, expected_cyclomatic) in slots { + for spelling in std::iter::once(identifier).chain(constructs.iter().copied()) { + let source = template.replace("{}", spelling); + assert_eq!( + conditions(lang, &source), + expected_conditions, + "{lang:?}: `{spelling}` conditions\n source: {source}" + ); + assert_eq!( + cyclomatic_sum(lang, &source), + expected_cyclomatic, + "{lang:?}: `{spelling}` cyclomatic_sum\n source: {source}" + ); + } + } + }); + } +} diff --git a/src/metrics/abc/csharp.rs b/src/metrics/abc/csharp.rs index 892004abf..36dbbed43 100644 --- a/src/metrics/abc/csharp.rs +++ b/src/metrics/abc/csharp.rs @@ -22,10 +22,14 @@ fn csharp_inspect_container(container_node: &Node, parent: &Node, conditions: &m let mut node_kind = node.kind_id().into(); // Seed the boolean-context flag from the parent: known-boolean - // contexts (loop / if / binary expression) imply the contained - // expression evaluates as a condition. + // contexts (loop / if / guard / binary expression) imply the + // contained expression evaluates as a condition. The two guard + // clauses joined this list with #1422 — a `when` guard is a boolean + // slot exactly as an `if` condition is, so `when (b)` and + // `catch (E e) when ((b))` count their parenthesised operand. let mut has_boolean_content = match parent.kind_id().into() { - BinaryExpression | IfStatement | WhileStatement | DoStatement | ForStatement => true, + BinaryExpression | IfStatement | WhileStatement | DoStatement | ForStatement + | WhenClause | CatchFilterClause => true, ConditionalExpression => parent .child_by_field_name("condition") .is_some_and(|condition| condition.id() == node.id()), @@ -107,10 +111,12 @@ fn csharp_count_unary_conditions(list_node: &Node, conditions: &mut f64) { // ABC token-level helpers for C#. Mirror of Java's helper layout with // C#-specific deltas: every aliased kind id is matched via the // `csharp_*_kinds!()` macros (lesson #2); `ObjectCreationExpression` -// joins `InvocationExpression*` as a branch; each of the four tokens a -// `relational_pattern` can spell (`<` `>` `<=` `>=`) excludes that -// parent, because a C# pattern's operator belongs to the arm that owns -// it rather than scoring on its own (#1383); +// joins `InvocationExpression*` as a branch; all six comparison tokens +// share one arm allowlisting a `binary_expression` parent, which excludes +// type syntax (#1275), the declared name of an operator overload (#1297, +// #1420) and a `relational_pattern`'s operator — the last because a C# +// pattern's operator belongs to the arm that owns it rather than scoring +// on its own (#1383); // `ConditionalExpression` replaces Java's `TernaryExpression`; // `for_statement` exposes its condition via the named `condition` // field rather than positional index. @@ -187,27 +193,84 @@ fn csharp_count_token_assignment<'a>( } // Counts branch tokens: every invocation, `new` allocation, and -// constructor delegation. +// constructor delegation in either of its two spellings. // `ConstructorInitializer` is the `: base(…)` / `: this(…)` delegation on a // constructor — a call by Fitzpatrick's rule, and the C# spelling of the // shape Java and Groovy count (#1279). Unlike the invocation kinds it // carries no numeric-suffix aliases, and it does not wrap an // `InvocationExpression` for the delegation itself, so no double count // arises; calls in its argument list are separate nodes counted on their own. -fn csharp_count_token_branch(node: &Node, stats: &mut Stats) -> bool { +fn csharp_count_token_branch<'a>( + node: &Node<'a>, + ancestors: Ancestors<'a, '_>, + stats: &mut Stats, +) -> bool { use Csharp::*; - if matches!( - node.kind_id().into(), + let is_branch = match node.kind_id().into() { + // Spelled out rather than reaching for `csharp_invocation_expr_kinds!()` + // (`big-code-analysis-ast/src/macros/kind_sets.rs`), which carries + // exactly these three: the macro *is* an or-pattern, so combining it + // with further alternatives here trips `clippy::unnested_or_patterns` + // at `-D warnings`. `Checker::is_csharp_call` can use it because the + // set is that predicate's whole answer. The §1 obligation is met by + // listing all three aliases, not by the macro. crate::Csharp::InvocationExpression - | crate::Csharp::InvocationExpression2 - | crate::Csharp::InvocationExpression3 - | ObjectCreationExpression - | ConstructorInitializer - ) { + | crate::Csharp::InvocationExpression2 + | crate::Csharp::InvocationExpression3 + | ObjectCreationExpression + | ConstructorInitializer => true, + // The C# 12 primary-constructor superclass call — `class + // Sub(int x) : Base(x)` — invokes the base constructor exactly as + // `: base(x)` does, and scored zero until #1406. Kotlin's + // equivalent was fixed in #1384; this is the C# sibling. + // + // tree-sitter-c-sharp 0.23.5 spells the two declaration families + // differently, so there is no single node to match (verified with + // `bca dump`, not inferred): + // + // `record R(int x) : Base(x);` base_list > primary_constructor_ + // base_type > argument_list + // `class C(int x) : Base(x) {}` base_list > argument_list (flat) + // + // Matching both kinds under a `base_list` parent therefore covers + // both families. This is §5's container-plus-containable shape — + // a `primary_constructor_base_type` *holds* an `argument_list` — + // and the parent gate is what makes it safe rather than a double + // count: in the record spelling the inner `argument_list`'s parent + // is the `primary_constructor_base_type`, not the `base_list`, so + // only the outer node fires. The two are never siblings, so + // neither spelling ever presents both. `record R(int x) : Base;` + // and `struct S(int x) : IBase {}` emit no `argument_list` and no + // `primary_constructor_base_type` at all, so an argument-less base + // type still costs nothing, and `enum E : byte` / `interface I : + // IBase` cost nothing for the same reason. + // + // Three grammar-reachable shapes are not valid C# and score 1 + // here: `interface I : IBase(x)`, `enum E : Base(x)`, and a + // `class NoCtor : Base(x)` with no primary constructor. Each + // parses cleanly — the grammar is more permissive than the + // language — and no valid program can tell the behaviours apart, + // so per §6 the gap is documented and left untested rather than + // pinned, which would make the grammar's present permissiveness + // the contract. + // + // The parent gate is load-bearing on `ArgumentList`, which is + // otherwise the argument list of every call in the file. It is + // also what keeps the sibling `attribute_argument_list` / + // `bracketed_argument_list` / `type_argument_list` kinds out — + // they are distinct kind ids, and none of them occurs under a + // `base_list`. `BaseList` and `BaseList2` both render to + // `"base_list"` and only the second is observed here; both are + // listed per `.claude/rules/grammar-dispatch.md` §1. + PrimaryConstructorBaseType | ArgumentList => ancestors + .parent(node) + .is_some_and(|parent| matches!(parent.kind_id().into(), BaseList | BaseList2)), + _ => false, + }; + if is_branch { stats.branches += 1.; - return true; } - false + is_branch } fn csharp_count_token_condition<'a>( @@ -222,32 +285,121 @@ fn csharp_count_token_condition<'a>( // arrow `default ->` forms) is the unconditional fallthrough and // is excluded, mirroring cyclomatic's `Case`-only count and the // expression-arm discard rule below (issues #456, #469). - EQEQ | BANGEQ | Else | Case | Try | Catch => { + // + // These four stay ungated; `EQEQ` / `BANGEQ` shared the arm until + // #1420 and moved to the gated one below. `Else`, `Try` and + // `Catch` come from one production each (`if_statement`, + // `try_statement`, `catch_clause`), so there is nothing to gate on. + // `Case` comes from two — `switch_section` and `goto_statement` — + // and the second is over-counted: FIXME(#1450), `goto case 2;` + // scores a condition without being an arm. Left alone rather than + // missed: C# cyclomatic counts the same token + // (`src/metrics/cyclomatic/csharp.rs`), so gating it here alone + // would break the §8 parity `conditions == cyclomatic() - 1` that + // a two-arm-plus-`goto case` method currently satisfies at + // 3 == 4 - 1. Both arms have to move together. `goto default;` + // costs nothing already, because `Default` is excluded as the + // switch's unconditional fallthrough. + // + // `QMARKQMARK` joined them in #1459 and is ungated for the same + // reason: `??` comes from `binary_expression` alone. It is a + // decision — `a ?? b` evaluates `b` only when `a` is null, the + // `a != null ? a : b` the language lets you not spell — and C# + // cyclomatic has always counted it + // (`src/metrics/cyclomatic/csharp.rs`), so ABC sat one *below* + // C#'s own decision count wherever a `??` appeared. + // + // #1422 made that gap visible rather than merely present. Its + // guard slot claims every spelling of a `when` guard is worth + // one condition, and `when b ?? false` was worth zero: the slot + // sees a `binary_expression`, which it leaves to the operator + // arms because a comparison guard's operator is already counted + // here — and for `??` there was nothing to leave it to. Counting + // the token is what levels the spellings *and* keeps a compound + // guard (`when a > 1 && b < 2`) at its two conditions; a blanket + // `+1` on the slot would have done neither. + // + // No double count (§5): `??` is one token, distinct from the + // bare `QMARK` below and from the `??=` compound assignment + // (`QMARKQMARKEQ`, counted as an assignment), and every + // condition slot declines a `binary_expression` outright. + Else | Case | Try | Catch | QMARKQMARK => { stats.conditions += 1.; } - // The other half of #1383. A `relational_pattern` spells its - // operator as any of `<` `>` `<=` `>=`, and the last two are - // distinct tokens that never reach the `GT | LT` arm below, so - // fixing only that arm would leave `x switch { >= 10 => … }` - // double-counted while `x switch { > 10 => … }` was not — the - // same bug surviving under a different token. The arm that owns - // the pattern is the decision; see the `GT | LT` comment. + // All six C# comparison tokens, counted only where they *apply* + // an operator. A `grammar.json` sweep of tree-sitter-c-sharp + // 0.23.5 finds each one in these productions, and only + // `binary_expression` (`a < b`) qualifies: + // + // `<` / `>` — `binary_expression`, `relational_pattern`, + // `operator_declaration`, `type_argument_list`, + // `type_parameter_list`, `function_pointer_type` + // `<=` / `>=` — `binary_expression`, `relational_pattern`, + // `operator_declaration` + // `==` / `!=` — `binary_expression`, `preproc_binary_expression` + // (`#if A == B`), `operator_declaration` + // + // Three reasons to exclude, one per production family: // - // Denylist polarity here rather than the allowlist the `GT` / - // `LT` arm uses, because these two tokens have no type syntax - // to fail closed against: `<=` and `>=` reach this arm from - // `binary_expression`, `relational_pattern` and - // `operator_declaration` only. That third parent is the - // `>=` / `<=` spelling of #1297's operator-overload bug, which - // that fix reached only through the `GT` / `LT` allowlist and - // which `==` / `!=` share. + // `type_argument_list` (`Dictionary`), `type_parameter_list` + // (`class Foo`) and `function_pointer_type` (`delegate*`) are type syntax, no more a decision than the `?` of a + // nullable type (#1275). // - // FIXME(#1420): `operator <=` / `>=` / `==` / `!=` still score - // a spurious condition each, measured. Left alone here so this - // commit changes one behaviour; the fix is to give all four - // tokens the `BinaryExpression` allowlist polarity, which - // subsumes the denial below. - GTEQ | LTEQ if !ancestors.parent_has_kind(node, RelationalPattern as u16) => { + // `operator_declaration` — `public static bool operator <(V a, + // V b)` — names the operator being *defined* rather than applying + // it. #1297 found this for `<` / `>`, whose denylist at the time + // named the three type-syntax parents and not this one, so every + // comparison-operator overload scored a condition. C# overloads + // six operators, though, and the other four are distinct tokens + // that reached a different arm: `<=` `>=` `==` `!=` each still + // scored 1 against `<` / `>`'s 0 on a class overloading all six, + // every member of which has `cyclomatic()` 1 (#1420). + // + // `relational_pattern` is excluded for a third reason (#1383): a + // pattern's comparison operator is not its own decision, since the + // enclosing `switch_expression_arm` or `if` condition slot already + // scores one. Counting the operator too charged + // `x switch { > 5 => … }` twice what the constant arm + // `x switch { 5 => … }` scores, and twice C#'s own cyclomatic + // decision count. + // + // Allowlist polarity, matching Java's #1274 fix and unlike the + // `QMARK` arm below: one decision parent against six excluded + // productions, and a grammar bump that grows a seventh should fail + // closed (`.claude/rules/grammar-dispatch.md` §1). #1383 landed + // half of this as a denylist naming `relational_pattern` alone, + // which is what let `operator_declaration` through for four of the + // six tokens; the allowlist subsumes that denial. The `QMARK` arm + // takes the opposite polarity for a reason specific to that token + // — see its comment. + // + // `BinaryExpression2` is the second enum id carrying the + // `binary_expression` kind string, listed per §1. Measured at this + // pin it is unreachable from here: the `preproc_binary_expression` + // of `#if A == B` parses as `BinaryExpression` (369), so the + // preprocessor spelling counts through the first entry, and C#'s + // preprocessor admits only `== != && || !` in any case. + // + // A failed guard returns `false` and falls through to + // `csharp_walk_for_conditions`, which matches none of these six + // token kinds — so the fall-through is a no-op. + // + // Failing closed also covers error recovery, where the sweep + // above says nothing: a token the parser reparents under `{ERROR}` + // stops counting. `<` / `>` have behaved that way since #1297 and + // the other four now agree, valid input is unaffected, and no C# + // corpus file parses with an `{ERROR}` node — measured. Nothing + // pins it, because a fixture the language rejects would make the + // grammar's present recovery the contract (§6). + GT | LT | GTEQ | LTEQ | EQEQ | BANGEQ + if ancestors.parent(node).is_some_and(|parent| { + matches!( + parent.kind_id().into(), + BinaryExpression | BinaryExpression2 + ) + }) => + { stats.conditions += 1.; } // tree-sitter-c-sharp emits a bare `?` from exactly four @@ -270,10 +422,15 @@ fn csharp_count_token_condition<'a>( // ABC *below* C#'s own cyclomatic decision count on a safe- // navigation chain. Denying the two type-syntax parents keeps // that count without an allowlist entry a later "consistency" - // pass could drop. It is agreement on this one token, not on - // the metric: `??` is a C# cyclomatic decision (`QMARKQMARK` - // there) and no ABC condition here, a pre-existing gap the - // JS-family arms do not share. + // pass could drop. The sibling `??` gap this comment used to + // record — a C# cyclomatic decision that was no ABC condition — + // is closed for `??` by `QMARKQMARK` joining the ungated + // condition-token arm at the top of this match (#1459). It is + // *not* closed for `??=`: `QMARKQMARKEQ` is a cyclomatic + // decision and stays an ABC assignment only, so `a ??= b` still + // sits one below C#'s decision count. That is deliberate rather + // than missed — the JS family scores `??=` the same way — but it + // is a live divergence, not a closed one. // // The agreement being protected is C#-internal, not cross- // language: `a?.b?.c` scores ABC conditions 2 in C# and 0 in @@ -312,57 +469,6 @@ fn csharp_count_token_condition<'a>( { stats.conditions += 1.; } - // Counts `<` / `>` only where they score a decision of their - // own. A `grammar.json` sweep of tree-sitter-c-sharp 0.23.5 - // finds a bare `<` / `>` in exactly six productions, and only - // `binary_expression` (`a < b`) qualifies. Four are type - // syntax — `type_argument_list` (`Dictionary`), - // `type_parameter_list` (`class Foo`), `function_pointer_type` - // (`delegate*`) and `operator_declaration` - // (`public static bool operator <(V a, V b)`), whose `<` names - // the operator being *defined* rather than applying it. The - // sixth is `relational_pattern`, excluded for a different - // reason — see below. - // - // The previous denylist named three of those four and not - // `operator_declaration`, so every comparison-operator overload - // scored a condition per declaration (#1297). Allowlist - // polarity, matching Java's #1274 fix and unlike the `QMARK` - // arm above: `<` / `>` have one decision parent against five - // excluded productions, and a grammar bump that grows a seventh - // should fail closed - // (`.claude/rules/grammar-dispatch.md` §1). The `QMARK` arm - // takes the opposite polarity for a reason specific to that - // token — see its comment. - // - // `relational_pattern` is deliberately *not* in the allowlist - // (#1383). A pattern's comparison operator is not its own - // decision: the enclosing `switch_expression_arm` or `if` - // condition slot already scores one, so counting the operator - // too charged `x switch { > 5 => … }` twice what the constant - // arm `x switch { 5 => … }` scores and twice C#'s own - // cyclomatic decision count. #1297 kept the entry to avoid an - // unmeasured behaviour change riding along with it; this is - // that measured change. - // - // `BinaryExpression2` is the id the grammar aliases - // `preproc_binary_expression` to, and it is listed defensively - // per §1 rather than because it is reachable: C#'s preprocessor - // admits only `== != && || !`, so no bare `<` / `>` can have - // that parent at this pin. It is genuinely reachable in the - // C / C++ arms this one mirrors, where `#if A < B` is legal. - // `<=` / `>=` and the shifts are distinct tokens and never - // reach this arm. - GT | LT - if ancestors.parent(node).is_some_and(|parent| { - matches!( - parent.kind_id().into(), - BinaryExpression | BinaryExpression2 - ) - }) => - { - stats.conditions += 1.; - } _ => return false, } true @@ -381,6 +487,14 @@ fn csharp_walk_for_conditions<'a>( csharp_count_unary_conditions(&parent, conds); } } + // `compute` returns as soon as `csharp_count_token_branch` fires, + // so since #1406 an `argument_list` under a `base_list` no longer + // reaches this arm. Measured harmless: the arm is dead for *every* + // argument list, because an `argument_list`'s children are + // `argument` wrappers that `csharp_inspect_container` rejects on + // the first iteration — `Helper(!b)` and `Helper((b))` both score + // zero conditions today. Repairing it means revisiting that + // exclusion, not just this arm. ArgumentList => csharp_count_unary_conditions(node, conds), // tree-sitter-c-sharp `if_statement` / `while_statement` shape: // [`if`/`while`, `(`, condition, `)`, body, …]. The parens are @@ -404,6 +518,52 @@ fn csharp_walk_for_conditions<'a>( csharp_count_condition(&condition, node, conds); } } + // C#'s two guard spellings, each modelled as a condition slot + // exactly like the `if` / `while` / `do` slots above (#1422). + // Before this, a guard scored whatever operator happened to sit + // inside it: `when x % 2 == 0` and `when x > 2` counted one via + // the comparison-token arm while `when IsEven(x)` counted zero, + // so three semantically identical guards produced two different + // numbers. As a slot every spelling contributes exactly one — + // a call / `is` test / bare identifier through + // `csharp_bool_terminal_kinds!()`, a comparison through the + // token arm that already owned it — and a compound guard + // (`when a > 1 && b < 2`) keeps its sub-structure rather than + // collapsing to one. + // + // Suppressing the guard's operator instead would have reached + // the same internal agreement one count *below* C#'s own + // cyclomatic decision count, which #1422 fixes upward in + // `src/metrics/cyclomatic/csharp.rs`. + // + // By role, not index (`.claude/rules/grammar-dispatch.md` §3): + // the two clauses carry their guard as the only *expression* + // child but disagree on where it sits, because + // `catch_filter_clause` spells its parentheses as anonymous + // tokens (`when`, `(`, expr, `)`) the way `if_statement` does, + // while `when_clause` has none (`when`, expr). Neither exposes + // a field for the slot. + // + // Every named child, not the first: tree-sitter `extra`s are + // named nodes and may precede the expression, so `when /*c*/ g` + // hands a `comment` to a first-child read and silently restores + // the spelling-dependence this fix removes. C#'s extras at this + // pin are `comment` plus nine `preproc_*` kinds — none of them a + // `csharp_bool_terminal_kinds!()` member, and none a paren or + // `!`-prefix wrapper — so passing them through the slot adds + // nothing and the loop cannot double count a clause that holds + // one expression by construction. + // + // FIXME(#1455): the sibling `if` / `while` / `do` slots read a + // fixed child index and so still lose their condition to a + // leading comment (`if (/*c*/ g)` scores 0). That is the same + // class of bug and predates this arm; it is left to its own + // change rather than widened into here. + WhenClause | CatchFilterClause => { + for guard in node.children().filter(Node::is_named) { + csharp_count_condition(&guard, node, conds); + } + } // `return value;` — child(1) is the value expression. ReturnStatement => csharp_inspect_child(node, 1, conds), // Child 2: declarator / assignment RHS, lambda body @@ -467,7 +627,7 @@ impl Abc for CsharpCode { if csharp_count_token_assignment(node, ancestors, stats) { return; } - if csharp_count_token_branch(node, stats) { + if csharp_count_token_branch(node, ancestors, stats) { return; } if csharp_count_token_condition(node, ancestors, stats) { diff --git a/src/metrics/abc/groovy.rs b/src/metrics/abc/groovy.rs index f419816b8..2e1f2f1fb 100644 --- a/src/metrics/abc/groovy.rs +++ b/src/metrics/abc/groovy.rs @@ -162,11 +162,42 @@ fn groovy_count_token_assignment<'a>( fn groovy_count_token_branch(node: &Node, stats: &mut Stats) -> bool { use Groovy::*; - if matches!(node.kind_id().into(), MethodInvocation | CommandChain | New) { + let is_branch = match node.kind_id().into() { + MethodInvocation | CommandChain | New => true, + // An enum constant carrying constructor arguments — `A(1)` in + // `enum E { A(1), B, C(2) }` — invokes the enum's constructor, so + // it is an object construction under Fitzpatrick's "function + // invocation or object construction" rule. It scored zero until + // #1407, which is the inconsistent position once #1279 decided + // Java's `super(…)` / `this(…)` — already counted here through + // `MethodInvocation` — and #1384 decided Kotlin's `class Sub : + // Base(1, 2)`: all three are a constructor call the source spells + // out. + // + // The counter-argument is that an enum constant is a declaration, + // not a call site a reader navigates to. It loses because the same + // is true of a superclass delegation, and because the arguments + // still have to be understood as a constructor's, which is the + // effort ABC is measuring. + // + // The child gate is what makes this a §6 narrowing rather than a + // new node: a bare `B` is `enum_constant > identifier` with no + // `argument_list` child and must stay at zero. Verified with `bca + // dump`: the dekobon Groovy grammar gives the constant an + // `identifier` plus an optional `argument_list`. Java's annotated + // spelling has no analogue to guard against here — this grammar + // cannot parse `@Deprecated A(1)` inside an enum body at all and + // recovers into `ERROR` nodes, so no `enum_constant` is produced. + // An argument that is itself a call (`A(f())`) scores 2: + // `argument_list` is not a branch node, so the inner + // `method_invocation` is the only other node counted (§5). + EnumConstant => node.is_child(ArgumentList as u16), + _ => false, + }; + if is_branch { stats.branches += 1.; - return true; } - false + is_branch } // The `default` arm of a `switch` is excluded (issue #469): it is the @@ -189,7 +220,37 @@ fn groovy_count_token_condition<'a>( // C / PHP short-ternary reading that also walks the left operand // as a unary condition — it keeps `abc.conditions` equal to // `cyclomatic() - 1` on the chain (grammar-dispatch §8). - GTEQ | LTEQ | EQEQ | BANGEQ | Else | Case | Try | Catch | QMARKCOLON => { + // `EQEQEQ` / `BANGEQEQ` (`===`, `!==`) and `EQTILDE` / + // `EQEQTILDE` (`=~`, `==~`) are Groovy's identity and regex + // comparisons. They sit in this ungated arm beside `==` / `!=` + // for the reason those do, and a `grammar.json` sweep of + // dekobon-tree-sitter-groovy 0.2.2 is what makes ungating + // safe: each of the four tokens is emitted by exactly one + // production — `identity_expression` for the first pair, + // `regex_find_expression` / `regex_match_expression` for the + // second — so unlike `GT` / `LT` there is no type-argument or + // loop-header spelling to exclude. + // + // Their own expression kinds are therefore **not** in + // `groovy_bool_terminal_kinds!()`; counting both would score + // each twice (§5). The token is the better half of that choice + // because it scores outside a boolean slot as well, where + // `def r = (a == b)` already scored 1 and `def r = (a === b)` + // scored 0 — a within-language asymmetry between two + // equality operators. It is also how every other language here + // spells these: `EQEQEQ | BANGEQEQ` are token arms in Kotlin, + // the JS family, PHP and Elixir, and `EQTILDE` in Perl, Ruby + // and Bash. `getter/groovy.rs` already classifies all four as + // Halstead operators. + // + // Groovy's membership (`in` / `!in`) is the one relational + // form that cannot come through here — see + // `groovy_bool_terminal_kinds!()` for why. The spaceship `<=>` + // is deliberately still absent: it yields -1 / 0 / 1 rather + // than a boolean, so whether it is a condition at all is the + // open question in FIXME(#1461) item 4, not an oversight here. + GTEQ | LTEQ | EQEQ | BANGEQ | EQEQEQ | BANGEQEQ | EQTILDE | EQEQTILDE | Else | Case + | Try | Catch | QMARKCOLON => { stats.conditions += 1.; } // As in Java: a bare `?` is either a ternary head or the head of diff --git a/src/metrics/abc/java.rs b/src/metrics/abc/java.rs index 4bb690173..70570aadd 100644 --- a/src/metrics/abc/java.rs +++ b/src/metrics/abc/java.rs @@ -196,14 +196,41 @@ fn java_count_token_assignment<'a>( // are separate nodes and still count on their own. fn java_count_token_branch(node: &Node, stats: &mut Stats) -> bool { use Java::*; - if matches!( - node.kind_id().into(), - MethodInvocation | New | ExplicitConstructorInvocation - ) { + let is_branch = match node.kind_id().into() { + MethodInvocation | New | ExplicitConstructorInvocation => true, + // An enum constant carrying constructor arguments — `A(1)` in + // `enum E { A(1), B, C(2); }` — invokes the enum's constructor, so + // it is an object construction under Fitzpatrick's "function + // invocation or object construction" rule. It scored zero until + // #1407, which is the inconsistent position once #1279 decided + // `super(…)` / `this(…)` above: both are a constructor call the + // source spells out, and Kotlin's `class Sub : Base(1, 2)` was + // settled the same way in #1384. + // + // The counter-argument is that an enum constant is a declaration, + // not a call site a reader navigates to. It loses because the same + // is true of the delegation forms already counted, and because the + // arguments still have to be understood as a constructor's, which + // is the effort ABC is measuring. + // + // The child gate is what makes this a §6 narrowing rather than a + // new node: a bare `B` is `enum_constant > identifier` with no + // `argument_list` child and must stay at zero. The gate is + // specifically on `argument_list`, so an annotated constant + // (`@Deprecated A` / `@Foo(1) A`) cannot satisfy it — an + // annotation's arguments are a distinct `annotation_argument_list` + // production, and they hang off the constant's `modifiers` child + // rather than off the constant directly. Both verified with `bca + // dump`. An argument that is itself a call (`A(f())`) scores 2: + // `argument_list` is not a branch node, so the inner + // `method_invocation` is the only other node counted (§5). + EnumConstant => node.is_child(ArgumentList as u16), + _ => false, + }; + if is_branch { stats.branches += 1.; - return true; } - false + is_branch } // Counts condition tokens: comparison operators, control-flow keywords, diff --git a/src/metrics/abc/kotlin.rs b/src/metrics/abc/kotlin.rs index eeed3af63..e7856cc24 100644 --- a/src/metrics/abc/kotlin.rs +++ b/src/metrics/abc/kotlin.rs @@ -20,8 +20,15 @@ use crate::*; // `else` / `when`-entry / `catch` arms. Compared with the Java impl we // stay token-level (matching the leaf kind_ids) rather than walking // `Modifiers` children; the Kotlin grammar exposes the relevant -// operators directly as token nodes inside `binary_expression`, -// `assignment`, `prefix_expression`, and `postfix_expression`. +// operators directly as token nodes inside `binary_expression` and +// `assignment`. +// +// It does *not* have `prefix_expression` / `postfix_expression`, which +// this comment named until #1459. tree-sitter-kotlin-ng spells both +// unary positions as one `unary_expression` told apart only by its +// `operator` field — and believing the two-production version is how +// `kotlin_inspect_container` came to handle the prefix `!` and drop the +// postfix `!!` while reading as though it covered both. // Returns true when this `=` token initialises an *immutable* (`val`) // binding, whose initialiser is part of the declaration and therefore not @@ -47,47 +54,111 @@ fn kotlin_eq_initializes_immutable_binding<'a>( parent.children().any(|child| child.kind_id() == Val) } +// The operand a transparent wrapper wraps, and whether the wrapper +// itself proves that operand is boolean. `None` means the node is not a +// wrapper and the peel stops there. +// +// Only the prefix `!` proves booleanness: `!x` is boolean whatever `x` +// is, while parentheses, a null assertion and a cast are all +// type-preserving (`a!!` and `a as T` are boolean exactly when the slot +// they sit in is). So they inherit the caller's verdict rather than +// setting it. +// +// Two of the four shapes were not here from the start, and both were +// #1459 fallout from #1421 turning the `when` entry's blanket count +// into a condition slot: before that, `when { a!! -> … }` was paid for +// by the entry, and after it by this peel, which scored zero. +// +// - `unary_expression` is one kind for *both* Kotlin unary spellings, +// and the peel recognised only the prefix one. A postfix `a!!` stores +// its operand *before* the token, so the positional read for `!x` +// found the `!!` and stopped on a node the slot had already routed +// here as handled. +// - `as_expression` was not recognised at all, so a cast fell off the +// end of the slot's `else if`. +// +// By field, not index (`.claude/rules/grammar-dispatch.md` §3): both +// kinds name their parts (`operator` / `argument`, `left` / `right`), so +// one read serves the prefix and postfix spellings alike instead of the +// per-spelling index that produced the defect, and it survives a grammar +// re-order. It also survives an interposed `extra`: `when { ! /*c*/ a }` +// scores its condition, where the positional read scored zero. +// +// `parenthesized_expression` names nothing — its only child in +// node-types.json is the unlabelled inner `expression` — so it keeps the +// positional read it has always had, and with it the comment bug: +// `when { ( /*c*/ a) -> … }` still scores zero, because child(1) is the +// comment. Measured, not assumed. That is the same class as the C# +// `if` / `while` / `do` slots tracked in #1455 and left to their own +// change there; it predates #1459 and is recorded here rather than +// widened into it. +fn kotlin_wrapper_operand<'a>(node: &Node<'a>) -> Option<(Node<'a>, bool)> { + use Kotlin::*; + + match node.kind_id().into() { + // `(expr)` — the inner expression follows the `(` token. + ParenthesizedExpression => Some((node.child(1)?, false)), + UnaryExpression => { + let operand = node.child_by_field_name("argument")?; + match node.child_by_field_name("operator")?.kind_id().into() { + BANG => Some((operand, true)), + BANGBANG => Some((operand, false)), + // `-x`, `x++` and friends: arithmetic, never a boolean + // slot's operand, so the peel declines rather than + // reaching a bare `identifier` and counting it. + _ => None, + } + } + // `x as T` — `left` is the operand, `right` the target type. + // + // The *safe* cast `x as? T` is excluded on purpose (§5, one kind + // per operator): `AsQMARK` is already a condition token below, so + // peeling through it too would score `when { a as? T -> … }` two + // where every other spelling scores one. The plain `as` has no + // such token — `As` is not in that arm — so the peel is the only + // thing that can count it, and the two spellings come out level. + AsExpression if !node.is_child(AsQMARK as u16) => { + Some((node.child_by_field_name("left")?, false)) + } + _ => None, + } +} + // Kotlin ABC unary-conditional walker (Fitzpatrick Rule 9; issue #557). // tree-sitter-kotlin-ng parses `a && b || c` as a left-nested chain of // flat `binary_expression` nodes carrying `&&` / `||` operator tokens, -// the same shape as the Java template. Negation surfaces as -// `unary_expression` whose child(0) is the `!` token; the condition slot -// may also be wrapped in `parenthesized_expression`. Both are unwrapped -// by `kotlin_inspect_container` to reach the inner bare operand. +// the same shape as the Java template. Negation surfaces as a +// `unary_expression` whose `operator` field is the `!` token; the +// condition slot may also be wrapped in `parenthesized_expression`, a +// postfix `!!` null assertion, or an `as` cast. All are unwrapped by +// `kotlin_inspect_container` to reach the inner bare operand, and they +// chain: `(a!! as Boolean)` peels three wrappers to one `identifier`. fn kotlin_inspect_container(container_node: &Node, parent: &Node, conditions: &mut f64) { use Kotlin::*; let mut node = *container_node; - let mut node_kind = node.kind_id().into(); // A parenthesised / negated operand only contributes when it sits in // a boolean-evaluating slot. The chain wrapper (`binary_expression`) // and the control-flow headers all qualify; a `!`-operator anywhere - // also proves the operand is boolean (`if (!x)`). + // also proves the operand is boolean (`if (!x)`). `WhenEntry` joined + // the list with #1421: a subject-less `when` arm's condition is an + // ordinary boolean expression, so `when { (a) -> … }` is the same + // slot as `if ((a))`. let mut has_boolean_content = matches!( parent.kind_id().into(), - BinaryExpression | IfExpression | WhileStatement | DoWhileStatement | ForStatement + BinaryExpression + | IfExpression + | WhileStatement + | DoWhileStatement + | ForStatement + | WhenEntry ); - loop { - let is_parens = matches!(node_kind, ParenthesizedExpression); - let is_not = matches!(node_kind, UnaryExpression) - && node.child(0).is_some_and(|c| c.kind_id() == BANG as u16); - - if !is_parens && !is_not { - break; - } - if !has_boolean_content && is_not { - has_boolean_content = true; - } - - // Parenthesised expressions wrap their inner expression at child - // index 1 (after the `(` token); a `!` unary stores its operand - // at index 1 (after the `!` token). - let Some(child) = node.child(1) else { break }; - node = child; - node_kind = node.kind_id().into(); + while let Some((operand, proves_boolean)) = kotlin_wrapper_operand(&node) { + has_boolean_content |= proves_boolean; + node = operand; - if matches!(node_kind, kotlin_bool_terminal_kinds!()) { + if matches!(node.kind_id().into(), kotlin_bool_terminal_kinds!()) { if has_boolean_content { *conditions += 1.; } @@ -96,12 +167,14 @@ fn kotlin_inspect_container(container_node: &Node, parent: &Node, conditions: &m } } -// Counts each non-comparison operand of a Kotlin `&&` / `||` chain once. -// Mirrors `java_count_unary_conditions`: comparison operands are nested -// `binary_expression` nodes (absent from `kotlin_bool_terminal_kinds!()`) -// and so contribute nothing, while bare identifiers / calls / member -// accesses each add one. Inner chain links and `!` / paren wrappers are -// routed through `kotlin_inspect_container`. +// Counts each operand of a Kotlin `&&` / `||` chain that no token arm +// already owns. Mirrors `java_count_unary_conditions`: a comparison +// operand is a nested `binary_expression`, absent from +// `kotlin_bool_terminal_kinds!()`, and contributes nothing here because +// its operator token was counted directly; bare identifiers / calls / +// member accesses, and the `is` / `in` tests no token arm sees, each add +// one. Inner chain links and `!` / paren wrappers are routed through +// `kotlin_inspect_container`. fn kotlin_count_unary_conditions(list_node: &Node, conditions: &mut f64) { use Kotlin::*; @@ -133,25 +206,109 @@ fn kotlin_count_unary_conditions(list_node: &Node, conditions: &mut f64) { // mirroring `ruby_count_condition` / `rust_count_condition`. // tree-sitter-kotlin-ng exposes the predicate via the `condition` field on // `if_expression`, `while_statement`, and `do_while_statement`, so the -// field lookup is position-independent across all three forms. A bare -// terminal (`if (flag)`) counts directly; a comparison or boolean chain -// (`if (a == b)`, `if (a && b)`) is a nested `binary_expression` already -// counted by the comparison-token and `&&`/`||` walker arms, so it adds -// nothing here. A parenthesised or negated predicate (`if ((flag))`, -// `if (!flag)`) is unwrapped by `kotlin_inspect_container`. Without this +// field lookup is position-independent across all three forms. Since +// #1421 it also scores a subject-less `when` entry's condition, which is +// an `if` predicate in all but spelling. A bare terminal (`if (flag)`) +// counts directly, as does an `is` / `in` test that no token arm sees; a +// comparison or boolean chain (`if (a == b)`, `if (a && b)`) is a nested +// `binary_expression` already counted by the comparison-token and +// `&&`/`||` walker arms, so it adds +// nothing here. A predicate wrapped in parentheses, a negation, a +// postfix `!!` null assertion or an `as` cast (`if ((flag))`, +// `if (!flag)`, `if (flag!!)`, `if (v as Boolean)`) is unwrapped by +// `kotlin_inspect_container`. Without this // arm, idiomatic Kotlin bare predicates reported 0 ABC conditions while // Kotlin's own cyclomatic counted the decision, breaking the // conditions >= decisions invariant (#469/#473/#456/#696); issue #773. fn kotlin_count_condition(condition: &Node, parent: &Node, conditions: &mut f64) { - use Kotlin::*; - let kind = condition.kind_id().into(); - if matches!(kind, kotlin_bool_terminal_kinds!()) { + if matches!(condition.kind_id().into(), kotlin_bool_terminal_kinds!()) { *conditions += 1.; - } else if matches!(kind, ParenthesizedExpression | UnaryExpression) { + } else if kotlin_wrapper_operand(condition).is_some() { + // Asking the peel itself which kinds it unwraps, rather than + // restating the list here. The two spelled the list separately + // until #1459, and that is how `unary_expression` came to be + // routed here while the peel handled only half of it — the slot + // read as covering a shape the peel dropped on the floor + // (`.claude/rules/grammar-dispatch.md` §7). kotlin_inspect_container(condition, parent, conditions); } } +// Returns true when the `when_expression` enclosing `entry` carries a +// subject — `when (x) { … }` rather than `when { … }`. The distinction +// decides how the entry's condition is scored (#1421): a subject-ful +// entry lists a *pattern* compared against the subject, a subject-less +// one lists an ordinary boolean expression. +// +// A `when_entry`'s parent IS the `when_expression`, and the grammar +// exposes no field for the subject (`when_expression` has an empty +// `fields` map in node-types.json), so the membership test is a scan of +// the parent's children rather than a `child_by_field_name`. Scanning +// rather than reading a fixed index also keeps a leading `extra` — a +// comment between `when` and `(x)` — from displacing the answer. +// +// The scan stops at the opening brace, which is what keeps it `O(1)`. +// The subject can only sit in the header, between `when` and `{`, so +// everything past the brace is arms — and an unbounded `any()` over a +// *subject-less* `when` never short-circuits, so it walks every arm +// once per arm. That is quadratic in the arm count, and measurable: +// before the bound, a generated 8,000-arm subject-less `when` took +// 2.5 s against 0.03 s for the same-size subject-ful control. The +// bound preserves the `extra` tolerance above — `when /*c*/ (x) {` +// still finds the subject, `when /*c*/ {` still stops at the brace — +// and degrades to the old whole-child scan only when error recovery +// leaves the brace out entirely, where the answer is unchanged. +fn kotlin_enclosing_when_has_subject<'a>(entry: &Node<'a>, ancestors: Ancestors<'a, '_>) -> bool { + ancestors.parent(entry).is_some_and(|when_expression| { + when_expression + .children() + .take_while(|child| child.kind_id() != Kotlin::LBRACE) + .any(|child| child.kind_id() == Kotlin::WhenSubject) + }) +} + +// Scores one non-`else` `when` entry. Both `when` shapes contribute the +// same single decision that cyclomatic counts, but they pay for it in +// different places, and #1421 is the case where ABC charged for it twice. +// +// A subject-ful entry (`when (x) { in 1..2 -> … }`) lists a *pattern*, +// not an independent boolean expression: the decision is the implicit +// `x == pattern`, which nothing in the source spells, so the entry +// itself contributes the condition. A `range_test`, `type_test` or bare +// constant carries no token the comparison arms would count, and #1383's +// logic applies unchanged. +// +// A subject-less entry (`when { x > 5 -> … }`) lists an ordinary boolean +// expression — textually identical to an `if` predicate, and compiled as +// one — so it goes through the same slot `IfExpression` uses. Before +// this the entry added a blanket 1 *on top of* the comparison operator +// its condition already scored through the token arms, so +// `when { x > 5 -> 1; x < 0 -> 2; else -> 0 }` reported 4 conditions +// against a cyclomatic decision count of 2. The slot scores a bare +// terminal (`when { x -> … }`, `when { a is String -> … }`) directly and +// leaves a comparison or `&&` / `||` chain to the arms that already own +// it. Suppressing the operator instead would have been the wrong half to +// give way: a compound condition `when { a > 1 && b < 2 -> … }` needs +// both its comparisons, and suppression collapses it to one. +// +// The `condition` field is `multiple` — `when { a, b -> … }` lists +// alternatives — and only the first reaches the slot. That is +// deliberate: cyclomatic scores a multi-alternative entry as one +// decision, so routing every alternative through the slot would move +// `when { x, y -> … }` off that count rather than onto it. Alternatives +// after the first are still seen by the token arms. +fn kotlin_count_when_entry<'a>( + entry: &Node<'a>, + ancestors: Ancestors<'a, '_>, + conditions: &mut f64, +) { + if kotlin_enclosing_when_has_subject(entry, ancestors) { + *conditions += 1.; + } else if let Some(condition) = entry.child_by_field_name("condition") { + kotlin_count_condition(&condition, entry, conditions); + } +} + impl Abc for KotlinCode { fn compute<'a>( node: &Node<'a>, @@ -218,6 +375,35 @@ impl Abc for KotlinCode { { stats.branches += 1.; } + // An enum entry carrying constructor arguments — `A(1)` in + // `enum class E(val v: Int) { A(1), B(2) }` — invokes the enum's + // constructor, so it is an object construction under + // Fitzpatrick's "function invocation or object construction" + // rule. It scored zero until #1407, which is the inconsistent + // position once #1279 and #1384 decided the two sibling + // delegation forms (`: this(…)` and `class Sub : Base(1, 2)`): + // all three are a constructor call the source spells out. + // + // The counter-argument is that an enum entry is a declaration, + // not a call site a reader navigates to. It loses because the + // same is true of `class Sub : Base(1, 2)` — also a declaration + // — and because the arguments still have to be understood as a + // constructor's, which is the effort ABC is measuring. + // + // The child gate is what makes this a §6 narrowing rather than + // a new node: a bare `B` is `enum_entry > identifier` with no + // `value_arguments` child and must stay at zero, as must every + // entry of an enum with no constructor at all (`enum class E { + // A, B }`). Verified with `bca dump`. `value_arguments` is the + // only argument-list production the entry can carry, and a + // Kotlin `annotation` is a `constructor_invocation` above, not + // an entry, so nothing else satisfies the gate. An argument + // that is itself a call (`A(f())`) scores 2: `value_arguments` + // is not a branch node, so the inner `call_expression` is the + // only other node counted (no double count, §5). + EnumEntry if node.is_child(ValueArguments as u16) => { + stats.branches += 1.; + } // Conditions: comparison operators, identity equality, // ternary-elvis (`?:`), `as?` safe-cast, and the arms of // control-flow constructs (`else`, `catch`, `when` entries). @@ -252,9 +438,13 @@ impl Abc for KotlinCode { // fallback arm, which is the analogue of C-family `default:` // and Rust's wildcard `_ =>`. Cyclomatic already excludes it // (`WhenEntry if !kotlin_when_entry_is_else`); ABC must track - // the same decision count (issue #456, lesson 11). + // the same decision count (issue #456, lesson 11). Sharing + // that predicate is what keeps the two metrics from drifting. + // `kotlin_count_when_entry` decides where the decision is + // paid for — the entry, or the operators of its condition + // (#1421). WhenEntry if !crate::metrics::cyclomatic::kotlin_when_entry_is_else(node) => { - stats.conditions += 1.; + kotlin_count_when_entry(node, ancestors, &mut stats.conditions); } // `else` is a keyword token used in both `if_expression`'s // else-clause and `when`'s `else ->` entry. Only count it diff --git a/src/metrics/cyclomatic.rs b/src/metrics/cyclomatic.rs index c2b2c3507..2c2b34dea 100644 --- a/src/metrics/cyclomatic.rs +++ b/src/metrics/cyclomatic.rs @@ -1924,6 +1924,15 @@ mod tests { /// bare wildcard — the `when` guard adds a non-trivial decision — /// so the arm still contributes one standard decision, mirroring /// Rust's `_ if g` rule. + /// + /// Since #1422 the guard contributes a second decision of its own + /// (`WhenClause`), so the arm is worth 2: the pattern can match and + /// the guard still fail, which is a distinct way to fall through to + /// the next arm. The two counts have different owners — + /// `csharp_switch_expression_arm_is_bare_discard` keeps the arm out + /// of the `default:` exclusion, the `WhenClause` arm scores the + /// guard — so this test would still fail if #282's exclusion were + /// reintroduced, at sum 5 / max 3 against the asserted 6 / 4. #[test] fn csharp_switch_expression_guarded_discard_still_counts() { check_metrics::( @@ -1938,10 +1947,10 @@ mod tests { "foo.cs", |metric| { // expected: unit(1) + class(1) + fn(base 1 + 1 explicit + - // 1 guarded discard; bare `_ =>` skipped) = 5, - // max 3. - assert_eq!(metric.cyclomatic.cyclomatic_sum(), 5); - assert_eq!(metric.cyclomatic.cyclomatic_max(), 3); + // 1 guarded discard + 1 `when` guard (#1422); + // bare `_ =>` skipped) = 6, max 4. + assert_eq!(metric.cyclomatic.cyclomatic_sum(), 6); + assert_eq!(metric.cyclomatic.cyclomatic_max(), 4); }, ); } @@ -1979,6 +1988,11 @@ mod tests { /// so the arm still contributes one standard decision. Exercises /// the `DeclarationPattern` arm of `classify_pattern` combined /// with the post-pattern `WhenClause` sweep. + /// + /// As with `csharp_switch_expression_guarded_discard_still_counts`, + /// the guard has scored a decision of its own since #1422, so the + /// arm is worth 2. Reintroducing #303's exclusion would read sum 5 / + /// max 3 against the asserted 6 / 4. #[test] fn csharp_switch_expression_guarded_var_underscore_still_counts() { check_metrics::( @@ -1993,10 +2007,10 @@ mod tests { "foo.cs", |metric| { // expected: unit(1) + class(1) + fn(base 1 + 1 explicit `1` + - // 1 guarded `var _`; bare `_ =>` skipped) = 5, - // max 3. - assert_eq!(metric.cyclomatic.cyclomatic_sum(), 5); - assert_eq!(metric.cyclomatic.cyclomatic_max(), 3); + // 1 guarded `var _` + 1 `when` guard (#1422); + // bare `_ =>` skipped) = 6, max 4. + assert_eq!(metric.cyclomatic.cyclomatic_sum(), 6); + assert_eq!(metric.cyclomatic.cyclomatic_max(), 4); }, ); } diff --git a/src/metrics/cyclomatic/csharp.rs b/src/metrics/cyclomatic/csharp.rs index 205372329..85912d9fd 100644 --- a/src/metrics/cyclomatic/csharp.rs +++ b/src/metrics/cyclomatic/csharp.rs @@ -21,6 +21,13 @@ impl Cyclomatic for CsharpCode { // Standard-only: individual switch statement arms. The `case` // keyword token is what is matched here; `default:` uses a // distinct `Default` token and is correctly excluded. + // + // FIXME(#1450): `goto case 2;` spells the same token, so it + // scores a decision without being an arm. The ABC half of + // this is the matching arm in `src/metrics/abc/csharp.rs`; + // the two have to move together or the §8 parity + // `conditions == cyclomatic() - 1` breaks, which is why + // neither has been gated on its own. Case => { stats.cyclomatic += 1.; } @@ -39,12 +46,39 @@ impl Cyclomatic for CsharpCode { stats.cyclomatic_modified += 1.; } // Both standard and modified. + // + // `WhenClause` and `CatchFilterClause` are C#'s two guard + // spellings, and each is a decision the enclosing construct + // does not already pay for (#1422): a guarded arm fails two + // ways — the pattern does not match, or it matches and the + // guard is false — while contributing one decision, and + // `catch (E e) when (c)` tests the filter after the type. + // Rust already counts a match guard — `rust.rs`'s `If` arm + // catches the guard's own `if` token — so C# was the + // outlier, not the convention. + // + // The clause *nodes*, not the `when` keyword they share: + // `When` (119) is also a `_reserved_identifier` at this pin, + // so `int when = 1;` emits it under an `identifier` and the + // token would score a decision per mention of the variable + // (`bca dump`, not inferred). One `when_clause` serves both + // the `switch_expression_arm` and `switch_section` guards; + // neither kind carries a numeric-suffix alias. + // + // A guarded discard (`_ when g => …`) therefore scores two, + // and that is the intended reading: the guard is already why + // `csharp_switch_expression_arm_is_bare_discard` keeps the + // arm out of the `default:` exclusion, so the arm is no + // longer an unconditional fallthrough and the guard that + // makes it conditional is its own decision. IfStatement | ForStatement | ForeachStatement | WhileStatement | DoStatement | CatchClause + | WhenClause + | CatchFilterClause | ConditionalExpression | ConditionalAccessExpression | AMPAMP diff --git a/tests/repositories/big-code-analysis-output b/tests/repositories/big-code-analysis-output index 98c48d2cb..81abf5931 160000 --- a/tests/repositories/big-code-analysis-output +++ b/tests/repositories/big-code-analysis-output @@ -1 +1 @@ -Subproject commit 98c48d2cb9e1209edc6204ec868eef1e068d0e2f +Subproject commit 81abf5931c9d54dfc43a109149209d14cdf60cac