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..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[], @@ -95,7 +105,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 && + def.bodyless !== true + ) { const funcResult = matchResultToDef(byLine.get(def.line), def.name); if (!funcResult) continue; const { metrics } = funcResult; @@ -127,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[]); @@ -134,7 +152,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 && + 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 973d9c908..8f1be3b83 100644 --- a/src/features/complexity.ts +++ b/src/features/complexity.ts @@ -707,7 +707,21 @@ async function computeJsFallbackMetrics( if (def.complexity) { analyzed += upsertPrecomputedComplexity(db, upsert, def, relPath); - } else { + } 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); } } diff --git a/tests/unit/apply-results.test.ts b/tests/unit/apply-results.test.ts index 2d6f49ccf..34a161f89 100644 --- a/tests/unit/apply-results.test.ts +++ b/tests/unit/apply-results.test.ts @@ -126,6 +126,57 @@ 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 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 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: [ + { + funcNode: fakeFuncNode(4, 'foo'), + funcName: 'foo', + metrics: { cognitive: 0, cyclomatic: 1, maxNesting: 0 }, + }, + ], + }; + + storeComplexityResults(results, [def], 'csharp'); + + 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', () => { @@ -177,6 +228,42 @@ 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(); + }); + + 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)', () => {