Skip to content

fix(resolver): kind-filter same-file bare-name lookup for receiver-bearing calls#2026

Open
carlos-alm wants to merge 1 commit into
fix/issue-1887-native-engine-skips-object-definepropertyfrom
fix/issue-1888-same-file-bare-name-lookup-resolvecalltargets
Open

fix(resolver): kind-filter same-file bare-name lookup for receiver-bearing calls#2026
carlos-alm wants to merge 1 commit into
fix/issue-1887-native-engine-skips-object-definepropertyfrom
fix/issue-1888-same-file-bare-name-lookup-resolvecalltargets

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

resolveCallTargets's same-file bare-name lookup (lookup.byNameAndFile(call.name, relPath) in call-resolver.ts) and resolveByGlobal's exact-name fallback (resolveExactGlobalMatch in resolver/strategy.ts) both ran unconditionally for every call, unfiltered by symbol kind. A call with a receiver — this.x(), self.x(), super.x(), or obj.x() — is logically "invoke a member of some instance", so a same-file class/interface/struct/variable that merely shared the call's bare name could win outright, before any more specific resolution tier (receiver typing, the Object.defineProperty accessor fallback, etc.) ever got a chance to run.

Confirmed identical on both engines before the fix (WASM and native Rust) using the issue's own repro:

class Registry {
  bar() { return 1; }
}
const obj = new Registry();
function getter() { this.bar(); }
Object.defineProperty(obj, 'x', { get: getter });
class bar {}

Before: getter -> bar (kind: class) — the coincidentally-named class pre-empted the correctly-typed Registry.bar method. After: getter -> Registry.bar (kind: method), and bar no longer receives a spurious calls edge.

Fix

Both lookups now restrict receiver-bearing calls to a new shared CALLABLE_SYMBOL_KINDS constant (function/method) in src/shared/kinds.ts — consolidating what was already duplicated inline at several other call sites in this exact resolver for the identical purpose (resolveDefinePropertyAccessorTarget's tier 2, resolveKotlinReflectionPreQualified, resolveReflectionKeyExprFallback). A genuinely bare call (no receiver at all) is left unfiltered: at this resolution layer it's indistinguishable from a new ClassName() constructor invocation, which legitimately targets a class-kind definition — kind-filtering it would break constructor-call resolution across every language (verified empirically; an earlier draft that filtered unconditionally broke the obj -> Registry constructor-call edge in the repro above).

Native engine: the identical bug existed in the mirrored Rust resolve_call_targets / resolve_exact_global_match (crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs) — fixed the same way, reusing the existing is_callable_kind helper already in that file. Verified both engines now produce byte-identical calls edges for the repro.

What's out of scope (filed separately)

A broader fix — trying resolveByReceiver's type-aware cascade before the bare lookup whenever a concrete object receiver is present (obj.method(), not this/self/super) — was implemented and then reverted: it regressed tests/integration/issue-1517-computed-prop-resolution.test.ts. That fixture's computed-key object-literal methods ({ ['formatDate'](date) {} }) get two node representations for the same physical method (a bare-name node from #1517's own fix, and a qualified-name node from the general object-literal-function extraction pass); reordering caused the composite-pts-key fallback to find the less-precise qualified node instead of the correct bare one.

This means a concrete-receiver call can still, in principle, match an unrelated same-file function/method (not class/interface/etc. — those are now excluded) before resolveByReceiver gets a chance to type-check the receiver. Filed #2025 to track this narrower residual, with the #1517 regression details and a suggested fix direction (likely deduplicating the two node representations rather than reordering resolution).

Using Refs #1888 rather than Closes since the issue named both "unfiltered by kind" (fixed here, for all receiver shapes) and "runs before type-aware dispatch" (fixed for this/self/super; the concrete-receiver sub-case is #2025) — mirrors the precedent in PR #1943.

Test plan

  • New tests/integration/issue-1888-same-file-bare-name-kind-filter.test.ts: the issue's exact repro, run on both engines, asserting the correct Registry.bar edge, the absence of the false bar (class) edge, and that the new Registry() constructor-call edge still resolves.
  • New unit tests in tests/unit/call-resolver.test.ts covering resolveCallTargets and resolveByMethodOrGlobal/resolveByGlobal's kind filter directly, plus the bare-call (no receiver) constructor-compatibility case.
  • npm test — full suite green (245 files, 3927 passed, 0 failed) with both WASM and a locally-built+codesigned native addon exercised; one unrelated, already-documented flaky test (Flaky cycle-node ordering in checkNoNewCycles full-suite run (tests/integration/check.test.ts) #1990, cycle-node ordering) confirmed flaky in isolation, not a regression.
  • tests/benchmarks/resolution/resolution-benchmark.test.ts — all 206 tests pass across all 34 languages, no precision/recall regression.
  • npm run lint — clean (one pre-existing, unrelated warning in an untouched fixture file).
  • codegraph diff-impact --staged -T — impact contained to the three changed functions (resolveCallTargets, resolveExactGlobalMatch, and the incidental CALLABLE_KINDSCALLABLE_SYMBOL_KINDS rename in findEnclosingCallable), 21 transitive callers across 4 files, all expected.

Stacked on #1887 (base branch fix/issue-1887-native-engine-skips-object-defineproperty) — only the resolver/kinds/test diff is this issue's change.

…aring calls

resolveCallTargets's same-file bare-name lookup (call-resolver.ts) and
resolveByGlobal's exact-name fallback (resolver/strategy.ts) ran
unconditionally for every call, unfiltered by symbol kind. A call with
a receiver (this.x(), obj.x()) is logically "invoke a member of some
instance" — a same-file class/interface/struct/variable that merely
shared the call's bare name could win outright, before any more
specific resolution tier (receiver typing, the Object.defineProperty
accessor fallback, etc.) ever got a chance to run.

Both lookups now restrict receiver-bearing calls to CALLABLE_SYMBOL_KINDS
(function/method), a new shared constant in shared/kinds.ts. A genuinely
bare call (no receiver at all) stays unfiltered, since at this layer it
is indistinguishable from a `new ClassName()` constructor invocation,
which legitimately targets a class-kind definition.

Mirrored the identical fix in the native Rust engine
(resolve_call_targets / resolve_exact_global_match in build_edges.rs),
which reproduced the same bug.

A broader fix (trying receiver-typed resolution before the bare lookup
for concrete object receivers) was attempted but reverted: it regressed
issue-1517's computed-property object-literal test, since that pattern
emits two node representations for the same method and the bare lookup
finds the more precise one. Filed #2025 to track that narrower residual.

Refs #1888
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a bug where receiver-bearing calls (this.x(), obj.x(), etc.) could incorrectly resolve to a same-named class/interface/struct declaration in the same-file bare-name lookup and the resolveExactGlobalMatch global-name fallback, because neither tier applied any kind filter. The fix adds a CALLABLE_SYMBOL_KINDS guard (function/method) to both tiers whenever a receiver is present, while leaving bare calls (no receiver) unfiltered to preserve constructor-call resolution.

  • src/shared/kinds.ts: Adds the shared CALLABLE_SYMBOL_KINDS constant, consolidating what was previously an inline-duplicated local set.
  • src/domain/graph/builder/call-resolver.ts and src/domain/graph/resolver/strategy.ts: Apply the kind filter at the same-file bare-name lookup (step 2) and at resolveExactGlobalMatch (inside resolveByGlobal), guarded on call.receiver being truthy.
  • crates/codegraph-core/…/build_edges.rs: Mirrors the identical fix for the native Rust engine, reusing the pre-existing is_callable_kind helper.

Confidence Score: 4/5

Safe to merge; the change is narrowly scoped to two fallback tiers of the resolution cascade, both engines are patched consistently, and the regression guard for constructor-call resolution is explicitly tested.

The logic is correct and the guard condition (truthy receiver → filter; no receiver → unfiltered) is consistently applied across both the TypeScript same-file lookup and the Rust mirror. All three key scenarios — false class match, correct method match, and bare constructor call — are covered by new unit and integration tests on both engines. No edge cases were found that would cause incorrect resolution under normal graph inputs.

No files require special attention. The Rust and TypeScript changes are symmetrical and independently verified by the integration test running on both engines.

Important Files Changed

Filename Overview
src/shared/kinds.ts Adds CALLABLE_SYMBOL_KINDS = Set(['function', 'method']), typed as ReadonlySet (consistent with TYPE_ERASED_SYMBOL_KINDS); consolidates the previously inline CALLABLE_KINDS constant from call-resolver.ts.
src/domain/graph/builder/call-resolver.ts Same-file bare-name lookup (step 2 in resolveCallTargets) now filters by CALLABLE_SYMBOL_KINDS when call.receiver is truthy; removes the now-redundant local CALLABLE_KINDS constant.
src/domain/graph/resolver/strategy.ts resolveExactGlobalMatch accepts a receiver field and filters global candidates by CALLABLE_SYMBOL_KINDS when call.receiver is truthy; resolveByGlobal correctly threads the receiver through.
crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs Native-engine mirror: resolve_call_targets step 2 and resolve_exact_global_match both now apply is_callable_kind when call.receiver is Some; reuses the pre-existing helper consistently.
tests/integration/issue-1888-same-file-bare-name-kind-filter.test.ts New integration test covering the exact issue repro on both WASM and native engines; asserts correct Registry.bar edge, absence of the false class-bar edge, and that the constructor-call edge still resolves.
tests/unit/call-resolver.test.ts New unit tests cover the kind filter in resolveCallTargets and resolveByMethodOrGlobal/resolveByGlobal for this/obj receivers and the bare-call constructor case.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[resolveCallTargets] --> B{call.receiver\npresent?}
    B -->|Yes| C[Same-file lookup\nfiltered by CALLABLE_SYMBOL_KINDS\nfunction / method only]
    B -->|No| D[Same-file lookup\nunfiltered\nallows class for constructor calls]
    C --> E{matches\nfound?}
    D --> E
    E -->|Yes| F[Return targets]
    E -->|No| G[resolveByMethodOrGlobal]
    G --> H{receiver is\nconcrete object?}
    H -->|Yes| I[resolveByReceiver\ntype-aware cascade]
    H -->|No - bare / this / self / super| J[resolveByGlobal]
    J --> K[resolveViaAccessorThisDispatch]
    K --> L{found?}
    L -->|Yes| F
    L -->|No| M[resolveExactGlobalMatch\nfiltered by CALLABLE_SYMBOL_KINDS\nwhen receiver present]
    M --> N{unique best\ncandidate?}
    N -->|Yes| F
    N -->|No / tie| O[resolveViaSameClassSibling]
    O --> F

    style C fill:#d4edda,stroke:#28a745
    style M fill:#d4edda,stroke:#28a745
Loading
%%{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] --> B{call.receiver\npresent?}
    B -->|Yes| C[Same-file lookup\nfiltered by CALLABLE_SYMBOL_KINDS\nfunction / method only]
    B -->|No| D[Same-file lookup\nunfiltered\nallows class for constructor calls]
    C --> E{matches\nfound?}
    D --> E
    E -->|Yes| F[Return targets]
    E -->|No| G[resolveByMethodOrGlobal]
    G --> H{receiver is\nconcrete object?}
    H -->|Yes| I[resolveByReceiver\ntype-aware cascade]
    H -->|No - bare / this / self / super| J[resolveByGlobal]
    J --> K[resolveViaAccessorThisDispatch]
    K --> L{found?}
    L -->|Yes| F
    L -->|No| M[resolveExactGlobalMatch\nfiltered by CALLABLE_SYMBOL_KINDS\nwhen receiver present]
    M --> N{unique best\ncandidate?}
    N -->|Yes| F
    N -->|No / tie| O[resolveViaSameClassSibling]
    O --> F

    style C fill:#d4edda,stroke:#28a745
    style M fill:#d4edda,stroke:#28a745
Loading

Reviews (1): Last reviewed commit: "fix(resolver): kind-filter same-file bar..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

3 functions changed21 callers affected across 4 files

  • findEnclosingCallable in src/domain/graph/builder/call-resolver.ts:132 (11 transitive callers)
  • resolveCallTargets in src/domain/graph/builder/call-resolver.ts:263 (15 transitive callers)
  • resolveExactGlobalMatch in src/domain/graph/resolver/strategy.ts:372 (3 transitive callers)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant