Skip to content

fix: correct switch/case complexity shadowing and comment-prefix undercounting (#2058) #2286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 1 commit into from
Aug 5, 2026
Merged

fix: correct switch/case complexity shadowing and comment-prefix undercounting (#2058) #2286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 150 additions & 17 deletions crates/codegraph-core/src/ast_analysis/complexity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"),
Expand All @@ -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"],
Expand All @@ -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,
Expand All @@ -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"],
Expand Down Expand Up @@ -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" => &["//"],
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) ──────────────────────────────────────────
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) ───────────────────────────────────────────
Expand Down
29 changes: 14 additions & 15 deletions src/ast-analysis/metrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string[]>([
['javascript', C_STYLE_PREFIXES],
['typescript', C_STYLE_PREFIXES],
Expand All @@ -79,13 +74,17 @@ const COMMENT_PREFIXES = new Map<string, string[]>([
['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],
Comment on lines +81 to +87

@greptile-apps greptile-apps Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Bare star misclassifies source lines

When an affected-language function contains an executable line beginning with *, such as *ptr = value;, the trimmed-line prefix check classifies it as a comment, inflating commentLines, reducing SLOC, and distorting the maintainability index. Comment continuation detection needs to account for block-comment state rather than treating every leading * as a comment.

Knowledge Base Used:

Fix in Claude Code

['bash', ['#']],
['lua', ['--']],
['zig', ['//']],
Expand Down
Loading
Loading