fix: prevent resolveThisDispatch from matching an unrelated same-named class in another file - #2291
Conversation
Sorry, something went wrong.
…d class in another file chaCtx.parents is keyed by bare class name, so a this/super hierarchy walk could accept a cross-file method match whenever the caller's own same-named ancestor had no explicit definition of the dispatched method (e.g. an implicit default constructor). Two independent files each defining their own unrelated class with the same name triggered false cross-file edges. Before accepting a cross-file match, check whether a class of the same name is also declared in the caller's own file; if so, keep walking instead of accepting the unrelated match. Genuine cross-file heritage (the class is not declared anywhere in the caller's file) is unaffected. docs check acknowledged: bug fix only, no language/feature/architecture surface change. Impact: 1 functions changed, 12 affected
Greptile SummaryThe PR disambiguates this and super hierarchy traversal when unrelated files declare classes with identical names.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix: disambiguate CHA parent-chain trave..." | Re-trigger Greptile |
Sorry, something went wrong.
| .byNameAndFile(current, callerFile) | ||
| .some((n) => RECEIVER_KINDS.has(n.kind ?? '')); | ||
| if (!sameNameInCallerFile) return found; |
There was a problem hiding this comment.
Ambiguous parent chain traversal
When duplicate intermediate class names have different parents, this guard rejects the unrelated intermediate method but then continues through the same bare-name-keyed parents entry, which can belong to the other file and produce a false call edge to an unrelated grandparent.
Knowledge Base Used: Graph Build Pipeline
Sorry, something went wrong.
| const sameNameInCallerFile = lookup | ||
| .byNameAndFile(current, callerFile) | ||
| .some((n) => RECEIVER_KINDS.has(n.kind ?? '')); |
There was a problem hiding this comment.
Re-exports masquerade as declarations
When the caller file re-exports a cross-file ancestor, byNameAndFile returns that receiver-kind export node and this guard treats it as a local declaration, causing the resolver to skip the legitimate inherited method and omit its CHA call edge.
Knowledge Base Used: Graph Build Pipeline
Sorry, something went wrong.
Codegraph Impact Analysis6 functions changed → 18 callers affected across 6 files
|
Sorry, something went wrong.
chaCtx.parents was keyed by bare class name only (first-write-wins),
so once resolveThisDispatch rejected a same-named cross-file collision
and continued the walk, the next hop could follow an unrelated file's
parent mapping for that same class name and misdirect into a wholly
different hierarchy (false or missing edges).
Add a file-scoped parents map (parentsByFile, keyed by
"${className}|${file}") alongside the existing bare-name map, and
prefer it whenever a class's home file is known: the initial super()
hop, and every hop taken after rejecting a cross-file collision.
Wired through all three ChaContext construction sites (in-memory
full-build, DB-driven incremental rebuild, native this/super post-pass).
docs check acknowledged: bug fix only, no language/feature/architecture
surface change.
Impact: 7 functions changed, 18 affected
|
Addressed both findings: Issue 1 (ambiguous parent chain traversal) — fixed. This was real: chaCtx.parents is first-write-wins bare-name-keyed, so once the collision guard rejects a cross-file match and the walk takes another hop, that hop could land on an unrelated file's parent mapping for the same class name. Fixed by adding a file-scoped parentsByFile map (${className}|${file}) and using it for the initial super() hop and for every hop taken after rejecting a collision, wired through all three ChaContext construction sites (in-memory full build, DB-driven incremental rebuild, native this/super post-pass). Added a dedicated regression test (does not misdirect the walk through a colliding class whose bare-name parent belongs to a different file) that constructs the exact scenario — two files with same-named Middle classes extending different parents — and asserts the caller-file-scoped parent wins. Issue 2 (re-exports masquerade as declarations) — not reproducible, traced through the pipeline. export { X } from './other' is captured by the extractor as an imports entry with reexport: true (src/extractors/javascript.ts, handleExportCapture/handleExportStmt), which becomes a reexports edge, not an Export pushed to exps. It never produces a nodes table row for X at the reexporting file. All three byNameAndFile implementations (build-edges.ts's in-memory nodesByNameAndFile, incremental.ts's findNodeInFile DB query, native-orchestrator.ts's DB query) select only from the nodes table, filtered to declaration kinds — so a re-exported class can't appear as a phantom "declaration" in the caller's own file. If you have a concrete syntax (not export { X } from '...') that does insert such a row, please point to it and I'll fix it; based on the extractor code I couldn't construct a reproduction. Full test suite (274 files / 4454 tests) + lint + typecheck all pass. |
Sorry, something went wrong.
Sorry, something went wrong.
Summary
resolveThisDispatch's this/super hierarchy walk resolved to the wrong file whenever two independent files each declared a class with the same name and the caller's own same-named ancestor had no explicit definition of the dispatched method (e.g. an implicit default constructor). chaCtx.parents is keyed by bare class name, so the walk had no way to tell "genuine cross-file heritage" apart from "unrelated class that merely shares a name."
Fix: before accepting a cross-file match with no same-file candidate, check whether a class of the same name is also declared in the caller's own file. If so, that same-file class is the caller's real ancestor at this step — it simply doesn't define the method — so keep walking instead of accepting the unrelated cross-file match. When the class genuinely isn't declared anywhere in the caller's file, the cross-file match is legitimate heritage and is returned as before.
This is TS-only logic (src/domain/graph/builder/cha.ts) shared by both engines — the native path runs this same JS CHA post-pass on top of native edges, so no Rust mirror change is needed.
Test plan
Closes #2062