fix(resolution): a binding in a module that exports nothing is not a cross-file candidate (#1719) (#1746)
* fix(resolution): a binding in a module that exports nothing is not a cross-file candidate
On vitejs/vite, 157 cross-file `imports` refs — every `import { defineConfig }
from 'vite'` in the playground and the create-vite templates — resolved onto
`playground/ssr-html/test-stacktrace.js::vite`, which is `const vite = await
createServer(...)` at module scope in a file with zero exports.
Neither existing guard can see it. `isLexicallyReachable` returns early for any
candidate that is not a `function`, and the bare-import guard correctly declines
because `vite` IS a workspace member, so the specifier really is project-local.
What is wrong is only which node the name lands on.
A JS/TS file that contains an `import` statement and no export of any form
offers nothing to any other file, so none of its bindings is a candidate for a
cross-file name match. Applied in both name-based strategies: declining in
matchByExactName alone just hands the same target to matchFuzzy, which resolves
a unique candidate on its own.
Narrow on three axes, each a class this would otherwise get wrong in the
opposite direction: a classic script is exempt (a top-level binding really is a
reachable global), CommonJS is exempt (`module.exports` and `exports.x` count as
exports), and every non-JS/TS language is exempt. The export test reads source
rather than the node's `isExported` flag, because that flag is set only from an
`export_statement` ancestor and so reads false for `const x = ...; export { x }`.
* fix(resolution): count bracket CommonJS exports and `declare global` as exports
A file writing `exports["x"] = …` exports x, and a file with a `declare
global` block contributes every name in it to every other file whether or not
it exports anything of its own — the extractor emits nodes for the ambient
`var` and `interface` members, so sealing such a file would hide names that
really are reachable everywhere. Neither shape occurs on the vite corpus, so
this changes no measured count; both are now covered by the test.
* test(resolution): bind the #1719 fixture without a bare import
The consumer bound every name from 'some-external-pkg'. A bare specifier
names a package that is not in the graph, so no project node is the right
target for such a reference and #1715 declines it -- which made four of the
five positive assertions depend on a resolution that should not happen, and
they failed the moment this branch was stacked on #1715. Free references
reach the same exact-match path without asserting that.
`strayVar` was not testable at all: a bare identifier read emits no edge, so
that assertion only ever passed through the bare-import binding. The
`declare global` coverage moves to an interface reached through a type
annotation, paired with an identical file whose interface is not in a
`declare global` -- so the assertion turns on that clause rather than
passing whichever way the guard goes.
* docs(changelog): record the sealed-module guard under Unreleased
* fix(resolution): the sealed test rejects fuzzy's survivor, never filters its set
matchFuzzy declines an ambiguous name outright, so filtering sealed
candidates out of its set can leave a lone survivor and manufacture a 0.5
edge from an ambiguity that would have been declined. Testing the single
survivor instead closes that path; matchByExactName keeps the filter,
because it ranks a crowd rather than declining one.
No instance on vitejs/vite either way (row-identical, LOST 0 / GAINED 0
per #1720 review). It also declines one shape the filter form resolved: a
sealed same-language survivor no longer yields to a cross-language
candidate at 0.3.
* fix(resolution): reject invalid fallback targets without retargeting
---------
Co-authored-by: Aaron Queen <bompus@users.noreply.github.com>
Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
This commit is contained in:
co-authored by
Aaron Queen
Colby McHenry
parent
2c251e2c61
commit
bffd50e4f1
@@ -3,6 +3,10 @@ import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import { CodeGraph } from '../src';
|
||||
import { DatabaseConnection, getDatabasePath } from '../src/db';
|
||||
import { QueryBuilder } from '../src/db/queries';
|
||||
import { createResolver } from '../src/resolution';
|
||||
import type { Node } from '../src/types';
|
||||
import { initGrammars, loadAllGrammars } from '../src/extraction/grammars';
|
||||
|
||||
beforeAll(async () => {
|
||||
@@ -10,6 +14,54 @@ beforeAll(async () => {
|
||||
await loadAllGrammars();
|
||||
});
|
||||
|
||||
describe('Express middleware imports', () => {
|
||||
it('does not resolve package imports into license headings', async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-express-doc-import-'));
|
||||
let cg: CodeGraph | undefined;
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify({ dependencies: { express: '*', cors: '*' } }));
|
||||
fs.writeFileSync(path.join(tmpDir, 'LICENSE.md'), '# cors\n\n# host-validation-middleware\n');
|
||||
fs.writeFileSync(path.join(tmpDir, 'local.js'), 'export function localMiddleware() {}\n');
|
||||
fs.writeFileSync(path.join(tmpDir, 'server.js'), [
|
||||
"import corsMiddleware from 'cors'",
|
||||
"import { hostValidationMiddleware as originalHostValidationMiddleware } from 'host-validation-middleware'",
|
||||
"import { localMiddleware } from './local.js'",
|
||||
'localMiddleware()',
|
||||
].join('\n'));
|
||||
cg = await CodeGraph.init(tmpDir, { index: true });
|
||||
const local = cg.getNodesByKind('function').find((n) => n.name === 'localMiddleware');
|
||||
expect(local).toBeDefined();
|
||||
expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'imports')).toBe(true);
|
||||
expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'calls')).toBe(true);
|
||||
cg.close();
|
||||
cg = undefined;
|
||||
const db = DatabaseConnection.open(getDatabasePath(tmpDir));
|
||||
try {
|
||||
const queries = new QueryBuilder(db.getDb());
|
||||
for (const name of ['cors', 'host-validation-middleware']) {
|
||||
queries.insertNode({
|
||||
id: `heading:${name}`, name, qualifiedName: `LICENSE.md#${name}`,
|
||||
kind: 'module', language: 'markdown' as Node['language'], filePath: 'LICENSE.md',
|
||||
startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0,
|
||||
});
|
||||
}
|
||||
const resolver = createResolver(tmpDir, queries);
|
||||
for (const referenceName of ['cors', 'corsMiddleware', 'host-validation-middleware']) {
|
||||
expect(resolver.resolveOne({
|
||||
fromNodeId: 'file:server.js', referenceName, referenceKind: 'imports',
|
||||
filePath: 'server.js', language: 'javascript', line: 1, column: 0,
|
||||
})).toBeNull();
|
||||
}
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
} finally {
|
||||
cg?.close();
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Django end-to-end framework extraction', () => {
|
||||
let tmpDir: string | undefined;
|
||||
afterEach(() => {
|
||||
|
||||
Reference in New Issue
Block a user