fix: resolve renamed-import receiver calls in qualified-method lookup#1971
Open
carlos-alm wants to merge 1 commit into
Conversation
`resolveByReceiver`'s qualified-name lookups (typed-method, prototype-alias,
and direct-qualified-method) built the symbol-table key from the call
receiver's local text or its typeMap-resolved type name, neither of which
accounts for a renamed import binding (`import { X as Y } from './mod'; Y.method()`).
The symbol table stores definitions under their declared name (X), not the
importing file's local alias (Y), so the lookup silently missed and no
`calls` edge was created.
Thread `importedOriginalNames` (already computed for direct call-name
resolution, #1730) through `resolveByMethodOrGlobal` into `resolveByReceiver`,
and de-alias the qualifier immediately before building each qualified lookup
key. Mirrored in the native Rust resolver (`resolve_call_targets`).
Contributor
Greptile SummaryThis PR fixes receiver-based calls through renamed imports. The main changes are:
Confidence Score: 5/5This looks safe to merge after a small test cleanup hardening.
tests/integration/issue-1825-renamed-import-receiver.test.ts Important Files Changed
Reviews (1): Last reviewed commit: "fix: resolve renamed-import receiver cal..." | Re-trigger Greptile |
Comment on lines
+77
to
+80
| afterAll(() => { | ||
| fs.rmSync(tmpWasm, { recursive: true, force: true }); | ||
| fs.rmSync(tmpNative, { recursive: true, force: true }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Cleanup Uses Uninitialized Paths
If setup throws before both temp directory variables are assigned, this cleanup calls fs.rmSync with undefined. The original setup error is then followed by a teardown TypeError, and any directory created before the failure can be left behind.
Suggested change
| afterAll(() => { | |
| fs.rmSync(tmpWasm, { recursive: true, force: true }); | |
| fs.rmSync(tmpNative, { recursive: true, force: true }); | |
| }); | |
| afterAll(() => { | |
| if (tmpWasm) fs.rmSync(tmpWasm, { recursive: true, force: true }); | |
| if (tmpNative) fs.rmSync(tmpNative, { recursive: true, force: true }); | |
| }); |
Contributor
Codegraph Impact Analysis3 functions changed → 15 callers affected across 3 files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
resolveByReceiver's qualified-name resolution steps (typed-method lookup, prototype-alias lookup, and direct-qualified-method lookup) built the symbol-table lookup key from either the call receiver's local text or its typeMap-resolved type name. Neither accounted for a renamed import binding (import { X as Y } from './mod'; Y.method()) — the symbol table stores definitions under their declared name (X), not the importing file's local alias (Y), so the qualified lookup (Y.method) missed and the call was silently dropped with nocallsedge.importedOriginalNames(the local-alias → original-name map already built for direct call-name resolution, codegraph exports does not credit consumers that import via a rename (import { X as Y }) #1730) throughresolveByMethodOrGlobalintoresolveByReceiver, and de-aliasing the qualifier immediately before each qualified-name is built:NsAlias.doThing()whereNsAliasis a renamed import of an object literalNamespaceObj— now resolves viaNamespaceObj.doThing.new Bar()whereBaris a renamed import of classFoo— the typeMap records the receiver's type as the local alias text (Bar), soresolveViaTypedMethod/resolveViaPrototypeAliashad the identical bug; now resolves viaFoo.method.resolve_call_targetsincrates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs) — both engines now produce identical edges.Test plan
tests/integration/issue-1825-renamed-import-receiver.test.ts— verifies both the direct-qualified and typed-method paths resolve correctly on both WASM and native engines, and that no spurious edge is created against the local alias name.npm test— full suite green (228 test files, 3761 passed, 0 failed)npm run lint— cleancargo test --lib— 529 passedFixes #1825