Skip to content
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In VB.NET, a type or member read through a namespace, like `Billing.Invoice.MaxLines`, `StaxRip.UI.ButtonEx.ButtonSymbol.Delete` with the project's root namespace in front, or a Module's member read through its namespace like `RedGifs.RedGifsSiteKey`, now links to what it names, and a method called without parentheses, like `Me.Load` or a constructor's `Me.New`, now links to the overload that takes no arguments instead of the first one declared or nothing. The namespace is looked up as VB.NET does, so a namespace of the same name in another project is not mistaken for it. Thanks @serkanince for the report. (#2305)
- In VB.NET, a field, property or Module variable written by its bare name in front of a member, like `Button1` in `Button1.Text = "x"` or `Settings` in `Settings.Plugins.Count`, now counts as used by that code, so `codegraph callers` on a form's control or a shared settings object lists the code that reads through it. Nothing is linked for a local or parameter of that name, for the member of a same-named class in another project, or for a member declared with an outside type of its own name, like `Responser As Responser`, whose `Shared` members VB.NET reads through the type; a call through the name, like `Button1.Focus()`, still links only the method. Re-index VB.NET projects after upgrading. (#2305)
- In JavaScript and TypeScript, an import whose name contains a `$`, like `import items$ from './store'`, `import * as api$ from './api'` or `import { load as load$ } from './data'`, now links the calls made through it, such as `items$.getState().inc()`, to the code it imports, and so does a `$` name in a `require(…)` destructuring or an `export { … } from` barrel. Before, such an import was skipped or bound under a cut-off name (`load` for `load$`), so those calls reached nothing and the functions they call could look unused. A local variable or parameter that reuses an imported `$` name, like `const items$ = …` inside a function, is no longer mistaken for the import. Re-index JavaScript and TypeScript projects after upgrading.
- In JavaScript and TypeScript, a function declared inside another function now takes the calls made to its name inside that function, even when the file also imports that name from a package, like a nested `resolve` beside `import { resolve } from 'node:path'`. Before, such calls linked to nothing. Thanks @danusha2345. (#1715)
- In Dart, calling a parameter or a local variable, like `create(ref, arg)` inside `overrideWith(… create)`, a closure's parameter, a loop variable or a test's mock `listener(0, 1)`, no longer links the call to an unrelated function or constant of the same name in another file, or to a method the class inherits: a Bloc event handler's `emit(…)` is the handler's own `emit` parameter, not `Bloc.emit`. So `codegraph callers` and impact no longer list code that never calls them, while calls the parameter or variable doesn't cover, and calls to a local function, link as before. Re-index Dart projects after upgrading.
- In Dart, `const` constructors and redirecting factories, like flutter_bloc's `const BlocProvider.value(…)` or freezed's `const factory LoginState.loading() = _Loading;`, are now indexed, so a call such as `BlocProvider.value(…)` or `BlocProvider<CounterCubit>.value(…)` links to that constructor instead of another class's member with the same name. A call through a class name now reaches only that class's own constructors and static members, never a look-alike class, a parent class, an instance getter of the same name, or a project method that merely shares its name with an SDK call like `Uri.parse(…)`, and a constructor is no longer linked from a bare name, the end of a call chain or an annotation like `@internal`. Re-index Dart projects after upgrading.
- In Dart, a call that passes a type argument and arguments, like `ref.read<Repo>(repoProvider)`, `ref.watch<int>(counterProvider)`, `Provider<User>((ref) => …)`, `BlocProvider.of<CounterCubit>(context)` or `RepositoryProvider<Repo>(create: …)`, now links to the method or class it calls, and the type it passes counts as used there. Depending on the code around it, the parser often read such a call as two comparisons, so it linked nothing, and Riverpod providers, repository reads and bloc lookups were missing from `callers`, impact and `codegraph_explore`. A real comparison like `a < b` is still left alone. Re-index Dart projects after upgrading.
Expand Down
99 changes: 99 additions & 0 deletions __tests__/out-of-repo-import-shadow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* A bare JS/TS call through a name imported from outside the repository
* (`import { resolve } from 'node:path'`) means the package's function, never
* another file's same-named symbol. A declaration of that name in an inner
* scope of the calling file still shadows the import, and the call is that
* local declaration's.
*/

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;

function project(files: Record<string, string>): void {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-import-shadow-'));
for (const [rel, content] of Object.entries(files)) {
const abs = path.join(tempDir, rel);
fs.mkdirSync(path.dirname(abs), { recursive: true });
fs.writeFileSync(abs, content);
}
}

async function callTargets(caller: string): Promise<string[]> {
cg = await CodeGraph.init(tempDir, { index: true });
cg.resolveReferences();
const from = cg.getNodesByKind('function').find((n) => n.name === caller)!;
expect(from).toBeDefined();
return cg.getOutgoingEdges(from.id).filter((e) => e.kind === 'calls')
.map((e) => { const t = cg!.getNode(e.target)!; return `${t.filePath}:${t.name}:${t.startLine}`; });
}

afterEach(() => {
cg?.close();
cg = null;
fs.rmSync(tempDir, { recursive: true, force: true });
});

describe('a name imported from outside the repository', () => {
it('is shadowed by a declaration in an inner scope of the calling file', async () => {
project({
'plugin.ts': "export function resolve() { return 'plugin'; }",
'config.ts':
"import { resolve } from 'node:path';\n" +
"export function configure() {\n function resolve() { return 'local'; }\n return resolve();\n}\n",
});
expect(await callTargets('configure')).toEqual(['config.ts:resolve:3']);
});

it('is not shadowed by a declaration nested in a sibling function', async () => {
// `b` calls the package's `resolve`: the one nested in `a` is out of scope there.
project({
'plugin.ts': "export function resolve() { return 'plugin'; }",
'config.ts':
"import { resolve } from 'node:path';\n" +
"export function a() {\n function resolve() { return 'local'; }\n return resolve();\n}\n" +
"export function b() {\n return resolve('src');\n}\n",
});
expect(await callTargets('b')).toEqual([]);
});

it('is not shadowed by a declaration nested a level below the call', async () => {
project({
'config.ts':
"import { resolve } from 'node:path';\n" +
"export function outer() {\n function mid() { function resolve() { return 1; } return resolve(); }\n return resolve('x') + mid();\n}\n",
});
expect((await callTargets('outer')).filter((t) => t.includes(':resolve:'))).toEqual([]);
});

it('is shadowed in a function nested inside the declaring one', async () => {
project({
'config.ts':
"import { resolve } from 'node:path';\n" +
"export function configure() {\n function resolve() { return 'local'; }\n function inner() { return resolve(); }\n return inner();\n}\n",
});
expect(await callTargets('inner')).toEqual(['config.ts:resolve:3']);
});

it('binds no other file\'s symbol of that name', async () => {
project({
'package.json': JSON.stringify({ name: 'app', dependencies: { 'external-resolver': '^1.0.0' } }),
'plugin.ts': "export function resolve() { return 'plugin'; }",
'config.ts': "import { resolve } from 'external-resolver';\nexport function configure() { return resolve('src'); }\n",
});
expect(await callTargets('configure')).toEqual([]);
});

it('is not shadowed by its own top-level require binding', async () => {
project({
'package.json': JSON.stringify({ name: 'app', devDependencies: { supertest: '^6' } }),
'app.js': "var request = require('supertest');\nfunction run(app) { return request(app); }\nmodule.exports = run;\n",
});
expect(await callTargets('run')).toEqual([]);
});
});
29 changes: 25 additions & 4 deletions src/resolution/name-matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11787,6 +11787,23 @@ function bindsAtSites(source: string, name: string, sites: LocalBindingSites): b
return false;
}

/**
* The calling file's declarations of an out-of-repo imported name that sit
* inside a function or method — only those can shadow the import. A top-level
* one is the import's own binding (`request = require('supertest')`), not a
* shadow of it.
*/
function innerScopeShadows(nodes: Node[], ref: UnresolvedRef, context: ResolutionContext): Node[] {
const local = nodes.filter((n) => n.filePath === ref.filePath);
if (local.length === 0) return local;
const scopes = context.getNodesInFile(ref.filePath).filter((n) => n.kind === 'function' || n.kind === 'method');
return local.filter((n) => scopes.some((s) =>
s.id !== n.id &&
s.startLine <= n.startLine &&
(s.endLine ?? s.startLine) >= (n.endLine ?? n.startLine) &&
(s.startLine < n.startLine || (s.endLine ?? s.startLine) > (n.endLine ?? n.startLine))));
}

/**
* Try to resolve a reference by exact name match
*/
Expand Down Expand Up @@ -11814,14 +11831,16 @@ export function matchByExactName(
/^[A-Za-z_$][\w$]*$/.test(ref.referenceName) && isOutOfRepoBinding(ref.referenceName, ref, context)) {
return null;
}
// `import { useQuery } from '@tanstack/react-query'`: the call means the
// package's, and no other file's same-named symbol. A declaration of the
// name in this file's inner scope still shadows the import.
let outOfRepoCall = false;
if (bareJs) {
const storeAction = matchJsStoreBindingCall(ref, context);
if (storeAction) return storeAction;
const returned = matchDestructuredCallResult(ref, context);
if (returned) return returned;
// `import { useQuery } from '@tanstack/react-query'`: the call means the
// package's, and no same-named project symbol.
if (isOutOfRepoBinding(ref.referenceName, ref, context)) return null;
outOfRepoCall = isOutOfRepoBinding(ref.referenceName, ref, context);
}
// Every rule below judges one candidate on its own, so they run as ONE pass,
// the kind/language checks before the ones that read source: a common name
Expand All @@ -11833,7 +11852,9 @@ export function matchByExactName(
// Read once, and only for a candidate in another file: it scans the file.
let bareTypeName: boolean | undefined;
const tsBareTypeName = (): boolean => (bareTypeName ??= isTsBareTypeName(ref, context));
const sameName = context.getNodesByName(ref.referenceName);
const sameName = outOfRepoCall
? innerScopeShadows(context.getNodesByName(ref.referenceName), ref, context)
: context.getNodesByName(ref.referenceName);
// `NAME(...)` where NAME is a function-like macro somewhere in the project is
// an expansion or a call to a same-named function — never the macro itself
// (#1839), and never a type that happens to share the name (#2070: expat's
Expand Down