Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
148 changes: 148 additions & 0 deletions __tests__/bare-call-no-method.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
95 changes: 93 additions & 2 deletions src/resolution/name-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,85 @@ function isLexicallyReachable(
);
}

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
*/
Expand All @@ -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;
Expand Down Expand Up @@ -1299,6 +1384,7 @@ function getInferScanStates(context: ResolutionContext): Map<string, InferScanSt
/** Drop the per-context scan states (see ReferenceResolver.clearCaches). */
export function clearNameMatcherMemos(context: ResolutionContext): void {
INFER_SCAN_STATES.delete(context);
LOCAL_BINDING_MEMO.delete(context);
}

function memoPatterns(key: string, build: () => RegExp[]): RegExp[] {
Expand Down Expand Up @@ -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,
Expand Down