fix(complexity): stop using dotted names as a proxy for signature-only stubs#2056
Conversation
…y stubs
hasFuncBody() (and its duplicated logic in features/complexity.ts and
features/cfg.ts) excluded any definition whose name contained a dot,
intended to filter out bodyless TS interface method signatures. Dotted
names are also the normal qualified name for real, bodied class/struct/
impl/module methods in every extractor (Lua's `M.foo` module-table
pattern, Go/Java/C#/PHP/Rust receiver or impl methods, any `Class.method`
in any language) — so when *every* function/method in a file happened to
have a dotted name, the file-level "does this file need WASM complexity"
gate (`defs.some(hasFuncBody)`) went false for the whole file, and none
of its functions got complexity or CFG data.
Replace the name-shape heuristic with `Definition.bodyless`, a direct
signal each extractor sets from the AST node's body field at the exact
sites that produce genuinely bodyless dotted definitions (TS/JS interface
method_signature, Go interface method_elem, Java/C#/PHP interface and
abstract methods, Rust trait function_signature_item). `hasFuncBody`,
`initWasmParsersIfNeeded`, `classifyDefinitionForNativeBulk`, and
`hasNativeCfgForFile` now check `!d.bodyless` instead of
`!d.name.includes('.')`; the latter two were also folded onto the shared
`hasFuncBody` to remove duplicated logic.
Mirrors the same fix into the native Rust engine's `Definition` struct
and every extractor that produces dotted names, so both engines agree on
which definitions are real (bodied) vs. signature-only. Native already
computed complexity unconditionally and was unaffected by the original
bug, but without `bodyless` its bridged Definitions would have looked
identical to WASM's pre-fix state to the JS-side gate, silently forcing
unnecessary WASM/JS fallback re-analysis on every native build touching
an interface, trait, or abstract method.
Fixes #1922
Impact: 14 functions changed, 54 affected
Codegraph Impact Analysis16 functions changed → 52 callers affected across 10 files
|
Greptile SummaryThis PR replaces a fragile name-shape heuristic (
Confidence Score: 4/5Safe to merge — the core complexity gate fix is correct and well-tested across Lua, Go, TypeScript, Java, and Rust fixtures; the only issue is that unit test object literals still carry a The fix is mechanically applied across all 47 files with consistent logic and backed by both unit and integration tests. After narrowing tests/unit/apply-results.test.ts — all calls to Important Files Changed
|
| it('is true for a dotted name with a real body (Class.method, module-table function, receiver method) — issue #1922', () => { | ||
| // A dotted name alone must never disqualify a real, bodied function: it's the normal | ||
| // qualified name for class/struct/impl methods (`Class.method`) and module-table | ||
| // functions (Lua's `M.foo`, Go/Java/C#/PHP/Rust receiver or impl methods) across every | ||
| // extractor. Regression guard for the bug where the file-level "does this file need | ||
| // complexity" gate (`defs.some(hasFuncBody)`) went false for an entire file when every | ||
| // function in it happened to have a dotted name. | ||
| expect(hasFuncBody({ name: 'Foo.bar', kind: 'method', line: 5, endLine: 10 })).toBe(true); | ||
| expect(hasFuncBody({ name: 'M.foo', kind: 'method', line: 5, endLine: 10 })).toBe(true); | ||
| }); | ||
|
|
||
| it('is false when the extractor marks the definition bodyless (interface/trait/abstract signature)', () => { | ||
| expect( | ||
| hasFuncBody({ name: 'Foo.bar', kind: 'method', line: 5, endLine: 10, bodyless: true }), | ||
| ).toBe(false); | ||
| // Even a non-dotted signature-only declaration is excluded via `bodyless`. | ||
| expect( | ||
| hasFuncBody({ name: 'bar', kind: 'function', line: 5, endLine: 10, bodyless: true }), | ||
| ).toBe(false); | ||
| }); |
There was a problem hiding this comment.
All test calls to
hasFuncBody still pass a name property even though the function's parameter type was narrowed to remove name. TypeScript performs excess property checking on fresh object literals passed as arguments, so every call site here — including pre-existing ones at lines 42–52 and the newly added lines — would be flagged as Object literal may only specify known properties, and 'name' does not exist in type '{ kind: string; line: number; endLine?: number | null | undefined; bodyless?: boolean | undefined; }'. The tests are harmless at runtime (JavaScript ignores extra properties), but if test files are ever included in a strict tsc --noEmit pass the build will break. Remove name from the object literals to match the updated function signature.
| it('is true for a dotted name with a real body (Class.method, module-table function, receiver method) — issue #1922', () => { | |
| // A dotted name alone must never disqualify a real, bodied function: it's the normal | |
| // qualified name for class/struct/impl methods (`Class.method`) and module-table | |
| // functions (Lua's `M.foo`, Go/Java/C#/PHP/Rust receiver or impl methods) across every | |
| // extractor. Regression guard for the bug where the file-level "does this file need | |
| // complexity" gate (`defs.some(hasFuncBody)`) went false for an entire file when every | |
| // function in it happened to have a dotted name. | |
| expect(hasFuncBody({ name: 'Foo.bar', kind: 'method', line: 5, endLine: 10 })).toBe(true); | |
| expect(hasFuncBody({ name: 'M.foo', kind: 'method', line: 5, endLine: 10 })).toBe(true); | |
| }); | |
| it('is false when the extractor marks the definition bodyless (interface/trait/abstract signature)', () => { | |
| expect( | |
| hasFuncBody({ name: 'Foo.bar', kind: 'method', line: 5, endLine: 10, bodyless: true }), | |
| ).toBe(false); | |
| // Even a non-dotted signature-only declaration is excluded via `bodyless`. | |
| expect( | |
| hasFuncBody({ name: 'bar', kind: 'function', line: 5, endLine: 10, bodyless: true }), | |
| ).toBe(false); | |
| }); | |
| it('is true for a dotted name with a real body (Class.method, module-table function, receiver method) — issue #1922', () => { | |
| // A dotted name alone must never disqualify a real, bodied function: it's the normal | |
| // qualified name for class/struct/impl methods (`Class.method`) and module-table | |
| // functions (Lua's `M.foo`, Go/Java/C#/PHP/Rust receiver or impl methods) across every | |
| // extractor. Regression guard for the bug where the file-level "does this file need | |
| // complexity" gate (`defs.some(hasFuncBody)`) went false for an entire file when every | |
| // function in it happened to have a dotted name. | |
| expect(hasFuncBody({ kind: 'method', line: 5, endLine: 10 })).toBe(true); | |
| expect(hasFuncBody({ kind: 'method', line: 5, endLine: 10 })).toBe(true); | |
| }); | |
| it('is false when the extractor marks the definition bodyless (interface/trait/abstract signature)', () => { | |
| expect( | |
| hasFuncBody({ kind: 'method', line: 5, endLine: 10, bodyless: true }), | |
| ).toBe(false); | |
| // Even a non-dotted signature-only declaration is excluded via `bodyless`. | |
| expect( | |
| hasFuncBody({ kind: 'function', line: 5, endLine: 10, bodyless: true }), | |
| ).toBe(false); | |
| }); |
Summary
hasFuncBody()insrc/ast-analysis/apply-results.ts(plus duplicated copies of the same check infeatures/complexity.tsandfeatures/cfg.ts) excluded any definition whosenamecontained a dot, intended to filter out bodyless TS interface method signatures (InterfaceName.methodName).Dotted names are also the normal qualified name for every real, bodied method across the entire codebase's extractors: Lua's
M.foomodule-table pattern, Go/Java/C#/PHP/Rust receiver or impl methods, and plainClass.methodin any OOP language. The dot check was a proxy that happened to be true for the one case that motivated it, but false for everything else that uses.as a namespacing convention.Since the file-level "does this file need WASM complexity" gate is
defs.some(hasFuncBody), a file where every function/method definition happens to have a dotted name (a Lua module file, a Go file with only receiver methods, a Python/Java/etc. file with a single class and no free functions) made the gate false for the entire file — none of its functions got complexity or CFG data via WASM.Fix
Replaced the name-shape heuristic with
Definition.bodyless, a direct signal each extractor sets from the AST node's actual body field, at the specific sites verified (via each language'snode-types.json) to produce genuinely bodyless dotted definitions:method_signature(interface stub) —extractInterfaceMethodsmethod_elem(interface stub, nobodyfield at all) —handleGoInterfaceType; also defensively marks external/asm-linked receiver methods (method_declarationwith an optionalbodyfield) inhandleGoMethodDeclextractJavaInterfaceMethods) and abstract class methods (handleJavaMethodDecl, both usemethod_declaration, body optional)handleCsInterfaceDecl) and abstract class methods (handleCsMethodDecl)handlePhpInterfaceDecl) and abstract class methods (handlePhpMethodDecl)function_signature_item(required, no body field) vs.function_item(default impl, body required) —extractTraitMethodsPython, Ruby, and Lua don't need this: their grammars structurally require a body on every function/method definition (even
pass/.../end), so there's no bodyless-but-dotted case to guard against.hasFuncBody,initWasmParsersIfNeeded, andclassifyDefinitionForNativeBulk(features/complexity.ts) andhasNativeCfgForFile(features/cfg.ts) now check!d.bodyless. The latter two previously duplicated the dot-check logic inline — folded onto the sharedhasFuncBodyinstead, removing the duplication.Native (Rust) mirror: native already computed complexity unconditionally at extraction time (ignoring dots entirely), so the original bug never affected it directly. But
allNativeDataComplete/detectNativeNeedsinast-analysis/engine.tsevaluatehasFuncBodyagainst native-bridgedDefinitionobjects too (fromparseFilesFull) — without a native-sidebodylesssignal, those interface/trait/abstract stub definitions would look identical to WASM's pre-fix "possibly-needs-complexity" defs to the JS-side gate, silently forcing an unnecessary WASM/JS fallback re-parse on every native build touching an interface, trait, or abstract method (the exact perf regression #606 originally fixed, reintroduced through the native path). Added the samebodylessfield + extraction-site logic to the nativeDefinitionstruct and every extractor listed above.Testing
tests/unit/apply-results.test.ts— updatedhasFuncBodyunit tests (dotted-with-body is nowtrue; explicitbodyless: trueisfalse)tests/integration/issue-1922-wasm-complexity-dotted-names.test.ts— new integration test building a Lua module-table file, a Go receiver-only file, and a mixed TS file (real dotted class method + genuinely bodyless multi-line interface signature) via the WASM engine, asserting complexity is correctly computed/excludedgo.rs,javascript.rs,rust_lang.rs,java.rs) assertingbodylesson representative interface/trait/abstract/receiver-method fixturesfunctions/cyclomaticoutput) for Lua, Go, Python, TypeScript, and Java fixtures reproducing the reported bugnpm test: 255 files, 4027 passed (0 failed — onetests/integration/check.test.tsflake unrelated to this change and already tracked as Flaky cycle-node ordering in checkNoNewCycles full-suite run (tests/integration/check.test.ts) #1990)npm run lint: clean (pre-existing unrelated warning intests/fixtures/issue-1833-plain-type-import/consumer.ts)cargo test(codegraph-core): 607 passed, 0 failednpm run build/cargo check: cleanOut-of-scope findings filed separately
While auditing every native extractor site that mirrors a WASM interface/trait stub extractor (to verify
bodylesscorrectness and avoid regressing the original interface-stub-filtering behavior), found and filed three independent, pre-existing bugs — not fixed here to keep this PR scoped to the file-level complexity gate:function_signature_item); WASM correctly omits themhandle_method_decl)Stacked on #1920 (base branch
fix/issue-1920-wasm-dynamic-import-destructure-extraction).Fixes #1922