prop-flow: stop reporting live props as caller-dead

packages/prop-flow/CHANGELOG.md

@@ -5,6 +5,54 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## To Be Released
 
+## 3.1.0
+
+Three corrections to one failure: a prop that every caller passes, reported
+`caller-dead` — "inline the default, remove the prop" on live code. Two are
+holes in the walk; the third is the guard for whatever holes are left. Nothing
+in the library API or the CLI arguments moved, and `verdictOf` keeps its
+signature; `Site.kind` gains a value.
+
+- Fixed: `children` passed by JSX nesting was read as an omission. The value of
+  `children` is the one a call site writes *between* the tags rather than in the
+  attributes, and only the attributes were being read — so `<Panel><Body/>
+  </Panel>` counted as a caller that passes nothing, and a `children?` nested at
+  every call site came back `caller-dead`. Nesting now wins over both an
+  attribute of that name and any spread, matching what JSX itself does;
+  `<Panel>{slot}</Panel>` is a pass-through like any other, while whitespace
+  between the tags, a lone `{/* comment */}` and `<Panel></Panel>` correctly
+  reach nothing
+- Fixed: a component whose wrapper call only NAMES the function it wraps —
+  `function CardComponent(…) {}` plus `export const Card = memo(CardComponent)`,
+  the shape every wrapped component takes once it outgrows being written inline
+  — was two components to the walk. JSX renders `Card`, while a pass-through
+  climbing out of the body arrives at `CardComponent`, and the call sites filed
+  under the one name were invisible from the other. The climb ended on an empty
+  usage list, contributed nothing, and a prop that every caller passes came back
+  `caller-dead`: "delete the prop" on live code. The same gap made
+  `findComponents` skip such a file entirely, so pointing prop-flow at
+  `Card.tsx` reported no components at all. The wrapped function may live in
+  another file — `memo(CardImpl)` over an import resolves through the alias
+- A `caller-dead` whose sites include a pass-through that contributed nothing is
+  now reported `manual`. Both fixes above were one shape of the same failure:
+  the subtree came back a silent 0/0/0, the omissions written elsewhere were all
+  that was left, and the walk concluded nobody passes a prop that is passed on
+  every render. There are three reasons a subtree comes back empty — the
+  component really is dead, it is *called* rather than rendered (already
+  `manual`), or the walk filed its call sites under a different key. The first
+  and the third are indistinguishable, and only the first may safely end in
+  "delete this", so neither does. Scoped to `caller-dead` on purpose: under
+  `justified` or `unnecessary-optional` a silent pass-through changes nothing
+  anyone acts on, and downgrading those would throw away good verdicts to guard
+  against a risk that only exists where the advice is destructive
+- Such a pass-through is reported under a new `Site.kind`, `silent`, and the
+  hint names it, so the row says which line to go and check rather than leaving
+  the reader to notice that one of them is empty. The kind is printed under
+  every verdict, as evidence; only `caller-dead` is downgraded by it. The counts
+  never move — a downgraded row still reads `passes=0`, and what changed is the
+  conclusion drawn from it. A self-recursive pass-through is not a silence: the
+  first visit counted that subtree, and the repeat is meant to add nothing
+
 ## 3.0.0
 
 - **BREAKING CHANGE**: a file with no exported component is no longer a failure.

packages/prop-flow/README.md

@@ -105,7 +105,7 @@ cannot reach either.
 | `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`               | an unreadable spread or a contested override blocks a static conclusion  |
+| `manual`               | an unreadable spread, a contested override, or a pass-through that came back empty, blocks a static conclusion |
 | `required`             | the prop has no `?` to judge — listed only for its constant value        |
 
 Exit codes: `0` success, `1` nothing to do (usage printed), `2` a handled
@@ -214,22 +214,60 @@ 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`.
+### When a pass-through comes back empty
+
+A pass-through whose subtree moved no counter at all is reported as a `silent`
+site rather than a `passthrough`. There are three reasons a subtree comes back
+empty, and only one of them is an answer:
+
+1. **The component really is dead** — nothing renders it, nothing calls it.
+   Contributing nothing is the honest result.
+2. **The component is *called* rather than rendered** — a `renderX({ … })` test
+   helper. Its callers exist but are invisible to a JSX walk, so the site is
+   `manual` on its own, before any verdict is formed.
+3. **The walk filed its call sites under a different key.** A bug, by
+   definition — and not one you can see from the output.
+
+(1) and (3) are indistinguishable, so `caller-dead` — the one verdict whose
+advice is destructive — is not allowed to rest on either. **A `caller-dead`
+with at least one `silent` site is reported `manual` instead**, and the hint
+names the pass-through to go and check. The counts are untouched: the row still
+reads `passes=0`, and what changed is only the conclusion drawn from it.
+
+The guard is scoped to `caller-dead` on purpose. Under `justified` or
+`unnecessary-optional` a silent pass-through changes nothing anyone acts on, so
+those verdicts stand as they are — the `silent` line is still printed, as
+evidence rather than as a downgrade. In practice most `caller-dead` rows bottom
+out in direct omissions and carry no pass-through at all, so the rule is a
+no-op on them.
 
 `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.
 
+`children` is read off the nesting, which is where JSX puts it rather than in
+the attributes. Nesting wins over an attribute of that name and over every
+spread, exactly as JSX resolves it: `<Panel children={a}>{b}</Panel>` passes
+`b`. A lone `<Panel>{slot}</Panel>` is a pass-through like any other value, so
+a forwarded `children` is traced to where it comes from. Whitespace between the
+tags, a lone `{/* comment */}` and `<Panel></Panel>` reach nothing and stay
+omissions.
+
 Components are picked up from `export function C`, `export const C = …`
 (including `memo()` / `forwardRef()` wrappers), `export default function C` and
 `export { C }` at the bottom of the file. A component re-exported through a
 barrel is still found at its call sites, but must be inspected in the file that
 declares it.
 
+A wrapper whose argument only *names* the function — `function CardComponent(…)
+{}` above `export const Card = memo(CardComponent)` — gives one component two
+names. Call sites are written as `<Card/>`, while a pass-through climbing out of
+the body arrives at `CardComponent`; both reach the same entry, so a prop fed
+through the wrapper is counted at every call site rather than at none of them.
+The wrapped function may sit in another file — `memo(CardImpl)` over an import
+is followed through the alias.
+
 An exported `useX` taking an options object is skipped. It is indistinguishable
 from a component to the AST and has no JSX call sites, so every one of its
 options would come back `unused-component` — a statement about the walk, not

packages/prop-flow/fixtures/basic/app.tsx

@@ -9,6 +9,7 @@ import { Panel } from './panel';
 import { Renamed } from './renamed';
 import { Rest } from './rest';
 import { Tree } from './tree';
+import { Chrome, Remote, Stripe } from './wrapped';
 
 const badgeProps: BadgeProps = { text: 'spread', tone: 'info' };
 
@@ -34,6 +35,12 @@ export function App() {
       <Ghost label={undefined} />
       <Ghost label={undefined} />
       <Badge {...badgeProps} />
+      <Chrome highlight />
+      <Chrome />
+      {/* The omission that makes Stripe.loud caller-dead as long as the walk
+          cannot see through <Chrome/> to the function it wraps. */}
+      <Stripe />
+      <Remote tint="cross-file" />
     </main>
   );
 }

packages/prop-flow/fixtures/basic/children.tsx

@@ -0,0 +1,80 @@
+// What reaches `children`, and what only looks like it does. Nesting is the one
+// prop value written outside the attributes, so every shape below is invisible
+// to a walk that reads attributes alone — and `children` is the prop where
+// concluding "nobody passes it" does the most damage.
+//
+// `ChildrenApp` at the bottom is the only render root in this file, so nothing
+// here moves the counts the other fixtures assert.
+
+export interface SlotProps {
+  /** justified: nested at most call sites, genuinely absent at others */
+  children?: JSX.Element;
+  id: string;
+}
+
+export function Slot({ children, id }: SlotProps) {
+  return <div data-id={id}>{children}</div>;
+}
+
+export interface RelayedProps {
+  /** justified: forwarded by nesting, which is a pass-through like any other */
+  children?: JSX.Element;
+  id: string;
+}
+
+export function Relayed({ children, id }: RelayedProps) {
+  return <Slot id={id}>{children}</Slot>;
+}
+
+export interface ShellProps {
+  /** unnecessary-optional: nested once, and never omitted */
+  children?: JSX.Element;
+  id: string;
+}
+
+/** Spreads its whole props object AND nests: the nesting is what arrives. */
+export function Shell(props: ShellProps) {
+  return <Slot {...props}>
+    <b />
+  </Slot>;
+}
+
+export function ChildrenApp() {
+  return (
+    <div>
+      {/* Values that reach `children` */}
+      <Slot id="element">
+        <b />
+      </Slot>
+      <Slot id="text">plain text</Slot>
+      <Slot id="expression">{'from an expression'}</Slot>
+      <Slot id="several">
+        <b />
+        <i />
+      </Slot>
+      {/* An attribute is still read where nothing is nested… */}
+      <Slot id="attribute" children={<i />} />
+      {/* …and loses to the nesting where there is some. */}
+      <Slot id="both" children={<i />}>
+        <u />
+      </Slot>
+
+      {/* Nestings that reach nothing */}
+      <Slot id="selfclosing" />
+      <Slot id="empty"></Slot>
+      <Slot id="blank">
+      </Slot>
+      <Slot id="comment">{/* nothing to render */}</Slot>
+      <Slot id="explicit">{undefined}</Slot>
+
+      {/* Forwarded rather than originated */}
+      <Relayed id="relayed">
+        <em />
+      </Relayed>
+      <Relayed id="relayed-bare" />
+      <Shell id="shell">
+        <s />
+      </Shell>
+    </div>
+  );
+}

packages/prop-flow/fixtures/basic/silent.tsx

@@ -0,0 +1,45 @@
+// A `caller-dead` that a silent pass-through pulls back to MANUAL. `Base.note`
+// is omitted at every call site the walk can read, and the one remaining site
+// climbs into `Relayed`, which is rendered nowhere — so the subtree comes back
+// empty and the counts say "nobody passes it" on the strength of a hole.
+
+export interface BaseProps {
+  id: string;
+  /** manual: every readable caller omits it, one pass-through said nothing */
+  note?: string;
+}
+
+export function Base({ id, note }: BaseProps) {
+  return <b data-id={id}>{note}</b>;
+}
+
+export interface RelayedProps {
+  id: string;
+  note?: string;
+}
+
+/** Exported and forwards `note`, but rendered nowhere in the Program. */
+export function Relayed(props: RelayedProps) {
+  return <Base {...props} />;
+}
+
+export interface LoudProps {
+  id: string;
+  /** caller-dead: nothing but direct omissions — the guard is a no-op here */
+  note?: string;
+}
+
+export function Loud({ id, note }: LoudProps) {
+  return <i data-id={id}>{note}</i>;
+}
+
+export function SilentApp() {
+  return (
+    <div>
+      <Base id="a" />
+      <Base id="b" />
+      <Loud id="c" />
+      <Loud id="d" />
+    </div>
+  );
+}

packages/prop-flow/fixtures/basic/wrapped-impl.tsx

@@ -0,0 +1,22 @@
+export interface TintedProps {
+  /** unnecessary-optional, and only reachable across the wrapper AND the file
+   *  boundary: the climb lands on RemoteImpl here, while every call site is a
+   *  <Remote/> written against the binding wrapped.tsx exports. */
+  shade?: string;
+}
+
+export function Tinted({ shade }: TintedProps) {
+  return <q data-shade={shade} />;
+}
+
+export interface RemoteProps {
+  /** unnecessary-optional: the one call site passes it, through the wrapper */
+  tint?: string;
+}
+
+// The implementation half of a wrapper split across two files — wrapped.tsx
+// imports this name and hands it to memo(). Resolving the wrapper argument has
+// to follow the import alias, not just the local name.
+export function RemoteImpl({ tint }: RemoteProps) {
+  return <Tinted shade={tint} />;
+}

packages/prop-flow/fixtures/basic/wrapped.tsx

@@ -0,0 +1,41 @@
+import { RemoteImpl } from './wrapped-impl';
+
+// The wrapper shape panel.tsx does not cover: the wrapped function is
+// DECLARED, and the wrapper call only names it. That gives one component two
+// symbols — JSX renders `Chrome`, while a pass-through climbs into
+// `ChromeComponent` — and a walk that keeps them apart finds no call site on
+// the way up, which reads as "nobody passes this prop".
+
+export interface StripeProps {
+  /** justified: passed through Chrome, and omitted at a direct call site */
+  loud?: boolean;
+}
+
+export function Stripe({ loud }: StripeProps) {
+  return <hr data-loud={loud} />;
+}
+
+export interface ChromeProps {
+  /** the prop the pass-through climbs to — declared on the inner function */
+  highlight?: boolean;
+}
+
+function ChromeComponent({ highlight }: ChromeProps) {
+  return <Stripe loud={highlight} />;
+}
+
+// Stand-in for React.memo, as in panel.tsx — here wrapping a name, not a
+// function written out inside the call.
+const memo = <T,>(component: T): T => component;
+
+export const Chrome = memo(ChromeComponent);
+
+// The same wrapper, but the name it hands to memo() is an IMPORT. Following it
+// means resolving the alias first — the local name resolves to nothing.
+export const Remote = memo(RemoteImpl);
+
+// Wrapper arguments that name no function: one that does not exist at all, and
+// one that names a type. Following a name must not turn either into a
+// component — nor stop the walk on the way past.
+export const Missing = memo(Absent);
+export const Typed = memo(ChromeProps);

packages/prop-flow/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@fxone/prop-flow",
-  "version": "3.0.0",
+  "version": "3.1.0",
   "description": "trace an optional prop across every JSX call site and tell whether its `?` is justified",
   "keywords": [
     "typescript",