From dd79d86505d77f967e5e28f9c57747ffdc4f1119 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 5 Aug 2026 03:18:06 -0600 Subject: [PATCH 1/2] fix: skip complexity/CFG for bodyless functions in the WASM engine (#2055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit storeComplexityResults/storeCfgResults (ast-analysis/apply-results.ts) merged the shared visitor's per-file walk results onto every function/ method Definition without checking whether it actually has a body. The visitor walks by node-TYPE membership in functionNodes, which for C#/ Java shares one node type (method_declaration) between a bodied class method and a bodyless interface/abstract signature — so it computes a trivial-but-real result for the bodyless one too, and this merge step blindly attached it. Native's csharp.rs already explicitly skips this case; the WASM "mirror the WASM extractor" comment in csharp.rs described intent, not actual behavior. Add the same `hasFuncBody` gate already used elsewhere in this codebase (classifyDefinitionForNativeBulk, initWasmParsersIfNeeded) to both merge functions — this is the shared code both ast-analysis/ engine.ts and domain/wasm-worker-entry.ts import, so it fixes every caller in one place. Also gate computeJsFallbackMetrics's independent per-definition loop in features/complexity.ts for defense-in-depth on that separate fallback path. Fixing the shared engine generically also affects Java, which the issue's own discovery context flagged as a related inconsistency: native's java.rs ALSO computed complexity/cfg unconditionally for interface methods (the same bug shape #2053/#2054 already fixed for Rust/PHP), so leaving it unfixed here would have introduced a *new* WASM/native divergence for Java where the fix made WASM correct but native still wrong. Fixed java.rs's handle_interface_decl to match csharp.rs's existing pattern. Verified native and WASM produce identical output on both the issue's C# repro and a Java interface+default-method repro (across single-file and multi-file/worker-pool builds). Added regression tests: 2 new cases in tests/unit/apply-results.test.ts (bodyless def not attached despite a matching visitor result) and 1 in java.rs (interface signature vs. default method). docs check acknowledged — bug fix restoring engine parity, no new language, feature, or architecture surface to document. Impact: 3 functions changed, 10 affected --- crates/codegraph-core/src/extractors/java.rs | 48 ++++++++++++++++++-- src/ast-analysis/apply-results.ts | 10 +++- src/features/complexity.ts | 14 +++++- tests/unit/apply-results.test.ts | 42 +++++++++++++++++ 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/crates/codegraph-core/src/extractors/java.rs b/crates/codegraph-core/src/extractors/java.rs index 27312f935..2167d8c2b 100644 --- a/crates/codegraph-core/src/extractors/java.rs +++ b/crates/codegraph-core/src/extractors/java.rs @@ -167,16 +167,30 @@ fn handle_interface_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) let Some(child) = body.child(i) else { continue }; if child.kind() != "method_declaration" { continue; } if let Some(meth_name) = child.child_by_field_name("name") { + // Interface method declarations have no body (unless it's a + // default/static method with one) — skip CFG and complexity + // for the bodyless case to mirror the WASM extractor and + // avoid producing meaningless metrics for body-less + // declarations. Mirrors csharp.rs's handle_interface_decl. + let is_bodyless = child.child_by_field_name("body").is_none(); symbols.definitions.push(Definition { name: format!("{}.{}", iface_name, node_text(&meth_name, source)), kind: "method".to_string(), line: start_line(&child), end_line: Some(end_line(&child)), decorators: None, - complexity: compute_all_metrics(&child, source, "java"), - cfg: build_function_cfg(&child, "java", source), + complexity: if is_bodyless { + None + } else { + compute_all_metrics(&child, source, "java") + }, + cfg: if is_bodyless { + None + } else { + build_function_cfg(&child, "java", source) + }, children: None, - bodyless: Some(child.child_by_field_name("body").is_none()), + bodyless: Some(is_bodyless), content_hash: None, accessor_kind: None, }); @@ -580,6 +594,34 @@ mod tests { assert_ne!(concrete_save.bodyless, Some(true)); } + /// Regression test for #2055: an interface method with no body must not + /// get a fabricated complexity/CFG entry — mirrors csharp.rs's + /// handle_interface_decl, which already skips these. A default method + /// (has a body) must still get real metrics. + #[test] + fn interface_method_has_no_complexity_default_method_does() { + let s = parse_java( + "interface Repo {\n\ + boolean save(String id, int value);\n\ + default boolean saveOrDefault(String id, int value) {\n\ + if (value < 0) { return false; }\n\ + return true;\n\ + }\n\ + }\n", + ); + let save = s.definitions.iter().find(|d| d.name == "Repo.save").unwrap(); + assert!(save.complexity.is_none()); + assert!(save.cfg.is_none()); + + let default_method = s + .definitions + .iter() + .find(|d| d.name == "Repo.saveOrDefault") + .unwrap(); + assert!(default_method.complexity.is_some()); + assert!(default_method.cfg.is_some()); + } + #[test] fn extracts_class_fields() { let s = parse_java("class User { String name; int age; }"); diff --git a/src/ast-analysis/apply-results.ts b/src/ast-analysis/apply-results.ts index 09985a420..19fab150e 100644 --- a/src/ast-analysis/apply-results.ts +++ b/src/ast-analysis/apply-results.ts @@ -95,7 +95,12 @@ export function storeComplexityResults( ): void { const byLine = indexByLine((results.complexity || []) as ComplexityFuncResult[]); for (const def of defs) { - if ((def.kind === 'function' || def.kind === 'method') && def.line && !def.complexity) { + if ( + (def.kind === 'function' || def.kind === 'method') && + def.line && + !def.complexity && + hasFuncBody(def) + ) { const funcResult = matchResultToDef(byLine.get(def.line), def.name); if (!funcResult) continue; const { metrics } = funcResult; @@ -134,7 +139,8 @@ export function storeCfgResults(results: WalkResults, defs: Definition[]): void if ( (def.kind === 'function' || def.kind === 'method') && def.line && - !def.cfg?.blocks?.length + !def.cfg?.blocks?.length && + hasFuncBody(def) ) { const cfgResult = matchResultToDef(byLine.get(def.line), def.name); if (!cfgResult) continue; diff --git a/src/features/complexity.ts b/src/features/complexity.ts index 973d9c908..ef64b674f 100644 --- a/src/features/complexity.ts +++ b/src/features/complexity.ts @@ -707,9 +707,21 @@ async function computeJsFallbackMetrics( if (def.complexity) { analyzed += upsertPrecomputedComplexity(db, upsert, def, relPath); - } else { + } else if (hasFuncBody(def)) { analyzed += upsertAstComplexity(db, upsert, def, relPath, tree, langId, rules); } + // Signature-only declarations (interface/abstract method stubs, marked + // `bodyless` by the extractor) correctly get no complexity row. + // storeComplexityResults (apply-results.ts) is the primary fix for + // #2055 and already prevents `def.complexity` from ever being set + // wrong for these; this gate is defensive-in-depth for this + // independent fallback path, which would otherwise call + // upsertAstComplexity's _findFunctionNode unconditionally — matching + // the same node type used for bodied methods (e.g. C#'s + // `method_declaration` covers both an interface signature and a real + // method body) — and fabricate a trivial-but-meaningless entry, the + // same way this file's own classifyDefinitionForNativeBulk / + // initWasmParsersIfNeeded already gate the native-bulk path. } } }); diff --git a/tests/unit/apply-results.test.ts b/tests/unit/apply-results.test.ts index 2d6f49ccf..e71265d7b 100644 --- a/tests/unit/apply-results.test.ts +++ b/tests/unit/apply-results.test.ts @@ -126,6 +126,31 @@ describe('storeComplexityResults', () => { expect(def.complexity?.cyclomatic).toBe(1); }); + + it('does not attach a result to a bodyless definition even when the visitor computed one for that line (#2055)', () => { + // The visitor walks by node-TYPE membership in functionNodes, which for + // C#/Java shares one node type (method_declaration) between a bodied + // class method and a bodyless interface/abstract signature — so the + // visitor produces a trivial-but-real result for the bodyless one too. + // Without the hasFuncBody gate, this would fabricate a meaningless + // complexity entry that native's csharp.rs/java.rs explicitly skip. + // endLine (10) > line (5) deliberately, so this exercises the `bodyless` + // exclusion itself, not just the endLine-heuristic half of hasFuncBody. + const def = fakeDef({ bodyless: true }); + const results: WalkResults = { + complexity: [ + { + funcNode: fakeFuncNode(4, 'foo'), + funcName: 'foo', + metrics: { cognitive: 0, cyclomatic: 1, maxNesting: 0 }, + }, + ], + }; + + storeComplexityResults(results, [def], 'csharp'); + + expect(def.complexity).toBeUndefined(); + }); }); describe('storeCfgResults', () => { @@ -177,6 +202,23 @@ describe('storeCfgResults', () => { expect(def.cfg).toBe(existingCfg); }); + + it('does not attach CFG blocks to a bodyless definition even when the visitor computed one for that line (#2055)', () => { + const def = fakeDef({ bodyless: true }); + const results: WalkResults = { + cfg: [ + { + funcNode: fakeFuncNode(4, 'foo'), + blocks: [{ id: 0, label: 'entry', startLine: 5, endLine: 5 }], + edges: [], + }, + ], + }; + + storeCfgResults(results, [def]); + + expect(def.cfg).toBeUndefined(); + }); }); describe('shared module is actually used by both call sites (drift guard, #1850)', () => { From d01121bf1a68da4a4f23bf2601e5dcf2a3bdf609 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 5 Aug 2026 03:49:30 -0600 Subject: [PATCH 2/2] fix: gate #2055's bodyless check on bodyless alone, not endLine>line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile caught a real regression in the previous commit: the fix used hasFuncBody(def), which also requires endLine > line. That heuristic is fine for the FILE-level "does this file need a WASM fallback pass at all" decision hasFuncBody was originally designed for, but wrong for deciding whether to attach an ALREADY-COMPUTED visitor result — it silently discarded real complexity/CFG data for any genuinely bodied function whose entire body fits on one line (a plain single-line method, a C# expression-bodied member, etc.), which is common. Switch storeComplexityResults/storeCfgResults (apply-results.ts) and the computeJsFallbackMetrics fallback (complexity.ts) to check `def.bodyless !== true` directly instead. That's the actual, reliable signal every extractor sets from the AST body field. Verified: a single-line method in a file that also has a multi-line method (the case that exposed the bug) now correctly gets its complexity/CFG attached. Confirmed the original C#/Java interface fixes are unaffected. Added 2 regression tests covering a genuinely bodied single-line definition. Filed #2285 for a related-but-distinct, pre-existing bug found while stress-testing this: a file where EVERY function is single-line never gets the visitor added at all (setupComplexityVisitorForFile has the same hasFuncBody misuse) — out of scope here, predates this PR. docs check acknowledged — bug fix, no new language/feature/architecture surface to document. Impact: 3 functions changed, 10 affected --- src/ast-analysis/apply-results.ts | 19 ++++++++++-- src/features/complexity.ts | 28 ++++++++++-------- tests/unit/apply-results.test.ts | 49 +++++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/src/ast-analysis/apply-results.ts b/src/ast-analysis/apply-results.ts index 19fab150e..d1fd89ede 100644 --- a/src/ast-analysis/apply-results.ts +++ b/src/ast-analysis/apply-results.ts @@ -87,7 +87,17 @@ export function matchResultToDef( ); } -/** Merge visitor-walk complexity results onto `defs`, matched by line + name. */ +/** + * Merge visitor-walk complexity results onto `defs`, matched by line + name. + * + * Deliberately checks `def.bodyless !== true` directly rather than calling + * `hasFuncBody()`: that helper also requires `endLine > line`, which is a + * fine coarse signal for "does this FILE contain anything worth a WASM + * fallback pass" (its other call sites), but wrongly excludes a genuinely + * bodied single-line function/method (`endLine === line`) from having an + * already-computed visitor result attached — losing real data rather than + * skipping a bodyless stub. + */ export function storeComplexityResults( results: WalkResults, defs: Definition[], @@ -99,7 +109,7 @@ export function storeComplexityResults( (def.kind === 'function' || def.kind === 'method') && def.line && !def.complexity && - hasFuncBody(def) + def.bodyless !== true ) { const funcResult = matchResultToDef(byLine.get(def.line), def.name); if (!funcResult) continue; @@ -132,6 +142,9 @@ export function storeComplexityResults( * metric for any function using `&&`/`||`/`??`/`?.` or containing a closure * (issue #1743) — CFG blocks/edges are stored here purely for CFG * queries/visualization (`codegraph cfg`), not as a complexity source. + * + * Uses `def.bodyless !== true` directly rather than `hasFuncBody()` for the + * same reason `storeComplexityResults` does, above. */ export function storeCfgResults(results: WalkResults, defs: Definition[]): void { const byLine = indexByLine((results.cfg || []) as CfgFuncResult[]); @@ -140,7 +153,7 @@ export function storeCfgResults(results: WalkResults, defs: Definition[]): void (def.kind === 'function' || def.kind === 'method') && def.line && !def.cfg?.blocks?.length && - hasFuncBody(def) + def.bodyless !== true ) { const cfgResult = matchResultToDef(byLine.get(def.line), def.name); if (!cfgResult) continue; diff --git a/src/features/complexity.ts b/src/features/complexity.ts index ef64b674f..8f1be3b83 100644 --- a/src/features/complexity.ts +++ b/src/features/complexity.ts @@ -707,21 +707,23 @@ async function computeJsFallbackMetrics( if (def.complexity) { analyzed += upsertPrecomputedComplexity(db, upsert, def, relPath); - } else if (hasFuncBody(def)) { + } else if (def.bodyless !== true) { + // Checks `bodyless` directly rather than `hasFuncBody(def)`: that + // helper also requires `endLine > line`, which would wrongly skip + // upsertAstComplexity for a genuinely bodied single-line function + // (endLine === line) — losing real data rather than skipping a + // bodyless stub. Signature-only declarations (interface/abstract + // method stubs, marked `bodyless` by the extractor) correctly get + // no complexity row: without this gate, upsertAstComplexity's + // _findFunctionNode would match the same node type used for + // bodied methods (e.g. C#'s `method_declaration` covers both an + // interface signature and a real method body) and fabricate a + // trivial-but-meaningless entry (#2055) — storeComplexityResults + // (apply-results.ts) is the primary fix and already prevents + // `def.complexity` from being set wrong for these; this gate is + // defensive-in-depth for this independent fallback path. analyzed += upsertAstComplexity(db, upsert, def, relPath, tree, langId, rules); } - // Signature-only declarations (interface/abstract method stubs, marked - // `bodyless` by the extractor) correctly get no complexity row. - // storeComplexityResults (apply-results.ts) is the primary fix for - // #2055 and already prevents `def.complexity` from ever being set - // wrong for these; this gate is defensive-in-depth for this - // independent fallback path, which would otherwise call - // upsertAstComplexity's _findFunctionNode unconditionally — matching - // the same node type used for bodied methods (e.g. C#'s - // `method_declaration` covers both an interface signature and a real - // method body) — and fabricate a trivial-but-meaningless entry, the - // same way this file's own classifyDefinitionForNativeBulk / - // initWasmParsersIfNeeded already gate the native-bulk path. } } }); diff --git a/tests/unit/apply-results.test.ts b/tests/unit/apply-results.test.ts index e71265d7b..34a161f89 100644 --- a/tests/unit/apply-results.test.ts +++ b/tests/unit/apply-results.test.ts @@ -132,10 +132,11 @@ describe('storeComplexityResults', () => { // C#/Java shares one node type (method_declaration) between a bodied // class method and a bodyless interface/abstract signature — so the // visitor produces a trivial-but-real result for the bodyless one too. - // Without the hasFuncBody gate, this would fabricate a meaningless + // Without checking `bodyless`, this would fabricate a meaningless // complexity entry that native's csharp.rs/java.rs explicitly skip. // endLine (10) > line (5) deliberately, so this exercises the `bodyless` - // exclusion itself, not just the endLine-heuristic half of hasFuncBody. + // exclusion itself, not the endLine-heuristic (see the test below for why + // that heuristic must NOT be used here). const def = fakeDef({ bodyless: true }); const results: WalkResults = { complexity: [ @@ -151,6 +152,31 @@ describe('storeComplexityResults', () => { expect(def.complexity).toBeUndefined(); }); + + it('attaches a result to a genuinely bodied single-line function (#2055 fix-of-a-fix)', () => { + // Regression guard: an earlier version of the #2055 fix gated this merge + // on `hasFuncBody(def)`, which ALSO requires `endLine > line` — wrongly + // discarding a real, already-computed visitor result for any function + // whose entire body fits on one line (`bool IsPositive(int x) { return + // x > 0; }`, a C# expression-bodied member, etc.), even though it is not + // bodyless at all. Caught by Greptile review before merge. The gate must + // check `bodyless` alone, never the line span. + const def = fakeDef({ bodyless: false, line: 5, endLine: 5 }); + const results: WalkResults = { + complexity: [ + { + funcNode: fakeFuncNode(4, 'foo'), + funcName: 'foo', + metrics: { cognitive: 0, cyclomatic: 1, maxNesting: 0 }, + }, + ], + }; + + storeComplexityResults(results, [def], 'csharp'); + + expect(def.complexity).toBeDefined(); + expect(def.complexity?.cyclomatic).toBe(1); + }); }); describe('storeCfgResults', () => { @@ -219,6 +245,25 @@ describe('storeCfgResults', () => { expect(def.cfg).toBeUndefined(); }); + + it('attaches CFG blocks to a genuinely bodied single-line function (#2055 fix-of-a-fix)', () => { + // Same regression as storeComplexityResults' equivalent test above: the + // gate must check `bodyless` alone, never `endLine > line`. + const def = fakeDef({ bodyless: false, line: 5, endLine: 5 }); + const results: WalkResults = { + cfg: [ + { + funcNode: fakeFuncNode(4, 'foo'), + blocks: [{ id: 0, label: 'entry', startLine: 5, endLine: 5 }], + edges: [], + }, + ], + }; + + storeCfgResults(results, [def]); + + expect(def.cfg?.blocks).toHaveLength(1); + }); }); describe('shared module is actually used by both call sites (drift guard, #1850)', () => {