diff --git a/.gitignore b/.gitignore index 6886bb44..caac5714 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,9 @@ .yarnrc node_modules +# a checked-in stand-in for a dependency, so prop-flow can be tested against +# props it must treat as vendored +!packages/prop-flow/fixtures/basic/node_modules # build /lib/ diff --git a/packages/prop-flow/CHANGELOG.md b/packages/prop-flow/CHANGELOG.md index 978fe912..6c628847 100644 --- a/packages/prop-flow/CHANGELOG.md +++ b/packages/prop-flow/CHANGELOG.md @@ -5,6 +5,29 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## To Be Released +- JSX spreads are resolved instead of being blanket-reported as `manual`: a + spread whose type provably lacks the prop is skipped, `{...props}` and + `{...rest}` are followed one level up, and a spread of an object literal (or + of a `const` bound to one) is read key by key. Only a spread whose type cannot + answer the question, or an optional prop in a spread contesting an earlier + value, still requires a human +- Fixed attribute precedence: `` reported `"x"`, but + JSX resolves last-wins, so the spread overrides the attribute +- Fixed pass-throughs inside render callbacks: `items.map(() => )` + was classified as a local value, which could turn into a wrong `justified` or + `unnecessary-optional` +- Optional props declared only in a dependency are no longer reported. A + component spreading `React.ComponentProps<'button'>` inherits some 250 + optional DOM and ARIA props; a verdict on those is true but not actionable, + and it buried the props the author actually owns +- A pass-through that climbs into a function which is called rather than + rendered (a `renderX({ … })` test helper) now reports `manual` instead of + counting its invisible callers as zero, which would report a live prop as + `caller-dead` +- `--json` output: `SiteKind` no longer has a `spread` member — former spread + sites are now reported as `passthrough` / `real` / `omit` / `manual` with a + note + ## 1.0.0 - Initial release diff --git a/packages/prop-flow/README.md b/packages/prop-flow/README.md index e9a11621..7d7ff251 100644 --- a/packages/prop-flow/README.md +++ b/packages/prop-flow/README.md @@ -60,7 +60,7 @@ justified Button.title | `unnecessary-optional` | every call site passes it → could be required | | `caller-dead` | no call site passes it → optional and always `undefined` | | `unused-component` | the component itself has no call sites in the Program | -| `manual` | a spread / rename / dynamic value on the path blocks a static conclusion | +| `manual` | an unreadable spread or a contested override blocks a static conclusion | Exit codes: `0` success, `1` nothing to do (usage printed), `2` a handled failure (message on stderr). @@ -83,12 +83,33 @@ from `cwd`. ## Limitations Pass-throughs are followed through plain identifiers and `props.x` member -access. A spread (`{...rest}`) and a rest element in the props destructure are -reported as `manual` rather than guessed at. `prop={undefined}` counts as an -omission — it is an omission dressed up as a pass, so a prop that is only ever -fed `undefined` still comes out as `caller-dead`. A conditional expression that -can evaluate to `undefined` counts as a real source — the one false positive the -tool accepts on purpose. +access, including inside render callbacks — a `props.x` in `items.map(…)` is +still traced to the surrounding component. + +A spread is only ambiguous when it can actually reach the prop. `{...x}` whose +type provably lacks the prop is skipped; `{...props}` and `{...rest}` are +followed one level up, and a spread of an object literal (or of a `const` bound +to one) is read key by key. What stays `manual`: a spread whose type cannot +answer the question (`any`, `Record`, a union that carries the +prop in only some constituents), and an *optional* prop in a spread that +contests an earlier value — both outcomes are possible at runtime, so neither is +concluded. JSX ordering is respected throughout: in `` +the spread wins. + +Optional props a component only *inherits* from a dependency — the ~250 DOM and +ARIA props behind `React.ComponentProps<'button'>`, say — are not reported. A +verdict on them is true but useless: the `?` is not yours to drop, and they bury +the props that are. A prop redeclared in your own type is still reported. + +A pass-through that climbs into a function which is *called* rather than +rendered — a `renderX({ … })` test helper, typically — also stays `manual`: its +callers exist but are invisible to a JSX walk, and counting them as zero would +report a live prop as `caller-dead`. + +`prop={undefined}` counts as an omission — it is an omission dressed up as a +pass, so a prop that is only ever fed `undefined` still comes out as +`caller-dead`. A conditional expression that can evaluate to `undefined` counts +as a real source — the one false positive the tool accepts on purpose. Components are picked up from `export function C`, `export const C = …` (including `memo()` / `forwardRef()` wrappers), `export default function C` and diff --git a/packages/prop-flow/fixtures/basic/badge.tsx b/packages/prop-flow/fixtures/basic/badge.tsx index 0eb4356f..ab0576c8 100644 --- a/packages/prop-flow/fixtures/basic/badge.tsx +++ b/packages/prop-flow/fixtures/basic/badge.tsx @@ -1,6 +1,6 @@ export interface BadgeProps { text: string; - /** manual: reached through a spread and through an explicit undefined */ + /** justified: passed directly, through a resolved spread, and left undefined once */ tone?: string; } diff --git a/packages/prop-flow/fixtures/basic/node_modules/vendor/index.d.ts b/packages/prop-flow/fixtures/basic/node_modules/vendor/index.d.ts new file mode 100644 index 00000000..351016c2 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/node_modules/vendor/index.d.ts @@ -0,0 +1,6 @@ +// Stands in for React's DOM prop types: optional props the author inherits +// but does not own. The analyzer must not list them. +export interface VendorProps { + hidden?: boolean; + lang?: string; +} diff --git a/packages/prop-flow/fixtures/basic/node_modules/vendor/package.json b/packages/prop-flow/fixtures/basic/node_modules/vendor/package.json new file mode 100644 index 00000000..bd3b0b03 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/node_modules/vendor/package.json @@ -0,0 +1,5 @@ +{ + "name": "vendor", + "version": "1.0.0", + "types": "index.d.ts" +} diff --git a/packages/prop-flow/fixtures/basic/rest.tsx b/packages/prop-flow/fixtures/basic/rest.tsx index f2285fea..ce3138c2 100644 --- a/packages/prop-flow/fixtures/basic/rest.tsx +++ b/packages/prop-flow/fixtures/basic/rest.tsx @@ -5,7 +5,8 @@ export interface RestProps { extra?: string; } -// Rest element in the props destructure: what `rest` holds is not decidable. +// The rest object is passed as a VALUE, not spread: it always exists, so +// Sink.data counts as a real pass at this site. export function Rest({ ...rest }: RestProps) { return ; } diff --git a/packages/prop-flow/fixtures/basic/sources.tsx b/packages/prop-flow/fixtures/basic/sources.tsx new file mode 100644 index 00000000..47a52287 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/sources.tsx @@ -0,0 +1,180 @@ +// Every value shape a prop can be fed from, and what the classifier concludes +// about each. Three leaves keep the families apart: `Literal` collects the +// spreads that resolve down to a value, `Blocked` the ones that must not +// resolve, and `Direct` the values written on the element itself. +// +// A few shapes here deliberately do not type-check. prop-flow analyses whatever +// the compiler was handed — mid-refactor code included — so surviving them is +// part of the contract, and each one is called out where it appears. + +interface NotedProps { + id: string; + /** manual: fed one of every shape the classifier knows */ + note?: string; +} + +/** Collects the object-literal spreads that resolve down to a value. */ +export function Literal({ id, note }: NotedProps) { + return {note}; +} + +/** Collects the spreads the classifier must refuse to resolve. */ +export function Blocked({ id, note }: NotedProps) { + return {note}; +} + +/** Collects the values written as attributes on the element itself. */ +export function Direct({ id, note }: NotedProps) { + return {note}; +} + +// ── object literals a spread resolves through ──────────────────────────────── + +const note = 'a local const'; +const spreadIn = { note: 'from a nested spread' }; +const key = 'note'; + +const shorthand: NotedProps = { id: 'shorthand', note }; +const quoted: NotedProps = { id: 'quoted', 'note': 'a quoted key' }; +const unset: NotedProps = { id: 'unset' }; +const nested: NotedProps = { id: 'nested', ...spreadIn }; +const computed = { id: 'computed', [key]: 'a computed key' }; +const getter = { + id: 'getter', + get note() { + return 'from a getter'; + }, +}; + +export function LiteralSites() { + return ( +
+ + + + + + + +
+ ); +} + +// ── spreads that must stay unresolved ──────────────────────────────────────── + +interface WithNote { + id: string; + note?: string; +} + +interface WithoutNote { + id: string; +} + +interface OtherOptional { + id: string; + other?: string; +} + +/** No declared properties to read at all. */ +declare const anything: any; +/** Only one constituent carries `note`, so the union cannot answer for it. */ +declare const partlyNoted: WithNote | WithoutNote; +/** No constituent carries `note`, so the spread is provably irrelevant. */ +declare const neverNoted: OtherOptional | WithoutNote; + +// A `let` could be reassigned between its declaration and the call site below. +let reassignable: NotedProps = { id: 'reassignable', note: 'for now' }; + +const wrapper = { inner: { id: 'inner', note: 'from a destructured local' } }; +const { inner } = wrapper; + +function makeProps(): NotedProps { + return { id: 'made', note: 'from a call' }; +} + +// A const, but initialised to something other than an object literal. +const fromCall = makeProps(); + +/** An anonymous function is bound to no name, so it names no component. */ +export const renderers = [(props: NotedProps) => ]; + +/** `options` is not the first parameter, so it names no component's props. */ +function renderSecond(id: string, options: NotedProps) { + return ; +} + +/** A class method is no component function, so its parameter is not props. */ +class LegacyRenderer { + render(props: NotedProps) { + return ; + } +} + +export function BlockedSites() { + return ( +
+ + + + + + + +
+ ); +} + +// ── values written on the element itself ───────────────────────────────────── + +/** `config` is a destructured prop that is itself an object, not the props. */ +export function Member({ config, id }: { config: { note?: string }; id: string }) { + return ; +} + +/** The destructure is a class method's, so it binds no component's props. */ +class LegacyDestructured { + render({ id, note }: NotedProps) { + return ; + } +} + +export function DirectSites() { + return ( +
+ {/* `absent` is undeclared on purpose: an identifier that resolves to + nothing must not stop the walk. */} + + {/* The value was commented out, leaving an attribute form with nothing + to read. */} + + {/* A tag that resolves to no symbol at all is not a call site. */} + +
+ ); +} + +// ── props the checker synthesises ──────────────────────────────────────────── + +type AllOptional = { [K in keyof NotedProps]?: NotedProps[K] }; + +/** A mapped type: the prop symbols belong to no declaration of their own, and + * must not be taken for a dependency's. */ +export function Mapped({ id, note }: AllOptional) { + return {note}; +} + +// ── export shapes that name no component ───────────────────────────────────── + +const catalogueName = 'sources'; + +// A call whose callee resolves to no symbol. +export const rendered = (() => 'nothing')(); +// A name that does not exist: the specifier has no declaration to resolve to. +export { Absent }; +// A type in an export list names no function. +export { NotedProps }; +// A const in an export list, but not one that holds a function. +export { catalogueName }; +// A destructured export: the declaration binds no identifier to look up. +export const { id: catalogueId } = shorthand; diff --git a/packages/prop-flow/fixtures/basic/spread.tsx b/packages/prop-flow/fixtures/basic/spread.tsx new file mode 100644 index 00000000..a1a20460 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/spread.tsx @@ -0,0 +1,126 @@ +// Every JSX spread shape the analyzer resolves, plus the two it must refuse to. +// `Leaf` collects the resolvable cases, `Murky` the ones that stay MANUAL. + +export interface LeafProps { + id: string; + /** justified: fed through spreads that resolve to passes and to omissions */ + note?: string; +} + +export function Leaf({ id, note }: LeafProps) { + return {note}; +} + +export interface MurkyProps { + id: string; + /** manual: an unreadable spread type, and a contested optional override */ + note?: string; +} + +export function Murky({ id, note }: MurkyProps) { + return {note}; +} + +export interface ForwardProps { + id: string; + note?: string; +} + +export interface RequiredNoteProps { + id: string; + note: string; +} + +interface DecorProps { + className?: string; +} + +const decor: DecorProps = { className: 'decor' }; +const leafProps: LeafProps = { id: 'lit', note: 'from a literal' }; +const record: Record = { note: 'unreadable' }; + +/** Whole-object parameter spread. */ +export function Forward(props: ForwardProps) { + return ; +} + +/** Rest binding that still carries `note` — it was not destructured out. */ +export function ForwardRest({ id, ...rest }: ForwardProps) { + return ; +} + +/** `note` is destructured away, so the rest type provably lacks it. */ +export function DropNote({ note, ...rest }: ForwardProps) { + return ; +} + +/** The first spread cannot carry `note` and must not make the site ambiguous. */ +export function Multi(props: ForwardProps) { + return ; +} + +/** The spread carries `note` as REQUIRED, so it overrides the attribute. */ +export function Override(props: RequiredNoteProps) { + return ; +} + +/** Never rendered and never called: the inside it is dead code, and + * contributing nothing is the honest answer — unlike renderMurky below. */ +export function Unrendered(props: ForwardProps) { + return ; +} + +export interface ListProps { + items: readonly string[]; + note?: string; +} + +/** Render callback: the nearest enclosing function is not the component. */ +export function List(props: ListProps) { + return
    {props.items.map((item) => )}
; +} + +/** Same, but the mapped array is bound to a const first. */ +export function ListConst(props: ListProps) { + const rows = props.items.map((item) => ); + return
    {rows}
; +} + +/** The spread type has no readable properties at all. */ +export function Opaque({ id }: MurkyProps) { + return ; +} + +/** An optional `note` in the spread may or may not override the attribute. */ +export function Contested(props: ForwardProps) { + return ; +} + +interface MurkyOptions { + note?: string; +} + +// Not a component: it is CALLED, never rendered as JSX. Climbing into it would +// find no call sites, and counting that as "nobody passes note" is a lie. +function renderMurky(options: MurkyOptions) { + return ; +} + +export const murkySnapshots = [renderMurky({ note: 'from a helper' }), renderMurky({})]; + +export function SpreadApp() { + return ( +
+ + + + + + + + + + +
+ ); +} diff --git a/packages/prop-flow/fixtures/basic/vendored.tsx b/packages/prop-flow/fixtures/basic/vendored.tsx new file mode 100644 index 00000000..5d3ae197 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/vendored.tsx @@ -0,0 +1,11 @@ +import type { VendorProps } from 'vendor'; + +export interface VendoredProps extends VendorProps { + id: string; + /** the only optional prop declared in this project rather than inherited */ + caption?: string; +} + +export function Vendored({ caption, id }: VendoredProps) { + return {caption}; +} diff --git a/packages/prop-flow/src/analyzer.test.ts b/packages/prop-flow/src/analyzer.test.ts index a56cc613..30aaa9ba 100644 --- a/packages/prop-flow/src/analyzer.test.ts +++ b/packages/prop-flow/src/analyzer.test.ts @@ -39,9 +39,23 @@ function describeSite(site: Site): string { return `${site.kind} ${site.via ?? site.note ?? ''}`.trim(); } +/** + * How often each site description occurs. Several distinct code shapes collapse + * to the same verdict on purpose, so the multiset — not a sorted list — is what + * says which of them were seen. + */ +function tally(sites: readonly Site[]): Record { + const out: Record = {}; + for (const site of sites) { + const key = describeSite(site); + out[key] = (out[key] ?? 0) + 1; + } + return out; +} + describe('createAnalyzer', () => { it.each<[string, string, string, Verdict]>([ - ['badge.tsx', 'Badge', 'tone', 'manual'], + ['badge.tsx', 'Badge', 'tone', 'justified'], ['button.tsx', 'Button', 'disabled', 'justified'], ['button.tsx', 'Button', 'icon', 'caller-dead'], ['button.tsx', 'Button', 'size', 'unnecessary-optional'], @@ -57,7 +71,9 @@ describe('createAnalyzer', () => { ['panel.tsx', 'Panel', 'note', 'justified'], ['renamed.tsx', 'Renamed', 'caption', 'unnecessary-optional'], ['rest.tsx', 'Rest', 'extra', 'caller-dead'], - ['sink.tsx', 'Sink', 'data', 'manual'], + ['sink.tsx', 'Sink', 'data', 'unnecessary-optional'], + ['spread.tsx', 'Leaf', 'note', 'justified'], + ['spread.tsx', 'Murky', 'note', 'manual'], ['tree.tsx', 'Tree', 'depth', 'unnecessary-optional'], ])('%s: %s.%s → %s', (file, component, prop, verdict) => { expect(analyse(file, component, prop).verdict).toBe(verdict); @@ -99,18 +115,132 @@ describe('createAnalyzer', () => { expect(ghost.sites.map(describeSite)).toEqual(['omit explicit undefined', 'omit explicit undefined']); }); - it('flags spreads and rest elements for a human', () => { + it('resolves every spread shape it can see through', () => { + const result = analyse('spread.tsx', 'Leaf', 'note'); + + expect(result).toMatchObject({ ambiguous: 0, omit: 3, real: 5 }); + expect(result.sites.map(describeSite).sort()).toEqual([ + 'omit', + 'passthrough Forward.note', + 'passthrough ForwardRest.note', + 'passthrough List.note', + 'passthrough ListConst.note', + 'passthrough Multi.note', + 'passthrough Override.note', + // Never rendered and never called — the site below it is dead code. + 'passthrough Unrendered.note', + 'real string literal', + ]); + }); + + it('keeps the three spread shapes it must not resolve MANUAL', () => { + // An unreadable spread type, a contested optional override, and a climb + // into a plain helper whose callers this analysis cannot see. + const result = analyse('spread.tsx', 'Murky', 'note'); + + expect(result).toMatchObject({ ambiguous: 3, omit: 0, real: 0 }); + expect(result.sites.map(describeSite).sort()).toEqual([ + 'manual an optional prop in a spread contests an earlier value', + 'manual renderMurky.note: called, never rendered as JSX', + 'manual spread of a type that cannot be read', + ]); + }); + + it('reads a prop through a render callback instead of stopping at it', () => { + // `items.map((item) => )` — the nearest enclosing + // function is the callback, so a lexical match would call this a local. + const { sites } = analyse('spread.tsx', 'Leaf', 'note'); + + expect(sites.filter(({ via }) => via === 'List.note')).toHaveLength(1); + expect(sites.filter(({ via }) => via === 'ListConst.note')).toHaveLength(1); + }); + + it('resolves a spread of a typed const through its object literal', () => { + // in app.tsx, where badgeProps sets `tone`. const badge = analyse('badge.tsx', 'Badge', 'tone'); - expect(badge).toMatchObject({ ambiguous: 1, omit: 1, real: 2 }); + + expect(badge).toMatchObject({ ambiguous: 0, omit: 1, real: 3 }); expect(badge.sites.map(describeSite).sort()).toEqual([ 'omit explicit undefined', 'real local value', 'real string literal', - 'spread', + 'real string literal', ]); + // The rest object is passed as a VALUE — it always exists, so it is real. const sink = analyse('sink.tsx', 'Sink', 'data'); - expect(sink.sites.map(describeSite)).toEqual(['manual rest element in props destructure']); + expect(sink.sites.map(describeSite)).toEqual(['real local value']); + }); + + it('resolves a spread of an object literal down to the key it sets', () => { + // sources.tsx feeds one object-literal shape per call site. + const result = analyse('sources.tsx', 'Literal', 'note'); + + expect(tally(result.sites)).toEqual({ + // A nested spread and a computed key can both still be setting `note`. + 'manual a nested spread or computed key in the spread object': 2, + // A getter is a value the classifier cannot follow to its source. + 'manual unrecognised object literal member': 1, + 'omit not set in the spread object': 1, + // The shorthand `{ id, note }`, where `note` is a module-level const. + 'real local value': 1, + // Written inline at the call site, and behind a quoted key. + 'real string literal': 2, + }); + }); + + it('refuses to resolve a spread whose value it cannot pin down', () => { + const result = analyse('sources.tsx', 'Blocked', 'note'); + + expect(tally(result.sites)).toEqual({ + 'manual spread of CallExpression cannot be resolved': 1, + // A reassignable `let`, a const holding a call's result, a destructured + // local, a second parameter, a class method's parameter and an anonymous + // function's — six ways to name an object the classifier cannot read. + 'manual spread of Identifier cannot be resolved': 6, + // `any`, and a union carrying `note` in only some of its constituents. + 'manual spread of a type that cannot be read': 2, + // A union no constituent of which has `note`: provably nothing to carry. + 'omit': 1, + }); + expect(result.verdict).toBe('manual'); + }); + + it('classifies attribute values that resolve to nothing', () => { + const result = analyse('sources.tsx', 'Direct', 'note'); + + expect(tally(result.sites)).toEqual({ + // `note={/* nothing */}` — an expression container with no expression. + 'manual unrecognised attribute form': 1, + // An identifier resolving to no declaration, and one destructured in a + // class method — neither names a prop of an enclosing component. + 'real local value': 2, + // `config.note`, where `config` is a destructured prop, not the props. + 'real member value': 1, + }); + }); + + it('skips exports and tags that name no component', () => { + // A missing name, a type, a plain const and a destructured declaration all + // sit in sources.tsx and must be walked past rather than reported. + expect(analyzer.findComponents(sourceFile('sources.tsx')).map(({ name }) => name)).toEqual([ + 'Literal', + 'Blocked', + 'Direct', + 'Member', + 'Mapped', + ]); + }); + + it('lists optional props a mapped type synthesised', () => { + // `{ [K in keyof P]?: P[K] }` — the props are the checker's, not written + // out anywhere, and `id` becomes optional only through the mapping. + const mapped = analyzer.findComponents(sourceFile('sources.tsx')).find(({ name }) => name === 'Mapped'); + + expect(mapped && analyzer.listOptionalProps(mapped)).toEqual([ + { hasDefault: false, name: 'id' }, + { hasDefault: false, name: 'note' }, + ]); }); it('terminates on a self-recursive component instead of looping', () => { @@ -140,6 +270,14 @@ describe('createAnalyzer', () => { { hasDefault: false, name: 'title' }, ]); }); + + it('skips optional props that are inherited from a dependency', () => { + // VendoredProps extends an interface from node_modules: `hidden` and + // `lang` are not the author's to drop, and they drown the ones that are. + const [vendored] = analyzer.findComponents(sourceFile('vendored.tsx')); + + expect(vendored && analyzer.listOptionalProps(vendored)).toEqual([{ hasDefault: false, name: 'caption' }]); + }); }); describe('verdictOf', () => { diff --git a/packages/prop-flow/src/analyzer.ts b/packages/prop-flow/src/analyzer.ts index c699d92b..9ebbc246 100644 --- a/packages/prop-flow/src/analyzer.ts +++ b/packages/prop-flow/src/analyzer.ts @@ -1,25 +1,12 @@ import type * as TS from 'typescript'; -import { - bindingKey, - bindingNameOfFn, - defaultedBindingNames, - enclosingComponentFn, - findAttr, - isExported, - unwrapToFn, -} from './ast.js'; -import type { ComponentFn } from './ast.js'; +import { defaultedBindingNames, isExported, unwrapToFn } from './ast.js'; +import { createClassifier } from './classify.js'; +import { createComponentFactory } from './component.js'; +import type { Component } from './component.js'; import { locOf } from './paths.js'; import type { OptionalProp, PropAnalysis, Site, Verdict } from './types.js'; import type { TypeScriptApi } from './typescript-api.js'; -export interface Component { - readonly fn: ComponentFn; - readonly name: string; - readonly paramNode: TS.ParameterDeclaration; - readonly symbol: TS.Symbol; -} - export interface AnalyzerOptions { /** Paths in the output are printed relative to this directory. */ readonly cwd: string; @@ -27,6 +14,14 @@ export interface AnalyzerOptions { readonly ts: TypeScriptApi; } +/** What one JSX call site adds to the analysis of a prop. */ +interface Contribution { + readonly ambiguous: number; + readonly omit: number; + readonly real: number; + readonly site: Site; +} + export interface Analyzer { /** Walk every call site of `component` and classify how `propName` is fed. */ analyse(component: Component, propName: string): PropAnalysis; @@ -35,36 +30,10 @@ export interface Analyzer { listOptionalProps(component: Component): OptionalProp[]; } -/** The value is the enclosing component's own prop — climb one level up. */ -interface Passthrough { - readonly component: Component; - readonly kind: 'passthrough'; - readonly prop: string; -} - -/** Static analysis cannot decide this one; a human has to look. */ -interface Manual { - readonly kind: 'manual'; - readonly note: string; -} - -/** The attribute is written out but carries nothing: `prop={undefined}`. */ -interface Omission { - readonly kind: 'omit'; - readonly note: string; -} - -/** A value originates right here. */ -interface Real { - readonly kind: 'real'; - readonly note: string; -} - -/** Where a JSX attribute's value comes from. */ -type Classification = Manual | Omission | Passthrough | Real; - export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer { const checker = program.getTypeChecker(); + const components = createComponentFactory({ checker, ts }); + const classifier = createClassifier({ checker, components, ts }); // Symbol identity. Symbols are not primitives, so a Map keyed by symbol // works — but the index is keyed by a plain id to keep it printable. @@ -72,8 +41,9 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer let nextSymbolId = 1; // Every JSX usage in the Program, indexed once: component symbol → call - // sites. Building this eagerly costs one walk and saves one per prop. - const usageIndex = buildUsageIndex(); + // sites, plus the symbols that are CALLED rather than rendered. Building + // this eagerly costs one walk and saves one per prop. + const { calledIds, usageIndex } = indexProgram(); return { analyse, findComponents, listOptionalProps }; @@ -95,165 +65,90 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer let ambiguous = 0; for (const el of usages) { - const attr = findAttr(ts, el, propName); - if (attr === 'spread') { - ambiguous += 1; - sites.push({ kind: 'spread', loc: locOf(el, cwd) }); - continue; - } - if (attr === null) { - omit += 1; - sites.push({ kind: 'omit', loc: locOf(el, cwd) }); - continue; - } - - const classified = classifyAttrValue(attr); - if (classified.kind === 'passthrough') { - // The value is the enclosing component's own prop — climb. - const sub = analyse(classified.component, classified.prop, visited); - real += sub.real; - omit += sub.omit; - ambiguous += sub.ambiguous; - sites.push({ - kind: 'passthrough', - loc: locOf(el, cwd), - via: `${classified.component.name}.${classified.prop}`, - }); - } else if (classified.kind === 'real') { - real += 1; - sites.push({ kind: 'real', loc: locOf(el, cwd), note: classified.note }); - } else if (classified.kind === 'omit') { - omit += 1; - sites.push({ kind: 'omit', loc: locOf(el, cwd), note: classified.note }); - } else { - ambiguous += 1; - sites.push({ kind: 'manual', loc: locOf(el, cwd), note: classified.note }); - } + const contribution = contributionOf(el, propName, visited); + real += contribution.real; + omit += contribution.omit; + ambiguous += contribution.ambiguous; + sites.push(contribution.site); } return { ambiguous, omit, real, sites, verdict: verdictOf(usages.length, real, omit, ambiguous) }; } - function classifyAttrValue(attr: TS.JsxAttribute): Classification { - const init = attr.initializer; - // Shorthand boolean: → always a concrete `true`. - if (init === undefined) { - return { kind: 'real', note: 'boolean shorthand' }; - } - if (ts.isStringLiteral(init)) { - return { kind: 'real', note: 'string literal' }; - } - if (!ts.isJsxExpression(init) || init.expression === undefined) { - return { kind: 'manual', note: 'unrecognised attribute form' }; + /** Classify one call site, expanding a pass-through into what it bottoms out in. */ + function contributionOf(el: TS.JsxOpeningLikeElement, propName: string, visited: Set): Contribution { + const classified = classifier.classifyElement(el, propName); + if (classified.kind === 'real') { + return { ambiguous: 0, omit: 0, real: 1, site: { kind: 'real', loc: locOf(el, cwd), note: classified.note } }; + } + if (classified.kind === 'omit') { + return { ambiguous: 0, omit: 1, real: 0, site: { kind: 'omit', loc: locOf(el, cwd), note: classified.note } }; + } + if (classified.kind === 'manual') { + return { ambiguous: 1, omit: 0, real: 0, site: { kind: 'manual', loc: locOf(el, cwd), note: classified.note } }; + } + + // The value is the enclosing component's own prop — climb. + const sub = analyse(classified.component, classified.prop, visited); + if (sub.verdict === 'unused-component' && calledIds.has(symbolId(classified.component.symbol))) { + // The climb landed on a plain function taking an options object — a test + // helper, say. It has callers, they just are not JSX, so the walk cannot + // see them. Counting the empty subtree would turn an invisible pass into + // a caller-dead: "delete the prop" on live code. + return { + ambiguous: 1, + omit: 0, + real: 0, + site: { + kind: 'manual', + loc: locOf(el, cwd), + note: `${classified.component.name}.${classified.prop}: called, never rendered as JSX`, + }, + }; } - const expr = init.expression; + return { + ambiguous: sub.ambiguous, + omit: sub.omit, + real: sub.real, + site: { + kind: 'passthrough', + loc: locOf(el, cwd), + via: `${classified.component.name}.${classified.prop}`, + }, + }; + } - // `prop={undefined}` is an omission dressed up as a pass. - if (ts.isIdentifier(expr) && expr.text === 'undefined') { - return { kind: 'omit', note: 'explicit undefined' }; - } + // ── component + prop discovery ──────────────────────────────────────────── - // Identifier — the enclosing component's own prop (climb) or a local - // value (a real source). - if (ts.isIdentifier(expr)) { - return asEnclosingProp(expr, expr.text) ?? { kind: 'real', note: 'local value' }; - } - // `props.foo` — climb when `props` is the enclosing props parameter. - if (ts.isPropertyAccessExpression(expr) && ts.isIdentifier(expr.expression)) { - return asEnclosingProp(expr, expr.name.text, expr.expression.text) ?? { kind: 'real', note: 'member value' }; - } - // Any other expression (call, object, conditional, JSX, template…) — the - // value is produced right here. Treat as a real source; a conditional that - // can yield undefined is the one false positive we accept (conservative: - // counts as "present"). - return { kind: 'real', note: ts.SyntaxKind[expr.kind] }; + function findComponents(sourceFile: TS.SourceFile): Component[] { + return sourceFile.statements.flatMap((stmt) => componentsOfStatement(stmt)); } - /** - * If `name` (optionally accessed as `objName.name`) refers to a prop of the - * component that lexically encloses `node`, describe the pass-through. - * Returns null when it is a local — i.e. a real source. - */ - function asEnclosingProp(node: TS.Node, name: string, objName?: string): Classification | null { - const fn = enclosingComponentFn(ts, node); - const param = fn?.parameters[0]; - if (!fn || !param) { - return null; - } - const component = componentOfFn(fn); - if (!component) { - return null; - } - - // Destructured props: function C({ foo, bar: baz }: Props) - if (ts.isObjectBindingPattern(param.name)) { - if (objName) { - // `props.x` while props are destructured → not the parameter. Local. - return null; - } - for (const element of param.name.elements) { - if (!ts.isIdentifier(element.name) || element.name.text !== name) { - continue; - } - if (element.dotDotDotToken) { - return { kind: 'manual', note: 'rest element in props destructure' }; - } - return { component, kind: 'passthrough', prop: bindingKey(ts, element) ?? name }; - } - return null; + /** The components one top-level statement declares or exports. */ + function componentsOfStatement(stmt: TS.Statement): Component[] { + if (ts.isFunctionDeclaration(stmt) && stmt.name && isExported(ts, stmt)) { + return compact([components.fromNode(stmt, stmt.name)]); } - - // Whole-object parameter: function C(props: Props) … used as props.x - if (ts.isIdentifier(param.name) && objName === param.name.text) { - return { component, kind: 'passthrough', prop: name }; + if (ts.isVariableStatement(stmt) && isExported(ts, stmt)) { + return compact(stmt.declarationList.declarations.map((decl) => componentOfDeclaration(decl))); } - return null; - } - - // ── component + prop discovery ──────────────────────────────────────────── - - function findComponents(sourceFile: TS.SourceFile): Component[] { - const out: Component[] = []; - for (const stmt of sourceFile.statements) { - if (ts.isFunctionDeclaration(stmt) && stmt.name && isExported(ts, stmt)) { - push(out, componentFrom(stmt, stmt.name)); - } else if (ts.isVariableStatement(stmt) && isExported(ts, stmt)) { - for (const decl of stmt.declarationList.declarations) { - if (!ts.isIdentifier(decl.name) || !decl.initializer) { - continue; - } - const fn = unwrapToFn(ts, decl.initializer); - if (fn) { - push(out, componentFrom(fn, decl.name)); - } - } - } else if (ts.isExportDeclaration(stmt) && !stmt.moduleSpecifier && stmt.exportClause) { - // Declared first, exported later: `export { Card, Inner as Public }`. - if (ts.isNamedExports(stmt.exportClause)) { - for (const element of stmt.exportClause.elements) { - push(out, componentFromExport(element)); - } - } + if (ts.isExportDeclaration(stmt) && !stmt.moduleSpecifier) { + const clause = stmt.exportClause; + // Declared first, exported later: `export { Card, Inner as Public }`. + if (clause && ts.isNamedExports(clause)) { + return compact(clause.elements.map((element) => components.fromExport(element))); } } - return out; + return []; } - /** Resolve an `export { X }` specifier back to the function it names. */ - function componentFromExport(element: TS.ExportSpecifier): Component | null { - const exported = checker.getSymbolAtLocation(element.propertyName ?? element.name); - const declaration = exported && resolveAlias(exported).declarations?.[0]; - if (!declaration) { + /** The component `export const C = memo(() => …)` binds, if it binds one. */ + function componentOfDeclaration(decl: TS.VariableDeclaration): Component | null { + if (!ts.isIdentifier(decl.name) || !decl.initializer) { return null; } - if (ts.isFunctionDeclaration(declaration) && declaration.name) { - return componentFrom(declaration, declaration.name); - } - if (ts.isVariableDeclaration(declaration) && ts.isIdentifier(declaration.name) && declaration.initializer) { - const fn = unwrapToFn(ts, declaration.initializer); - return fn ? componentFrom(fn, declaration.name) : null; - } - return null; + const fn = unwrapToFn(ts, decl.initializer); + return fn ? components.fromNode(fn, decl.name) : null; } function listOptionalProps(component: Component): OptionalProp[] { @@ -261,64 +156,63 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer const defaults = defaultedBindingNames(ts, component.paramNode); const out: OptionalProp[] = []; for (const sym of type.getProperties()) { - if ((sym.getFlags() & ts.SymbolFlags.Optional) !== 0) { + if ((sym.getFlags() & ts.SymbolFlags.Optional) !== 0 && !isVendored(sym)) { out.push({ hasDefault: defaults.has(sym.getName()), name: sym.getName() }); } } return out; } - function componentFrom(fn: ComponentFn, nameNode: TS.Identifier): Component | null { - const paramNode = fn.parameters[0]; - const symbol = checker.getSymbolAtLocation(nameNode); - if (!paramNode || !symbol) { - return null; - } - return { fn, name: nameNode.text, paramNode, symbol: resolveAlias(symbol) }; - } - - /** Rebuild a component descriptor from the function alone. */ - function componentOfFn(fn: ComponentFn): Component | null { - const nameNode = bindingNameOfFn(ts, fn); - return nameNode ? componentFrom(fn, nameNode) : null; + /** + * Whether the prop is declared only in a dependency. A component spreading + * `React.ComponentProps<'button'>` inherits some 250 optional DOM and ARIA + * props; a verdict on those is true but useless — the `?` is not the + * author's to drop, and it drowns the props that are. + */ + function isVendored(sym: TS.Symbol): boolean { + const declarations = sym.getDeclarations() ?? []; + return ( + declarations.length > 0 && + declarations.every((declaration) => program.isSourceFileFromExternalLibrary(declaration.getSourceFile())) + ); } // ── JSX usage index ─────────────────────────────────────────────────────── - function buildUsageIndex(): Map { - const index = new Map(); + function indexProgram(): { calledIds: Set; usageIndex: Map } { + const calledIds = new Set(); + const usageIndex = new Map(); + const visit = (node: TS.Node): void => { + if (ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node)) { + const sym = jsxTagSymbol(node.tagName); + if (sym) { + const id = symbolId(sym); + const list = usageIndex.get(id) ?? []; + list.push(node); + usageIndex.set(id, list); + } + } else if (ts.isCallExpression(node)) { + const sym = checker.getSymbolAtLocation(node.expression); + if (sym) { + calledIds.add(symbolId(components.resolveAlias(sym))); + } + } + ts.forEachChild(node, visit); + }; + for (const sourceFile of program.getSourceFiles()) { - if (sourceFile.isDeclarationFile || sourceFile.fileName.includes('/node_modules/')) { - continue; + // Declarations and dependencies hold no call site this analysis owns. + if (!sourceFile.isDeclarationFile && !sourceFile.fileName.includes('/node_modules/')) { + visit(sourceFile); } - const visit = (node: TS.Node): void => { - if (ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node)) { - const sym = jsxTagSymbol(node.tagName); - if (sym) { - const id = symbolId(sym); - const list = index.get(id); - if (list) { - list.push(node); - } else { - index.set(id, [node]); - } - } - } - ts.forEachChild(node, visit); - }; - visit(sourceFile); } - return index; + return { calledIds, usageIndex }; } function jsxTagSymbol(tagName: TS.JsxTagNameExpression): TS.Symbol | null { // Intrinsics (
) have no symbol; resolves on the property. const sym = checker.getSymbolAtLocation(tagName); - return sym ? resolveAlias(sym) : null; - } - - function resolveAlias(sym: TS.Symbol): TS.Symbol { - return (sym.getFlags() & ts.SymbolFlags.Alias) !== 0 ? checker.getAliasedSymbol(sym) : sym; + return sym ? components.resolveAlias(sym) : null; } function symbolId(sym: TS.Symbol): number { @@ -351,8 +245,6 @@ export function verdictOf(usageCount: number, real: number, omit: number, ambigu return 'manual'; } -function push(out: T[], value: T | null): void { - if (value) { - out.push(value); - } +function compact(values: readonly (T | null)[]): T[] { + return values.filter((value) => value !== null); } diff --git a/packages/prop-flow/src/ast.test.ts b/packages/prop-flow/src/ast.test.ts index 9959924b..071c3fc0 100644 --- a/packages/prop-flow/src/ast.test.ts +++ b/packages/prop-flow/src/ast.test.ts @@ -1,10 +1,18 @@ import ts from 'typescript'; import { describe, expect, it } from 'vitest'; -import { bindingNameOfFn, defaultedBindingNames, enclosingComponentFn, isExported, unwrapToFn } from './ast.js'; +import { + bindingKey, + bindingNameOfFn, + defaultedBindingNames, + findAttr, + isComponentFn, + isExported, + unwrapToFn, +} from './ast.js'; import type { ComponentFn } from './ast.js'; -function parse(code: string): ts.SourceFile { - return ts.createSourceFile('fixture.tsx', code, ts.ScriptTarget.ES2023, true, ts.ScriptKind.TSX); +function parse(code: string, setParentNodes = true): ts.SourceFile { + return ts.createSourceFile('fixture.tsx', code, ts.ScriptTarget.ES2023, setParentNodes, ts.ScriptKind.TSX); } function firstStatement(code: string): ts.Statement { @@ -16,7 +24,11 @@ function firstStatement(code: string): ts.Statement { } /** First node in `code` matching `predicate`, depth-first. */ -function findNode(code: string, predicate: (node: ts.Node) => node is T): T { +function findNode( + code: string, + predicate: (node: ts.Node) => node is T, + setParentNodes = true, +): T { let found: T | undefined; const visit = (node: ts.Node): void => { if (!found && predicate(node)) { @@ -24,7 +36,7 @@ function findNode(code: string, predicate: (node: ts.Node) => } ts.forEachChild(node, visit); }; - visit(parse(code)); + visit(parse(code, setParentNodes)); if (!found) { throw new Error(`no matching node in: ${code}`); } @@ -56,6 +68,17 @@ describe('isExported', () => { }); }); +describe('isComponentFn', () => { + it('accepts the three function shapes a component can take, and nothing else', () => { + expect(isComponentFn(ts, firstStatement('function C(p: P) {}'))).toBe(true); + expect(isComponentFn(ts, initializerOf('const C = (p: P) => null;'))).toBe(true); + expect(isComponentFn(ts, initializerOf('const C = function (p: P) {};'))).toBe(true); + // A class method takes a props argument too, but it is none of the three. + expect(isComponentFn(ts, findNode('class C { render(p: P) {} }', ts.isMethodDeclaration))).toBe(false); + expect(isComponentFn(ts, firstStatement('const C = 1;'))).toBe(false); + }); +}); + describe('unwrapToFn', () => { it.each([ ['const C = () => null;', ts.SyntaxKind.ArrowFunction], @@ -80,19 +103,45 @@ describe('defaultedBindingNames', () => { it('has nothing to collect for a whole-object parameter', () => { expect([...defaultedBindingNames(ts, firstParameter('function C(props: P) {}'))]).toEqual([]); }); -}); -describe('enclosingComponentFn', () => { - it('walks up to the nearest function that takes a parameter', () => { - const identifier = findNode('function C(p: P) { return marker; }', (node): node is ts.Identifier => { - return ts.isIdentifier(node) && node.text === 'marker'; - }); + it('skips a defaulted binding whose props key cannot be read', () => { + // A computed key over a nested pattern: neither half names a props key. + expect([...defaultedBindingNames(ts, firstParameter('function C({ [k]: { x } = {} }: P) {}'))]).toEqual([]); + }); +}); - expect(enclosingComponentFn(ts, identifier)?.kind).toBe(ts.SyntaxKind.FunctionDeclaration); +describe('bindingKey', () => { + it.each<[string, string | null]>([ + ['function C({ label }: P) {}', 'label'], + // The props key is what the binding reads FROM, not what it binds to. + ['function C({ label: text }: P) {}', 'label'], + // Neither half is an identifier: a computed key over a nested pattern. + ['function C({ [key]: { inner } }: P) {}', null], + ])('%s → %s', (code, expected) => { + expect(bindingKey(ts, findNode(code, ts.isBindingElement))).toBe(expected); }); +}); - it('returns null at the top level', () => { - expect(enclosingComponentFn(ts, firstStatement('const a = 1;'))).toBeNull(); +describe('findAttr', () => { + /** What the lookup found for `title`, compacted: the value and the spreads. */ + function lookup(code: string): { spreads: number; value: string | null } { + const { attr, spreadsAfter } = findAttr(ts, findNode(code, ts.isJsxSelfClosingElement), 'title'); + const init = attr?.initializer; + return { spreads: spreadsAfter.length, value: init && ts.isStringLiteral(init) ? init.text : null }; + } + + it.each([ + ['const a = ;', { spreads: 0, value: 'x' }], + ['const a = ;', { spreads: 0, value: null }], + ['const a = ;', { spreads: 1, value: null }], + ['const a = ;', { spreads: 1, value: 'x' }], + ['const a = ;', { spreads: 2, value: null }], + // A spread BEFORE the attribute cannot win under JSX last-wins — dropped. + ['const a = ;', { spreads: 0, value: 'x' }], + // Duplicates resolve to the last one, exactly as JSX itself does. + ['const a = ;', { spreads: 0, value: 'y' }], + ])('%s', (code, expected) => { + expect(lookup(code)).toEqual(expected); }); }); @@ -114,4 +163,11 @@ describe('bindingNameOfFn', () => { expect(bindingNameOfFn(ts, fn)).toBeNull(); }); + + it('returns null when the tree carries no parent pointers to walk up', () => { + // The walk climbs `parent`; parsed without them there is nowhere to go. + const fn = findNode('const C = (p: P) => null;', ts.isArrowFunction, false); + + expect(bindingNameOfFn(ts, fn)).toBeNull(); + }); }); diff --git a/packages/prop-flow/src/ast.ts b/packages/prop-flow/src/ast.ts index b5bb73ed..21d9246d 100644 --- a/packages/prop-flow/src/ast.ts +++ b/packages/prop-flow/src/ast.ts @@ -4,8 +4,13 @@ import type { TypeScriptApi } from './typescript-api.js'; /** Every function shape that can back a component. */ export type ComponentFn = TS.ArrowFunction | TS.FunctionDeclaration | TS.FunctionExpression; -/** What `findAttr` found: the attribute, `null` (absent), or an unresolvable spread. */ -export type AttrLookup = TS.JsxAttribute | 'spread' | null; +/** What `findAttr` found on a JSX element for one prop name. */ +export interface AttrLookup { + /** The winning attribute of that name, or `null` when none is written out. */ + readonly attr: TS.JsxAttribute | null; + /** Spreads written AFTER `attr` — the only ones that can still override it. */ + readonly spreadsAfter: readonly TS.JsxSpreadAttribute[]; +} export function isExported(ts: TypeScriptApi, node: TS.Node): boolean { if (!ts.canHaveModifiers(node)) { @@ -32,20 +37,13 @@ export function unwrapToFn(ts: TypeScriptApi, expr: TS.Expression): ComponentFn /** Names in `function C({ foo = 1 }: P)` that carry a default initializer. */ export function defaultedBindingNames(ts: TypeScriptApi, paramNode: TS.ParameterDeclaration): Set { - const names = new Set(); if (!ts.isObjectBindingPattern(paramNode.name)) { - return names; - } - for (const element of paramNode.name.elements) { - if (!element.initializer) { - continue; - } - const key = bindingKey(ts, element); - if (key) { - names.add(key); - } + return new Set(); } - return names; + const keys = paramNode.name.elements + .filter((element) => element.initializer !== undefined) + .map((element) => bindingKey(ts, element)); + return new Set(keys.filter((key) => key !== null)); } /** The props key a binding element reads: `{ label: text }` → `label`. */ @@ -57,32 +55,22 @@ export function bindingKey(ts: TypeScriptApi, element: TS.BindingElement): strin } /** - * On a JSX element, find the attribute named `propName`: the JsxAttribute, or - * `null` when absent, or `'spread'` when a `{...x}` could be supplying it. + * On a JSX element, find what feeds `propName`. JSX resolves attributes + * last-wins, so anything before the winning attribute — including spreads — + * cannot influence the value and is dropped here. */ export function findAttr(ts: TypeScriptApi, el: TS.JsxOpeningLikeElement, propName: string): AttrLookup { - let sawSpread = false; - for (const attr of el.attributes.properties) { - if (ts.isJsxAttribute(attr) && ts.isIdentifier(attr.name) && attr.name.text === propName) { - return attr; - } - if (ts.isJsxSpreadAttribute(attr)) { - sawSpread = true; + let attr: TS.JsxAttribute | null = null; + let spreadsAfter: TS.JsxSpreadAttribute[] = []; + for (const property of el.attributes.properties) { + if (ts.isJsxSpreadAttribute(property)) { + spreadsAfter.push(property); + } else if (ts.isJsxAttribute(property) && ts.isIdentifier(property.name) && property.name.text === propName) { + attr = property; + spreadsAfter = []; } } - return sawSpread ? 'spread' : null; -} - -/** Nearest enclosing function that takes a parameter — i.e. could be a component. */ -export function enclosingComponentFn(ts: TypeScriptApi, node: TS.Node): ComponentFn | null { - let current: TS.Node | undefined = node.parent; - while (current) { - if (isComponentFn(ts, current) && current.parameters.length > 0) { - return current; - } - current = current.parent; - } - return null; + return { attr, spreadsAfter }; } /** @@ -93,16 +81,14 @@ export function bindingNameOfFn(ts: TypeScriptApi, fn: ComponentFn): TS.Identifi if (ts.isFunctionDeclaration(fn) && fn.name) { return fn.name; } + // Wrapper calls are the only thing worth climbing past; whatever sits above + // them either binds the function to a name or ends the search. let current: TS.Node | undefined = fn.parent; - while (current) { - if (ts.isVariableDeclaration(current) && ts.isIdentifier(current.name)) { - return current.name; - } - if (ts.isCallExpression(current)) { - current = current.parent; - continue; - } - return null; + while (current && ts.isCallExpression(current)) { + current = current.parent; + } + if (current && ts.isVariableDeclaration(current) && ts.isIdentifier(current.name)) { + return current.name; } return null; } diff --git a/packages/prop-flow/src/classify.ts b/packages/prop-flow/src/classify.ts new file mode 100644 index 00000000..485e96df --- /dev/null +++ b/packages/prop-flow/src/classify.ts @@ -0,0 +1,307 @@ +import type * as TS from 'typescript'; +import { bindingKey, findAttr, isComponentFn } from './ast.js'; +import type { Component, ComponentFactory } from './component.js'; +import type { TypeScriptApi } from './typescript-api.js'; + +/** The value is the enclosing component's own prop — climb one level up. */ +interface Passthrough { + readonly component: Component; + readonly kind: 'passthrough'; + readonly prop: string; +} + +/** Static analysis cannot decide this one; a human has to look. */ +interface Manual { + readonly kind: 'manual'; + readonly note: string; +} + +/** Nothing reaches the prop: no attribute, or `prop={undefined}`. */ +interface Omission { + readonly kind: 'omit'; + readonly note?: string; +} + +/** A value originates right here. */ +interface Real { + readonly kind: 'real'; + readonly note: string; +} + +/** Where the value a JSX element feeds to one prop comes from. */ +export type Classification = Manual | Omission | Passthrough | Real; + +/** + * What a `{...x}` can contribute to one prop: + * + * definite the spread type has the prop, and it is required → always wins + * maybe the spread type has the prop, but it is optional → may win + * no the spread type provably lacks the prop → irrelevant + * opaque the type cannot answer the question → nothing may be concluded + */ +type Carry = 'definite' | 'maybe' | 'no' | 'opaque'; + +/** A binding that names the props of the component it is declared in. */ +interface PropsBinding { + readonly component: Component; + /** The destructured element, or null when the whole object is the binding. */ + readonly element: TS.BindingElement | null; +} + +export interface ClassifierOptions { + readonly checker: TS.TypeChecker; + readonly components: ComponentFactory; + readonly ts: TypeScriptApi; +} + +export interface Classifier { + /** Where the value `el` feeds to `propName` comes from, spreads included. */ + classifyElement(el: TS.JsxOpeningLikeElement, propName: string): Classification; +} + +export function createClassifier({ checker, components, ts }: ClassifierOptions): Classifier { + return { classifyElement }; + + // ── call site ───────────────────────────────────────────────────────────── + + function classifyElement(el: TS.JsxOpeningLikeElement, propName: string): Classification { + const { attr, spreadsAfter } = findAttr(ts, el, propName); + const candidates = spreadsAfter + .map((spread) => ({ carry: carryOf(spread, propName), spread })) + .filter(({ carry }) => carry !== 'no'); + const winner = candidates.at(-1); + + if (!winner) { + // Either no spread at all, or every one of them provably lacks the prop. + return attr ? classifyAttribute(attr) : { kind: 'omit' }; + } + if (winner.carry === 'opaque') { + return { kind: 'manual', note: 'spread of a type that cannot be read' }; + } + // An optional prop in the last spread may or may not override what comes + // before it — two possible runtime outcomes, so no static verdict. A + // `definite` winner overrides everything before it and needs no such check. + if (winner.carry === 'maybe' && (candidates.length > 1 || attr !== null)) { + return { kind: 'manual', note: 'an optional prop in a spread contests an earlier value' }; + } + return classifySpread(winner.spread, propName); + } + + /** What `{...x}` can contribute to `propName`, judged by its type alone. */ + function carryOf(spread: TS.JsxSpreadAttribute, propName: string): Carry { + const type = checker.getTypeAtLocation(spread.expression); + if (isOpaque(type, propName)) { + return 'opaque'; + } + const sym = type.getProperty(propName); + if (!sym) { + return 'no'; + } + return (sym.getFlags() & ts.SymbolFlags.Optional) !== 0 ? 'maybe' : 'definite'; + } + + /** + * Whether `getProperty` returning nothing would be uninformative rather than + * a real absence. Every one of these makes the prop possibly-present without + * a symbol to prove it, so treating them as "absent" would under-report. + */ + function isOpaque(type: TS.Type, propName: string): boolean { + if ((type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) !== 0) { + return true; + } + // Record and friends: no declared properties at all. + if (checker.getIndexInfoOfType(type, ts.IndexKind.String)) { + return true; + } + // A union carries the prop in only some constituents: `getProperty` on the + // union returns undefined, indistinguishable from an all-round absence. + if (type.isUnion() && !type.getProperty(propName)) { + return type.types.some((member) => isOpaque(member, propName) || member.getProperty(propName) !== undefined); + } + return false; + } + + // ── spread resolution ───────────────────────────────────────────────────── + + function classifySpread(spread: TS.JsxSpreadAttribute, propName: string): Classification { + const expr = spread.expression; + // `{...props}` / `{...rest}` — the enclosing component forwards its own. + if (ts.isIdentifier(expr)) { + const binding = propsBindingOf(expr); + if (binding && (binding.element === null || binding.element.dotDotDotToken !== undefined)) { + return { component: binding.component, kind: 'passthrough', prop: propName }; + } + } + const literal = objectLiteralOf(expr); + if (literal) { + return classifyLiteralKey(literal, propName); + } + return { kind: 'manual', note: `spread of ${ts.SyntaxKind[expr.kind]} cannot be resolved` }; + } + + /** The object literal `expr` is, or that a `const` it names is initialised to. */ + function objectLiteralOf(expr: TS.Expression): TS.ObjectLiteralExpression | null { + if (ts.isObjectLiteralExpression(expr)) { + return expr; + } + if (!ts.isIdentifier(expr)) { + return null; + } + const declaration = declarationOf(expr); + if (!declaration || !ts.isVariableDeclaration(declaration) || !declaration.initializer) { + return null; + } + // A `let` could be reassigned between declaration and use. + if ((declaration.parent.flags & ts.NodeFlags.Const) === 0) { + return null; + } + return ts.isObjectLiteralExpression(declaration.initializer) ? declaration.initializer : null; + } + + /** What the object literal sets `propName` to, under last-wins ordering. */ + function classifyLiteralKey(literal: TS.ObjectLiteralExpression, propName: string): Classification { + let member: TS.ObjectLiteralElementLike | null = null; + let contested = false; + for (const property of literal.properties) { + const key = memberKey(property); + if (key === null) { + // A nested spread or a computed key could still be setting propName. + contested = true; + } else if (key === propName) { + member = property; + contested = false; + } + } + + if (contested) { + return { kind: 'manual', note: 'a nested spread or computed key in the spread object' }; + } + if (!member) { + return { kind: 'omit', note: 'not set in the spread object' }; + } + if (ts.isPropertyAssignment(member)) { + return classifyExpression(member.initializer); + } + if (ts.isShorthandPropertyAssignment(member)) { + return classifyExpression(member.name); + } + return { kind: 'manual', note: 'unrecognised object literal member' }; + } + + /** The static key a literal member writes, or null when it is not readable. */ + function memberKey(property: TS.ObjectLiteralElementLike): string | null { + const name = property.name; + if (!name) { + return null; + } + if (ts.isIdentifier(name) || ts.isStringLiteral(name) || ts.isNumericLiteral(name)) { + return name.text; + } + return null; + } + + // ── attribute values ────────────────────────────────────────────────────── + + function classifyAttribute(attr: TS.JsxAttribute): Classification { + const init = attr.initializer; + // Shorthand boolean: → always a concrete `true`. + if (init === undefined) { + return { kind: 'real', note: 'boolean shorthand' }; + } + if (ts.isStringLiteral(init)) { + return { kind: 'real', note: 'string literal' }; + } + if (!ts.isJsxExpression(init) || init.expression === undefined) { + return { kind: 'manual', note: 'unrecognised attribute form' }; + } + return classifyExpression(init.expression); + } + + /** Where the value an expression evaluates to originates. */ + function classifyExpression(expr: TS.Expression): Classification { + if (ts.isStringLiteral(expr)) { + return { kind: 'real', note: 'string literal' }; + } + // `prop={undefined}` is an omission dressed up as a pass. + if (ts.isIdentifier(expr) && expr.text === 'undefined') { + return { kind: 'omit', note: 'explicit undefined' }; + } + // Identifier — a destructured prop of the enclosing component (climb) or a + // local value (a real source). + if (ts.isIdentifier(expr)) { + return asDestructuredProp(expr) ?? { kind: 'real', note: 'local value' }; + } + // `props.foo` — climb when `props` is the enclosing props parameter. + if (ts.isPropertyAccessExpression(expr) && ts.isIdentifier(expr.expression)) { + return asMemberProp(expr.expression, expr.name.text) ?? { kind: 'real', note: 'member value' }; + } + // Any other expression (call, object, conditional, JSX, template…) — the + // value is produced right here. Treat as a real source; a conditional that + // can yield undefined is the one false positive we accept (conservative: + // counts as "present"). + return { kind: 'real', note: ts.SyntaxKind[expr.kind] }; + } + + /** `` where `text` is destructured from the props parameter. */ + function asDestructuredProp(id: TS.Identifier): Classification | null { + const binding = propsBindingOf(id); + // A rest binding used as a value passes the rest OBJECT, not one prop — and + // that object always exists, so it is a real source rather than a climb. + if (!binding?.element || binding.element.dotDotDotToken) { + return null; + } + return { component: binding.component, kind: 'passthrough', prop: bindingKey(ts, binding.element) ?? id.text }; + } + + /** `` where `props` is the whole props parameter. */ + function asMemberProp(objId: TS.Identifier, name: string): Classification | null { + const binding = propsBindingOf(objId); + if (binding?.element !== null) { + return null; + } + return { component: binding.component, kind: 'passthrough', prop: name }; + } + + // ── props bindings ──────────────────────────────────────────────────────── + + /** + * Resolve `id` to the props binding it names, if any. Symbol resolution, not + * a lexical name match: inside a render callback the nearest enclosing + * function is the callback, whose parameter is an item — not props. + */ + function propsBindingOf(id: TS.Identifier): PropsBinding | null { + const declaration = declarationOf(id); + if (!declaration) { + return null; + } + if (ts.isParameter(declaration)) { + const component = componentOfParam(declaration); + return component ? { component, element: null } : null; + } + if (ts.isBindingElement(declaration)) { + const pattern = declaration.parent; + // Only a top-level destructure of the props parameter counts; a nested + // one has a BindingElement, not a ParameterDeclaration, as its parent. + if (!ts.isObjectBindingPattern(pattern) || !ts.isParameter(pattern.parent)) { + return null; + } + const component = componentOfParam(pattern.parent); + return component ? { component, element: declaration } : null; + } + return null; + } + + /** The component whose FIRST parameter `param` is — props sit in no other. */ + function componentOfParam(param: TS.ParameterDeclaration): Component | null { + const fn = param.parent; + if (!isComponentFn(ts, fn) || fn.parameters[0] !== param) { + return null; + } + return components.fromFn(fn); + } + + function declarationOf(id: TS.Identifier): TS.Declaration | null { + const symbol = checker.getSymbolAtLocation(id); + return symbol?.valueDeclaration ?? symbol?.declarations?.[0] ?? null; + } +} diff --git a/packages/prop-flow/src/component.ts b/packages/prop-flow/src/component.ts new file mode 100644 index 00000000..3b9b5fec --- /dev/null +++ b/packages/prop-flow/src/component.ts @@ -0,0 +1,70 @@ +import type * as TS from 'typescript'; +import { bindingNameOfFn, unwrapToFn } from './ast.js'; +import type { ComponentFn } from './ast.js'; +import type { TypeScriptApi } from './typescript-api.js'; + +export interface Component { + readonly fn: ComponentFn; + readonly name: string; + readonly paramNode: TS.ParameterDeclaration; + readonly symbol: TS.Symbol; +} + +export interface ComponentFactoryOptions { + readonly checker: TS.TypeChecker; + readonly ts: TypeScriptApi; +} + +export interface ComponentFactory { + /** Resolve an `export { X }` specifier back to the function it names. */ + fromExport(element: TS.ExportSpecifier): Component | null; + /** Rebuild a component descriptor from the function alone. */ + fromFn(fn: ComponentFn): Component | null; + /** Describe the component `fn`, named by `nameNode`. */ + fromNode(fn: ComponentFn, nameNode: TS.Identifier): Component | null; + /** Follow an import alias to the symbol it ultimately names. */ + resolveAlias(sym: TS.Symbol): TS.Symbol; +} + +/** + * Turns functions and export specifiers into `Component` descriptors. The + * checker is needed for symbol identity, which is what the usage index and the + * cycle guard key on. + */ +export function createComponentFactory({ checker, ts }: ComponentFactoryOptions): ComponentFactory { + return { fromExport, fromFn, fromNode, resolveAlias }; + + function fromExport(element: TS.ExportSpecifier): Component | null { + const exported = checker.getSymbolAtLocation(element.propertyName ?? element.name); + const declaration = exported && resolveAlias(exported).declarations?.[0]; + if (!declaration) { + return null; + } + if (ts.isFunctionDeclaration(declaration) && declaration.name) { + return fromNode(declaration, declaration.name); + } + if (ts.isVariableDeclaration(declaration) && ts.isIdentifier(declaration.name) && declaration.initializer) { + const fn = unwrapToFn(ts, declaration.initializer); + return fn ? fromNode(fn, declaration.name) : null; + } + return null; + } + + function fromFn(fn: ComponentFn): Component | null { + const nameNode = bindingNameOfFn(ts, fn); + return nameNode ? fromNode(fn, nameNode) : null; + } + + function fromNode(fn: ComponentFn, nameNode: TS.Identifier): Component | null { + const paramNode = fn.parameters[0]; + const symbol = checker.getSymbolAtLocation(nameNode); + if (!paramNode || !symbol) { + return null; + } + return { fn, name: nameNode.text, paramNode, symbol: resolveAlias(symbol) }; + } + + function resolveAlias(sym: TS.Symbol): TS.Symbol { + return (sym.getFlags() & ts.SymbolFlags.Alias) !== 0 ? checker.getAliasedSymbol(sym) : sym; + } +} diff --git a/packages/prop-flow/src/index.ts b/packages/prop-flow/src/index.ts index b9ea26d7..714bf54a 100644 --- a/packages/prop-flow/src/index.ts +++ b/packages/prop-flow/src/index.ts @@ -1,9 +1,10 @@ export { createAnalyzer, verdictOf } from './analyzer.js'; -export type { Analyzer, AnalyzerOptions, Component } from './analyzer.js'; +export type { Analyzer, AnalyzerOptions } from './analyzer.js'; export { parseArgs } from './args.js'; export type { CliArgs } from './args.js'; export { runCli, USAGE } from './cli.js'; export type { CliContext, OutputStream } from './cli.js'; +export type { Component } from './component.js'; export { PropFlowError } from './errors.js'; export { analyseProps } from './prop-flow.js'; export type { AnalyseOptions } from './prop-flow.js'; diff --git a/packages/prop-flow/src/report.ts b/packages/prop-flow/src/report.ts index 2dce244c..8de0ff6f 100644 --- a/packages/prop-flow/src/report.ts +++ b/packages/prop-flow/src/report.ts @@ -61,7 +61,7 @@ export function hint(row: PropReport): string { case 'justified': return ' → genuinely sometimes-absent. The `?` is correct.\n'; case 'manual': - return ' → a spread / rename / dynamic value blocks a static verdict. Check the MANUAL sites by hand.\n'; + return ' → an unreadable spread or a contested override blocks a static verdict. Check the MANUAL sites by hand.\n'; case 'unused-component': return ' → the component has no call sites in this Program. Verify the tsconfig spans its callers.\n'; default: diff --git a/packages/prop-flow/src/types.ts b/packages/prop-flow/src/types.ts index 308ad09f..0fd8eb1d 100644 --- a/packages/prop-flow/src/types.ts +++ b/packages/prop-flow/src/types.ts @@ -5,14 +5,15 @@ * unnecessary-optional every call site passes it → could be required * caller-dead NO call site passes it → optional + always undefined * unused-component the component itself has no call sites in the Program - * manual a spread / rename / render-prop on the path blocks a - * static conclusion → listed for a human to check + * manual an unreadable spread, a dynamic value or a contested + * override blocks a static conclusion → listed for a + * human to check * cycle the pass-through graph looped back on itself; the * repeat visit contributes no new information */ export type Verdict = 'caller-dead' | 'cycle' | 'justified' | 'manual' | 'unnecessary-optional' | 'unused-component'; -export type SiteKind = 'manual' | 'omit' | 'passthrough' | 'real' | 'spread'; +export type SiteKind = 'manual' | 'omit' | 'passthrough' | 'real'; /** One JSX call site, classified. */ export interface Site { diff --git a/packages/prop-flow/vitest.config.ts b/packages/prop-flow/vitest.config.ts index ada03332..80484537 100644 --- a/packages/prop-flow/vitest.config.ts +++ b/packages/prop-flow/vitest.config.ts @@ -11,7 +11,8 @@ export default defineConfig({ ], include: ['src/**/*.ts'], provider: 'v8', - reporter: ['text', 'lcov'], + // json-summary is what per-file coverage tooling reads off disk. + reporter: ['text', 'lcov', 'json-summary'], }, environment: 'node', include: ['src/**/*.test.ts'],