prop-flow: resolve JSX spreads instead of reporting them as manual

.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/

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: `<C title="x" {...props} />` reported `"x"`, but
+  JSX resolves last-wins, so the spread overrides the attribute
+- Fixed pass-throughs inside render callbacks: `items.map(() => <C x={props.x}/>)`
+  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

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<string, unknown>`, 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 `<C title="x" {...props} />`
+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

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;
 }
 

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 <Sink data={rest} />;
 }

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 <b data-id={id}>{note}</b>;
+}
+
+/** Collects the spreads the classifier must refuse to resolve. */
+export function Blocked({ id, note }: NotedProps) {
+  return <i data-id={id}>{note}</i>;
+}
+
+/** Collects the values written as attributes on the element itself. */
+export function Direct({ id, note }: NotedProps) {
+  return <u data-id={id}>{note}</u>;
+}
+
+// ── 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 (
+    <div>
+      <Literal {...{ id: 'inline', note: 'an inline object literal' }} />
+      <Literal {...shorthand} />
+      <Literal {...quoted} />
+      <Literal {...unset} />
+      <Literal {...nested} />
+      <Literal {...computed} />
+      <Literal {...getter} />
+    </div>
+  );
+}
+
+// ── 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) => <Blocked {...props} />];
+
+/** `options` is not the first parameter, so it names no component's props. */
+function renderSecond(id: string, options: NotedProps) {
+  return <Blocked id={id} {...options} />;
+}
+
+/** A class method is no component function, so its parameter is not props. */
+class LegacyRenderer {
+  render(props: NotedProps) {
+    return <Blocked {...props} />;
+  }
+}
+
+export function BlockedSites() {
+  return (
+    <div>
+      <Blocked {...reassignable} />
+      <Blocked {...fromCall} />
+      <Blocked {...makeProps()} />
+      <Blocked {...inner} />
+      <Blocked id="any" {...anything} />
+      <Blocked id="union" {...partlyNoted} />
+      <Blocked id="disjoint" {...neverNoted} />
+    </div>
+  );
+}
+
+// ── 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 <Direct id={id} note={config.note} />;
+}
+
+/** The destructure is a class method's, so it binds no component's props. */
+class LegacyDestructured {
+  render({ id, note }: NotedProps) {
+    return <Direct id={id} note={note} />;
+  }
+}
+
+export function DirectSites() {
+  return (
+    <div>
+      {/* `absent` is undeclared on purpose: an identifier that resolves to
+          nothing must not stop the walk. */}
+      <Direct id="absent" note={absent} />
+      {/* The value was commented out, leaving an attribute form with nothing
+          to read. */}
+      <Direct id="empty" note={/* nothing */} />
+      {/* A tag that resolves to no symbol at all is not a call site. */}
+      <Unknown.Tag note="ignored" />
+    </div>
+  );
+}
+
+// ── 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 <em data-id={id}>{note}</em>;
+}
+
+// ── 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;