fix(resolver): attribute new ClassName() calls to the constructor method#2028
Conversation
`new ClassName()` (and bare `ClassName()` in languages with no `new` keyword) always resolved only to the class declaration node, never to the class's own constructor method, even when one was explicitly declared. Every constructor in the codebase therefore reported zero dependents via `fn-impact`/`roles --role dead`, regardless of how many call sites actually constructed the class. resolveCallTargets (call-resolver.ts) and resolve_call_targets (build_edges.rs) now additionally resolve the class's own constructor method for any bare (no-receiver) call that lands on a class-kind target, and emit a second calls edge to it alongside the existing class-node edge (attachConstructorTargets / attach_constructor_targets in strategy.ts / build_edges.rs). The class-node edge is kept: buildChaContextFromDb's RTA fallback for incremental rebuilds reads instantiation evidence from calls edges targeting class-kind nodes. The constructor's local name is resolved per-language (JS/TS: "constructor", Python: "__init__", PHP: "__construct", Java/C#/Dart/ Groovy: same identifier as the class itself), covering every language whose extractor both tracks object-construction call sites and emits an explicit constructor definition. Updates the javascript/python/csharp/dynamic-typescript resolution benchmark fixtures, whose expected-edges.json ground truth predated this fix and only encoded the class-node edge. Impact: 4 functions changed, 18 affected
Greptile SummaryThis PR fixes a long-standing attribution gap where
Confidence Score: 4/5The change is well-contained, additive-only, and mirrors identically between the TS and Rust engines. The existing class-node edge is always preserved, so incremental-rebuild RTA evidence is unaffected. The core logic is correct and the Rust/TS symmetry is carefully maintained. The only gaps are missing integration test coverage for PHP, Dart, and Groovy constructors (all use the same extension-map mechanism as the tested languages), and a minor type-precision issue in call-resolver.ts that does not affect runtime behaviour. The benchmark fixture updates are all legitimate true positives verified against both engines. The integration test file could benefit from a PHP/Dart fixture to close the coverage gap; src/domain/graph/builder/call-resolver.ts has a minor type-precision nit around the kind property on targets. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["resolveCallTargets / resolve_call_targets"] --> B["resolve_call_targets_core\n(existing multi-tier resolution)"]
B --> C{call.receiver?}
C -- yes --> D[Return targets as-is]
C -- no --> E["attachConstructorTargets(targets, className)"]
E --> F{For each target}
F -- kind != class --> G[Skip]
F -- kind == class --> H["constructorLocalName(target.file, className)"]
H -- ext not in map --> I[No constructor edge]
H -- JS/TS --> J["className.constructor"]
H -- Python --> K["className.__init__"]
H -- PHP --> L["className.__construct"]
H -- Java/C#/Dart/Groovy --> M["className.className"]
J & K & L & M --> N["lookup.byNameAndFile(qualifiedName, target.file)\nfilter kind==method"]
N -- found, not duplicate --> O["Append constructor node to augmented list"]
N -- not found --> I
O --> P["Return original class-node edge + new constructor-method edge"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["resolveCallTargets / resolve_call_targets"] --> B["resolve_call_targets_core\n(existing multi-tier resolution)"]
B --> C{call.receiver?}
C -- yes --> D[Return targets as-is]
C -- no --> E["attachConstructorTargets(targets, className)"]
E --> F{For each target}
F -- kind != class --> G[Skip]
F -- kind == class --> H["constructorLocalName(target.file, className)"]
H -- ext not in map --> I[No constructor edge]
H -- JS/TS --> J["className.constructor"]
H -- Python --> K["className.__init__"]
H -- PHP --> L["className.__construct"]
H -- Java/C#/Dart/Groovy --> M["className.className"]
J & K & L & M --> N["lookup.byNameAndFile(qualifiedName, target.file)\nfilter kind==method"]
N -- found, not duplicate --> O["Append constructor node to augmented list"]
N -- not found --> I
O --> P["Return original class-node edge + new constructor-method edge"]
Reviews (1): Last reviewed commit: "fix(resolver): attribute new ClassName()..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
| }); | ||
|
|
||
| it('Java: new Corge() resolves only to Corge (class) — no explicit constructor to attribute to', () => { | ||
| expect(edges).toContainEqual({ | ||
| src: 'Wrapper.run', | ||
| srcKind: 'method', | ||
| tgt: 'Corge', | ||
| tgtKind: 'class', | ||
| }); | ||
| expect(edges.some((e) => e.src === 'Wrapper.run' && e.tgt === 'Corge.Corge')).toBe(false); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| runScenario('wasm'); | ||
| describe.skipIf(!isNativeAvailable())('native engine coverage', () => { | ||
| runScenario('native'); | ||
| }); |
There was a problem hiding this comment.
__construct not covered by any integration test
The extension map in both strategy.ts and build_edges.rs includes .php/.phtml → __construct, but neither the new integration test nor any existing benchmark fixture exercises this path. PHP's constructor convention (__construct) is a fixed keyword (like __init__), unlike the Java/C#/Dart/Groovy "same as class name" pattern, so a typo in the keyword or a mismatch between the TS and Rust arms would go undetected. Dart and Groovy are in the same position.
A minimal fixture analogous to the Python Baz/Qux scenario would validate all three extension families: keyword-fixed (PHP), class-name-identical (Java/C#/Dart/Groovy), and Python-style (__init__). Java is already tested; PHP/Dart/Groovy are not.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const entry = CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION.get(file.slice(dotIdx)); | ||
| if (!entry) return null; |
There was a problem hiding this comment.
!entry guard conflates undefined with empty string
CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION values are string | typeof CTOR_NAME_SAME_AS_CLASS. Map.get() returns undefined when the key is absent. The guard if (!entry) return null would also short-circuit on an empty string "", which cannot appear in the current map but could silently corrupt a future extension entry. Prefer an explicit undefined check to make the intent clear and guard against accidental falsy values.
| const entry = CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION.get(file.slice(dotIdx)); | |
| if (!entry) return null; | |
| const entry = CONSTRUCTOR_LOCAL_NAME_BY_EXTENSION.get(file.slice(dotIdx)); | |
| if (entry === undefined) return null; |
| let resolved = [...(targets ?? [])]; | ||
| // #1892: `new ClassName()` / bare `ClassName()` (keyword-less languages) | ||
| // always resolves as a bare (no-receiver) call — augment any class-kind | ||
| // match with the class's own constructor method, if it declares one. | ||
| if (!call.receiver) { | ||
| resolved = attachConstructorTargets(lookup, resolved, targetName); | ||
| } |
There was a problem hiding this comment.
targets declared without kind, but attachConstructorTargets relies on it at runtime
targets is typed as ReadonlyArray<{ id: number; file: string }> (no kind), so resolved inherits that narrower type. When attachConstructorTargets is called, TypeScript infers T = { id: number; file: string }, satisfying the generic constraint only because kind is optional in the bound. At runtime, the actual objects returned by lookup.byNameAndFile do carry kind, so the target.kind !== 'class' guard inside attachConstructorTargets works correctly. However, if downstream code ever tries to read kind from items of resolved after this call, TypeScript won't catch that access as safe. Widening the targets declaration to include kind?: string would close the gap.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codegraph Impact Analysis4 functions changed → 18 callers affected across 4 files
|
1 participant
Summary
new ClassName()(and bareClassName()in languages with nonewkeyword) always resolved only to the class declaration node, never to the class's own constructor method, even when the class declared one explicitly. Every constructor in the codebase therefore reported zero dependents viafn-impact/roles --role dead, no matter how many call sites actually constructed the class.Root cause
handleNewExpr(JS/TS) and its per-language equivalents emit a bare (no-receiver) call named after the class.resolveCallTargets/resolveByGlobalto the class declaration node (stored under the bare name), never to the constructor method (stored under a separate qualified name —ClassName.constructorfor JS/TS,ClassName.__init__for Python,ClassName.ClassNamefor Java/C#/Dart/Groovy, whose constructor identifier equals the class name).Fix
resolveCallTargets(src/domain/graph/builder/call-resolver.ts) andresolve_call_targets(crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs) now additionally resolve the class's own constructor method for any bare call that lands on a class-kind target, and emit a secondcallsedge to it alongside the existing class-node edge (attachConstructorTargets/attach_constructor_targetsinstrategy.ts/build_edges.rs).This is additive, not a replacement: the class-node edge is always kept, because
buildChaContextFromDb's RTA fallback (incremental rebuilds,builder/cha.ts) reads instantiation evidence fromcallsedges targeting class-kind nodes. A class with no explicit constructor still resolves only to the class node — there's nothing else to attribute the call to, and the fix doesn't fabricate an edge.The constructor's local name is resolved per-language via a small extension-keyed table (mirrored identically in both engines):
.js .mjs .cjs .jsx .ts .tsx .mts .ctsconstructor.py .pyi__init__.php .phtml__construct.java .cs .dart .groovy .gvyKotlin, Swift, and Scala are deliberately excluded — their extractors don't emit an explicit constructor definition at all, so there's nothing to attribute to. C++ is excluded because its extractor doesn't track object-construction call sites at all.
Test plan
tests/integration/issue-1892-constructor-call-attribution.test.ts) covering JS/TS, Python, and Java, on both native and WASM engines: verifies the constructor-method edge is added when a class declares one, and verifies no edge is fabricated when it doesn't.javascript/python/csharp/dynamic-typescriptresolution benchmark fixtures — theirexpected-edges.jsonground truth predated this fix and only encoded the class-node edge; the new constructor-method edges are legitimate true positives, not regressions.npm test— full suite green except a single pre-existing flake tracked separately (Flaky cycle-node ordering in checkNoNewCycles full-suite run (tests/integration/check.test.ts) #1990), unrelated to this change.npm run lint— clean on touched files.cargo test— 577 passed.cargo check --workspace/cargo clippy --workspaceclean on touched files (pre-existing warnings elsewhere untouched).codegraph diff-impact --staged -T— contained impact (4 functions changed, 18 transitive callers, all within the resolver).Out of scope
Found and filed separately while testing, not touched here:
findEnclosingBindingwidest-span tie-break collision from missingendLine).Fixes #1892