prop-flow: make output batch-safe for a loop over changed files

packages/prop-flow/CHANGELOG.md

@@ -5,6 +5,38 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## To Be Released
 
+## 3.0.0
+
+- **BREAKING CHANGE**: a file with no exported component is no longer a failure.
+  It exits `0` with an empty report instead of `2` with a message on stderr —
+  a route module or a props-less page has nothing to analyse, and that is an
+  answer, not a broken input. Exit `2` now means only what actually blocked the
+  analysis: a missing file, no resolvable compiler, a tsconfig that does not
+  span the file. A caller looping over changed files can finally stop on a real
+  failure without stopping on a page component
+- **BREAKING CHANGE**: `Report` carries a required `components` field — the
+  number of exported components with a typed props object. It is what tells the
+  two empty reports apart: no component to look at (`0`), versus a component
+  whose props are all required
+- **BREAKING CHANGE**: under `--json` a handled failure is now a `{"error": …}`
+  object on **stdout** instead of a line on stderr, so an invocation that names
+  a file emits exactly one JSON object whatever happens. A `for f in …; do
+  prop-flow "$f" --json; done | jq -s` no longer breaks on the whole stream
+  because one file failed. `--json` is read off the raw argv, so a bad argument
+  reaches the envelope too. Without `--json` the stderr line is unchanged.
+  Usage output is deliberately left outside the envelope: `--help`, and exit `1`
+  for an invocation with no file, still print plain text — both answer a person,
+  and neither is reachable from a loop that passes a file every time
+- An exported `useX` taking an options object is no longer reported. It is
+  indistinguishable from a component to the AST and has no JSX call sites, so
+  every one of its options came back `unused-component` — a statement about the
+  walk rather than about the hook. Only discovery is narrowed: a prop passing
+  through a hook on its way down is still traced, and still reported at the
+  component that declares it
+- The text output keeps its header on an empty report, so the tsconfig and the
+  Program's file count are visible in the case where they are most worth
+  checking
+
 ## 2.1.0
 
 - The `typescript` peer dependency is now marked optional. prop-flow never

packages/prop-flow/README.md

@@ -67,6 +67,36 @@ justified         Button.title
    → genuinely sometimes-absent. The `?` is correct.
 ```
 
+## Batching over files
+
+Under `--json` an invocation **that names a file** always emits exactly one
+object on **stdout** — a report, or `{"error": "…"}` for a handled failure. So
+a loop over the files you changed stays parsable even where one of them fails,
+and the whole batch goes through a single `jq`:
+
+```bash
+$ for f in $CHANGED_TSX; do prop-flow "$f" --json; done | jq -s '
+    [ .[] | select(.error | not) | .props[]
+      | select(.verdict == "caller-dead" or .verdict == "unnecessary-optional"
+               or .verdict == "manual" or .constant != null) ]'
+```
+
+An empty array means nothing was found — not that a filter missed it. Do not
+filter the *text* output instead: the verdict labels are deliberately mixed
+case (`justified` reads quietly, `CALLER-DEAD` does not), so a grep anchored on
+one case silently drops the other, and an empty result then means either
+"nothing found" or "wrong pattern".
+
+Each invocation builds its own Program, which is the bulk of the runtime (~6 s
+on a 4 000-file monorepo). So batch by *file* — passing a `propName` saves
+nothing, and re-running a file to reformat its output costs a second Program.
+Capture once.
+
+The one thing `--json` does not wrap is the usage text: `--help`, and exit `1`
+for an invocation with no file at all, print it plain. Both are answers to a
+person rather than to a pipeline — and a loop that passes a file every time
+cannot reach either.
+
 ## Verdicts
 
 | verdict                | meaning                                                                  |
@@ -79,7 +109,15 @@ justified         Button.title
 | `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
-failure (message on stderr).
+failure (message on stderr, or `{"error": …}` on stdout under `--json`).
+
+Nothing to analyse is a **success**, not a failure: a file with no exported
+component — a route module, a props-less page — and a component whose props are
+all required both come back with an empty report and exit `0`. They are
+distinguishable: `components` is `0` in the first case. Exit `2` is reserved for
+what genuinely blocked the analysis (a missing file, no resolvable compiler, a
+tsconfig that does not span the file), so a loop over changed files can stop on
+a real failure without stopping on a page component.
 
 ## Constant values
 
@@ -191,3 +229,11 @@ Components are picked up from `export function C`, `export const C = …`
 `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.
+
+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
+about the hook. Only discovery is narrowed: a prop that passes *through* a hook
+on its way down is still traced, and still reported at the component that
+declares it. The `use` prefix is the one naming convention safe to key on;
+lower-cased components are rare but legal, so PascalCase is not.

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

@@ -0,0 +1,21 @@
+export interface UseFilterOptions {
+  /** A hook option is shaped like a prop and is not one: no JSX site passes it. */
+  initial?: string;
+}
+
+export function useFilter({ initial }: UseFilterOptions) {
+  return initial ?? '';
+}
+
+export function FilterChip({ label }: { label?: string }) {
+  return <span>{label}</span>;
+}
+
+/**
+ * One character off the hook pattern, and a component. It is what the `[A-Z]`
+ * in the pattern is for — and being lower-cased, it is also the case that rules
+ * PascalCase out as the thing to key on.
+ */
+export function used({ tone }: { tone?: string }) {
+  return <b>{tone}</b>;
+}

packages/prop-flow/package.json

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

packages/prop-flow/src/analyzer.test.ts

@@ -261,6 +261,12 @@ describe('createAnalyzer', () => {
     expect(analyzer.findComponents(sourceFile('late.tsx')).map(({ name }) => name)).toEqual(['Aliased', 'Late']);
     // App takes no props, so there is nothing to analyse in it.
     expect(analyzer.findComponents(sourceFile('app.tsx'))).toEqual([]);
+    // A `useX` taking an options object is indistinguishable from a component
+    // to the AST, and has no JSX call sites — its options would come back
+    // `unused-component`, which says nothing about the hook. `used` is the
+    // near miss the pattern has to survive: `use` alone is not the signal, and
+    // a lower-cased component is legal.
+    expect(analyzer.findComponents(sourceFile('hooks.tsx')).map(({ name }) => name)).toEqual(['FilterChip', 'used']);
 
     const [button] = analyzer.findComponents(sourceFile('button.tsx'));
     expect(button && analyzer.listProps(button)).toEqual([

packages/prop-flow/src/analyzer.ts

@@ -196,7 +196,7 @@ export function createAnalyzer({ cwd, program, ts }: AnalyzerOptions): Analyzer
   // ── component + prop discovery ────────────────────────────────────────────
 
   function findComponents(sourceFile: TS.SourceFile): Component[] {
-    return sourceFile.statements.flatMap((stmt) => componentsOfStatement(stmt));
+    return sourceFile.statements.flatMap((stmt) => componentsOfStatement(stmt)).filter(({ name }) => !isHook(name));
   }
 
   /** The components one top-level statement declares or exports. */
@@ -320,3 +320,18 @@ export function verdictOf(usageCount: number, real: number, omit: number, ambigu
 function compact<T>(values: readonly (T | null)[]): T[] {
   return values.filter((value) => value !== null);
 }
+
+/**
+ * A `useX` taking an options object looks exactly like a component to the AST,
+ * and it is not one: a hook has no JSX call sites, so every one of its options
+ * comes back `unused-component`. That is not a finding, it is the walk pointed
+ * at the wrong kind of function — the callers exist, they are just invisible to
+ * a JSX walk. The `use` prefix is the one naming convention safe to key on;
+ * lower-cased components are rare but legal, so PascalCase is not.
+ *
+ * Only *discovery* is narrowed. A prop that passes through a hook on its way
+ * down is still traced, and still reported at the component that declares it.
+ */
+function isHook(name: string): boolean {
+  return /^use[A-Z]/.test(name);
+}

packages/prop-flow/src/cli.test.ts

@@ -69,6 +69,40 @@ describe('runCli', () => {
     ]);
   });
 
+  it('exits 0 on a file with nothing to analyse, in both output modes', () => {
+    // The whole point of the exit code: a loop over changed files must be able
+    // to stop on a real failure without stopping on a page component.
+    const text = run(['app.tsx']);
+    const json = run(['--json', 'app.tsx']);
+
+    expect(text.code).toBe(0);
+    expect(text.stderr).toBe('');
+    expect(text.stdout).toContain('No exported component with a typed props object.');
+    expect(json.code).toBe(0);
+    expect(JSON.parse(json.stdout)).toMatchObject({ components: 0, file: 'app.tsx', props: [] });
+  });
+
+  it('puts a handled failure into the JSON stream instead of onto stderr', () => {
+    // One object per invocation, whatever happens — a bare error line in the
+    // middle of a `for f in …; do prop-flow "$f" --json; done` would break the
+    // parse for every other file in the stream, not just the failing one.
+    const { code, stderr, stdout } = run(['--json', 'missing.tsx']);
+
+    expect(code).toBe(2);
+    expect(stderr).toBe('');
+    expect((JSON.parse(stdout) as { error: string }).error).toContain('File not found');
+  });
+
+  it('reaches the JSON envelope even when it is the arguments that are bad', () => {
+    // `--json` is read off the raw argv, so a parse failure lands in the
+    // envelope too — which is when a caller is least able to guess the shape.
+    const { code, stderr, stdout } = run(['--json', '--nope', 'button.tsx']);
+
+    expect(code).toBe(2);
+    expect(stderr).toBe('');
+    expect(JSON.parse(stdout)).toEqual({ error: 'Unknown option: --nope' });
+  });
+
   it('prints usage on --help and exits 0', () => {
     const { code, stdout } = run(['--help']);
 
@@ -83,6 +117,23 @@ describe('runCli', () => {
     expect(stdout).toContain('Usage: prop-flow <file>');
   });
 
+  it('keeps usage human-readable under --json', () => {
+    // The one outcome that is not a JSON object: usage is an answer to a person
+    // who asked nothing analysable, and exit 1 is unreachable from a loop that
+    // passes a file every time. Wrapping it would make the text an escaped
+    // string in a field nobody reads.
+    const noFile = run(['--json']);
+    const help = run(['--json', '--help']);
+
+    expect(noFile.code).toBe(1);
+    expect(noFile.stdout).toContain('Usage: prop-flow <file>');
+    expect(help.code).toBe(0);
+    expect(help.stdout).toContain('Usage: prop-flow <file>');
+    expect(() => {
+      JSON.parse(help.stdout);
+    }).toThrow();
+  });
+
   it.each([
     [['--nope', 'button.tsx'], 'prop-flow: Unknown option: --nope\n'],
     [['missing.tsx'], 'prop-flow: File not found: '],

packages/prop-flow/src/cli.ts

@@ -1,7 +1,7 @@
 import { parseArgs } from './args.js';
 import { PropFlowError } from './errors.js';
 import { analyseProps } from './prop-flow.js';
-import { formatJson, formatText } from './report.js';
+import { formatJson, formatJsonError, formatText } from './report.js';
 import type { TypeScriptApi } from './typescript-api.js';
 
 export interface OutputStream {
@@ -30,8 +30,15 @@ export const USAGE =
   '`?` is justified, needless, or the prop is never passed (caller-dead),\n' +
   'plus whether every call site sends it one and the same value.\n';
 
-/** Exit codes: 0 ok, 1 nothing to do (usage printed), 2 a handled failure. */
+/**
+ * Exit codes: 0 ok — including a file with nothing to analyse — 1 nothing to do
+ * (usage printed), 2 a handled failure.
+ */
 export function runCli(argv: readonly string[], context: CliContext): number {
+  // One read of the flag, and off the raw argv rather than off the parsed args:
+  // the envelope has to cover a failure to parse the arguments themselves, and
+  // at that point there are no parsed args left to ask.
+  const json = argv.includes('--json');
   try {
     const args = parseArgs(argv);
     if (args.help || args.file === null) {
@@ -46,11 +53,15 @@ export function runCli(argv: readonly string[], context: CliContext): number {
       ts: context.ts,
       tsconfig: args.tsconfig,
     });
-    context.stdout.write(args.json ? formatJson(report) : formatText(report));
+    context.stdout.write(json ? formatJson(report) : formatText(report));
     return 0;
   } catch (error) {
     if (error instanceof PropFlowError) {
-      context.stderr.write(`prop-flow: ${error.message}\n`);
+      if (json) {
+        context.stdout.write(formatJsonError(error.message));
+      } else {
+        context.stderr.write(`prop-flow: ${error.message}\n`);
+      }
       return 2;
     }
     throw error;

packages/prop-flow/src/index.ts

@@ -8,7 +8,7 @@ export type { Component } from './component.js';
 export { PropFlowError } from './errors.js';
 export { analyseProps } from './prop-flow.js';
 export type { AnalyseOptions } from './prop-flow.js';
-export { formatJson, formatText } from './report.js';
+export { formatJson, formatJsonError, formatText } from './report.js';
 export { discoverTsconfig, readTsConfig } from './tsconfig.js';
 export type {
   ConstantValue,

packages/prop-flow/src/prop-flow.test.ts

@@ -25,10 +25,35 @@ describe('analyseProps', () => {
       'Button.size': 'unnecessary-optional',
       'Button.title': 'justified',
     });
-    expect(report).toMatchObject({ configPath: 'tsconfig.json', file: 'button.tsx' });
+    expect(report).toMatchObject({ components: 1, configPath: 'tsconfig.json', file: 'button.tsx' });
     expect(report.fileCount).toBeGreaterThan(1);
   });
 
+  it('reports a file without components as an empty report, not as a failure', () => {
+    // A route module or a props-less page is not a broken input — the honest
+    // answer is "nothing to analyse", and a caller looping over changed files
+    // must be able to tell that apart from "could not analyse" without reading
+    // a message. `components` is what says which empty this is.
+    const report = analyse('app.tsx');
+
+    expect(report).toMatchObject({ components: 0, file: 'app.tsx', props: [] });
+  });
+
+  it('leaves hooks out of discovery, and counts what is left', () => {
+    // `useFilter({ initial })` is shaped like a component and has no JSX call
+    // sites, so every option of it would come back `unused-component`. What is
+    // narrowed is the `use` + capital shape, not everything spelled `use…`:
+    // `used` is a component and stays one, and `components` counts the two that
+    // survive rather than reducing to "found something".
+    const report = analyse('hooks.tsx');
+
+    expect(report.components).toBe(2);
+    expect(verdicts(report)).toEqual({
+      'FilterChip.label': 'unused-component',
+      'used.tone': 'unused-component',
+    });
+  });
+
   it('narrows to a single prop when one is named', () => {
     const report = analyse('button.tsx', 'size');
 
@@ -81,7 +106,9 @@ describe('analyseProps', () => {
   it.each([
     ['nope.tsx', undefined, /File not found/],
     ['button.tsx', 'nope', /'nope' is not an optional prop/],
-    ['app.tsx', undefined, /No exported component/],
+    // A named prop that no component declares is a typo in the argument, and
+    // stays a failure even where the file has no component to declare it.
+    ['app.tsx', 'nope', /'nope' is not an optional prop/],
   ])('rejects %s %s', (file, prop, message) => {
     expect(() => analyse(file, prop)).toThrow(PropFlowError);
     expect(() => analyse(file, prop)).toThrow(message);