Просмотр исходного кода

fix(resolution): a receiver-less JS/TS call never binds to a method (#1759)

`serialize(this.raw)` inside `Record.serialize`, with a module-scope
`function serialize` in the same file, resolved onto the method itself:
both were exact-name candidates, both same-file, and findBestMatch's
line-proximity term always prefers the enclosing method (#1714). In JS/TS a
call written without a receiver cannot reach a method at all — methods
need `this.`, an object, or a bound reference.

The extractor emits `this.m()` and `super.m()` under the bare method name,
so the receiver is read back from the call site's own line: when the text
there begins with the name itself and nothing but whitespace, an operator
or an opener precedes it, the call is bare, and `method` nodes leave the
candidate set before ranking. matchFuzzy declines a lone `method` survivor
for the same ref. A name the file binds itself also has no cross-file
candidate for a bare call. `this.serialize()` and `other.serialize()` are
unchanged.

Rebased #1735 onto current main (resolved conflicts with sealed-module /
cross-file visibility guards from #1719/#1730/#1731).

Fixes #1714

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Co-authored-by: danusha2345 <ewidusoc498@gmail.com>
Colby Mchenry 5 часов назад
Родитель
Сommit
cd4e65b59c
3 измененных файлов с 242 добавлено и 2 удалено
  1. 1 0
      CHANGELOG.md
  2. 148 0
      __tests__/bare-call-no-method.test.ts
  3. 93 2
      src/resolution/name-matcher.ts

+ 1 - 0
CHANGELOG.md

@@ -218,6 +218,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - 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)
 - **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)
 
 
 - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges.
 - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges.
 
 

+ 148 - 0
__tests__/bare-call-no-method.test.ts

@@ -0,0 +1,148 @@
+/**
+ * In JS/TS a receiver-less call can never bind to a class method: `serialize(x)`
+ * inside `Record.serialize` means the module-scope function, and the method
+ * itself — which the same-file proximity term used to pick, producing a
+ * self-edge — is not a candidate (#1714). `this.serialize(x)` still is.
+ */
+
+import { describe, it, expect, afterEach } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import CodeGraph from '../src/index';
+
+let tempDir: string;
+let cg: CodeGraph | null = null;
+
+async function callsFromMethod(source: string, methodName: string): Promise<string[]> {
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1714-'));
+  fs.writeFileSync(path.join(tempDir, 'record.ts'), source);
+  cg = await CodeGraph.init(tempDir, { index: true });
+  cg.resolveReferences();
+  const from = cg.getNodesByKind('method').find((n) => n.name === methodName)!;
+  expect(from).toBeDefined();
+  return cg
+    .getOutgoingEdges(from.id)
+    .filter((e) => e.kind === 'calls')
+    .map((e) => cg!.getNode(e.target))
+    .filter((n): n is NonNullable<typeof n> => !!n)
+    .map((n) => `${n.kind}:${n.qualifiedName ?? n.name}`);
+}
+
+afterEach(() => {
+  cg?.close();
+  cg = null;
+  fs.rmSync(tempDir, { recursive: true, force: true });
+});
+
+describe('a receiver-less JS/TS call never binds to a method (#1714)', () => {
+  it('resolves the bare call onto the module-scope function, not the enclosing method', async () => {
+    const callees = await callsFromMethod(
+      [
+        'function serialize(value: string): string {',
+        '  return value.trim();',
+        '}',
+        '',
+        'export class Record {',
+        '  constructor(private readonly raw: string) {}',
+        '  serialize(): string {',
+        '    return serialize(this.raw);',
+        '  }',
+        '}',
+        '',
+      ].join('\n'),
+      'serialize'
+    );
+    expect(callees).toContain('function:serialize');
+    expect(callees).not.toContain('method:Record::serialize');
+  });
+
+  it('keeps `this.serialize()` — a real recursive self-call', async () => {
+    const callees = await callsFromMethod(
+      [
+        'function serialize(value: string): string {',
+        '  return value.trim();',
+        '}',
+        '',
+        'export class Record {',
+        '  constructor(private readonly raw: string, private depth = 0) {}',
+        '  serialize(): string {',
+        '    if (this.depth > 0) return this.serialize();',
+        '    return this.raw;',
+        '  }',
+        '}',
+        '',
+      ].join('\n'),
+      'serialize'
+    );
+    expect(callees).toContain('method:Record::serialize');
+  });
+
+  it('a bare call to a name the file binds itself has no cross-file candidate', async () => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1714-'));
+    fs.writeFileSync(path.join(tempDir, 'config.ts'), 'export function resolve(p: string) { return p; }\nexport function transform(c: string) { return c; }\nexport function now() { return 0; }\n');
+    fs.writeFileSync(
+      path.join(tempDir, 'client.ts'),
+      [
+        'const transform = makeTransform();',
+        'export function ping(): Promise<void> {',
+        '  return new Promise((resolve, reject) => {',
+        '    setTimeout(() => resolve(), 10);',
+        '  });',
+        '}',
+        'export function run(options: { now?: () => number }) {',
+        '  const now = options.now || (() => Date.now());',
+        '  return now() + transform("x").length;',
+        '}',
+        '',
+      ].join('\n')
+    );
+    cg = await CodeGraph.init(tempDir, { index: true });
+    cg.resolveReferences();
+    const targets = cg.getNodesByKind('function').filter((n) => n.filePath === 'config.ts').map((n) => n.id);
+    const callers = cg.getNodesByKind('function').filter((n) => n.filePath === 'client.ts');
+    const crossFile = callers.flatMap((c) => cg!.getOutgoingEdges(c.id)).filter((e) => e.kind === 'calls' && targets.includes(e.target));
+    expect(crossFile).toEqual([]);
+  });
+
+  it('a destructured require or a string mentioning the name is not a local binding', async () => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1714-'));
+    fs.writeFileSync(path.join(tempDir, 'public-ip.js'), 'function lookupPublicIPv4() { return "1.2.3.4"; }\nfunction test(name, fn) { return fn(); }\nmodule.exports = { lookupPublicIPv4, test };\n');
+    fs.writeFileSync(
+      path.join(tempDir, 'main.js'),
+      [
+        'const { lookupPublicIPv4 } = require("./public-ip");',
+        'const { test } = require("./public-ip");',
+        'async function prepare() {',
+        '  const ip = await lookupPublicIPv4();',
+        '  test("a test of the thing", () => {});',
+        '  return ip;',
+        '}',
+        'module.exports = { prepare };',
+        '',
+      ].join('\n')
+    );
+    cg = await CodeGraph.init(tempDir, { index: true });
+    cg.resolveReferences();
+    const prepare = cg.getNodesByKind('function').find((n) => n.name === 'prepare')!;
+    const names = cg.getOutgoingEdges(prepare.id).filter((e) => e.kind === 'calls').map((e) => cg!.getNode(e.target)?.name);
+    expect(names).toContain('lookupPublicIPv4');
+    expect(names).toContain('test');
+  });
+
+  it('keeps `other.serialize()` — a call through a receiver', async () => {
+    const callees = await callsFromMethod(
+      [
+        'export class Record {',
+        '  serialize(): string { return ""; }',
+        '  copyOf(other: Record): string {',
+        '    return other.serialize();',
+        '  }',
+        '}',
+        '',
+      ].join('\n'),
+      'copyOf'
+    );
+    expect(callees).toContain('method:Record::serialize');
+  });
+});

+ 93 - 2
src/resolution/name-matcher.ts

@@ -643,6 +643,85 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex
   return isCrossFileReachable(candidate, ref, context);
   return isCrossFileReachable(candidate, ref, context);
 }
 }
 
 
+const JS_FAMILY = new Set<string>(['typescript', 'tsx', 'javascript', 'jsx']);
+
+/**
+ * Whether a JS/TS `calls` ref is a RECEIVER-LESS call — `serialize(x)`, not
+ * `this.serialize(x)` / `obj.serialize(x)`. The extractor emits `this.m()`
+ * and `super.m()` under the bare method name, so the receiver is read back
+ * from the call site's own line: the text at the ref's column is the call
+ * expression, and it starts with the name itself only when nothing precedes
+ * it. In JS/TS a bare call can never bind to a class method (methods need a
+ * receiver), so a `method` node is not a candidate for it (#1714) — the
+ * enclosing method itself least of all, which the same-file proximity term
+ * used to pick over the module-scope function the call actually means.
+ */
+function isBareJsCall(ref: UnresolvedRef, context: ResolutionContext): boolean {
+  if (ref.referenceKind !== 'calls' || !JS_FAMILY.has(ref.language)) return false;
+  if (ref.referenceName.includes('.')) return false;
+  const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1]
+    ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1];
+  if (line === undefined) return false;
+  const at = line.slice(ref.column);
+  const nameEsc = ref.referenceName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  if (!new RegExp('^' + nameEsc + '\\s*[(<]').test(at)) return false;
+  // Nothing but whitespace, an operator or an opener may precede a bare call.
+  return !/[.\w$\]\)]\s*$/.test(line.slice(0, ref.column)) || /\b(?:return|await|yield|typeof|void|new|else|case|throw|in|of|instanceof)\s*$/.test(line.slice(0, ref.column));
+}
+
+/** Per-context memo: `file\0name` → "the file binds this name locally". */
+const LOCAL_BINDING_MEMO = new WeakMap<ResolutionContext, Map<string, boolean>>();
+
+/**
+ * Whether a JS/TS file binds `name` itself — as a `const`/`let`/`var`/
+ * `function`/`class` declaration (destructuring included) or as a parameter
+ * of a function or arrow. Such a binding shadows every same-named symbol in
+ * other files, so a bare call to it has no cross-file candidate: the
+ * `resolve` of `new Promise((resolve, reject) => …)`, a spec's
+ * `const transform = await makeTransform()`, a factory's `const now =
+ * options.now || (() => new Date())`. None of these is a node the graph
+ * holds (a parameter, a const bound to a call result), so without this the
+ * matcher hands the call to whichever other file defines the name — and
+ * once methods stop being candidates for a bare call (#1714), the function
+ * that was out-ranked steps in. Read from source, memoised per file+name.
+ */
+function isLocallyBoundJsName(name: string, filePath: string, context: ResolutionContext): boolean {
+  let memo = LOCAL_BINDING_MEMO.get(context);
+  if (!memo) {
+    memo = new Map();
+    LOCAL_BINDING_MEMO.set(context, memo);
+  }
+  const key = filePath + '\0' + name;
+  const hit = memo.get(key);
+  if (hit !== undefined) return hit;
+  const source = context.readFile(filePath) ?? '';
+  const n = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  // `const { name } = require('./m')` / `= await import('./m')` binds an IMPORT,
+  // not a shadow: the symbol lives in the other file and the call means it.
+  const declRe = new RegExp(
+    '\\b(?:const|let|var)\\s+(?:' + n + '\\b|[{\\[][^;=]*?\\b' + n + '\\b[^;=]*?[}\\]])\\s*(?:=\\s*([^;\\n]*))?',
+    'g'
+  );
+  let bound = false;
+  for (const m of source.matchAll(declRe)) {
+    if (!/^\s*(?:await\s+)?(?:require|import)\s*\(/.test(m[1] ?? '')) { bound = true; break; }
+  }
+  if (!bound) {
+    bound =
+      new RegExp('\\b(?:function|class)\\s+' + n + '\\b').test(source) ||
+      // a parameter: every token before the name in the list is itself a
+      // parameter (identifier, optional type, optional default) — so a string
+      // argument containing the word cannot match.
+      new RegExp(
+        '\\(\\s*(?:(?:\\.\\.\\.)?[\\w$]+(?:\\s*\\??\\s*:\\s*[^,()]+)?(?:\\s*=\\s*[^,()]+)?\\s*,\\s*)*' +
+          n + '\\b(?:\\s*\\??\\s*:[^,()]*)?(?:\\s*=[^,()]*)?(?:\\s*,\\s*[^()]*)?\\)\\s*(?::[^=;{]*)?(?:=>|\\{)'
+      ).test(source) ||
+      new RegExp('(?:^|[^\\w$.])' + n + '\\s*=>').test(source);
+  }
+  memo.set(key, bound);
+  return bound;
+}
+
 /**
 /**
  * Try to resolve a reference by exact name match
  * Try to resolve a reference by exact name match
  */
  */
@@ -659,13 +738,19 @@ export function matchByExactName(
   // unresolved import refs each scored K same-named import candidates through
   // unresolved import refs each scored K same-named import candidates through
   // findBestMatch — O(K²) per package, the dominant cost of "Resolving refs" on
   // findBestMatch — O(K²) per package, the dominant cost of "Resolving refs" on
   // large import-heavy (front-end + back-end) repos (#915).
   // large import-heavy (front-end + back-end) repos (#915).
+  const bareJs = isBareJsCall(ref, context);
   const candidates = applyLanguageGate(context.getNodesByName(ref.referenceName), ref)
   const candidates = applyLanguageGate(context.getNodesByName(ref.referenceName), ref)
     .filter((n) => n.kind !== 'import')
     .filter((n) => n.kind !== 'import')
     // Nested locals are only reachable from inside their container (#1230).
     // Nested locals are only reachable from inside their container (#1230).
     .filter((n) => isLexicallyReachable(n, ref, context))
     .filter((n) => isLexicallyReachable(n, ref, context))
     // Preserve import ranking; calls reject the winner without promoting another.
     // Preserve import ranking; calls reject the winner without promoting another.
     .filter((n) => ref.referenceKind !== 'imports' || n.filePath === ref.filePath ||
     .filter((n) => ref.referenceKind !== 'imports' || n.filePath === ref.filePath ||
-      !ESM_FAMILY.has(n.language) || !isSealedModule(n.filePath, context));
+      !ESM_FAMILY.has(n.language) || !isSealedModule(n.filePath, context))
+    // A receiver-less JS/TS call cannot reach a method (#1714).
+    .filter((n) => !(bareJs && n.kind === 'method'))
+    // A name the file binds itself (a parameter, a const) shadows every other
+    // file's symbol of that name, so a bare call has no cross-file candidate.
+    .filter((n) => !(bareJs && n.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context)));
 
 
   if (candidates.length === 0) {
   if (candidates.length === 0) {
     return null;
     return null;
@@ -1561,6 +1646,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
   C_STATIC_MEMO.delete(context);
   C_STATIC_MEMO.delete(context);
   RUST_TRAIT_IMPL_MEMO.delete(context);
   RUST_TRAIT_IMPL_MEMO.delete(context);
   SEALED_MODULES.delete(context);
   SEALED_MODULES.delete(context);
+  LOCAL_BINDING_MEMO.delete(context);
 }
 }
 
 
 function memoPatterns(key: string, build: () => RegExp[]): RegExp[] {
 function memoPatterns(key: string, build: () => RegExp[]): RegExp[] {
@@ -2711,10 +2797,15 @@ export function matchFuzzy(
   // module). The sealed-module test rejects the survivor and never filters the
   // module). The sealed-module test rejects the survivor and never filters the
   // set that produced it: removing a sealed candidate from a crowd would leave
   // set that produced it: removing a sealed candidate from a crowd would leave
   // a lone one and manufacture a 0.5 guess out of an ambiguity fuzzy declines.
   // a lone one and manufacture a 0.5 guess out of an ambiguity fuzzy declines.
+  // Also decline a bare JS/TS call whose only survivor is a method or a
+  // cross-file name the file already binds locally (#1714).
   if (
   if (
     finalCandidates.length === 1 &&
     finalCandidates.length === 1 &&
     isVisibleAcrossFiles(finalCandidates[0]!, ref, context) &&
     isVisibleAcrossFiles(finalCandidates[0]!, ref, context) &&
-    isCrossFileReachable(finalCandidates[0]!, ref, context)
+    isCrossFileReachable(finalCandidates[0]!, ref, context) &&
+    !(isBareJsCall(ref, context) &&
+      (finalCandidates[0]!.kind === 'method' ||
+        (finalCandidates[0]!.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context))))
   ) {
   ) {
     const isCrossLanguage = finalCandidates[0]!.language !== ref.language;
     const isCrossLanguage = finalCandidates[0]!.language !== ref.language;
     return {
     return {