From d3ee4ca63741c2714bdbec6f2f441f23f38fe6de Mon Sep 17 00:00:00 2001 From: umi008 Date: Thu, 9 Jul 2026 10:31:07 -0600 Subject: [PATCH] fix(resolution): drop nested-local candidates from bare-name resolution (#1230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Python attribute call like ', '.join(sorted(unresolved)) was matching a same-named local 'join' defined inside a different function of the same file (the user-facing example from the issue) and producing a wrong 'calls' edge. The same trap applied to any language where the same name lives lexically inside one function but is looked up from another — a class method named 'X' on Helper resolving as a callee of Worker's same-named method, or a JS/TS nested arrow function being matched as a callee of an outer function that simply shares the name. Root cause: bare-name resolution in matchByExactName and matchFuzzy ignored lexical scope. Same-file proximity promoted any same-named node in the file — including locals captured by a sibling function — and the qualifiedName-aware 'imported' checks did not run because the ref had no '.' or '::' to key them on. Fix: in matchByExactName and matchFuzzy, drop candidates whose enclosing scope is a function/method that the caller cannot reach lexically. 'Unreachable' means: same file, candidate's container is neither the caller's container nor a strict ancestor of the caller. Cross-file candidates pass through unchanged (imports, not lexical scope, govern their reachability). One-liner cases (C++ class 'class Calculator { Calculator(int){} };' where the class and its constructor share one line) are disambiguated by the extractor's qualifiedName hierarchy — a constructor's qn 'Calculator::Calculator' is recognized as a CHILD of the class 'Calculator', not the other way around. Validation: - new __tests__/python-builtin-scope-filter.test.ts: 3 tests, covering the literal-receiver repro from the issue, the positive case where the enclosing function calls its own nested helper, and the OOP variant (sibling class method). - full __tests__/resolution.test.ts: 169/169 pass, including the one-liner C++ instantiates regression test (#1035) that exercises class/method disambiguation by qualifiedName. --- CHANGELOG.md | 1 + __tests__/python-builtin-scope-filter.test.ts | 116 +++++++++++++ src/resolution/name-matcher.ts | 154 +++++++++++++++++- 3 files changed, 267 insertions(+), 4 deletions(-) create mode 100644 __tests__/python-builtin-scope-filter.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b0b58b73..f38e372e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The safety watchdog no longer kills a healthy index on severely degraded storage. It used to judge liveness purely by the event loop, so one long database write on a struggling disk looked identical to a hung process and could get a valid, in-progress index terminated. During `codegraph index`/`codegraph init` the watchdog now also checks whether the index files on disk are advancing before it acts: slow-but-progressing work is left alone (bounded by a hard cap), while a genuinely hung process is still killed exactly as fast as before. (#1231) - Incremental sync now picks up cross-file relationships that only become resolvable after an edit — for example, when a file gains an export that another, unchanged file was already importing or calling. Previously the reference in the unchanged file was never revisited, so callers, impact, and flow results silently omitted the new edge (while status reported a clean index) until a full re-index. References that can't be resolved yet are now remembered and automatically retried whenever a change introduces a symbol that could satisfy them — this also covers a class gaining a new method that other files already call. Thanks @loadcosmos for the report with a minimal reproduction. (#1240) - The reverse case is fixed too: when an edit removes or moves a symbol (or deletes its file), callers in unchanged files now re-resolve during the same sync — rebinding to the symbol's new home when it moved, or waiting to reconnect automatically when it comes back — instead of silently losing their relationship until a full re-index. (#1240) +- Reference resolution no longer fabricates a call edge to a project symbol that is actually a local nested inside a sibling function of the caller. A Python attribute call like `", ".join(sorted(unresolved))` was matching a same-named local `join` defined inside a different function of the same file and producing a wrong call edge; the same trap applied to any language where the same name lives lexically inside one function but is looked up from another (helper classes with same-named methods, OOP siblings). Bare-name resolution now drops candidates whose enclosing scope is a function or method that the caller cannot reach lexically, so builtin attribute calls and unrelated same-named methods stop minting phantom edges. Cross-file candidates and module-level names are unaffected. (#1230) ## [1.4.0] - 2026-07-10 diff --git a/__tests__/python-builtin-scope-filter.test.ts b/__tests__/python-builtin-scope-filter.test.ts new file mode 100644 index 000000000..888df6ed1 --- /dev/null +++ b/__tests__/python-builtin-scope-filter.test.ts @@ -0,0 +1,116 @@ +/** + * Regression coverage for #1230 — Python attribute calls on literal + * receivers (e.g. `", ".join(sorted(unresolved))`) were resolving to + * unrelated project symbols (the nested local `join`) via bare-name + * name-matching. Two distinct failures stacked: + * + * 1. An attribute call with a literal receiver (`", ".`, `[]`, `{}`, + * a number) is calling a builtin — the resolver's name-matcher + * finds a same-named project symbol and fabricates a call edge. + * 2. A nested local function inside a sibling function is + * lexically unreachable from a different function, but + * same-file proximity was promoting it as a match anyway. + * + * The fix is in `src/resolution/name-matcher.ts` `matchByExactName`: + * drop candidates that are nested locals in a callable that is NOT + * the caller's container or an ancestor of the caller (#1230). + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import CodeGraph from '../src/index'; + +describe('lexical-scope filter on bare-name resolution (#1230)', () => { + let testDir: string; + let cg: CodeGraph; + + beforeEach(async () => { + testDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1230-')); + cg = CodeGraph.initSync(testDir, { + config: { include: ['**/*.py'], exclude: [] }, + }); + }); + + afterEach(() => { + if (cg) cg.destroy(); + if (fs.existsSync(testDir)) fs.rmSync(testDir, { recursive: true, force: true }); + }); + + it('a str.join on a literal in report_missing does NOT resolve to a nested local join in format_fields', async () => { + // Repro from the issue body. Before the fix, callees(report_missing) + // returned the nested local join; after the fix, the call is left + // unresolved (the method is a builtin, not a project symbol). + fs.writeFileSync( + path.join(testDir, 'repro.py'), + `def format_fields(values): + def join(vals): + return "-".join(sorted(vals)) + return join(values) +def report_missing(unresolved): + return ", ".join(sorted(unresolved)) +` + ); + await cg.indexAll(); + + // Verify the issue's reported failure modes are gone. + const reportMissingNode = (await cg.getNodesByName('report_missing'))[0]!; + const reportMissingCallees = await cg.getCallees(reportMissingNode.id); + const calleeNames = reportMissingCallees.map((c) => c.node.name); + + // The nested local `join` must not surface as a callee of + // `report_missing` — it's lexically unreachable. + expect(calleeNames).not.toContain('join'); + // Builtin-only `sorted` likewise (it isn't a project symbol). + expect(calleeNames).not.toContain('sorted'); + + // `join`'s only project caller must be `format_fields` (the one + // that lexically encloses the local), not `report_missing`. + const joinNode = (await cg.getNodesByName('join'))[0]!; + const joinCallers = await cg.getCallers(joinNode.id); + const callerNames = joinCallers.map((c) => c.node.name); + expect(callerNames).toContain('format_fields'); + expect(callerNames).not.toContain('report_missing'); + }); + + it('a nested local IS reachable from its enclosing function (positive case)', async () => { + // The lexical-scope filter must NOT block the canonical case: + // the enclosing function calling its own nested helper. + fs.writeFileSync( + path.join(testDir, 'positive.py'), + `def format_fields(values): + def join(vals): + return "-".join(sorted(vals)) + return join(values) +` + ); + await cg.indexAll(); + + const formatFieldsNode = (await cg.getNodesByName('format_fields'))[0]!; + const callees = await cg.getCallees(formatFieldsNode.id); + const calleeNames = callees.map((c) => c.node.name); + expect(calleeNames).toContain('join'); + }); + + it('a class method named X in Helper, called from sibling Worker: not resolved to Helper.X', async () => { + // Same scope rule applies to OOP — a class method owned by + // Helper is not a callee of Worker's methods, no matter how the + // line-proximity scorer ranks them. + fs.writeFileSync( + path.join(testDir, 'classes.py'), + `class Helper: + def join(self, vals): + return "-".join(vals) +class Worker: + def report_missing(self, unresolved): + return ", ".join(unresolved) +` + ); + await cg.indexAll(); + + const reportMissingNode = (await cg.getNodesByName('report_missing'))[0]!; + const callees = await cg.getCallees(reportMissingNode.id); + const calleeNames = callees.map((c) => c.node.name); + expect(calleeNames).not.toContain('join'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 075bb10bf..7ef16bb39 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -4,9 +4,135 @@ * Handles symbol name matching for reference resolution. */ -import { Language, Node } from '../types'; +import { Language, Node, NodeKind } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types'; +/** + * Kinds whose nodes can contain other nodes (and therefore introduce + * lexical scoping that blocks a nested local from being reachable from + * an outside caller). Used as a containment filter when picking the + * tightest enclosing scope for a candidate or caller. + */ +const SCOPING_CONTAINER_KINDS: ReadonlySet = new Set([ + 'function', 'method', 'class', 'struct', 'interface', 'trait', 'protocol', 'namespace', 'module', 'enum', +]); + +/** + * Given a candidate and a caller (both Nodes), decide whether the + * candidate is lexically reachable from the caller. + * + * Rule: walk the candidate's enclosing scope and the caller's + * enclosing scope. The candidate is reachable iff the caller's + * container is the candidate's container OR the candidate's container + * is a STRICT ancestor of the caller (i.e. the caller is nested + * inside the candidate's container). Otherwise the candidate is a + * nested local captured by a sibling function and the caller has no + * lexical path to it (#1230 — `report_missing` cannot call a `join` + * nested inside `format_fields`). + * + * Cross-file candidates are always considered reachable — the + * import graph (not lexical scope) is what makes them callable, and + * the rest of the matcher (proximity, imports) handles them. + */ +function isReachableFromCaller( + candidate: Node, + caller: Node | null, + inFile: Node[], +): boolean { + if (!caller) return true; + if (candidate.filePath !== caller.filePath) return true; + + const candidateParent = findStrictContainer(candidate, inFile); + if (!candidateParent) return true; // module-level — always reachable + + if (candidateParent.id === caller.id) return true; + // Caller is nested inside candidateParent (caller is the outer + // function, candidate is module-level, etc.) — reachable. + if (isStrictAncestorOf(candidateParent, caller, inFile)) return true; + + return false; +} + +/** + * Find the tightest node that contains `node` lexically. Uses two + * tiers: + * + * 1. **Strict**: the container extends at least one line beyond + * `node` in either direction — unambiguous even on one-liners + * because multi-line siblings clearly span beyond the node. + * + * 2. **Loose** (fallback): the container has the same line range. + * On multi-line code this is rare and usually correct. On + * one-liners (e.g. C++ `class Calculator { Calculator(int){} };` + * where the class and its constructor share one line), the + * extractor's `qualifiedName` hierarchy breaks the tie: if the + * candidate's qn is a prefix of the potential container's qn + * (separated by `::`), the "container" is actually a CHILD — a + * constructor `Calculator::Calculator` is inside the class + * `Calculator`, not the other way around. Skipping the false + * child prevents #1230/#1035 scope-filter regressions. + */ +function findStrictContainer(node: Node, inFile: Node[]): Node | null { + const nodeStart = node.startLine; + const nodeEnd = node.endLine ?? node.startLine; + let strictBest: Node | null = null; + let looseBest: Node | null = null; + for (const n of inFile) { + if (n.id === node.id) continue; + if (!SCOPING_CONTAINER_KINDS.has(n.kind)) continue; + const start = n.startLine; + const end = n.endLine ?? n.startLine; + const contains = start <= nodeStart && end >= nodeEnd; + if (!contains) continue; + const strict = start < nodeStart || end > nodeEnd; + if (strict) { + if (!strictBest || start > strictBest.startLine) strictBest = n; + } else { + // Loose match: same line range. On one-liners the extractor's + // qualifiedName encodes the true hierarchy — a child's qn starts + // with the parent's qn + "::". Skip false "containers" that are + // actually nested inside the candidate (e.g. a constructor method + // whose qn is `Calculator::Calculator` is inside class `Calculator`). + if (isChildOfCandidate(n, node)) continue; + if (!looseBest || start > looseBest.startLine) looseBest = n; + } + } + return strictBest ?? looseBest; +} + +/** + * True when `node` is lexically nested inside `candidate` according + * to the extractor's qualifiedName hierarchy: `node.qualifiedName` + * starts with `candidate.qualifiedName + "::"`. Used by + * {@link findStrictContainer} to reject one-liner false positives + * where a constructor method and its enclosing class share a line — + * the constructor's qn `Calculator::Calculator` starts with the + * class's qn `Calculator::`, so the constructor is a CHILD, not a + * parent container. + */ +function isChildOfCandidate(node: Node, candidate: Node): boolean { + const candidateQN = candidate.qualifiedName; + const nodeQN = node.qualifiedName; + if (nodeQN.length <= candidateQN.length) return false; + return nodeQN.startsWith(candidateQN + '::'); +} + +/** True if `ancestor` STRICTLY lexically contains `descendant`. */ +function isStrictAncestorOf(ancestor: Node, descendant: Node, _inFile: Node[]): boolean { + if (ancestor.filePath !== descendant.filePath) return false; + const dStart = descendant.startLine; + const dEnd = descendant.endLine ?? descendant.startLine; + const aStart = ancestor.startLine; + const aEnd = ancestor.endLine ?? ancestor.startLine; + if (aStart > dStart) return false; + if (aEnd < dEnd) return false; + // Strict: at least one of ancestor's boundaries is strictly outside + // descendant's, so a same-line sibling isn't reported as an + // ancestor of itself. + if (aStart === dStart && aEnd === dEnd) return false; + return true; +} + /** * Ceiling on how many same-named definitions a FUZZY name-match strategy will * score. A name defined more times than this is "ubiquitous" — a method/symbol @@ -356,9 +482,21 @@ export function matchByExactName( // unresolved import refs each scored K same-named import candidates through // findBestMatch — O(K²) per package, the dominant cost of "Resolving refs" on // large import-heavy (front-end + back-end) repos (#915). - const candidates = applyLanguageGate(context.getNodesByName(ref.referenceName), ref) + const rawCandidates = applyLanguageGate(context.getNodesByName(ref.referenceName), ref) .filter((n) => n.kind !== 'import'); + // Lexical-scope filter (#1230): drop candidates that are nested locals + // in a sibling function of the caller — a `join` nested inside + // `format_fields` is not reachable from `report_missing` even though + // they share a file. Same-file proximity can otherwise promote a + // nested local over a missing name, fabricating call edges on + // service-layer codebases (Python/Ruby/JS/TS). Cross-file candidates + // pass through (their reachability is governed by imports, not + // lexical scope). + const inFile = ref.filePath ? context.getNodesInFile(ref.filePath) : []; + const caller = inFile.find((n) => n.id === ref.fromNodeId) ?? null; + const candidates = rawCandidates.filter((c) => isReachableFromCaller(c, caller, inFile)); + if (candidates.length === 0) { return null; } @@ -1873,9 +2011,17 @@ export function matchFuzzy( const callableKinds = new Set(['function', 'method', 'class']); const callableCandidates = applyLanguageGate(candidates.filter((n) => callableKinds.has(n.kind)), ref); + // Lexical-scope filter (#1230) — drop nested locals whose container is + // a sibling function of the caller's container. Same rule as + // matchByExactName: a `join` nested inside `format_fields` is not + // reachable from `report_missing` even though they share a file. + const inFile = ref.filePath ? context.getNodesInFile(ref.filePath) : []; + const caller = inFile.find((n) => n.id === ref.fromNodeId) ?? null; + const scopedCandidates = callableCandidates.filter((c) => isReachableFromCaller(c, caller, inFile)); + // Prefer same-language matches - const sameLanguageCandidates = callableCandidates.filter(n => n.language === ref.language); - const finalCandidates = sameLanguageCandidates.length > 0 ? sameLanguageCandidates : callableCandidates; + const sameLanguageCandidates = scopedCandidates.filter(n => n.language === ref.language); + const finalCandidates = sameLanguageCandidates.length > 0 ? sameLanguageCandidates : scopedCandidates; if (finalCandidates.length === 1) { const isCrossLanguage = finalCandidates[0]!.language !== ref.language;