mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-19 23:20:31 +08:00
fix(js,ts): link calls through an import whose name contains $ (#2378)
The ES import regex in extractJSImports captured default and namespace
bindings with \w+, which stops at a `$`. So `import items$ from './store'`,
`import $ from './dom'` and `import * as ns$ from './ns'` failed the whole
statement and produced no import mapping, and `items$.getState().inc()`
never reached the store action. The `as` split of named imports, the
`a: b` split of a CommonJS destructuring and the re-export checks had the
same \w-only shape. Some bound a cut-off name instead (`{ a as b$ }` bound
`b`, `{ default as items$ }` bound `items`), and `export { a$ } from` was
dropped from barrel chasing.
Capture every binding with the ASCII identifier class [A-Za-z_$][\w$]*,
keeping the `type`-modifier lookahead: `import type from './x'` still
binds `type`, and `import type $T` / `import type from$` now bind. A
default binding is never followed by `{`, so the `${` of a code
generator's template literal (`import ${x} from '${src}'`, as in qwik's
optimizer) is not taken for a binding.
importShadowedAt and hasParameterBinding bounded the name with \b, which
never fires beside a `$`, so a local `const items$ = ...` or an `items$`
parameter did not shadow the imported store. Named `$` imports already
got those wrong edges on main, and the mapping fix would have extended
them to default and namespace imports. Bound the name with (?<![\w$]) and
(?![\w$]) instead.
Resolution-only: the kernel has no import-mapping mirror. A store
exported by `export { items$ }` (the \b in isExportedLater's clause
branch, left out of #2366) is a separate extraction fix.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -46,6 +46,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In Dart, what a top-level variable's or a field's initializer calls, creates and reads is now linked. In Riverpod apps, where most wiring lives in provider closures like `final repoProvider = Provider((ref) => Repository(ref.watch(dioProvider)));`, `callers`, impact and `codegraph_explore` now follow a provider to the code it builds instead of stopping at the provider. A `final` or `const` initializer belongs to its constant, and a `var`'s or a field's to its file or class. A call through a type's static constant, like `FutureProvider.autoDispose(…)`, now links to that constant instead of an unrelated method that shares its name. Re-index Dart projects after upgrading.
|
||||
- In Dart, a call further down a chain, like Riverpod's `FutureProvider.autoDispose.family(…)` or `events.map(mapper).transform(…)` on a stream, now links to the method of the type the chain before it produces, worked out from what each step is declared to return, and to nothing when that type can't be worked out. Before, it linked to whichever project method shared its name, so `X.autoDispose.family(…)` reached the same provider builder whatever `X` was, and a stream's `transform` reached an unrelated `transform` method. `super.` calls now reach the superclass's method, `context.read<CounterBloc>().add(…)` reaches the bloc's own `add`, and the members of an extension marked `@internal` are found too. Re-index Dart projects after upgrading. (#750)
|
||||
- In VB.NET, reading or setting a field or property through a variable, parameter, field, `Me` or `MyBase`, like `x.Normal = 3`, `Me._h.Title`, `.Value` inside a `With` block or `.Switch` in `New BoolParam With {…}`, now counts as a use of that member, and so do calls inside a `With` block, functions called without parentheses and reads through a class whose name starts in lower case. Before, `codegraph callers` on an instance field or property missed most of its uses. Only a member that the value's declared type has or inherits is linked, nothing when that type comes from outside your project, and a value named after its class, like `theme As Theme`, counts as a use of the class only when it reads a `Shared` member. Re-index VB.NET projects after upgrading.
|
||||
- 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.
|
||||
|
||||
## [1.6.2] - 2026-10-03
|
||||
|
||||
|
||||
@@ -6,13 +6,17 @@
|
||||
* `type.innerType()` (a parameter named `type`) was taken for a call on an
|
||||
* import. An inline `{ util, type objectUtil }` named its binding
|
||||
* `type objectUtil`.
|
||||
*
|
||||
* A binding is any JS identifier, `$` included. The import regexes matched
|
||||
* names with `\w`, which stops at a `$`: `import items$ from './store'` and
|
||||
* `import * as ns$` yielded no binding at all, and `{ a as b$ }` bound `b`.
|
||||
*/
|
||||
import { describe, it, expect, afterAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import { extractImportMappings } from '../src/resolution/import-resolver';
|
||||
import { extractImportMappings, extractReExports } from '../src/resolution/import-resolver';
|
||||
|
||||
const bindings = (source: string) =>
|
||||
extractImportMappings('src/a.ts', source, 'typescript').map((m) => `${m.localName}<${m.exportedName}${m.isNamespace ? ' ns' : ''}`);
|
||||
@@ -34,6 +38,90 @@ describe('import mappings and TypeScript type modifiers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('import bindings whose names contain `$`', () => {
|
||||
it('default, namespace and mixed imports bind the whole name', () => {
|
||||
expect(bindings(`import items$ from './store';\n`)).toEqual(['items$<default']);
|
||||
expect(bindings(`import $store from './store';\n`)).toEqual(['$store<default']);
|
||||
expect(bindings(`import * as ns$ from './ns';\n`)).toEqual(['ns$<* ns']);
|
||||
expect(bindings(`import $, { ajax } from 'jquery';\n`)).toEqual(['$<default', 'ajax<ajax']);
|
||||
expect(bindings(`import dflt$, * as $ns from './both';\n`)).toEqual(['dflt$<default', '$ns<* ns']);
|
||||
});
|
||||
|
||||
it('aliased and type-only imports bind the whole name', () => {
|
||||
expect(bindings(`import { a$ as b$, a as $b, default as items$ } from './ab';\n`)).toEqual(['b$<a$', '$b<a', 'items$<default']);
|
||||
expect(bindings(`import type $T from './t';\n`)).toEqual(['$T<default']);
|
||||
expect(bindings(`import { type $T, type U$ as V$ } from './t';\n`)).toEqual(['$T<$T', 'V$<U$']);
|
||||
// `from$` is a name, not the `from` that ends a bare `import type from …`.
|
||||
expect(bindings(`import type from$ from './t';\n`)).toEqual(['from$<default']);
|
||||
});
|
||||
|
||||
it("a code generator's `${…}` is no binding", () => {
|
||||
expect(bindings("const code = `import ${name} from '${src}'`;\n")).toEqual([]);
|
||||
expect(bindings("const code = `import type ${name} from '${src}'`;\n")).toEqual([]);
|
||||
});
|
||||
|
||||
it('a CommonJS destructuring binds the whole name', () => {
|
||||
expect(bindings(`const { a$: b$, a: $c, d$ } = require('./r');\n`)).toEqual(['b$<a$', '$c<a', 'd$<d$']);
|
||||
});
|
||||
|
||||
it('a re-export names the whole name', () => {
|
||||
expect(extractReExports(`export { a$, $b as c$, default as $store } from './x';\n`, 'typescript')).toEqual([
|
||||
{ kind: 'named', exportedName: 'a$', originalName: 'a$', source: './x' },
|
||||
{ kind: 'named', exportedName: 'c$', originalName: '$b', source: './x' },
|
||||
{ kind: 'named', exportedName: '$store', originalName: 'default', source: './x' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a `$` binding imported from another file', () => {
|
||||
const roots: string[] = [];
|
||||
afterAll(() => {
|
||||
for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('`items$.getState().inc()` reaches the store action unless a local or parameter shadows `items$`', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-import-dollar-'));
|
||||
roots.push(root);
|
||||
const files: Record<string, string> = {
|
||||
'package.json': JSON.stringify({ name: 'shop', dependencies: { zustand: '*' } }),
|
||||
'src/store.ts': `import { create } from 'zustand';
|
||||
|
||||
const items$ = create((set) => ({ inc: () => set({}) }));
|
||||
export default items$;
|
||||
`,
|
||||
'src/util.ts': `export function helper() { return 1; }
|
||||
`,
|
||||
'src/decoy.ts': `export function helper() { return 2; }
|
||||
`,
|
||||
'src/use.ts': `import items$ from './store';
|
||||
import * as util$ from './util';
|
||||
|
||||
export function clickInc() { items$.getState().inc(); }
|
||||
export function shadowed() { const items$ = other(); items$.getState().inc(); }
|
||||
export function param(items$: Store) { items$.getState().inc(); }
|
||||
export function run() { return util$.helper(); }
|
||||
`,
|
||||
};
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
const cg = await CodeGraph.init(root, { index: true });
|
||||
try {
|
||||
const callers = (file: string, name: string) => {
|
||||
const target = cg.getNodesInFile(file).find((n) => n.kind === 'function' && n.name === name);
|
||||
expect(target, `${file}: ${name}`).toBeDefined();
|
||||
return cg.getCallers(target!.id).filter((c) => c.edge.kind === 'calls').map((c) => c.node.name).sort();
|
||||
};
|
||||
expect(callers('src/store.ts', 'inc')).toEqual(['clickInc']);
|
||||
expect(callers('src/util.ts', 'helper')).toEqual(['run']);
|
||||
expect(callers('src/decoy.ts', 'helper')).toEqual([]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('a name imported from a package names nothing in the project', () => {
|
||||
const roots: string[] = [];
|
||||
afterAll(() => {
|
||||
|
||||
@@ -964,7 +964,11 @@ function extractJSImports(content: string): ImportMapping[] {
|
||||
// type-only form, not a default import named `type` — which every such
|
||||
// line used to add, making `type.innerType()` a call on an import.
|
||||
// (`import type from './x'` still binds `type`: backtracking gives it back.)
|
||||
const importRegex = /import\s+(?:type\s+(?=[{*]|(?!from\b)\w))?(?:(\w+)\s*,?\s*)?(?:\{([^}]+)\})?\s*(?:(\*)\s+as\s+(\w+))?\s*from\s*['"]([^'"]+)['"]/g;
|
||||
// A binding is any JS identifier, `$` included: `\w` stops at the `$` of
|
||||
// `import $store from './store'`, failing the whole statement. A default
|
||||
// binding is never followed by `{`, so the `${` of a code generator's
|
||||
// `` `import ${name} from '${src}'` `` is no binding.
|
||||
const importRegex = /import\s+(?:type\s+(?=[{*]|(?!from(?![\w$]))[A-Za-z_$]))?(?:([A-Za-z_$][\w$]*)(?![\w${])\s*,?\s*)?(?:\{([^}]+)\})?\s*(?:(\*)\s+as\s+([A-Za-z_$][\w$]*))?\s*from\s*['"]([^'"]+)['"]/g;
|
||||
|
||||
let match;
|
||||
while ((match = importRegex.exec(content)) !== null) {
|
||||
@@ -984,9 +988,9 @@ function extractJSImports(content: string): ImportMapping[] {
|
||||
// Named imports
|
||||
if (namedImports) {
|
||||
// `{ util, type objectUtil }`: an inline `type` modifier is not part of the name.
|
||||
const names = namedImports.split(',').map((s) => s.trim().replace(/^type\s+(?=\w)/, ''));
|
||||
const names = namedImports.split(',').map((s) => s.trim().replace(/^type\s+(?=[A-Za-z_$])/, ''));
|
||||
for (const name of names) {
|
||||
const aliasMatch = name.match(/(\w+)\s+as\s+(\w+)/);
|
||||
const aliasMatch = name.match(/([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)/);
|
||||
if (aliasMatch) {
|
||||
mappings.push({
|
||||
localName: aliasMatch[2]!,
|
||||
@@ -1038,7 +1042,7 @@ function extractJSImports(content: string): ImportMapping[] {
|
||||
if (destructured) {
|
||||
const names = destructured.split(',').map((s) => s.trim());
|
||||
for (const name of names) {
|
||||
const aliasMatch = name.match(/(\w+)\s*:\s*(\w+)/);
|
||||
const aliasMatch = name.match(/([A-Za-z_$][\w$]*)\s*:\s*([A-Za-z_$][\w$]*)/);
|
||||
if (aliasMatch) {
|
||||
mappings.push({
|
||||
localName: aliasMatch[2]!,
|
||||
@@ -1373,7 +1377,7 @@ export function extractReExports(content: string, language: Language): ReExport[
|
||||
for (const raw of inner.split(',')) {
|
||||
const item = raw.trim();
|
||||
if (!item) continue;
|
||||
const aliasMatch = item.match(/^(\w+)\s+as\s+(\w+)$/);
|
||||
const aliasMatch = item.match(/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/);
|
||||
if (aliasMatch) {
|
||||
out.push({
|
||||
kind: 'named',
|
||||
@@ -1381,7 +1385,7 @@ export function extractReExports(content: string, language: Language): ReExport[
|
||||
originalName: aliasMatch[1]!,
|
||||
source,
|
||||
});
|
||||
} else if (/^\w+$/.test(item)) {
|
||||
} else if (/^[A-Za-z_$][\w$]*$/.test(item)) {
|
||||
out.push({
|
||||
kind: 'named',
|
||||
exportedName: item,
|
||||
|
||||
@@ -11171,7 +11171,8 @@ function importShadowedAt(name: string, ref: UnresolvedRef, context: ResolutionC
|
||||
return stack;
|
||||
};
|
||||
const scope = stackAt(code.length);
|
||||
const declarations = new RegExp(`\\b(?:const|let|var|function|class)\\s+(?:${escaped}\\b|\\{[^}]*\\b${escaped}\\b)`, 'g');
|
||||
// Not `\b`: there is no word boundary beside the `$` of `items$` or `$store`.
|
||||
const declarations = new RegExp(`\\b(?:const|let|var|function|class)\\s+(?:${escaped}(?![\\w$])|\\{[^}]*(?<![\\w$])${escaped}(?![\\w$]))`, 'g');
|
||||
return [...code.matchAll(declarations)].some(m => stackAt(m.index!).every((p, i) => scope[i] === p));
|
||||
}
|
||||
|
||||
@@ -11179,8 +11180,8 @@ function importShadowedAt(name: string, ref: UnresolvedRef, context: ResolutionC
|
||||
* parentheses must not make the outer shadow invisible. Conservative when a
|
||||
* parameter's type mentions the same name: leave that call unresolved. */
|
||||
function hasParameterBinding(code: string, escapedName: string): boolean {
|
||||
const name = new RegExp(`\\b${escapedName}\\b`);
|
||||
if (new RegExp(`\\b${escapedName}\\s*=>`).test(code)) return true;
|
||||
const name = new RegExp(`(?<![\\w$])${escapedName}(?![\\w$])`);
|
||||
if (new RegExp(`(?<![\\w$])${escapedName}\\s*=>`).test(code)) return true;
|
||||
for (let i = 0; i < code.length; i++) {
|
||||
if (code[i] !== '(' || /\b(?:if|while|for|switch|with)\s*$/.test(code.slice(0, i))) continue;
|
||||
let depth = 1, j = i + 1;
|
||||
|
||||
Reference in New Issue
Block a user