diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ffa08d93..c67197ba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render()` or `render()`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587) - In Python, `import a.b as X` written inside a function now links its calls like `X.f()`, as the same import at module level already did. Thanks @Egrn for the report. (#2600) - In Python, `from app import email` no longer links the importing file to a project variable named `app`, such as a FastAPI or Flask `app = FastAPI()`. (#2584) - In Python, a call through a module-level instance, like `_svc.delete(obj)` after `_svc = SceneService()`, now links to the method its class has or inherits, even when other classes have a method of that name. (#2582) diff --git a/__tests__/jsx-render-outside-function.test.ts b/__tests__/jsx-render-outside-function.test.ts new file mode 100644 index 000000000..2965d7ba2 --- /dev/null +++ b/__tests__/jsx-render-outside-function.test.ts @@ -0,0 +1,237 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { CodeGraph } from '../src'; + +/** + * JSX written outside a named function, method or component (#2587): in an + * entry file's `createRoot(el).render()`, a top-level + * `const ICONS = { a: }`, or a test's `it('…', () => render())`. + * A plain call in those places is attributed to the file node or the `const`; + * a tag is attributed the same way. + */ +describe('JSX outside a named function (#2587)', () => { + let dir: string; + let cg: any; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'jsx-outside-')); + fs.writeFileSync(path.join(dir, 'package.json'), '{"dependencies":{"react":"^18.0.0"}}'); + }); + + afterEach(() => { + cg?.close?.(); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + const write = (rel: string, body: string) => { + const p = path.join(dir, rel); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, body); + }; + + /** Every jsx-render edge, as `source kind:name@file -> target name@file`. */ + async function renders(): Promise { + cg = await CodeGraph.init(dir, { silent: true }); + await cg.indexAll(); + const db = (cg as any).db.db; + return db + .prepare( + `SELECT s.kind || ':' || s.name || '@' || s.file_path || ' -> ' || t.name || '@' || t.file_path AS e + FROM edges e JOIN nodes s ON s.id = e.source JOIN nodes t ON t.id = e.target + WHERE json_extract(e.metadata, '$.synthesizedBy') = 'jsx-render' ORDER BY e` + ) + .all() + .map((r: any) => r.e); + } + + const CARD = [ + 'export function Card() { return
card
; }', + 'export function Icon() { return ; }', + 'export function Cell() { return ; }', + '', + ].join('\n'); + + it('links a tag at module scope from the file', async () => { + write('src/Card.tsx', CARD); + write('src/main.tsx', [ + 'import { createRoot } from "react-dom/client";', + 'import { Card } from "./Card";', + 'createRoot(document.getElementById("root")!).render();', + '', + ].join('\n')); + expect(await renders()).toEqual(['file:main.tsx@src/main.tsx -> Card@src/Card.tsx']); + }); + + it('links a tag in a top-level const from the const', async () => { + write('src/Card.tsx', CARD); + write('src/icons.tsx', [ + 'import { Icon } from "./Card";', + 'export const ICONS = {', + ' a: ,', + '};', + '', + ].join('\n')); + const out = await renders(); + expect(out).toHaveLength(1); + expect(out[0]).toMatch(/^(constant|variable):ICONS@src\/icons\.tsx -> Icon@src\/Card\.tsx$/); + }); + + it('links a tag in a test callback from the file', async () => { + write('src/Card.tsx', CARD); + write('src/Card.test.tsx', [ + 'import { it } from "vitest";', + 'import { render } from "@testing-library/react";', + 'import { Card } from "./Card";', + 'it("renders", () => {', + ' render();', + '});', + '', + ].join('\n')); + expect(await renders()).toEqual(['file:Card.test.tsx@src/Card.test.tsx -> Card@src/Card.tsx']); + }); + + it('keeps a tag inside a function on that function only', async () => { + write('src/Card.tsx', CARD); + write('src/Table.tsx', [ + 'import { Cell, Card } from "./Card";', + 'export const Row = () => ;', + 'export function Table() {', + ' return ;', + '}', + 'export const ROWS = [1].map(() => );', + '', + ].join('\n')); + const out = await renders(); + expect(out).toContain('function:Table@src/Table.tsx -> Card@src/Card.tsx'); + expect(out.filter((e) => e.startsWith('file:'))).toEqual([]); + expect(out.filter((e) => e.endsWith('-> Cell@src/Card.tsx'))).toHaveLength(1); + expect(out.filter((e) => e.endsWith('-> Card@src/Card.tsx'))).toHaveLength(1); + }); + + it('links an entry whose tags share lines with router routes', async () => { + write('src/Card.tsx', CARD); + write('src/main.tsx', [ + 'import { createRoot } from "react-dom/client";', + 'import { BrowserRouter, Routes, Route } from "react-router-dom";', + 'import { Card, Icon } from "./Card";', + 'createRoot(document.getElementById("root")!).render(', + ' ', + ' ', + ' } />', + ' } />', + ' ', + ' ', + ');', + '', + ].join('\n')); + expect(await renders()).toEqual([ + 'file:main.tsx@src/main.tsx -> Card@src/Card.tsx', + 'file:main.tsx@src/main.tsx -> Icon@src/Card.tsx', + ]); + }); + + it('is not shadowed by a parameter of another function, or by its own import', async () => { + write('src/Card.tsx', CARD); + write('src/Card.test.tsx', [ + 'import { it } from "vitest";', + 'import { render } from "@testing-library/react";', + 'import { Icon, Card, Cell } from "./Card";', + 'function wrap(Card: any) { return Card; }', + 'it("renders", () => {', + ' render();', + '});', + '', + ].join('\n')); + expect((await renders()).filter((e) => e.startsWith('file:'))).toEqual([ + 'file:Card.test.tsx@src/Card.test.tsx -> Card@src/Card.tsx', + ]); + }); + + it('does not link a name a test callback binds itself', async () => { + write('src/Card.tsx', CARD); + write('src/Card.test.tsx', [ + 'import { it, vi } from "vitest";', + 'import { render } from "@testing-library/react";', + 'it("renders a stub", () => {', + ' const Card = vi.fn(() => null);', + ' render();', + '});', + '', + ].join('\n')); + expect(await renders()).toEqual([]); + }); + + it('gives a line two top-level values share to neither', async () => { + write('src/Card.tsx', CARD); + write('src/icons.tsx', [ + 'import { Icon, Cell } from "./Card";', + 'export const a = , b = ;', + '', + ].join('\n')); + expect(await renders()).toEqual([]); + }); + + it('does not credit a value with a statement written after it on its line', async () => { + write('src/Card.tsx', CARD); + write('src/main.tsx', [ + 'import { createRoot } from "react-dom/client";', + 'import { Card } from "./Card";', + 'const root = createRoot(document.body); root.render();', + '', + ].join('\n')); + expect((await renders()).filter((e) => e.includes(':root@'))).toEqual([]); + }); + + it('links every entry of an icon map keyed by component name', async () => { + write('src/Card.tsx', CARD); + write('src/icons.tsx', [ + 'import { Icon, Cell } from "./Card";', + 'export const ICONS = {', + ' Icon: ,', + ' Cell: ,', + '};', + '', + ].join('\n')); + expect((await renders()).map((e) => e.replace(/^\w+:/, ''))).toEqual([ + 'ICONS@src/icons.tsx -> Cell@src/Card.tsx', + 'ICONS@src/icons.tsx -> Icon@src/Card.tsx', + ]); + }); + + it('links a component a test also passes as a value', async () => { + write('src/Card.tsx', CARD); + write('src/Card.test.tsx', [ + 'import { it, expect } from "vitest";', + 'import { render } from "@testing-library/react";', + 'import { Card } from "./Card";', + 'expect(Card).toBeDefined();', + 'it("renders", () => render());', + '', + ].join('\n')); + expect(await renders()).toEqual(['file:Card.test.tsx@src/Card.test.tsx -> Card@src/Card.tsx']); + }); + + it.each([ + ['a block comment before it', '/* note */ export const A = ;\n'], + ['a block comment after it', 'export const A = ; /* x */\n'], + ['a line comment and CRLF', 'export const A = ; // the icon\r\n'], + ])('keeps a value its own line with %s', async (_label, line) => { + write('src/Card.tsx', CARD); + write('src/a.tsx', `import { Icon } from "./Card";\n${line}`); + expect((await renders()).map((e) => e.replace(/^\w+:/, ''))).toEqual(['A@src/a.tsx -> Icon@src/Card.tsx']); + }); + + it('does not attribute a class field tag to the file', async () => { + write('src/Card.tsx', CARD); + write('src/Panel.tsx', [ + 'import { Icon } from "./Card";', + 'export class Panel {', + ' static icon = ;', + '}', + '', + ].join('\n')); + expect((await renders()).filter((e) => e.startsWith('file:'))).toEqual([]); + }); +}); diff --git a/src/resolution/callback-synthesizer.ts b/src/resolution/callback-synthesizer.ts index b0947d248..1fbd4327f 100644 --- a/src/resolution/callback-synthesizer.ts +++ b/src/resolution/callback-synthesizer.ts @@ -1917,6 +1917,16 @@ function writtenBare(src: string, name: string): boolean { return false; } +/** + * Whether the lines a file or top-level value owns (#2587) declare `name` + * (`const Card = vi.fn()` in a test callback). Only those lines are read, so + * a parameter or local of a function elsewhere in the file binds nothing here. + */ +function ownLinesBind(name: string, src: string): boolean { + const n = name.replace(/\$/g, '\\$'); + return new RegExp(`\\b(?:const|let|var)\\s+${n}\\b(?!\\s*[,\\]}])`).test(src); +} + /** * What a tag renders when its parent binds the name itself: a component the * parent declares in its own lines (`const Row = ({ row }) => ` in a @@ -2190,7 +2200,10 @@ function skipSpace(code: string, at: number, step: 1 | -1): number { * Phase 5: React JSX child rendering. A component that returns `` * mounts Child — React calls it — but JSX instantiation isn't a static call edge, * so a render tree (App.render → StaticCanvas → renderStaticScene) breaks at the - * JSX hop. Link parent → each capitalized JSX child it renders. File-oriented + * JSX hop. Link parent → each capitalized JSX child it renders. A parent is a + * method, function or component — or, for a tag outside all of those, the + * top-level value or the file that holds it (#2587), as for a plain call + * there. File-oriented * (read each JSX file once). Precision gates: the parent must write the name as * a tag in its code, not in a comment, a string or a template (react-router's * "You cannot render a inside another "; `jsxCode`), and @@ -2217,6 +2230,7 @@ async function reactJsxChildEdges(ctx: ResolutionContext, onYield: MaybeYield): const edges: Edge[] = []; const seen = new Set(); const PARENT_KINDS = new Set(['method', 'function', 'component']); + const VALUE_PARENT_KINDS = new Set(['constant', 'variable']); // Once per module and export for the whole pass: what a tag imported from // the project renders. The lookup reads and comment-strips the module. const exports = new Map(); @@ -2240,9 +2254,55 @@ async function reactJsxChildEdges(ctx: ResolutionContext, onYield: MaybeYield): // monorepos must not interpret `""` inside C as JSX (#1560). A // TypeScript `.ts` file holds no JSX either: a `` there is a type // assertion or text, like outline's OpenSearch XML template. - const parents = ctx.getNodesInFile(file).filter( - (n) => PARENT_KINDS.has(n.kind) && JSX_FAMILY.includes(n.language) - ); + const inFile = ctx.getNodesInFile(file); + const fns = inFile.filter((n) => PARENT_KINDS.has(n.kind) && JSX_FAMILY.includes(n.language)); + // JSX outside every function renders from where a plain call there is + // attributed (#2587): a top-level `const ICONS = { a: }` from the + // const, `createRoot(el).render()` or a test's + // `it('…', () => render())` from the file. Ownership is by line: + // a value owns the lines of its span no function or class holds and no + // other value shares; the file, the lines no node but itself, an import, + // an export or a framework's route holds. So no tag counts twice. + // The file's lines read as code, split once: a tag in a comment or a + // string is text (`jsxCode`). + let code: string | null | undefined; + const codeOf = () => (code === undefined ? (code = jsxCode(content)) : code); + let lines: string[] | null = null; + const holds = (n: Node) => PARENT_KINDS.has(n.kind) || n.kind === 'class'; + let fileNode = inFile.find((n) => n.kind === 'file' && JSX_FAMILY.includes(n.language)); + const owns = new Map boolean>(); + let values: Node[] = []; + if (fileNode) { + // Per line: 1 when a function or class holds it, 2 when any other node does. + const held = new Uint8Array((fileNode.endLine || 0) + 2); + const valuesOn = new Uint8Array(held.length); + for (const n of inFile) { + if (n.kind === 'file' || n.kind === 'import' || n.kind === 'export' || n.kind === 'route' || + !n.startLine || !n.endLine) continue; + const mark = holds(n) ? 1 : 2; + for (let l = n.startLine; l <= n.endLine && l < held.length; l++) if (held[l] !== 1) held[l] = mark; + } + // A top-level value: its first line is in no function or class. + values = inFile.filter((n) => VALUE_PARENT_KINDS.has(n.kind) && JSX_FAMILY.includes(n.language) && + held[n.startLine] !== 1); + // A line a value shares with another statement (`const root = + // createRoot(el); root.render()`) is no one's — not the value's, + // and not the file's, which owns only lines no node holds: ownership is + // by line, so its tags get no edge rather than a wrong one. Read as + // code, so a comment there is no statement. + const raw = (lines ??= (codeOf() ?? content).split('\n')); + for (const v of values) { + for (let l = v.startLine; l <= v.endLine && l < valuesOn.length; l++) if (valuesOn[l]! < 2) valuesOn[l]!++; + const before = (raw[v.startLine - 1] ?? '').slice(0, v.startColumn ?? 0); + const after = (raw[v.endLine - 1] ?? '').slice(v.endColumn ?? Infinity); + if (!/^\s*(?:(?:export|declare|default|const|let|var)\s+)*$/.test(before)) valuesOn[v.startLine] = 2; + if (!/^[\s;]*$/.test(after)) valuesOn[v.endLine] = 2; + } + for (const v of values) owns.set(v.id, (line) => held[line] !== 1 && valuesOn[line] === 1); + if (held.subarray(1, fileNode.endLine + 1).includes(0)) owns.set(fileNode.id, (line) => !held[line]); + else fileNode = undefined; + } + const parents = [...fns, ...values, ...(fileNode ? [fileNode] : [])]; if (parents.length === 0) continue; // Read once per file, and only when a name actually turns out ambiguous. let imports: Map | null = null; @@ -2260,18 +2320,18 @@ async function reactJsxChildEdges(ctx: ResolutionContext, onYield: MaybeYield): // over go-ethereum's graphiql.min.js (2,234 functions on 980 KB). The // file is split once, not once per function, and read as code once: a // tag in a comment or a string is text (`jsxCode`). - let code: string | null | undefined; - const codeOf = () => (code === undefined ? (code = jsxCode(content)) : code); - let lines: string[] | null = null; // Each name a span holds (`spanTags`), read when a function among the - // parents is, or a function around one. + // parents is, or a function around one. A parent that owns its lines + // (#2587) reads only those, the rest left blank, under its own key. const tagsBySpan = new Map(); const spanOf = (fn: Node): SpanTags => { - const span = `${fn.startLine}:${fn.endLine}`; + const own = owns.get(fn.id); + const span = own ? fn.id : `${fn.startLine}:${fn.endLine}`; let names = tagsBySpan.get(span); if (!names) { lines ??= (codeOf() ?? content).split('\n'); - names = spanTags(lines.slice(fn.startLine - 1, fn.endLine).join('\n'), fn.startLine); + const spanLines = lines.slice(fn.startLine - 1, fn.endLine); + names = spanTags((own ? spanLines.map((text, i) => (own(fn.startLine + i) ? text : '')) : spanLines).join('\n'), fn.startLine); tagsBySpan.set(span, names); } return names; @@ -2302,27 +2362,37 @@ async function reactJsxChildEdges(ctx: ResolutionContext, onYield: MaybeYield): if (!line) continue; let child = jsxChild(ctx, name, file, importsOf, ownValueOf, exportOf); if (child) { - // A name the parent, or a function around it, binds by the parent's - // first tag is that local: excalidraw's example app destructures - // `Sidebar` and `Footer` once and renders them in nested `render…` - // functions too. It renders a component that function declares, if - // any (`declaredInside`). - let binder = scopeBinds(names, parent, name, line, file, ctx) ? parent : undefined; - if (!binder) { - around ??= functionsAround(parents, parent); - for (const fn of around) { - if (scopeBinds(spanOf(fn), fn, name, line, file, ctx)) { - binder = fn; - break; + const own = owns.get(parent.id); + if (own) { + // A tag the file or a top-level value holds (#2587): a name its own + // lines declare (`const Card = vi.fn()` in a test's callback), or one + // the file destructures at its top, is that local. + if ((names.bare.has(name) && ownLinesBind(name, names.src)) || destructuredAtTop(name)) { + child = declaredInside(ctx, name, parent, child); + } + } else { + // A name the parent, or a function around it, binds by the parent's + // first tag is that local: excalidraw's example app destructures + // `Sidebar` and `Footer` once and renders them in nested `render…` + // functions too. It renders a component that function declares, if + // any (`declaredInside`). + let binder = scopeBinds(names, parent, name, line, file, ctx) ? parent : undefined; + if (!binder) { + around ??= functionsAround(fns, parent); + for (const fn of around) { + if (scopeBinds(spanOf(fn), fn, name, line, file, ctx)) { + binder = fn; + break; + } } } + if (binder) child = declaredInside(ctx, name, binder, child); + // A name the file destructures at its top is every function's local: + // refine's pages take `const { Title, Text } = Typography` from antd + // and rendered two other examples' `Title` and `Text`. A function + // that declares the name inside itself still renders its own. + else if (destructuredAtTop(name)) child = declaredInside(ctx, name, around?.[around.length - 1] ?? parent, child); } - if (binder) child = declaredInside(ctx, name, binder, child); - // A name the file destructures at its top is every function's local: - // refine's pages take `const { Title, Text } = Typography` from antd - // and rendered two other examples' `Title` and `Text`. A function - // that declares the name inside itself still renders its own. - else if (destructuredAtTop(name)) child = declaredInside(ctx, name, around?.[around.length - 1] ?? parent, child); } if (!child || child.id === parent.id || crossesCodeBoundary(parent.language, child.language)) continue; const key = `${parent.id}>${child.id}`; diff --git a/src/ui-server/api/steps.ts b/src/ui-server/api/steps.ts index 7dea2f7d3..f9eb86d4d 100644 --- a/src/ui-server/api/steps.ts +++ b/src/ui-server/api/steps.ts @@ -1621,7 +1621,10 @@ function renderParents(cg: CodeGraph, node: Node, memo: Map): nu const incoming = cg.getIncomingEdgesTo([node.id], ['calls']); const sources = new Set(); for (const e of incoming) { - if ((e.metadata as Record | undefined)?.synthesizedBy === 'jsx-render') sources.add(e.source); + // A file renders at module scope — an entry's root, a test's subject + // (#2587) — and is no parent a screen shares chrome with. + if ((e.metadata as Record | undefined)?.synthesizedBy === 'jsx-render' && + cg.getNode(e.source)?.kind !== 'file') sources.add(e.source); } parents = sources.size; memo.set(node.id, parents);