diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b3e62ce04..3622d52038 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 JavaScript and TypeScript, a call on a variable declared outside the calling function with a package's type, like react-dom's `root.render()` on a module-level `let root: Root | null`, no longer links to a project method of the same name, such as an error boundary's `render()`. (#2589) - In TypeScript, an awaited call with type arguments like `await fetchApi("/me")` or `await api.get(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586) - In JavaScript and TypeScript, a call or `new` in a parameter's default value, like `function renderWithNote(note = buildNote())`, a destructured `{ size = defaultSize() }` or `constructor(private svc = new Service())`, now counts as the function's own call. Before, it was dropped, so `buildNote` was missing that caller and impact on it came back incomplete. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and the pull request. (#2588) - 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) diff --git a/__tests__/ts-outer-declared-receiver.test.ts b/__tests__/ts-outer-declared-receiver.test.ts new file mode 100644 index 0000000000..3d4973f1f8 --- /dev/null +++ b/__tests__/ts-outer-declared-receiver.test.ts @@ -0,0 +1,245 @@ +/** + * A JS/TS receiver declared with a package's type in a function around the + * calling one, or at the module's top level, calls none of the project's + * methods (#2589). + * + * react-dom's `root.render()` went to the project's one class `render()` + * (an error boundary) when `root` was a module's `let root: Root | null`, or + * a `let popupRoot: Root | null` of an enclosing function a nested one + * renders through: the receiver's type was read only in the calling function. + * A receiver typed there already got no edge. + * + * Anything nearer that binds the name — a local, a parameter, a destructured + * name, a later declarator — decides instead, and a binding inside another + * function is not in scope. A project type declared this way is not used to + * resolve a call: those are guessed as before. + */ +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 }); +}); + +const FILES: Record = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { react: '^19.0.0', 'react-dom': '^19.0.0', typedi: '^0.10.0' } }), + 'src/Boundary.tsx': `import { Component } from "react"; +export class Boundary extends Component<{ children?: React.ReactNode }> { + render() { return this.props.children; } +} +export function makeBoundary() { return new Boundary(); } +`, + 'src/popup.tsx': `import { createRoot, type Root } from "react-dom/client"; +import { useState } from "react"; +import { Container } from "typedi"; +import { Boundary, makeBoundary } from "./Boundary"; + +let root: Root | null = null; +export function showPopup(el: HTMLElement) { + if (!root) root = createRoot(el); + root.render(
); +} + +let modRoot: Root; +export function a1(el: HTMLElement) { modRoot = createRoot(el); modRoot.render(
); } + +export function outer(el: HTMLElement) { + let popupRoot: Root | null = null; + function renderPopup() { popupRoot!.render(
); } + popupRoot = createRoot(el); + return renderPopup; +} + +export function typedHere(el: HTMLElement) { + const t: Root = createRoot(el); + t.render(
); +} + +let own: Boundary; +export function ownTyped() { own.render(); } + +export function ownNew() { const b = new Boundary(); b.render(); } + +export function ownCall() { const m = makeBoundary(); m.render(); } + +export function shadowed(root: any) { root.render(); } + +export function arrowParam(items: Boundary[]) { items.forEach((root) => root.render()); } + +export function destructured() { const [root] = useState(new Boundary()); root.render(); } + +export function caught() { try { makeBoundary(); } catch (root: any) { root.render(); } } + +export function injected() { const root = Container.get(Boundary); root.render(); } + +let loose: any; +export function untypedOuter() { loose = new Boundary(); loose.render(); } + +export function sibling() { let other: Root | null = null; return other; } +export function afterSibling() { const b = makeBoundary(); other.render(); } + +let wrapped: InstanceType; +export function viaWrapper() { wrapped.render(); } + +// let commented: Root; +let commented = makeBoundary(); +export function afterComment() { commented.render(); } + +let quoted = makeBoundary(); +const snippet = \` +let quoted: Root; +\`; +export function afterString() { quoted.render(); return snippet; } + +let blockRoot: Root | null = null; +export function inBlock(el: HTMLElement) { + if (!blockRoot) { + blockRoot = createRoot(el); + } + blockRoot.render(
); +} +`, + 'src/MapView.ts': `export default class MapView { draw() { return 1; } } +`, + 'src/map.ts': `import Map from "./MapView"; +let map: Map; +export function viaMap() { map.draw(); } +`, +}; + +/** The module's `ob` is a react-dom `Root`; the project's one `render()` is `Other`'s. */ +const SHADOW: Record = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { 'react-dom': '^19.0.0' } }), + 'src/Other.tsx': `export class Other { render() { return 2; } } +export function makeOther() { return new Other(); } +`, + 'src/shadow.tsx': `import type { Root } from "react-dom/client"; +import { Other, makeOther } from "./Other"; + +let ob: Root | null = null; +export function outside() { ob!.render(); } +export function fnExpr(items: Other[]) { items.forEach(function (ob) { ob.render(); }); } +export function laterDeclarator() { let n = 0, ob = makeOther(); ob.render(); } +export function multiLine() { + const { + ob, + } = { ob: makeOther() }; + ob.render(); +} +export function declaratorBelow() { + const a = 1, + ob = makeOther(); + ob.render(); +} +export function destructDefault(o: any) { const { ob = makeOther() } = o; ob.render(); } +export function multiDefault(o: any) { + const { + ob = makeOther(), + } = o; + ob.render(); +} +export function commentedDeclarator() { + let n = 0, // counter + ob = makeOther(); + ob.render(); +} +export const returnsArrow = () => { return ob => ob.render(); }; +`, +}; + +/** The project's one `render()` is a namesake of the package type a receiver is declared with. */ +const NAMESAKE: Record = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { 'react-dom': '^19.0.0' } }), + 'src/Root.ts': `export class Root { render() { return 3; } } +`, + 'src/use.ts': `import * as RD from "react-dom/client"; +let qualified: RD.Root | null = null; +export function viaNamespace() { qualified!.render(); } +`, +}; + +/** A project file declares the package, adding to its `Root`. */ +const AUGMENTED: Record = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { 'react-dom': '^19.0.0' } }), + 'src/aug.d.ts': `declare module "react-dom/client" { + interface Root { extra(): void } +} +`, + 'src/use.ts': `import type { Root } from "react-dom/client"; +let augmented: Root; +export function viaAugmentation() { augmented.extra(); } +`, +}; + +async function indexed(files: Record): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ts-outer-receiver-')); + roots.push(dir); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), content); + } + return CodeGraph.init(dir, { index: true }); +} + +/** The names of the functions with a `calls` edge to the method `qualifiedName`. */ +function callersOf(cg: CodeGraph, file: string, qualifiedName: string): Set { + const method = cg.getNodesInFile(file).find((n) => n.kind === 'method' && n.qualifiedName === qualifiedName)!; + expect(method).toBeDefined(); + return new Set(cg.getIncomingEdgesTo([method.id], ['calls']).map((e) => cg.getNode(e.source)?.name)); +} + +describe('JS/TS: a receiver typed in a scope around the call', () => { + it('has that type; a nearer binding, or one in another function, does not', async () => { + const cg = await indexed(FILES); + try { + const callers = callersOf(cg, 'src/Boundary.tsx', 'Boundary::render'); + for (const outside of ['showPopup', 'a1', 'renderPopup', 'typedHere', 'inBlock']) { + expect(callers.has(outside), outside).toBe(false); + } + for (const project of ['ownTyped', 'ownNew', 'ownCall', 'shadowed', 'arrowParam', 'destructured', 'caught', 'injected', 'untypedOuter', 'afterSibling', 'viaWrapper', 'afterComment', 'afterString']) { + expect(callers.has(project), project).toBe(true); + } + // A project class imported under a built-in's name is the project's. + expect(callersOf(cg, 'src/MapView.ts', 'MapView::draw').has('viaMap')).toBe(true); + } finally { + cg.close(); + } + }); + + it('gives way to a nearer binding of the name, which still reaches the project', async () => { + const cg = await indexed(SHADOW); + try { + const callers = callersOf(cg, 'src/Other.tsx', 'Other::render'); + expect(callers.has('outside'), 'outside').toBe(false); + for (const nearer of ['fnExpr', 'laterDeclarator', 'multiLine', 'declaratorBelow', 'destructDefault', 'multiDefault', 'commentedDeclarator', 'returnsArrow']) { + expect(callers.has(nearer), nearer).toBe(true); + } + } finally { + cg.close(); + } + }); + + it("leaves a package type the project declares to the project's declaration", async () => { + const cg = await indexed(AUGMENTED); + try { + const extra = cg.getNodesInFile('src/aug.d.ts').find((n) => n.name === 'extra')!; + expect(extra).toBeDefined(); + expect(cg.getIncomingEdgesTo([extra.id], ['calls']).map((e) => cg.getNode(e.source)?.name)).toContain('viaAugmentation'); + } finally { + cg.close(); + } + }); + + it("keeps a package namespace's type, not the project's namesake", async () => { + const cg = await indexed(NAMESAKE); + try { + expect(callersOf(cg, 'src/Root.ts', 'Root::render').has('viaNamespace')).toBe(false); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d300409f2e..90923f3518 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -17712,6 +17712,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { PYTHON_IMPORTED_FILES.delete(context); AWAITED_TYPE_MEMO.delete(context); AWAITED_FILES.delete(context); + OUTER_SCAN_FILES.delete(context); TS_IMPORTED_TYPE_FILES.delete(context); TS_NAMED_CLASSES.delete(context); TS_AMBIENT_MODULE_FILES.delete(context); @@ -18679,6 +18680,163 @@ export function inferLocalReceiverType( return null; } +/** + * Per JS/TS file, for the outer-scope receiver check below: the names some + * `const` / `let` / `var` declares with a written type, and — read only for + * a file that has one the check asks about — its lines with comments and + * string contents blanked (line breaks kept) and the innermost function or method node over + * each line. Kept for the last few files, since references arrive file by file. + */ +type OuterScanFile = { + typedNames: Set; + scopes?: { stripped: string[]; owner: (Node | undefined)[] }; + receivers: Map; +}; +/** A receiver name's typed `const` / `let` / `var` lines, and the lines that use it other than as a read (outerReceiverUseIsRead). */ +type OuterReceiverLines = { typed: number[]; unread: number[] }; +const OUTER_SCAN_FILES = new WeakMap>(); + +function outerScanFile(ref: UnresolvedRef, lines: string[], context: ResolutionContext): OuterScanFile { + let files = OUTER_SCAN_FILES.get(context); + if (!files) OUTER_SCAN_FILES.set(context, (files = new Map())); + const hit = files.get(ref.filePath); + if (hit) return hit; + const typedNames = new Set(); + for (const line of lines) { + if (line.length > 10_000) continue; + for (const m of line.matchAll(/(?= 16) files.delete(files.keys().next().value!); + files.set(ref.filePath, file); + return file; +} + +function outerScopes(file: OuterScanFile, ref: UnresolvedRef, lines: string[], context: ResolutionContext): NonNullable { + if (file.scopes) return file.scopes; + const stripped = blankStringContents(stripCommentsForRegex(lines.join('\n'), 'typescript')).split('\n'); + const owner: (Node | undefined)[] = new Array(lines.length); + // Outer nodes first, so a node nested in another overwrites its lines. + const fns = context.getNodesInFile(ref.filePath) + .filter((n) => (n.kind === 'function' || n.kind === 'method') && n.language === ref.language) + .sort((a, b) => a.startLine - b.startLine || (b.endLine ?? b.startLine) - (a.endLine ?? a.startLine)); + for (const n of fns) { + for (let i = n.startLine - 1; i <= (n.endLine ?? n.startLine) - 1 && i < lines.length; i++) owner[i] = n; + } + return (file.scopes = { stripped, owner }); +} + +/** + * Is this use of a receiver name, at `at` in `line`, one that can only read + * or assign it: a member read (`root.x`, `root!.x`, `root?.x`), a negation, a + * comparison or logical operand, a `return`, or the target of an assignment + * statement. Anything else may bind the name — a parameter, a declarator, a + * destructured name, a `catch` — and regex can't tell those from the reads + * they look like, so a scan that meets one decides nothing. + */ +function outerReceiverUseIsRead(line: string, at: number, name: string, previous: string | undefined): boolean { + const before = line.slice(0, at); + const after = line.slice(at + name.length); + if (/(?:\?\.|\.)\s*$/.test(before) && !/\.\.\.\s*$/.test(before)) return true; // a property of something else + if (/^\s*(?:!\s*)?(?:\?\.|\.(?!\.))/.test(after)) return true; + if (/!\s*$/.test(before) || (/(?/.test(after))) return true; + if (/^\s*(?:&&|\|\||\?\?(?!=)|[=!]==?)/.test(after)) return true; + // `root = …;` as a statement of its own, not a declarator continued from + // the line above (`const a = 1,` / `let`) nor a destructuring default + // (`{ root = x }`, `[root = x]`): a `{` before it opens a block, as one + // ending the line above does unless a pattern starts there (`const {`). + return /^\s*=(?![=>])/.test(after) && /(?:^|;|\)|(?:\)|=>|(? 10_000) { found.unread.push(i); continue; } + // Uses are read off the source as written, so one in a comment or a + // string can only make the scan decide less. + let typed = false; + for (const m of line.matchAll(use)) { + if (outerReceiverUseIsRead(line, m.index!, receiver, previous)) continue; + // The declaration itself; written in a comment or a string it declares nothing. + if (!typed && typedDecl.test(line) && typedDecl.test(bare)) typed = true; + else { found.unread.push(i); break; } + } + if (typed) found.typed.push(i); + if (bare.trim()) previous = bare; + } + file.receivers.set(receiver, found); + return found; +} + +/** + * Is a JS/TS receiver the calling function doesn't bind declared, in a + * function around it or at the module's top level, with a type from outside + * the project: one a package's import names (`let root: Root | null` after + * `import { type Root } from "react-dom/client"`, `let r: RD.Root`), or a + * built-in (`Map`, `Promise`) the project declares no namesake of? Then no + * project method is the one called (#2589). The nearest `const` / `let` / + * `var` of the name written with a type, on a line no function away from the + * call holds, decides — and only when every line from there to the call uses + * the name to read or assign it (outerReceiverUseIsRead). The answer only + * ever drops a call: a project type found this way is not used to resolve + * one, so a misread scope costs a guess, never adds an edge. + */ +function isOuterDeclaredOutsideReceiver(receiver: string, ref: UnresolvedRef, context: ResolutionContext): boolean { + if (!/^[A-Za-z_$][\w$]*$/.test(receiver) || receiver === 'this' || receiver === 'super') return false; + const lines = context.getFileLines + ? context.getFileLines(ref.filePath) + : (context.readFile(ref.filePath)?.split(/\r?\n/) ?? null); + if (!lines || lines.length === 0) return false; + const file = outerScanFile(ref, lines, context); + if (!file.typedNames.has(receiver)) return false; + if (context.getImportMappings?.(ref.filePath, ref.language)?.some((m) => m.localName === receiver)) return false; + const { stripped, owner } = outerScopes(file, ref, lines, context); + const { typed, unread } = outerReceiverLines(receiver, file, stripped, lines); + const callIdx = Math.max(0, Math.min(lines.length - 1, ref.line - 1)); + const holdsCall = (n: Node): boolean => n.startLine <= ref.line && (n.endLine ?? n.startLine) >= ref.line; + let declIdx = -1; + for (let k = typed.length - 1; k >= 0; k--) { + const i = typed[k]!; + if (i >= callIdx) continue; + // A line inside a function that doesn't hold the call is another scope's. + const at = owner[i]; + if (at && !holdsCall(at)) continue; + declIdx = i; + break; + } + if (declIdx < 0 || unread.some((i) => i > declIdx && i <= callIdx)) return false; + for (const re of localReceiverTypePatterns(ref.language, escapeRegExp(receiver))) { + const raw = lines[declIdx]!.match(re)?.[1]; + if (!raw) continue; + const type = normalizeInferredTypeName(raw); + if (!type) return false; + // As for a receiver typed in the calling function: a type the file + // imports from the project is the project's, whatever its name; one a + // package's import names is outside it unless a project file declares + // that package (`declare module 'pkg'`). + if (tsImportedTypeFile(raw, ref, context) !== undefined) return false; + const outside = tsOutsideTypeDeclaration(raw, type, ref, context); + if (outside !== undefined) return outside === null; + return raw.split(/[.<\[\s]/)[0] === type && JS_BUILT_INS.has(type) && isUndeclaredTypeName(type, ref, context); + } + return false; +} + /** Infer only a visible awaited binding and its actual local/imported callee. * The signature already carries the return annotation in both extractors, so * multiline declarations and neighboring declarations cannot donate a type. @@ -19137,6 +19295,12 @@ export function matchMethodCall( inferredType = null; } } + // A JS/TS receiver the calling function doesn't bind may be a `let` / + // `const` / `var` of a function around it or of the module, declared + // there with a package's type: react-dom's `root.render(…)` on a module's + // `let root: Root | null` went to the project's one class `render()` (#2589). + if (!inferredType && ESM_FAMILY.has(ref.language) && dotMatch && + nmTimedT('mc-outerdecl', ref, () => isOuterDeclaredOutsideReceiver(objectOrClass!, ref, context))) return null; // An Objective-C receiver the calling method doesn't declare may be an // instance variable or a property of its class (objcMemberReceiverType). if (!inferredType && ref.language === 'objc' && dotMatch) {