فهرست منبع

fix(resolution): reject Python module collection method guesses (#1652) (#1776)

Use same-file module variable initializers to filter builtin collection calls before class-name heuristics, and require a Python type node for the class escape. Preserve imported project module calls.

Add callers/callees regressions for dict, list, set, tuple, and frozenset, with a real instance control and same-name bindings across files.

Validation on Linux with Node 22.19.0: the new suite had 14 failures and one passing control on main at 8733c288; all 216 tests in the new suite, call-receiver-no-fabrication, and resolution now pass. TypeScript and copy-assets pass. Re-indexed /tmp/cg-1652-repro: callers get is empty and read_setting no longer calls a cache.py method.

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 5 ساعت پیش
والد
کامیت
195888d71f
3فایلهای تغییر یافته به همراه124 افزوده شده و 4 حذف شده
  1. 1 0
      CHANGELOG.md
  2. 106 0
      __tests__/python-module-scope-collection-methods.test.ts
  3. 17 4
      src/resolution/index.ts

+ 1 - 0
CHANGELOG.md

@@ -220,6 +220,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - **A C macro call written with designated initializers no longer swallows every function after it.** Betaflight resets each config struct with `RESET_CONFIG(type, dst, .field = value, …)`, a shape the C grammar cannot parse; past a hundred or so fields its error recovery ran the enclosing function to the end of the file, the next function vanished from the index and every later one was filed under the first, where name matching then treated it as an unreachable closure. The argument list of such a call is now blanked before parsing, offsets kept, so the file's functions come out with their real extents. On that tree 45 functions in `pid.c` alone moved back to top level and their 117 callers resolve at exact-match confidence. Re-index after upgrading. (#1729)
 - **A C macro call written with designated initializers no longer swallows every function after it.** Betaflight resets each config struct with `RESET_CONFIG(type, dst, .field = value, …)`, a shape the C grammar cannot parse; past a hundred or so fields its error recovery ran the enclosing function to the end of the file, the next function vanished from the index and every later one was filed under the first, where name matching then treated it as an unreachable closure. The argument list of such a call is now blanked before parsing, offsets kept, so the file's functions come out with their real extents. On that tree 45 functions in `pid.c` alone moved back to top level and their 117 callers resolve at exact-match confidence. Re-index after upgrading. (#1729)
 - A method called on the result of another call — `d.setdefault(k, []).append(v)`, `make().run()` — no longer produces a call edge to an unrelated top-level function that merely shares the name, in Python and JavaScript/TypeScript. The receiver is kept so the inner call still resolves; the outer method stays unresolved rather than guessed. Re-index after upgrading. (#1683, #1681)
 - A method called on the result of another call — `d.setdefault(k, []).append(v)`, `make().run()` — no longer produces a call edge to an unrelated top-level function that merely shares the name, in Python and JavaScript/TypeScript. The receiver is kept so the inner call still resolves; the outer method stays unresolved rather than guessed. Re-index after upgrading. (#1683, #1681)
 - A Python call through an imported project module whose name collides with a builtin collection method — `ledger.append(row)` after `from . import ledger` — is no longer dropped as `list.append`. The builtin-method filter now lets the receiver through when it is an imported module that resolves to a file in the project, so `resolveViaImport` can attach the real edge; a stdlib/PyPI receiver (`os.remove`) still produces none. Re-index after upgrading. (#1681, via #1704)
 - A Python call through an imported project module whose name collides with a builtin collection method — `ledger.append(row)` after `from . import ledger` — is no longer dropped as `list.append`. The builtin-method filter now lets the receiver through when it is an imported module that resolves to a file in the project, so `resolveViaImport` can attach the real edge; a stdlib/PyPI receiver (`os.remove`) still produces none. Re-index after upgrading. (#1681, via #1704)
+- Python method calls on module-scope builtin collections no longer create false calls or file dependencies to unrelated project methods with the same name; re-index after upgrading. (#1652)
 - **A definition its language makes file-local no longer captures calls from other files.** A C `static` in another source file (`.c`/`.cc`/… — not a header's `static inline`, which is textually included), a Kotlin/Java/C#/Swift/Scala/Dart/PHP `private` member, a Go unexported name in another package, and a Rust non-`pub` item outside its module subtree cannot be what a name in another file means, but name matching accepted them whenever the names agreed: an Android `editor.apply()` onto an unrelated class's `private fun apply`, a JavaScript `fail(...)` onto a Go `func fail`, a Rust `.count()` onto a private `fn count` in another crate, and C USB helpers onto a `static` in a `.c` they never link. Such a target is now declined after the whole name-matching pipeline settles — the reference stays unresolved rather than falling through to a fuzzy namesake. Same-file definitions, a child Rust module reaching its ancestors' private items, and Rust `impl Trait for Type` methods stay resolvable. Re-index after upgrading. (#1730, #1731)
 - **A definition its language makes file-local no longer captures calls from other files.** A C `static` in another source file (`.c`/`.cc`/… — not a header's `static inline`, which is textually included), a Kotlin/Java/C#/Swift/Scala/Dart/PHP `private` member, a Go unexported name in another package, and a Rust non-`pub` item outside its module subtree cannot be what a name in another file means, but name matching accepted them whenever the names agreed: an Android `editor.apply()` onto an unrelated class's `private fun apply`, a JavaScript `fail(...)` onto a Go `func fail`, a Rust `.count()` onto a private `fn count` in another crate, and C USB helpers onto a `static` in a `.c` they never link. Such a target is now declined after the whole name-matching pipeline settles — the reference stays unresolved rather than falling through to a fuzzy namesake. Same-file definitions, a child Rust module reaching its ancestors' private items, and Rust `impl Trait for Type` methods stay resolvable. Re-index after upgrading. (#1730, #1731)
 - **A binding in a module that exports nothing is no longer a cross-file target.** On vite, every `import { defineConfig } from 'vite'` across the playground resolved onto a `const vite = await createServer(…)` sitting at module scope in `playground/ssr-html/test-stacktrace.js` — a file with an import and no export, so that binding is reachable from nowhere but itself. Name matching commits as soon as one candidate survives, and nothing asked whether an import could reach the survivor; that one binding took 157 edges. A JS/TS file holding an `import` and no export of any kind now offers its locals to no other file. Classic scripts, CommonJS (including `exports["x"] = …`), a later `export { … }`, and names contributed through `declare global` are all unaffected. Across vite this removed 320 wrong edges and added 18, each addition a reference that was previously ambiguous rather than newly invented. Re-index after upgrading. (#1719)
 - **A binding in a module that exports nothing is no longer a cross-file target.** On vite, every `import { defineConfig } from 'vite'` across the playground resolved onto a `const vite = await createServer(…)` sitting at module scope in `playground/ssr-html/test-stacktrace.js` — a file with an import and no export, so that binding is reachable from nowhere but itself. Name matching commits as soon as one candidate survives, and nothing asked whether an import could reach the survivor; that one binding took 157 edges. A JS/TS file holding an `import` and no export of any kind now offers its locals to no other file. Classic scripts, CommonJS (including `exports["x"] = …`), a later `export { … }`, and names contributed through `declare global` are all unaffected. Across vite this removed 320 wrong edges and added 18, each addition a reference that was previously ambiguous rather than newly invented. Re-index after upgrading. (#1719)
 - **A bare call inside a JavaScript or TypeScript method no longer resolves to the method itself.** When a method and a module-scope function share a name, `serialize(this.raw)` written inside `Record.serialize` means the function, but the nearest same-named definition won the tie and the graph recorded the method calling itself. A call written without a receiver can never reach a method in JS/TS, so methods are no longer candidates for it; `this.serialize()` and `other.serialize()` resolve as before. (#1714)
 - **A bare call inside a JavaScript or TypeScript method no longer resolves to the method itself.** When a method and a module-scope function share a name, `serialize(this.raw)` written inside `Record.serialize` means the function, but the nearest same-named definition won the tie and the graph recorded the method calling itself. A call written without a receiver can never reach a method in JS/TS, so methods are no longer candidates for it; `this.serialize()` and `other.serialize()` resolve as before. (#1714)

+ 106 - 0
__tests__/python-module-scope-collection-methods.test.ts

@@ -0,0 +1,106 @@
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import { CodeGraph } from '../src';
+
+const collections = [
+  { name: 'dict_literal', value: '{"answer": "42"}', method: 'get' },
+  { name: 'empty_dict', value: '{}', method: 'get' },
+  { name: 'dict_constructor', value: 'dict()', method: 'get' },
+  { name: 'list_literal', value: '[1]', method: 'append' },
+  { name: 'empty_list', value: '[]', method: 'append' },
+  { name: 'list_constructor', value: 'list()', method: 'append' },
+  { name: 'set_literal', value: '{1}', method: 'add' },
+  { name: 'set_constructor', value: 'set()', method: 'add' },
+  { name: 'tuple_literal', value: '(1,)', method: 'index' },
+  { name: 'empty_tuple', value: '()', method: 'index' },
+  { name: 'tuple_constructor', value: 'tuple()', method: 'index' },
+  { name: 'frozenset_constructor', value: 'frozenset()', method: 'union' },
+];
+
+let dir: string;
+let cg: CodeGraph;
+
+beforeAll(async () => {
+  dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1652-'));
+  fs.writeFileSync(path.join(dir, 'settings.py'), `DEFAULTS = {"answer": "42"}
+
+def read_setting(name):
+    return DEFAULTS.get(name, None)
+`);
+  fs.writeFileSync(path.join(dir, 'cache.py'), `class LRUCache:
+    def __init__(self):
+        self._store = {}
+
+    def get(self, key):
+        return self._store.get(key)
+
+class ProjectCollection:
+    def append(self, item):
+        pass
+    def add(self, item):
+        pass
+    def index(self, item):
+        return 0
+    def union(self, item):
+        return self
+`);
+  for (const { name, value, method } of collections) {
+    // Capitalizing lRUCache matches a real class. Its name must not override
+    // the same-file binding's collection initializer (#1652).
+    fs.writeFileSync(path.join(dir, `${name}.py`), `lRUCache = ${value}
+
+def use_${name}(item):
+    return lRUCache.${method}(item)
+`);
+  }
+  fs.writeFileSync(path.join(dir, 'unknown.py'), `UNKNOWN = load_defaults()
+
+def read_unknown(name):
+    return UNKNOWN.get(name)
+`);
+  fs.writeFileSync(path.join(dir, 'client.py'), `from cache import LRUCache
+
+def read_cache(lRUCache: LRUCache, name):
+    return lRUCache.get(name)
+`);
+  cg = await CodeGraph.init(dir, { index: true });
+});
+
+afterAll(() => {
+  cg?.destroy();
+  fs.rmSync(dir, { recursive: true, force: true });
+});
+
+function expectNoMethodCall(callerName: string, file: string, methodName: string) {
+  const caller = cg.getNodesByName(callerName).find((n) => n.kind === 'function' && n.filePath === file);
+  const method = cg.getNodesByName(methodName).find((n) => n.kind === 'method' && n.filePath === 'cache.py');
+  expect(caller).toBeDefined();
+  expect(method).toBeDefined();
+  expect(cg.getCallers(method!.id).map(({ node }) => node.id)).not.toContain(caller!.id);
+  expect(cg.getCallees(caller!.id).map(({ node }) => node.id)).not.toContain(method!.id);
+}
+
+describe('Python module-scope collection methods (#1652)', () => {
+  it('does not connect DEFAULTS.get to the unrelated LRUCache.get method', () => {
+    expectNoMethodCall('read_setting', 'settings.py', 'get');
+  });
+
+  it.each(collections)('keeps $name ($method) external even when the receiver resembles a class', ({ name, method }) => {
+    expectNoMethodCall(`use_${name}`, `${name}.py`, method);
+  });
+
+  it('does not treat an unrelated variable as evidence of a project class', () => {
+    expectNoMethodCall('read_unknown', 'unknown.py', 'get');
+  });
+
+  it('preserves real instance calls despite same-named collections in other files', () => {
+    const caller = cg.getNodesByName('read_cache').find((n) => n.kind === 'function')!;
+    const method = cg.getNodesByName('get').find((n) => n.kind === 'method' && n.filePath === 'cache.py')!;
+    expect(caller).toBeDefined();
+    expect(method).toBeDefined();
+    expect(cg.getCallees(caller.id).map(({ node }) => node.id)).toContain(method.id);
+    expect(cg.getCallers(method.id).map(({ node }) => node.id)).toContain(caller.id);
+  });
+});

+ 17 - 4
src/resolution/index.ts

@@ -2104,8 +2104,22 @@ export class ReferenceResolver {
         // project dependency, not `list.append` (#1681). Without this, the
         // project dependency, not `list.append` (#1681). Without this, the
         // qualified ref never reaches resolveViaImport / resolvePythonModuleMember.
         // qualified ref never reaches resolveViaImport / resolvePythonModuleMember.
         if (PYTHON_BUILT_IN_METHODS.has(method)) {
         if (PYTHON_BUILT_IN_METHODS.has(method)) {
+          // A module-scope collection binding is stronger evidence than a
+          // coincidentally matching class name (#1652). Only use this file's
+          // binding: an unrelated module may reuse the receiver for a collection.
+          const isCollection = this.context.getNodesByName(receiver).some((node) =>
+            node.language === 'python' && node.filePath === ref.filePath &&
+            (node.kind === 'variable' || node.kind === 'constant') &&
+            node.qualifiedName === receiver &&
+            /^=\s*(?:[\[{]|(?:dict|list|set|tuple|frozenset)\s*\(|\(\s*\)|\([^()]*,)/.test(node.signature ?? '')
+          );
+          if (isCollection) return true;
+
           const capitalized = receiver.charAt(0).toUpperCase() + receiver.slice(1);
           const capitalized = receiver.charAt(0).toUpperCase() + receiver.slice(1);
-          const isKnownClass = this.knownNames?.has(capitalized) ?? false;
+          const isKnownClass = this.context.getNodesByName(capitalized).some((node) =>
+            node.language === 'python' &&
+            (node.kind === 'class' || node.kind === 'struct' || node.kind === 'interface')
+          );
           const isProjectModule =
           const isProjectModule =
             !isKnownClass && this.isPythonProjectModule(ref, receiver);
             !isKnownClass && this.isPythonProjectModule(ref, receiver);
           if (!isKnownClass && !isProjectModule) {
           if (!isKnownClass && !isProjectModule) {
@@ -2116,9 +2130,8 @@ export class ReferenceResolver {
       // A bare name colliding with a builtin method (index, get, update, count…)
       // A bare name colliding with a builtin method (index, get, update, count…)
       // is only a builtin when NOTHING in the codebase declares it. A declared
       // is only a builtin when NOTHING in the codebase declares it. A declared
       // symbol with that exact name — e.g. a Flask/FastAPI view `def index()` or
       // symbol with that exact name — e.g. a Flask/FastAPI view `def index()` or
-      // `def get()` — is a real reference target. Mirrors the knownNames guard on
-      // the dotted branch above; without it, every handler named after a builtin
-      // method silently loses its route→handler edge.
+      // `def get()` — is a real reference target. Without this guard, every
+      // handler named after a builtin method silently loses its route→handler edge.
       if (PYTHON_BUILT_IN_METHODS.has(name) && !this.knownNames?.has(name)) {
       if (PYTHON_BUILT_IN_METHODS.has(name) && !this.knownNames?.has(name)) {
         return true;
         return true;
       }
       }