From e00ae054f2195b9278162cf5e6b4ae745abc473b Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 5 Aug 2026 04:50:33 -0600 Subject: [PATCH] fix: correct switch/case complexity shadowing and comment-prefix undercounting (#2058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three pre-existing correctness issues in the native LangRules/HalsteadRules for c/cpp/objc/kotlin/swift/scala, mirrored verbatim into the TS rules by #1923 to keep engine parity, are now fixed in both engines: 1. Case-node shadowing (C_RULES, CPP_RULES, OBJC_RULES, KOTLIN_RULES, SCALA_RULES): the walk()/classifyNode DFS always returns after a branch_nodes/branchNodes match, so a node type listed in BOTH branch_nodes and case_nodes was always treated as a generic branch — the case_nodes arm never fired. Fixed by removing the case-arm node type from branch_nodes, keeping it in case_nodes only — matching the pattern JS/Java/C#/PHP/Ruby/Bash already use. 2. SWIFT_RULES dropped the switch container entirely: switch_statement was absent from both branch_nodes and nesting_nodes, so a Swift switch contributed zero nesting for its cases, and switch_entry was double-booked hitting the same shadowing bug as above. Fixed by adding switch_statement to branch_nodes + nesting_nodes and removing switch_entry from branch_nodes. 3. comment_prefixes()/COMMENT_PREFIXES: c/cpp/cuda/objc/kotlin/swift/scala used a 2-entry list instead of the 4-entry list JS/Java/C# use, missing bare `*`/`*/` continuation lines and undercounting commentLines for any multi-line Javadoc-style comment. ObjC (added after #1923/#2058 was filed) inherited the same bugs via copy-paste from C's rules — fixed here too. CUDA reuses CPP_RULES as-is, so it's auto-fixed. Cyclomatic values are numerically unaffected for existing tests (each case arm always contributes +1 whether via the buggy or correct path, container nets to zero either way) — the bug only showed up in cognitive complexity, which existing tests never asserted. Verified native and WASM produce byte-identical output on hand-built switch/when/match and multi-line-comment fixtures for all six languages. Added regression tests: switch/when/match cases for C/C++/ObjC/Swift (previously untested) plus multi-value-case variants; upgraded Kotlin's and Scala's existing tests to full assertions; a comment-prefix test per engine. docs check acknowledged — bug fix restoring engine parity and metric correctness, no new language/feature/architecture surface to document. --- .../src/ast_analysis/complexity.rs | 167 ++++++++++++++++-- src/ast-analysis/metrics.ts | 29 ++- src/ast-analysis/rules/b2.ts | 24 ++- src/ast-analysis/rules/c.ts | 21 ++- tests/unit/complexity.test.ts | 81 ++++++++- 5 files changed, 282 insertions(+), 40 deletions(-) diff --git a/crates/codegraph-core/src/ast_analysis/complexity.rs b/crates/codegraph-core/src/ast_analysis/complexity.rs index 331190321..c5796176f 100644 --- a/crates/codegraph-core/src/ast_analysis/complexity.rs +++ b/crates/codegraph-core/src/ast_analysis/complexity.rs @@ -344,13 +344,21 @@ pub static PHP_RULES: LangRules = LangRules { // style: an else_clause node wraps either a nested if_statement for // `else if` or the plain else body), NOT Pattern C (Go/Java style, where the // `alternative` field holds the substatement directly with no wrapper node). +// `walk()`'s node classification always returns after a `is_branch(kind)` +// match, so a type listed in BOTH branch_nodes and case_nodes is always +// treated as a generic branch — the case_nodes arm never fires (issue +// #2058). `switch_statement` (the container) belongs in branch_nodes + +// nesting_nodes (net-zero cyclomatic via switch_like_nodes, contributing +// nesting once, matching JS/Java/C#/PHP/Ruby/Bash); `case_statement` (each +// arm) belongs in case_nodes ONLY (flat `cyclomatic += 1`, no per-case +// cognitive/nesting weight) — not in branch_nodes. pub static C_RULES: LangRules = LangRules { - branch_nodes: &["if_statement", "else_clause", "for_statement", "while_statement", "do_statement", "case_statement", "conditional_expression"], + branch_nodes: &["if_statement", "else_clause", "for_statement", "while_statement", "do_statement", "switch_statement", "conditional_expression"], case_nodes: &["case_statement"], logical_operators: &["&&", "||"], logical_node_types: &["binary_expression"], optional_chain_type: None, - nesting_nodes: &["if_statement", "for_statement", "while_statement", "do_statement", "conditional_expression"], + nesting_nodes: &["if_statement", "for_statement", "while_statement", "do_statement", "switch_statement", "conditional_expression"], function_nodes: &["function_definition"], if_node_type: Some("if_statement"), else_node_type: Some("else_clause"), @@ -367,13 +375,14 @@ pub static C_RULES: LangRules = LangRules { // syntax), and parsing sample CUDA control flow confirms identical // if_statement/else_clause/for_statement/while_statement/switch_statement/ // binary_expression node kinds to plain C++. +// Same branch_nodes/case_nodes fix as C_RULES (issue #2058) — see comment there. pub static CPP_RULES: LangRules = LangRules { - branch_nodes: &["if_statement", "else_clause", "for_statement", "for_range_loop", "while_statement", "do_statement", "case_statement", "conditional_expression", "catch_clause"], + branch_nodes: &["if_statement", "else_clause", "for_statement", "for_range_loop", "while_statement", "do_statement", "switch_statement", "conditional_expression", "catch_clause"], case_nodes: &["case_statement"], logical_operators: &["&&", "||"], logical_node_types: &["binary_expression"], optional_chain_type: None, - nesting_nodes: &["if_statement", "for_statement", "for_range_loop", "while_statement", "do_statement", "catch_clause", "conditional_expression"], + nesting_nodes: &["if_statement", "for_statement", "for_range_loop", "while_statement", "do_statement", "switch_statement", "catch_clause", "conditional_expression"], function_nodes: &["function_definition"], if_node_type: Some("if_statement"), else_node_type: Some("else_clause"), @@ -397,13 +406,15 @@ pub static CPP_RULES: LangRules = LangRules { // also models as a dedicated try_statement/catch_clause/finally_clause // shape) is a branch/nesting node, same treatment as CPP_RULES's // catch_clause. +// Same branch_nodes/case_nodes fix as C_RULES (issue #2058) — see comment +// there. Inherited the bug via copy from C_RULES when ObjC was added. pub static OBJC_RULES: LangRules = LangRules { - branch_nodes: &["if_statement", "else_clause", "for_statement", "while_statement", "do_statement", "case_statement", "conditional_expression", "catch_clause"], + branch_nodes: &["if_statement", "else_clause", "for_statement", "while_statement", "do_statement", "switch_statement", "conditional_expression", "catch_clause"], case_nodes: &["case_statement"], logical_operators: &["&&", "||"], logical_node_types: &["binary_expression"], optional_chain_type: None, - nesting_nodes: &["if_statement", "for_statement", "while_statement", "do_statement", "catch_clause", "conditional_expression"], + nesting_nodes: &["if_statement", "for_statement", "while_statement", "do_statement", "switch_statement", "catch_clause", "conditional_expression"], function_nodes: &["function_definition", "method_definition"], if_node_type: Some("if_statement"), else_node_type: Some("else_clause"), @@ -412,8 +423,14 @@ pub static OBJC_RULES: LangRules = LangRules { switch_like_nodes: &["switch_statement"], }; +// `when_entry` (each case arm) must NOT also be in branch_nodes — `walk()` +// always treats a branch_nodes match as a generic branch and never falls +// through to the case_nodes arm, so having it in both shadowed the +// intended flat case treatment with nesting-weighted branch treatment +// (issue #2058). `when_expression` (the container) already correctly sits +// in branch_nodes + nesting_nodes + switch_like_nodes. pub static KOTLIN_RULES: LangRules = LangRules { - branch_nodes: &["if_expression", "for_statement", "while_statement", "do_while_statement", "catch_block", "when_expression", "when_entry"], + branch_nodes: &["if_expression", "for_statement", "while_statement", "do_while_statement", "catch_block", "when_expression"], case_nodes: &["when_entry"], logical_operators: &["&&", "||"], logical_node_types: &["conjunction_expression", "disjunction_expression"], @@ -432,13 +449,21 @@ pub static KOTLIN_RULES: LangRules = LangRules { // sharing one generic binary node — confirmed by parsing `a && b || a` and // inspecting the S-expression. `logical_node_types: &["binary_expression"]` // never matches either operator, so Swift && / || were never counted. +// `switch_statement` (the container) was missing from branch_nodes AND +// nesting_nodes entirely — only switch_like_nodes, which is only consulted +// from inside the branch handler, so a Swift `switch` contributed zero +// nesting for its cases. `switch_entry` (each case arm) was also +// double-booked in branch_nodes + case_nodes, hitting the same shadowing +// bug as Kotlin's when_entry (issue #2058). Fixed to match the +// container-in-branch+nesting+switch_like / case-in-case_nodes-only +// pattern every other switch-having language in this file uses. pub static SWIFT_RULES: LangRules = LangRules { - branch_nodes: &["if_statement", "for_in_statement", "while_statement", "repeat_while_statement", "catch_clause", "switch_entry", "ternary_expression", "guard_statement"], + branch_nodes: &["if_statement", "for_in_statement", "while_statement", "repeat_while_statement", "catch_clause", "switch_statement", "ternary_expression", "guard_statement"], case_nodes: &["switch_entry"], logical_operators: &["&&", "||"], logical_node_types: &["conjunction_expression", "disjunction_expression"], optional_chain_type: Some("optional_chaining_expression"), - nesting_nodes: &["if_statement", "for_in_statement", "while_statement", "repeat_while_statement", "catch_clause", "ternary_expression", "guard_statement"], + nesting_nodes: &["if_statement", "for_in_statement", "while_statement", "repeat_while_statement", "catch_clause", "switch_statement", "ternary_expression", "guard_statement"], function_nodes: &["function_declaration", "init_declaration"], if_node_type: Some("if_statement"), else_node_type: None, @@ -447,8 +472,11 @@ pub static SWIFT_RULES: LangRules = LangRules { switch_like_nodes: &["switch_statement"], }; +// `case_clause` must NOT also be in branch_nodes — same shadowing bug as +// Kotlin's when_entry (issue #2058). `match_expression` (the container) +// already correctly sits in branch_nodes + nesting_nodes + switch_like_nodes. pub static SCALA_RULES: LangRules = LangRules { - branch_nodes: &["if_expression", "for_expression", "while_expression", "do_while_expression", "catch_clause", "case_clause", "match_expression"], + branch_nodes: &["if_expression", "for_expression", "while_expression", "do_while_expression", "catch_clause", "match_expression"], case_nodes: &["case_clause"], logical_operators: &["&&", "||"], logical_node_types: &["infix_expression"], @@ -1272,15 +1300,14 @@ pub fn halstead_rules(lang_id: &str) -> Option<&'static HalsteadRules> { /// Comment line prefixes per language, used for LOC metrics. pub fn comment_prefixes(lang_id: &str) -> &'static [&'static str] { match lang_id { - "javascript" | "typescript" | "tsx" | "go" | "rust" | "java" | "csharp" => { - &["//", "/*", "*", "*/"] - } + // c/cpp/cuda/objc/kotlin/swift/scala all use the same `/** ... */` + // block-comment style as JS/Java/C# — the 2-entry list omitted + // bare `*`/`*/` continuation lines, undercounting commentLines for + // any multi-line Javadoc-style comment (issue #2058). + "javascript" | "typescript" | "tsx" | "go" | "rust" | "java" | "csharp" | "c" | "cpp" + | "cuda" | "objc" | "kotlin" | "swift" | "scala" => &["//", "/*", "*", "*/"], "python" | "ruby" => &["#"], "php" => &["//", "#", "/*", "*", "*/"], - "c" | "cpp" | "cuda" | "objc" => &["//", "/*"], - "kotlin" => &["//", "/*"], - "swift" => &["//", "/*"], - "scala" => &["//", "/*"], "bash" => &["#"], "lua" => &["--"], "zig" => &["//"], @@ -1595,6 +1622,19 @@ mod tests { use super::*; use tree_sitter::Parser; + #[test] + fn comment_prefixes_c_family_and_jvm_langs_match_continuation_lines() { + // Regression guard (issue #2058): c/cpp/cuda/objc/kotlin/swift/scala + // all use the same `/** ... */` block-comment style as JS/Java/C# — + // the old 2-entry list omitted bare `*`/`*/` continuation lines, + // undercounting commentLines for any multi-line Javadoc-style comment. + for lang in ["c", "cpp", "cuda", "objc", "kotlin", "swift", "scala"] { + let prefixes = comment_prefixes(lang); + assert!(prefixes.contains(&"*"), "{lang} should match bare '*' continuation lines"); + assert!(prefixes.contains(&"*/"), "{lang} should match closing '*/' lines"); + } + } + fn compute_js(code: &str) -> ComplexityMetrics { let mut parser = Parser::new(); parser @@ -2004,6 +2044,23 @@ mod tests { assert_eq!(m.cyclomatic, 2); } + #[test] + fn c_switch_with_multi_value_case() { + // Regression guard (issue #2058): switch_statement (the container) + // must be in branch_nodes + nesting_nodes (net-zero cyclomatic, + // contributing nesting once), and case_statement (each arm) must be + // in case_nodes ONLY (flat cyclomatic += 1, no per-case + // cognitive/nesting weight) — not also in branch_nodes, which + // previously shadowed the case treatment with a nesting-weighted + // generic branch treatment for every arm. + let m = compute_c( + "int f(int x) {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}", + ); + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 5); + assert_eq!(m.max_nesting, 1); + } + #[test] fn cpp_if_elseif_else() { let m = compute_cpp( @@ -2022,6 +2079,17 @@ mod tests { assert_eq!(m.max_nesting, 1); } + #[test] + fn cpp_switch_with_multi_value_case() { + // Same branch_nodes/case_nodes fix as C's equivalent test — see comment there. + let m = compute_cpp( + "int f(int x) {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}", + ); + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 5); + assert_eq!(m.max_nesting, 1); + } + // ─── CUDA tests (issue #1923) ──────────────────────────────────────────── // // tree-sitter-cuda is a C++-superset grammar (only adding qualifier @@ -2124,6 +2192,19 @@ mod tests { assert_eq!(m.cyclomatic, 2); } + #[test] + fn objc_switch_with_multi_value_case() { + // Same branch_nodes/case_nodes fix as C's equivalent test (see + // comment there) — inherited the bug via copy from C's rules when + // ObjC was added. + let m = compute_objc( + "@implementation Calculator\n- (NSInteger)classify:(NSInteger)x {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}\n@end", + ); + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 5); + assert_eq!(m.max_nesting, 1); + } + // ─── Zig tests (issue #1923) ───────────────────────────────────────────── // // tree-sitter-zig wraps its else branch in an else_clause node (Pattern @@ -2223,12 +2304,30 @@ mod tests { #[test] fn kotlin_when_expression() { + // Regression guard (issue #2058): when_entry (each case arm) must + // not also be in branch_nodes — that shadowed the flat case + // treatment with nesting-weighted branch treatment, inflating + // cognitive from 1 to 7 for this fixture even though cyclomatic + // happened to stay 4 either way (each arm contributes +1 via + // either code path). let m = compute_kotlin( "fun f(x: Int): Int {\n return when (x) {\n 1 -> 1\n 2 -> 2\n else -> 0\n }\n}", ); // base 1 + when container (0, switch-like) + 3 when_entry cases (+1 // each) = 4. + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 4); + assert_eq!(m.max_nesting, 1); + } + + #[test] + fn kotlin_when_expression_with_multi_value_case() { + let m = compute_kotlin( + "fun f(x: Int): Int {\n return when (x) {\n 1 -> 1\n 2, 3 -> 2\n else -> 0\n }\n}", + ); + assert_eq!(m.cognitive, 1); assert_eq!(m.cyclomatic, 4); + assert_eq!(m.max_nesting, 1); } // ─── Swift tests (issue #1923) ────────────────────────────────────────── @@ -2258,6 +2357,22 @@ mod tests { assert_eq!(m.cyclomatic, 2); } + #[test] + fn swift_switch_with_multi_value_case() { + // Regression guard (issue #2058): switch_statement (the container) + // was missing from branch_nodes AND nesting_nodes entirely — a + // Swift switch contributed ZERO nesting/cognitive from its own + // container, and switch_entry (each case arm) was double-booked in + // branch_nodes + case_nodes, hitting the same shadowing bug as + // Kotlin's when_entry. + let m = compute_swift( + "func f(_ x: Int) -> Int {\n switch x {\n case 1:\n return 1\n case 2, 3:\n return 2\n default:\n return 0\n }\n}", + ); + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 4); + assert_eq!(m.max_nesting, 1); + } + // ─── Scala tests (issue #1923) ────────────────────────────────────────── fn compute_scala(code: &str) -> ComplexityMetrics { @@ -2286,12 +2401,30 @@ mod tests { #[test] fn scala_match_expression() { + // Regression guard (issue #2058): case_clause (each case arm) must + // not also be in branch_nodes — that shadowed the flat case + // treatment with nesting-weighted branch treatment, inflating + // cognitive from 1 to 7 for this fixture even though cyclomatic + // happened to stay 4 either way (each arm contributes +1 via + // either code path). let m = compute_scala( "def f(x: Int): Int = {\n x match {\n case 1 => 1\n case 2 => 2\n case _ => 0\n }\n}", ); // base 1 + match container (0, switch-like) + 3 case_clause cases // (+1 each) = 4. + assert_eq!(m.cognitive, 1); + assert_eq!(m.cyclomatic, 4); + assert_eq!(m.max_nesting, 1); + } + + #[test] + fn scala_match_expression_with_alternative_pattern_case() { + let m = compute_scala( + "def f(x: Int): Int = {\n x match {\n case 1 => 1\n case 2 | 3 => 2\n case _ => 0\n }\n}", + ); + assert_eq!(m.cognitive, 1); assert_eq!(m.cyclomatic, 4); + assert_eq!(m.max_nesting, 1); } // ─── Bash tests (issue #1923) ─────────────────────────────────────────── diff --git a/src/ast-analysis/metrics.ts b/src/ast-analysis/metrics.ts index d837bd55d..7f0a79f5a 100644 --- a/src/ast-analysis/metrics.ts +++ b/src/ast-analysis/metrics.ts @@ -60,14 +60,9 @@ export function computeHalsteadDerived( const C_STYLE_PREFIXES = ['//', '/*', '*', '*/']; -// c/cpp/cuda/objc/kotlin/swift/scala intentionally mirror the native `comment_prefixes()` -// 2-entry list (`//`, `/*`) rather than the 4-entry C_STYLE_PREFIXES used by -// javascript/go/rust/java/csharp — see native `comment_prefixes()` in -// crates/codegraph-core/src/ast_analysis/complexity.rs for the source of truth -// this must stay byte-for-byte identical to (both engines must agree on which -// lines count as comments for the MI calculation). -const C_LIKE_PREFIXES = ['//', '/*']; - +// See native `comment_prefixes()` in crates/codegraph-core/src/ast_analysis/complexity.rs +// for the source of truth this must stay byte-for-byte identical to (both +// engines must agree on which lines count as comments for the MI calculation). const COMMENT_PREFIXES = new Map([ ['javascript', C_STYLE_PREFIXES], ['typescript', C_STYLE_PREFIXES], @@ -79,13 +74,17 @@ const COMMENT_PREFIXES = new Map([ ['python', ['#']], ['ruby', ['#']], ['php', ['//', '#', '/*', '*', '*/']], - ['c', C_LIKE_PREFIXES], - ['cpp', C_LIKE_PREFIXES], - ['cuda', C_LIKE_PREFIXES], - ['objc', C_LIKE_PREFIXES], - ['kotlin', C_LIKE_PREFIXES], - ['swift', C_LIKE_PREFIXES], - ['scala', C_LIKE_PREFIXES], + // c/cpp/cuda/objc/kotlin/swift/scala use the same `/** ... */` block-comment + // style as JS/Java/C# — the old 2-entry list omitted bare `*`/`*/` + // continuation lines, undercounting commentLines for any multi-line + // Javadoc-style comment (issue #2058). + ['c', C_STYLE_PREFIXES], + ['cpp', C_STYLE_PREFIXES], + ['cuda', C_STYLE_PREFIXES], + ['objc', C_STYLE_PREFIXES], + ['kotlin', C_STYLE_PREFIXES], + ['swift', C_STYLE_PREFIXES], + ['scala', C_STYLE_PREFIXES], ['bash', ['#']], ['lua', ['--']], ['zig', ['//']], diff --git a/src/ast-analysis/rules/b2.ts b/src/ast-analysis/rules/b2.ts index bf89f356f..ce234c640 100644 --- a/src/ast-analysis/rules/b2.ts +++ b/src/ast-analysis/rules/b2.ts @@ -57,6 +57,13 @@ export const dataflowKotlin: DataflowRulesConfig = makeDataflowRules({ // languages, which share one generic binary-expression node for every // operator) — both must be listed in logicalNodeTypes for cyclomatic/cognitive // counting to see both operators. Mirrors the native `KOTLIN_RULES`. +// when_entry (each case arm) must NOT also be in branchNodes — +// classifyBranchNode always treats a branchNodes match as a generic +// branch and never falls through to the caseNodes arm, so having it in +// both shadowed the intended flat case treatment with nesting-weighted +// branch treatment (issue #2058). when_expression (the container) already +// correctly sits in branchNodes + nestingNodes + switchLikeNodes. Mirrors +// the native KOTLIN_RULES fix. export const complexityKotlin: ComplexityRules = { branchNodes: new Set([ 'if_expression', @@ -65,7 +72,6 @@ export const complexityKotlin: ComplexityRules = { 'do_while_statement', 'catch_block', 'when_expression', - 'when_entry', ]), caseNodes: new Set(['when_entry']), logicalOperators: new Set(['&&', '||']), @@ -215,6 +221,14 @@ export const dataflowSwift: DataflowRulesConfig = makeDataflowRules({ // node types (conjunction_expression / disjunction_expression) rather than // sharing one generic binary node — confirmed by parsing `a && b || a` and // inspecting the S-expression. Mirrors the native `SWIFT_RULES`/`SWIFT_HALSTEAD`. +// switch_statement (the container) was missing from branchNodes AND +// nestingNodes entirely — only switchLikeNodes, which is only consulted +// from inside the branch handler, so a Swift `switch` contributed zero +// nesting for its cases. switch_entry (each case arm) was also +// double-booked in branchNodes + caseNodes, hitting the same shadowing +// bug as Kotlin's when_entry (issue #2058). Fixed to match the +// container-in-branch+nesting+switchLike / case-in-caseNodes-only pattern +// every other switch-having language uses. Mirrors the native SWIFT_RULES fix. export const complexitySwift: ComplexityRules = { branchNodes: new Set([ 'if_statement', @@ -222,7 +236,7 @@ export const complexitySwift: ComplexityRules = { 'while_statement', 'repeat_while_statement', 'catch_clause', - 'switch_entry', + 'switch_statement', 'ternary_expression', 'guard_statement', ]), @@ -236,6 +250,7 @@ export const complexitySwift: ComplexityRules = { 'while_statement', 'repeat_while_statement', 'catch_clause', + 'switch_statement', 'ternary_expression', 'guard_statement', ]), @@ -366,6 +381,10 @@ export const dataflowScala: DataflowRulesConfig = makeDataflowRules({ }); // Mirrors the native `SCALA_RULES`/`SCALA_HALSTEAD`. +// case_clause must NOT also be in branchNodes — same shadowing bug as +// Kotlin's when_entry (issue #2058). match_expression (the container) +// already correctly sits in branchNodes + nestingNodes + switchLikeNodes. +// Mirrors the native SCALA_RULES fix. export const complexityScala: ComplexityRules = { branchNodes: new Set([ 'if_expression', @@ -373,7 +392,6 @@ export const complexityScala: ComplexityRules = { 'while_expression', 'do_while_expression', 'catch_clause', - 'case_clause', 'match_expression', ]), caseNodes: new Set(['case_clause']), diff --git a/src/ast-analysis/rules/c.ts b/src/ast-analysis/rules/c.ts index 1cceb162d..229a8b80f 100644 --- a/src/ast-analysis/rules/c.ts +++ b/src/ast-analysis/rules/c.ts @@ -18,6 +18,15 @@ import { makeDataflowRules } from '../shared.js'; // `else if` or the plain else body), NOT Pattern C (Go/Java style, where the // `alternative` field holds the substatement directly with no wrapper node). +// classifyNode's classifyBranchNode always returns after a branchNodes +// match, so a type listed in BOTH branchNodes and caseNodes is always +// treated as a generic branch — the caseNodes arm never fires (issue +// #2058). switch_statement (the container) belongs in branchNodes + +// nestingNodes (net-zero cyclomatic via switchLikeNodes, contributing +// nesting once, matching JS/Java/C#/PHP/Ruby/Bash); case_statement (each +// arm) belongs in caseNodes ONLY (flat cyclomatic += 1, no per-case +// cognitive/nesting weight) — not in branchNodes. Mirrors the native +// C_RULES fix in complexity.rs. export const complexity: ComplexityRules = { branchNodes: new Set([ 'if_statement', @@ -25,7 +34,7 @@ export const complexity: ComplexityRules = { 'for_statement', 'while_statement', 'do_statement', - 'case_statement', + 'switch_statement', 'conditional_expression', ]), caseNodes: new Set(['case_statement']), @@ -37,6 +46,7 @@ export const complexity: ComplexityRules = { 'for_statement', 'while_statement', 'do_statement', + 'switch_statement', 'conditional_expression', ]), functionNodes: new Set(['function_definition']), @@ -53,6 +63,7 @@ export const complexity: ComplexityRules = { // on top of the C rule set; uses the same else_clause wrapper (Pattern A) as // C, confirmed by parsing the same if/else-if/else shape with tree-sitter-cpp. +// Same branchNodes/caseNodes fix as `complexity` above (issue #2058). export const complexityCpp: ComplexityRules = { branchNodes: new Set([ 'if_statement', @@ -61,7 +72,7 @@ export const complexityCpp: ComplexityRules = { 'for_range_loop', 'while_statement', 'do_statement', - 'case_statement', + 'switch_statement', 'conditional_expression', 'catch_clause', ]), @@ -75,6 +86,7 @@ export const complexityCpp: ComplexityRules = { 'for_range_loop', 'while_statement', 'do_statement', + 'switch_statement', 'catch_clause', 'conditional_expression', ]), @@ -258,6 +270,8 @@ export const halsteadCpp: HalsteadRules = { // handling, which tree-sitter-objc also models with a dedicated // try_statement/catch_clause/finally_clause shape) is a branch/nesting // node, same treatment as C++'s catch_clause. +// Same branchNodes/caseNodes fix as `complexity` above (issue #2058) — +// inherited the bug via copy from C's rules when ObjC was added. export const complexityObjC: ComplexityRules = { branchNodes: new Set([ 'if_statement', @@ -265,7 +279,7 @@ export const complexityObjC: ComplexityRules = { 'for_statement', 'while_statement', 'do_statement', - 'case_statement', + 'switch_statement', 'conditional_expression', 'catch_clause', ]), @@ -278,6 +292,7 @@ export const complexityObjC: ComplexityRules = { 'for_statement', 'while_statement', 'do_statement', + 'switch_statement', 'catch_clause', 'conditional_expression', ]), diff --git a/tests/unit/complexity.test.ts b/tests/unit/complexity.test.ts index 5b09008f5..c4bc8fd0d 100644 --- a/tests/unit/complexity.test.ts +++ b/tests/unit/complexity.test.ts @@ -1104,6 +1104,19 @@ describe('C complexity', () => { expect(r.cyclomatic).toBe(2); }); + it('switch with a multi-value case (issue #2058)', () => { + // Regression guard: switch_statement (the container) must be in + // branchNodes + nestingNodes (net-zero cyclomatic, contributing nesting + // once), and case_statement (each arm) must be in caseNodes ONLY (flat + // cyclomatic += 1, no per-case cognitive/nesting weight) — not also in + // branchNodes, which previously shadowed the case treatment with a + // nesting-weighted generic branch treatment for every arm. + const r = analyze( + 'int classify(int x) {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 5, maxNesting: 1 }); + }); + it('halstead: positive volume', () => { const h = halstead('int add(int a, int b) {\n return a + b;\n}\n'); expect(h).not.toBeNull(); @@ -1114,6 +1127,17 @@ describe('C complexity', () => { const l = loc('int f() {\n // slash comment\n return 1;\n}\n'); expect(l.commentLines).toBeGreaterThanOrEqual(1); }); + + it('LOC: /** ... */ continuation lines counted as comments (issue #2058)', () => { + // Regression guard: c/cpp/objc/kotlin/swift/scala previously used a + // 2-entry ["//", "/*"] prefix list that missed bare `*`/`*/` + // continuation lines, undercounting commentLines for any multi-line + // Javadoc-style comment. + const l = loc( + 'int f() {\n /**\n * Multi-line comment.\n * Second line.\n */\n return 1;\n}\n', + ); + expect(l.commentLines).toBe(4); + }); }); // ─── C++ (#1923) ────────────────────────────────────────────────────────── @@ -1135,6 +1159,14 @@ describe('C++ complexity', () => { expect(r).toEqual({ cognitive: 1, cyclomatic: 2, maxNesting: 1 }); }); + it('switch with a multi-value case (issue #2058)', () => { + // Same branchNodes/caseNodes fix as C's equivalent test — see comment there. + const r = analyze( + 'int classify(int x) {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 5, maxNesting: 1 }); + }); + it('halstead: positive volume', () => { const h = halstead('int add(int a, int b) {\n return a + b;\n}\n'); expect(h).not.toBeNull(); @@ -1222,6 +1254,15 @@ describe('ObjC complexity', () => { expect(r).toEqual({ cognitive: 1, cyclomatic: 2, maxNesting: 1 }); }); + it('switch with a multi-value case (issue #2058)', () => { + // Same branchNodes/caseNodes fix as C's equivalent test (see comment + // there) — inherited the bug via copy from C's rules when ObjC was added. + const r = analyze( + '@implementation Calculator\n- (NSInteger)classify:(NSInteger)x {\n switch (x) {\n case 1:\n return 1;\n case 2:\n case 3:\n return 2;\n default:\n return 0;\n }\n}\n@end\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 5, maxNesting: 1 }); + }); + it('halstead: message send and positive volume', () => { const h = halstead( '@implementation Calculator\n- (NSInteger)sum {\n return [self compute];\n}\n@end\n', @@ -1309,11 +1350,23 @@ describe('Kotlin complexity', () => { }); it('when expression', () => { + // Regression guard (issue #2058): when_entry (each case arm) must not + // also be in branchNodes — that shadowed the flat case treatment with + // nesting-weighted branch treatment, inflating cognitive from 1 to 7 for + // this fixture even though cyclomatic happened to stay 4 either way + // (each arm contributes +1 via either code path). const r = analyze( 'fun classify(x: Int): Int {\n return when (x) {\n 1 -> 1\n 2 -> 2\n else -> 0\n }\n}\n', ); // base 1 + when container (0, switch-like) + 3 when_entry cases (+1 each) = 4 - expect(r.cyclomatic).toBe(4); + expect(r).toEqual({ cognitive: 1, cyclomatic: 4, maxNesting: 1 }); + }); + + it('when expression with a multi-value case (issue #2058)', () => { + const r = analyze( + 'fun classify(x: Int): Int {\n return when (x) {\n 1 -> 1\n 2, 3 -> 2\n else -> 0\n }\n}\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 4, maxNesting: 1 }); }); it('halstead: positive volume', () => { @@ -1347,6 +1400,18 @@ describe('Swift complexity', () => { expect(r.cyclomatic).toBe(2); }); + it('switch with a multi-value case (issue #2058)', () => { + // Regression guard: switch_statement (the container) was missing from + // branchNodes AND nestingNodes entirely — a Swift switch contributed + // ZERO nesting/cognitive from its own container, and switch_entry (each + // case arm) was double-booked in branchNodes + caseNodes, hitting the + // same shadowing bug as Kotlin's when_entry. + const r = analyze( + 'func classify(_ x: Int) -> Int {\n switch x {\n case 1:\n return 1\n case 2, 3:\n return 2\n default:\n return 0\n }\n}\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 4, maxNesting: 1 }); + }); + it('halstead: positive volume', () => { const h = halstead('func add(_ a: Int, _ b: Int) -> Int {\n return a + b\n}\n'); expect(h).not.toBeNull(); @@ -1375,11 +1440,23 @@ describe('Scala complexity', () => { }); it('match expression', () => { + // Regression guard (issue #2058): case_clause (each case arm) must not + // also be in branchNodes — that shadowed the flat case treatment with + // nesting-weighted branch treatment, inflating cognitive from 1 to 7 for + // this fixture even though cyclomatic happened to stay 4 either way + // (each arm contributes +1 via either code path). const r = analyze( 'def classify(x: Int): Int = {\n x match {\n case 1 => 1\n case 2 => 2\n case _ => 0\n }\n}\n', ); // base 1 + match container (0, switch-like) + 3 case_clause cases (+1 each) = 4 - expect(r.cyclomatic).toBe(4); + expect(r).toEqual({ cognitive: 1, cyclomatic: 4, maxNesting: 1 }); + }); + + it('match expression with an alternative-pattern case (issue #2058)', () => { + const r = analyze( + 'def classify(x: Int): Int = {\n x match {\n case 1 => 1\n case 2 | 3 => 2\n case _ => 0\n }\n}\n', + ); + expect(r).toEqual({ cognitive: 1, cyclomatic: 4, maxNesting: 1 }); }); it('halstead: positive volume', () => {