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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixes

- Calls from your code no longer resolve into your tests. Tests depend on the code they exercise, never the other way round, but a name with no real definition to bind to — a value destructured from a hook, a helper that lives in a package — would land on any same-named helper inside a test file. On one project every `t(...)` call across 137 page components pointed at a translation stub defined in a single test, making it the second most-called symbol in the codebase and its callers and impact almost entirely fictional. Test files are recognised by the names their test runner enforces (`test_*.py`, `*.test.ts`, `*_test.go`, `*Test.java`), never by which folder they sit in, so a `spec/` directory holding an API document or a `testing/` utility library is left alone. Set `CODEGRAPH_TEST_TREE_GATE=0` to restore the old behaviour.


## [1.6.0] - 2026-08-26

Expand Down
170 changes: 169 additions & 1 deletion __tests__/resolution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as os from 'os';
import { CodeGraph } from '../src';
import { Node, UnresolvedReference } from '../src/types';
import { ReferenceResolver, createResolver, ResolutionContext } from '../src/resolution';
import { matchReference, resolveMethodOnType, matchByQualifiedName, preferCallSiteFile, matchMethodCall } from '../src/resolution/name-matcher';
import { matchReference, resolveMethodOnType, matchByQualifiedName, preferCallSiteFile, matchMethodCall, isRunnerNamedTestFile } from '../src/resolution/name-matcher';
import { resolveImportPath, extractImportMappings, resolveJvmImport, loadCppIncludeDirs, clearCppIncludeDirCache, isPhpIncludePathRef } from '../src/resolution/import-resolver';
import type { UnresolvedRef } from '../src/resolution/types';
import { detectFrameworks, getAllFrameworkResolvers } from '../src/resolution/frameworks';
Expand All @@ -36,6 +36,174 @@ describe('Resolution Module', () => {
}
});


describe('Production code never resolves into a test file', () => {
// Tests depend on the code they exercise; the code never depends on its
// tests. A production symbol resolving INTO a test file is therefore always
// wrong — and it happens constantly, because the names that reach the
// name-matching fallback are precisely the ones with no real definition to
// bind to.
const node = (over: Partial<Node>): Node => ({
id: 'n', kind: 'function', name: 'x', qualifiedName: 'x',
filePath: 'a.ts', language: 'typescript',
startLine: 1, endLine: 2, startColumn: 0, endColumn: 0,
updatedAt: Date.now(), ...over,
});
const contextOf = (nodes: Node[]): ResolutionContext => ({
getNodesInFile: () => nodes,
getNodesByName: (name: string) => nodes.filter((n) => n.name === name),
getNodesByQualifiedName: () => [],
getNodesByKind: () => [],
getNodesByLowerName: (lower: string) =>
nodes.filter((n) => n.name.toLowerCase() === lower),
getImportMappings: () => [],
fileExists: () => true,
readFile: () => null,
getProjectRoot: () => '/test',
getAllFiles: () => nodes.map((n) => n.filePath),
} as any);
const callFrom = (filePath: string, name: string, language = 'tsx') => ({
fromNodeId: `caller:${filePath}:c:1`,
referenceName: name,
referenceKind: 'calls' as const,
line: 5, column: 0, filePath, language: language as any,
});

it('declines a component call whose only candidate is a test stub', () => {
// `const { t } = useTranslation()` binds `t` from a package, so there is
// no local definition and no exported one — the only same-named symbol
// anywhere is a stub inside a test.
const nodes = [node({
id: 'stub:t', name: 't', qualifiedName: 'merge-dialog.test.tsx::t',
filePath: 'src/page-tests/merge-dialog.test.tsx', language: 'tsx',
})];
expect(matchReference(callFrom('src/pages/Planner.tsx', 't'), contextOf(nodes)))
.toBeNull();
});

it('prefers the production definition when both exist', () => {
const nodes = [
node({ id: 'test:format', name: 'format', qualifiedName: 'x.test.ts::format',
filePath: 'src/lib/x.test.ts' }),
node({ id: 'prod:format', name: 'format', qualifiedName: 'fmt.ts::format',
filePath: 'src/lib/fmt.ts' }),
];
const result = matchReference(callFrom('src/pages/Row.tsx', 'format'), contextOf(nodes));
expect(result?.targetNodeId).toBe('prod:format');
});

it('leaves a call made BY a test alone', () => {
// Tests calling shared fixtures is the normal direction.
const nodes = [node({
id: 'stub:renderPage', name: 'renderPage',
qualifiedName: 'helpers.test.ts::renderPage',
filePath: 'src/page-tests/helpers.test.ts',
})];
const result = matchReference(
callFrom('src/page-tests/planner.test.tsx', 'renderPage'), contextOf(nodes));
expect(result?.targetNodeId).toBe('stub:renderPage');
});

it('identifies a test by the name its runner enforces, not by its directory', () => {
// Which directories hold tests is a project's own decision — `spec/` is an
// OpenAPI document in one repo and RSpec in another, and `testing/` is
// often a shipped utility library. Deleting edges on that guess would be
// silently wrong, so only runner-enforced filenames count.
expect(isRunnerNamedTestFile('src/lib/panel.test.ts')).toBe(true);
expect(isRunnerNamedTestFile('tests/test_collect.py')).toBe(true);
expect(isRunnerNamedTestFile('internal/server_test.go')).toBe(true);
expect(isRunnerNamedTestFile('src/main/java/app/OrderTest.java')).toBe(true);
expect(isRunnerNamedTestFile('spec/models/user_spec.rb')).toBe(true);

// Directory alone is never enough.
expect(isRunnerNamedTestFile('spec/openapi.yaml')).toBe(false);
expect(isRunnerNamedTestFile('src/testing/harness.ts')).toBe(false);
expect(isRunnerNamedTestFile('tests/helpers.ts')).toBe(false);
// And a lowercase "test" inside an ordinary word is not a match.
expect(isRunnerNamedTestFile('src/latest.ts')).toBe(false);
expect(isRunnerNamedTestFile('src/manifest.ts')).toBe(false);
expect(isRunnerNamedTestFile('src/protest.py')).toBe(false);

// The bare CamelCase suffix belongs to the JVM/.NET/Swift runners that
// collect on it. vitest and jest do not — they want `.test.`/`.spec.` —
// so `useTests.ts` is an ordinary hook and must stay resolvable.
expect(isRunnerNamedTestFile('src/hooks/useTests.ts')).toBe(false);
expect(isRunnerNamedTestFile('src/RequestSpec.tsx')).toBe(false);
expect(isRunnerNamedTestFile('app/OrderTests.java')).toBe(true);
expect(isRunnerNamedTestFile('app/OrderTest.kt')).toBe(true);
});

it('applies to a method resolved through an inferred receiver type', () => {
// This path reaches a candidate through a receiver type rather than a
// bare name, but fails the same way: an unpinned receiver settles for a
// same-named method, and a test class's method is as good a match as any.
const nodes = [node({
id: 'test:say', kind: 'method', name: 'say',
qualifiedName: 'Reporter::say', filePath: 'src/tests/test_report.py',
language: 'python',
})];
const ctx = contextOf(nodes);
const ref = { ...callFrom('src/tools/collect.py', 'say', 'python') };
expect(resolveMethodOnType('Reporter', 'say', ref as any, ctx, 0.9, 'instance-method'))
.toBeNull();
// A test calling the same method still resolves.
const fromTest = { ...callFrom('src/tests/test_collect.py', 'say', 'python') };
expect(resolveMethodOnType('Reporter', 'say', fromTest as any, ctx, 0.9, 'instance-method')
?.targetNodeId).toBe('test:say');
});

it('applies to the receiver-name overlap fallback, which needs it most', () => {
// That strategy scores a receiver NAME against a class name, so a short
// receiver (`rep.say()`) matches a test's stand-in class (`_Rep`) on one
// shared word — and stand-ins are precisely the classes that share a name
// with the real collaborator they replace.
const nodes = [node({
id: 'test:_Rep.say', kind: 'method', name: 'say',
qualifiedName: 'test_collect::_Rep::say',
filePath: 'src/tests/test_collect.py', language: 'python',
})];
const ref = { ...callFrom('src/tools/answer.py', 'rep.say', 'python') };
expect(matchMethodCall(ref as any, contextOf(nodes))).toBeNull();
});

it('can be switched off for a project that relies on the old behaviour', async () => {
// The gate removes edges, so it carries the same escape hatch the
// ambiguity ceiling does. Read at module load, hence the reimport.
const prev = process.env.CODEGRAPH_TEST_TREE_GATE;
process.env.CODEGRAPH_TEST_TREE_GATE = '0';
try {
vi.resetModules();
const fresh = await import('../src/resolution/name-matcher');
const nodes = [node({
id: 'stub:t', name: 't', qualifiedName: 'merge.test.tsx::t',
filePath: 'src/page-tests/merge.test.tsx', language: 'tsx',
})];
expect(fresh.matchReference(
callFrom('src/pages/Planner.tsx', 't') as any, contextOf(nodes))
).not.toBeNull();
} finally {
if (prev === undefined) delete process.env.CODEGRAPH_TEST_TREE_GATE;
else process.env.CODEGRAPH_TEST_TREE_GATE = prev;
vi.resetModules();
}
});

it('declines rather than falling back when the only candidate is a test', () => {
// The fabricated edges are precisely the ones with no production
// alternative, so a "keep it if nothing else matches" fallback would
// leave the entire defect in place. Production cannot depend on a test
// file — it would not build — so no match is the truthful answer.
const nodes = [node({
id: 'only:assertGolden', name: 'assertGolden',
qualifiedName: 'golden.test.ts::assertGolden',
filePath: 'src/lib/golden.test.ts',
})];
expect(matchReference(
callFrom('src/lib/report.ts', 'assertGolden', 'typescript'), contextOf(nodes)))
.toBeNull();
});
});

describe('Name Matcher', () => {
it('should match exact name references', () => {
// Create a mock context
Expand Down
98 changes: 94 additions & 4 deletions src/resolution/name-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,81 @@ function isLexicallyReachable(
);
}

/**
* Whether to withhold test-file definitions from production references.
* On by default; `CODEGRAPH_TEST_TREE_GATE=0` restores the old behaviour, the
* same escape hatch `CODEGRAPH_AMBIGUOUS_NAME_CEILING` gives its own gate.
*/
const TEST_TREE_GATE = process.env.CODEGRAPH_TEST_TREE_GATE !== '0';

/**
* Files a test RUNNER will collect, identified by the naming its own toolchain
* enforces — not by where they sit.
*
* The distinction matters because this gate DELETES edges, and a wrong deletion
* is silent. Which directories hold a project's tests is a project's own
* decision (`spec/` is an OpenAPI document in one repo and RSpec in another;
* `testing/` is often a shipped utility library), so guessing at it would
* remove real edges from projects that merely name a folder unluckily. A
* FILENAME, by contrast, is not a preference: pytest collects `test_*.py` and
* `*_test.py` and nothing else, `go test` requires `_test.go`, vitest and jest
* default to `*.test.*` / `*.spec.*`, surefire to `*Test.java`. A file named
* this way IS a test, in every project, by the runner's own rule.
*
* Measured on a mixed Elixir/TypeScript/Python repository: of the impossible
* production→test call edges, the runner-enforced names accounted for all but
* one. Directory guessing would have added the risk without the reward.
*/
export function isRunnerNamedTestFile(filePath: string): boolean {
const fileName = filePath.slice(filePath.lastIndexOf('/') + 1);
const lower = fileName.toLowerCase();
if (
// pytest: test_foo.py
lower.startsWith('test_') ||
// vitest/jest/go/pytest/rspec: foo.test.ts, foo_test.go, foo-spec.rb, bar_spec.py
/[._-](test|tests|spec|specs)\.[a-z0-9]+$/.test(lower)
) {
return true;
}

// The bare CamelCase suffix — `OrderTest.java` — is a JVM/.NET/Swift
// convention (Maven surefire collects `**/*Test.java`, `**/*Tests.java`),
// NOT a JavaScript one: vitest and jest collect `*.test.ts` and would never
// pick up `useTests.ts`, which is an ordinary hook. Applying the suffix
// everywhere misreads any identifier that happens to end in "Test" or
// "Tests", so it is scoped to the languages whose runners actually use it.
return /\.(java|kt|kts|scala|cs|swift)$/i.test(fileName)
&& /(?:Test|Tests|TestCase|Spec|Specs)\.[A-Za-z0-9]+$/.test(fileName);
}

/**
* Drop test-file definitions when the reference comes from production code.
*
* Tests depend on the code they exercise; the code never depends on its tests.
* So a production symbol resolving INTO a test file is always wrong — and it
* happens constantly, because the names that reach this fallback are the ones
* with no real definition to bind to. A React component calling `t(...)` from a
* `const { t } = useTranslation()` destructure has no local `t` node and no
* exported one either (the hook comes from a package), so the only candidates
* anywhere are same-named helpers inside test files, and proximity picks one.
* Measured on one repository: 2,820 calls across 137 production page components
* all resolved to a single i18n stub defined in one test file.
*
* A reference from a test is left alone — tests calling test helpers is the
* normal direction, and shared fixtures legitimately live in the test tree.
*/
function applyTestTreeGate(candidates: Node[], ref: UnresolvedRef): Node[] {
if (!TEST_TREE_GATE) return candidates;
if (isRunnerNamedTestFile(ref.filePath)) return candidates;
// Unconditional, including when it empties the set. A production symbol that
// matches ONLY inside tests is the exact shape of the defect: production code
// cannot depend on a test file — it would not build or run — so a lone
// test-file candidate is a coincidence of naming, never the real target.
// Keeping it as a fallback would leave the whole defect in place, since the
// fabricated edges are precisely the ones with no production alternative.
return candidates.filter((c) => !isRunnerNamedTestFile(c.filePath));
}

/**
* Try to resolve a reference by exact name match
*/
Expand All @@ -404,7 +479,8 @@ 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 candidates = applyTestTreeGate(
applyLanguageGate(context.getNodesByName(ref.referenceName), ref), ref)
.filter((n) => n.kind !== 'import')
// Nested locals are only reachable from inside their container (#1230).
.filter((n) => isLexicallyReachable(n, ref, context));
Expand Down Expand Up @@ -706,6 +782,13 @@ export function resolveMethodOnType(
}
}
}
// Same rule as the name-based strategies: production code cannot depend on a
// test file, so a method that only matches inside one is a naming coincidence.
// This path reaches it through an inferred receiver type rather than a bare
// name, but the failure is identical — an unpinned receiver settles for a
// same-named method, and a test's class method is as good a match as any.
matches = applyTestTreeGate(matches, ref);

if (matches.length === 0) {
// Conformance fallback: the method may be defined on a supertype `typeName`
// extends, or on a protocol / trait it conforms to (e.g. a Swift protocol-
Expand Down Expand Up @@ -1998,8 +2081,14 @@ export function matchMethodCall(
if (methodCandidates.length > AMBIGUOUS_NAME_CEILING) {
return null;
}
const methods = methodCandidates.filter(
(n) => n.kind === 'method' && n.name === methodName
// Same rule as every other strategy: production code cannot depend on a
// test file. This one needs it most — it scores a receiver NAME against a
// class name, so a short receiver (`rep.say()`) matches any test's stand-in
// class (`_Rep`) on one shared word, and the stand-ins that tests define
// are exactly the classes that share a name with the real collaborator.
const methods = applyTestTreeGate(
methodCandidates.filter((n) => n.kind === 'method' && n.name === methodName),
ref
);

// Filter to same-language candidates first
Expand Down Expand Up @@ -2412,7 +2501,8 @@ export function matchFuzzy(

// Filter to callable kinds only (function, method, class)
const callableKinds = new Set(['function', 'method', 'class']);
const callableCandidates = applyLanguageGate(candidates.filter((n) => callableKinds.has(n.kind)), ref);
const callableCandidates = applyTestTreeGate(
applyLanguageGate(candidates.filter((n) => callableKinds.has(n.kind)), ref), ref);

// Prefer same-language matches
const sameLanguageCandidates = callableCandidates.filter(n => n.language === ref.language);
Expand Down