From fd26889334e146ec1e485432d9d38e3adf8dbeba Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Mon, 7 Sep 2026 11:10:10 +0300 Subject: [PATCH] fix(resolution): a receiver-less JS/TS call never binds to a method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 (the ref's column is the start of the call expression): 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. `this.serialize()` (recursion) and `other.serialize()` are unchanged. Standalone on vite this removes 566 method-bound bare calls (`log(…)` onto a spec file's `log` method, `import(…)` onto a runner method, `resolve(…)` onto PluginContainer.resolve) and lets 274 previously out-ranked candidates through — `resolve` bound by `import { resolve } from 'node:path'` and Promise-callback `resolve` parameters — which #1715 and a local-binding rule are for; measured in the stack it is purely subtractive. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 + __tests__/bare-call-no-method.test.ts | 148 ++++++++++++++++++++++++++ src/resolution/name-matcher.ts | 95 ++++++++++++++++- 3 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 __tests__/bare-call-no-method.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1815c4150..39d7e187c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -201,6 +201,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- **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. - **Production code under a `samples` or `examples` package path is no longer treated as test code.** A Kotlin or Java project whose package path runs through `com/google/samples/…` (Now in Android, for one) had nearly every file counted as a fixture, so the Map opened on `build-logic`, the entry points hid the app, and dead-code and test badges were wrong. Only the project layout above a `src/` folder decides now; the package path below it never does. diff --git a/__tests__/bare-call-no-method.test.ts b/__tests__/bare-call-no-method.test.ts new file mode 100644 index 000000000..ba5e1d3f8 --- /dev/null +++ b/__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 { + 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 => !!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 {', + ' 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'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c74d8f272..5236ac2fd 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -388,6 +388,85 @@ function isLexicallyReachable( ); } +const JS_FAMILY = new Set(['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>(); + +/** + * 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 */ @@ -404,10 +483,16 @@ 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 bareJs = isBareJsCall(ref, context); const candidates = applyLanguageGate(context.getNodesByName(ref.referenceName), ref) .filter((n) => n.kind !== 'import') // Nested locals are only reachable from inside their container (#1230). - .filter((n) => isLexicallyReachable(n, ref, context)); + .filter((n) => isLexicallyReachable(n, ref, 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) { return null; @@ -1299,6 +1384,7 @@ function getInferScanStates(context: ResolutionContext): Map RegExp[]): RegExp[] { @@ -2418,7 +2504,12 @@ export function matchFuzzy( const sameLanguageCandidates = callableCandidates.filter(n => n.language === ref.language); const finalCandidates = sameLanguageCandidates.length > 0 ? sameLanguageCandidates : callableCandidates; - if (finalCandidates.length === 1) { + if ( + finalCandidates.length === 1 && + !(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; return { original: ref,