mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-25 03:49:59 +08:00
fix(js,ts): a nested function takes calls to a name the file imports from a package
When a JavaScript or TypeScript file imports a name from outside the
repository, like import { resolve } from 'node:path', matchByExactName
returns no edge for a bare call of that name (the early return main added
for issue 2101). A call to a function declared inside the calling function
under that name, which shadows the import, lost its edge with it.
The candidates for such a call are now narrowed to the calling file's
declarations nested in a function or method (innerScopeShadows), instead
of giving up; a top-level one is the import's own binding. Lexical
reachability, applied later, keeps only the nested declarations visible
where the call is.
Our changes: tests for a sibling function's nested declaration and one
nested a level below the call, which neither may take the call (both hold
through the reachability rule), and for a call from a function nested in
the declaring one; the CHANGELOG bullet moved next to the JS/TS import
bullets, with credit.
Validation: the tests fail 2 of 6 on main and pass here. A/B against main
on 8 repos, kernel per arm: flask, gin, excalidraw, codegraph, pixijs,
pdf.js and three.js byte-identical; vite gains 1 right link, scanImports'
call to its nested scan() beside import { scan } from
'rolldown/experimental'. Index CPU
time on codegraph, test files full of vitest imports, within noise:
81.7 s to 81.9 s (median of 3, interleaved).
Original PR #1715 by @danusha2345.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
0d46dba08f
commit
428b3c487d
@@ -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.
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
*/
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user