mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-21 00:38:18 +08:00
fix(js,ts): a function value imported from a package no longer links a project function of that name (#2671)
A function passed as a value (`styled(Link)`, `createContext({ onClose:
noop })`, `Chart.register(Legend)`) whose name the file imports from a
package is that package's function. matchFunctionRef still bound it to
the one project function of that name whenever there was exactly one:
outline's react-router-dom `Link` landed on an email template's
`Footer.tsx:Link`, cord-field's lodash `noop` on a test's own stub.
The cross-file fallback now consults the same out-of-repo guard the
call and value paths use (isOutOfRepoBinding). A same-file declaration
still wins (a local that shadows the import is what the name means
there), a project import still resolves through its import, and a
specifier no package.json declares may be an alias and stays the
project's.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -164,6 +164,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In React, a component's JSX no longer links it to a class or component that only shares a name with one of its type arguments or with a variable of its own. A type in angle brackets, like `Document` in `<PaginatedList<Document> items={…} />`, `User` in `useState<User>()` or `Entry` in a generic `<Entry extends BaseEntity>(…) =>` component, is no longer read as a tag. A tag naming a variable or parameter the component sets itself, like `<Content>` after `const Content = isDropdown ? DropdownMenu.SubContent : ContextMenu.SubContent` or `<Widget>` in `widgets.map((Widget) => <Widget />)`, now links to nothing, unless the component declares a component of that name inside itself. Before, these linked to an unrelated class or component elsewhere in the repository: outline's document lists showed up among the callers of its `Document` model class, and its menus among the callers of the command bar's `Content`, so `codegraph_explore`, callers and impact followed renders that never happen. Re-index React projects after upgrading.
|
||||
- In React, a tag a component only mentions in a comment or a string no longer links it to that component: a JSDoc example, markup commented out like `{/* <Progress /> */}`, or an error message like react-router's "You cannot render a <Router> inside another <Router>" or shadcn's "useCarousel must be used within a <Carousel />". Nothing in a TypeScript `.ts` file, which can't hold JSX, is read as a tag either, like the XML in outline's OpenSearch template. Before, these counted as renders, so react-router's hooks showed up among the callers of `Router`, `Routes` and `Route`, and `codegraph_explore`, callers and impact followed renders that never happen. A tag after an apostrophe, a URL or a backtick in a component's text, like `<p>Don't have an account? <Link to="/signup">`, still links as before. Re-index React projects after upgrading.
|
||||
- In JavaScript and TypeScript, a name imported from a package your `package.json` lists no longer links to a project symbol that only shares its name when `tsconfig.json` or `jsconfig.json` has a catch-all path alias, like `"*": ["./typings/*"]` or `"*": ["src/*", "node_modules/*"]`, or an alias that points the package at a file in `node_modules`, like `"lit/decorators": ["./node_modules/lit/decorators.js"]`. Such an alias made every package look like part of the project, so `import { Typography } from '@mui/material'` was linked to the project's own `Typography` and every lit `@property()` decorator to an unrelated class's `property` field, and `codegraph callers`, impact and `codegraph affected` listed code that never used them. An import the alias does map to a file of your project, like a `.d.ts` you keep for an untyped package or `components/Button` through `"*": ["src/*"]`, links as before. Re-index affected projects after upgrading.
|
||||
- In JavaScript and TypeScript, a function the file imports from a package and passes along as a value, like react-router's `Link` in `styled(Link)`, lodash's `noop` in `createContext({ onClose: noop })` or chart.js's `Legend` in `Chart.register(Legend)`, no longer links to a project function that only shares its name. Before, whenever the project had exactly one function of that name, every such value linked to it: outline's breadcrumbs, menus and document lists showed up among the callers of an email template's `Link`, and a test's own `noop` stub in cord-field among the code the app's contexts use, so `codegraph callers`, impact and `codegraph_explore` followed references that never happen. A function the file declares itself, including one that hides the import inside a function, and one imported from a file of your project link as before. Re-index JavaScript and TypeScript projects after upgrading.
|
||||
- `codegraph_explore`'s Flow now goes through the implementation your query names when an interface method has several that lead to the same place. Before, it took whichever one came first in the index: asking about prometheus's `Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier` traced the call through the TSDB's `DB.Querier` and left the `fanout.Querier` you named off the Flow. Any two routes of the same length are now settled this way, in favor of the one that passes through more of the symbols you named.
|
||||
- In C and C++, a struct, class, union or enum defined in a variable declaration, like `struct Foo { … } foo;`, is now indexed with its methods and enum values, next to the variable. Before, at most the variable was, so the type was missing from search and `codegraph_explore`, a class deriving from it had no base, and calls to its methods linked to nothing: in redis, the Sentinel's `sentinelState` and `redis-cli`'s `config` struct were missing. A struct, union or enum with no name of its own now takes the name of the variable it declares, so redis's `static struct { … } SPT;` is found as `SPT`, and one declared inside a function is named after its variable instead of `<anonymous>`. The comment above the declaration becomes the type's description. Re-index C and C++ projects after upgrading.
|
||||
- In TypeScript React code, a styled-components or emotion component declared with a type argument, like `const Wrapper = styled.div<WrapperProps>` followed by its CSS, or `styled(Base)<Props>`, is now indexed as a component, as one written without a type argument already was. Before, it was indexed as a plain constant, so `codegraph callers` and impact found none of the code that renders it, and its tags could link to another file's component of the same name instead: in outline/outline, `NudeButton` and `HStack` had no callers, and `<Text>` tags linked to a different `Text` than the one each file imports. Re-index TypeScript React projects after upgrading.
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
/**
|
||||
* A function passed as a value (`styled(Link)`, `createContext({ onClose:
|
||||
* noop })`, `register(debounce)`) whose name the file imports from a package
|
||||
* is that package's function. The function-as-value matcher still bound it to
|
||||
* the one project function of that name whenever there was exactly one:
|
||||
* outline's `styled(Link)`, with `Link` from react-router-dom, landed on an
|
||||
* email template's `Footer.tsx:Link`, and cord-field's lodash `noop` on a
|
||||
* test file's local `const noop = () => {}`.
|
||||
*
|
||||
* The import says which `Link` it is. A name the file imports from a package
|
||||
* its package.json declares (or a Node built-in) names nothing in the
|
||||
* repository; a project import still resolves through its import, a
|
||||
* specifier nothing declares may be an alias the resolver doesn't know, and a
|
||||
* declaration in the same file still wins.
|
||||
*/
|
||||
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';
|
||||
|
||||
const roots: string[] = [];
|
||||
afterAll(() => {
|
||||
for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function indexProject(files: Record<string, string>): Promise<CodeGraph> {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-fnref-package-'));
|
||||
roots.push(root);
|
||||
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);
|
||||
}
|
||||
return CodeGraph.init(root, { index: true });
|
||||
}
|
||||
|
||||
/** `<value name> -> <target file>:<target qualified name>` for each function-as-value edge leaving `file`. */
|
||||
function fnRefsFrom(cg: CodeGraph, file: string): string[] {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return [...new Set(cg
|
||||
.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => e.metadata?.fnRef === true)
|
||||
.map((e) => {
|
||||
const target = cg.getNode(e.target);
|
||||
return `${String(e.metadata?.refName)} -> ${target?.filePath}:${target?.qualifiedName}`;
|
||||
}))]
|
||||
.sort();
|
||||
}
|
||||
|
||||
describe('a function value the file imports from a package', () => {
|
||||
it('never lands on a project function of the same name', async () => {
|
||||
const cg = await indexProject({
|
||||
'package.json': JSON.stringify({
|
||||
name: 'wiki',
|
||||
dependencies: { react: '^18.2.0', 'react-router-dom': '^6.22.0', 'styled-components': '^6.1.0', lodash: '^4.17.21' },
|
||||
}),
|
||||
'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', jsx: 'react-jsx', paths: { '~/*': ['./app/*'] } } }),
|
||||
// The project's only `Link`, `noop`, `debounce` and `format` functions.
|
||||
'server/emails/Footer.tsx': `export function Link(props: { href: string; children?: string }) {
|
||||
return <a href={props.href}>{props.children}</a>;
|
||||
}
|
||||
`,
|
||||
'app/utils/noop.ts': `export function noop(): void {}
|
||||
`,
|
||||
'app/utils/debounce.ts': `export function debounce(fn: () => void): () => void { return fn; }
|
||||
`,
|
||||
'app/utils/format.ts': `export function format(value: number): string { return String(value); }
|
||||
`,
|
||||
// outline's Breadcrumb: react-router-dom's `Link`, wrapped by styled-components.
|
||||
'app/components/Breadcrumb.tsx': `import { Link } from 'react-router-dom';
|
||||
import styled from 'styled-components';
|
||||
|
||||
const Item = styled(Link)\`
|
||||
display: flex;
|
||||
\`;
|
||||
|
||||
export function Crumb() {
|
||||
return <Item to="/">home</Item>;
|
||||
}
|
||||
`,
|
||||
// A named import, a default import of a package path, and a Node built-in.
|
||||
'app/hooks/useDeferred.ts': `import { noop } from 'lodash';
|
||||
import debounce from 'lodash/debounce';
|
||||
import { format } from 'node:util';
|
||||
|
||||
export const defaults = { onClose: noop };
|
||||
|
||||
export function useDeferred(register: (cb: unknown) => void): void {
|
||||
register(debounce);
|
||||
register(format);
|
||||
}
|
||||
`,
|
||||
});
|
||||
try {
|
||||
expect({
|
||||
breadcrumb: fnRefsFrom(cg, 'app/components/Breadcrumb.tsx'),
|
||||
deferred: fnRefsFrom(cg, 'app/hooks/useDeferred.ts'),
|
||||
}).toEqual({ breadcrumb: [], deferred: [] });
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it('never lands on one under a catch-all path alias either', async () => {
|
||||
const cg = await indexProject({
|
||||
'package.json': JSON.stringify({ name: 'cord', dependencies: { react: '^18.2.0', lodash: '^4.17.21' } }),
|
||||
// cord-field's shape: `~/…` for the project's own files, `*` for local
|
||||
// type declarations of untyped packages.
|
||||
'tsconfig.json': JSON.stringify({
|
||||
compilerOptions: { baseUrl: 'src', jsx: 'react-jsx', paths: { '~/*': ['./*'], '*': ['./typings/*'] } },
|
||||
}),
|
||||
'src/scenes/Users/UserForm.test.tsx': `const noop = () => {
|
||||
// a stub
|
||||
};
|
||||
|
||||
export function renderForm(onSubmit: () => void) {
|
||||
return onSubmit;
|
||||
}
|
||||
|
||||
renderForm(noop);
|
||||
`,
|
||||
'src/components/form/AllowClose.ts': `import { noop } from 'lodash';
|
||||
import { createContext } from 'react';
|
||||
|
||||
export const AllowFormCloseContext = createContext({ allow: noop });
|
||||
`,
|
||||
});
|
||||
try {
|
||||
expect(fnRefsFrom(cg, 'src/components/form/AllowClose.ts')).toEqual([]);
|
||||
// The test's own `noop` is still what the test passes.
|
||||
expect(fnRefsFrom(cg, 'src/scenes/Users/UserForm.test.tsx')).toEqual([
|
||||
'noop -> src/scenes/Users/UserForm.test.tsx:noop',
|
||||
]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it('keeps a project import, an undeclared specifier and a same-file declaration resolving', async () => {
|
||||
const cg = await indexProject({
|
||||
'package.json': JSON.stringify({ name: 'kit', dependencies: { react: '^18.2.0', lodash: '^4.17.21' } }),
|
||||
'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', jsx: 'react-jsx' } }),
|
||||
'src/emails/Footer.tsx': `export function Link(props: { href: string }) {
|
||||
return <a href={props.href} />;
|
||||
}
|
||||
`,
|
||||
'src/lib/handlers.ts': `export function onSave(): void {}
|
||||
`,
|
||||
// A project import resolves through the import.
|
||||
'src/menus/Menu.tsx': `import { Link } from '../emails/Footer';
|
||||
|
||||
export function menuItems(wrap: (c: unknown) => unknown) {
|
||||
return wrap(Link);
|
||||
}
|
||||
`,
|
||||
// SvelteKit's \`$lib\`: no package.json declares it, so it may be the
|
||||
// project's own folder under an alias this resolver doesn't read.
|
||||
'src/routes/page.ts': `import { onSave } from '$lib/handlers';
|
||||
|
||||
export function bind(register: (cb: () => void) => void): void {
|
||||
register(onSave);
|
||||
}
|
||||
`,
|
||||
// A declaration in the file shadows the package's \`noop\` where it is in scope.
|
||||
'src/hooks/stub.ts': `import { noop } from 'lodash';
|
||||
|
||||
export function setup(register: (cb: () => void) => void): void {
|
||||
function noop(): void {}
|
||||
register(noop);
|
||||
}
|
||||
`,
|
||||
});
|
||||
try {
|
||||
expect(fnRefsFrom(cg, 'src/menus/Menu.tsx')).toEqual(['Link -> src/emails/Footer.tsx:Link']);
|
||||
expect(fnRefsFrom(cg, 'src/routes/page.ts')).toEqual(['onSave -> src/lib/handlers.ts:onSave']);
|
||||
expect(fnRefsFrom(cg, 'src/hooks/stub.ts')).toEqual(['noop -> src/hooks/stub.ts:setup::noop']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
}, 60_000);
|
||||
});
|
||||
@@ -1194,9 +1194,13 @@ export function matchFunctionRef(
|
||||
// Cross-file (imported names the import resolver didn't already claim):
|
||||
// only an unambiguous match resolves — or, in Python, the one in reach of
|
||||
// a name the file imports (netbox's `sender=CustomField` beside a test's
|
||||
// own nested `CustomField`).
|
||||
// own nested `CustomField`). Never a name the file imports from a package
|
||||
// outside the repository, which no project function is: outline's
|
||||
// `styled(Link)`, with `Link` from react-router-dom, is not an email
|
||||
// template's `Link`, nor is cord-field's lodash `noop` a test's own stub.
|
||||
if (candidates.length === 1 && (named.length === 1 ||
|
||||
(ref.language === 'python' && pythonFromImports(ref.filePath, context).has(ref.referenceName)))) {
|
||||
(ref.language === 'python' && pythonFromImports(ref.filePath, context).has(ref.referenceName))) &&
|
||||
!(JS_FAMILY.has(ref.language) && isOutOfRepoBinding(ref.referenceName, ref, context))) {
|
||||
return {
|
||||
original: ref,
|
||||
targetNodeId: candidates[0]!.id,
|
||||
|
||||
Reference in New Issue
Block a user