From 59711ba2e956524f83a3e04d65e50cd4599ba6de Mon Sep 17 00:00:00 2001 From: Dominic Buetow Date: Tue, 4 Aug 2026 00:57:05 +0200 Subject: [PATCH 1/2] init --- packages/prop-flow/CHANGELOG.md | 29 ++++++++++++++ packages/prop-flow/README.md | 43 ++++++++++++++++++++- packages/prop-flow/fixtures/basic/hooks.tsx | 12 ++++++ packages/prop-flow/package.json | 2 +- packages/prop-flow/src/analyzer.test.ts | 4 ++ packages/prop-flow/src/analyzer.ts | 17 +++++++- packages/prop-flow/src/cli.test.ts | 34 ++++++++++++++++ packages/prop-flow/src/cli.ts | 16 ++++++-- packages/prop-flow/src/index.ts | 2 +- packages/prop-flow/src/prop-flow.test.ts | 25 +++++++++++- packages/prop-flow/src/prop-flow.ts | 10 +++-- packages/prop-flow/src/report.test.ts | 26 ++++++++++++- packages/prop-flow/src/report.ts | 16 ++++++++ packages/prop-flow/src/types.ts | 7 ++++ 14 files changed, 229 insertions(+), 14 deletions(-) create mode 100644 packages/prop-flow/fixtures/basic/hooks.tsx diff --git a/packages/prop-flow/CHANGELOG.md b/packages/prop-flow/CHANGELOG.md index fb79e2cc..e94f4b40 100644 --- a/packages/prop-flow/CHANGELOG.md +++ b/packages/prop-flow/CHANGELOG.md @@ -5,6 +5,35 @@ 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 every invocation 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 +- 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 diff --git a/packages/prop-flow/README.md b/packages/prop-flow/README.md index d2e83dd8..5e76243d 100644 --- a/packages/prop-flow/README.md +++ b/packages/prop-flow/README.md @@ -67,6 +67,31 @@ justified Button.title → genuinely sometimes-absent. The `?` is correct. ``` +## Batching over files + +Under `--json` every terminating outcome is 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. + ## Verdicts | verdict | meaning | @@ -79,7 +104,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 +224,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. diff --git a/packages/prop-flow/fixtures/basic/hooks.tsx b/packages/prop-flow/fixtures/basic/hooks.tsx new file mode 100644 index 00000000..b00f1431 --- /dev/null +++ b/packages/prop-flow/fixtures/basic/hooks.tsx @@ -0,0 +1,12 @@ +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 {label}; +} diff --git a/packages/prop-flow/package.json b/packages/prop-flow/package.json index 9ca222d2..323566d0 100644 --- a/packages/prop-flow/package.json +++ 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", diff --git a/packages/prop-flow/src/analyzer.test.ts b/packages/prop-flow/src/analyzer.test.ts index f61c5eb2..af92939b 100644 --- a/packages/prop-flow/src/analyzer.test.ts +++ b/packages/prop-flow/src/analyzer.test.ts @@ -261,6 +261,10 @@ 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. + expect(analyzer.findComponents(sourceFile('hooks.tsx')).map(({ name }) => name)).toEqual(['FilterChip']); const [button] = analyzer.findComponents(sourceFile('button.tsx')); expect(button && analyzer.listProps(button)).toEqual([ diff --git a/packages/prop-flow/src/analyzer.ts b/packages/prop-flow/src/analyzer.ts index 766cc45f..d36ce381 100644 --- a/packages/prop-flow/src/analyzer.ts +++ b/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(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); +} diff --git a/packages/prop-flow/src/cli.test.ts b/packages/prop-flow/src/cli.test.ts index 83ba88d9..3ef5bd9c 100644 --- a/packages/prop-flow/src/cli.test.ts +++ b/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']); diff --git a/packages/prop-flow/src/cli.ts b/packages/prop-flow/src/cli.ts index bdc7f84b..9a3b8164 100644 --- a/packages/prop-flow/src/cli.ts +++ b/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,14 @@ 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 { + // Read off the raw argv rather than off the parsed args, so the JSON envelope + // also covers a failure to parse the arguments themselves. + const json = argv.includes('--json'); try { const args = parseArgs(argv); if (args.help || args.file === null) { @@ -50,7 +56,11 @@ export function runCli(argv: readonly string[], context: CliContext): number { 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; diff --git a/packages/prop-flow/src/index.ts b/packages/prop-flow/src/index.ts index 57c19483..5503b7bc 100644 --- a/packages/prop-flow/src/index.ts +++ b/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, diff --git a/packages/prop-flow/src/prop-flow.test.ts b/packages/prop-flow/src/prop-flow.test.ts index e3590dcf..7ffe033e 100644 --- a/packages/prop-flow/src/prop-flow.test.ts +++ b/packages/prop-flow/src/prop-flow.test.ts @@ -25,10 +25,29 @@ 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', () => { + // `useFilter({ initial })` is shaped like a component and has no JSX call + // sites, so every option of it would come back `unused-component`. + const report = analyse('hooks.tsx'); + + expect(report.components).toBe(1); + expect(verdicts(report)).toEqual({ 'FilterChip.label': 'unused-component' }); + }); + it('narrows to a single prop when one is named', () => { const report = analyse('button.tsx', 'size'); @@ -81,7 +100,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); diff --git a/packages/prop-flow/src/prop-flow.ts b/packages/prop-flow/src/prop-flow.ts index 025195e4..4c976a00 100644 --- a/packages/prop-flow/src/prop-flow.ts +++ b/packages/prop-flow/src/prop-flow.ts @@ -59,10 +59,13 @@ export function analyseProps(options: AnalyseOptions): Report { } const analyzer = createAnalyzer({ cwd, program, ts }); + // No component is not a failure — it is the answer for a route module or a + // props-less page, and it belongs on the same path as a component whose props + // are all required. Both come back as an empty `props`; `components` says + // which one it was. Failing here instead would mean a caller looping over + // changed files has to tell "nothing to analyse" apart from "could not + // analyse" by reading a message. const components = analyzer.findComponents(sourceFile); - if (components.length === 0) { - throw new PropFlowError(`No exported component with a typed props object found in ${relativeTo(cwd, file)}.`); - } const allProps = options.allProps ?? false; const propName = options.prop ?? null; @@ -97,6 +100,7 @@ export function analyseProps(options: AnalyseOptions): Report { return { props, + components: components.length, configPath: relativeTo(cwd, configPath), file: relativeTo(cwd, file), fileCount: program.getSourceFiles().length, diff --git a/packages/prop-flow/src/report.test.ts b/packages/prop-flow/src/report.test.ts index 95616d97..7307fb63 100644 --- a/packages/prop-flow/src/report.test.ts +++ b/packages/prop-flow/src/report.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { constantHint, formatJson, formatText, hint } from './report.js'; +import { constantHint, formatJson, formatJsonError, formatText, hint } from './report.js'; import type { PropReport, Report, Verdict } from './types.js'; function makeRow(overrides: Partial = {}): PropReport { @@ -22,6 +22,7 @@ function makeRow(overrides: Partial = {}): PropReport { function makeReport(overrides: Partial = {}): Report { return { + components: 1, configPath: 'tsconfig.json', file: 'src/button.tsx', fileCount: 42, @@ -104,8 +105,20 @@ describe('formatText', () => { expect(formatText(report)).toContain('every passing call site sends "primary"'); }); - it('says so when a component has nothing to report', () => { + it('tells the two empty reports apart', () => { + // Same empty `props`, different answers: one component's props are all + // required, the other file has no component at all — and only the first + // could ever grow a finding. expect(formatText(makeReport({ props: [] }))).toContain('No props to report.'); + expect(formatText(makeReport({ components: 0, props: [] }))).toContain( + 'No exported component with a typed props object.', + ); + }); + + it('keeps the header on an empty report, so the Program is still accounted for', () => { + // Without it, "nothing found" and "the tsconfig pulled in three files" + // look the same — and the second is the one worth noticing. + expect(formatText(makeReport({ components: 0, props: [] }))).toContain('(42 files in Program)'); }); }); @@ -146,3 +159,12 @@ describe('formatJson', () => { expect(json.endsWith('\n')).toBe(true); }); }); + +describe('formatJsonError', () => { + it('wraps the message so a failed file stays parsable in a stream', () => { + const json = formatJsonError('File not found: /x/nope.tsx'); + + expect(JSON.parse(json)).toEqual({ error: 'File not found: /x/nope.tsx' }); + expect(json.endsWith('\n')).toBe(true); + }); +}); diff --git a/packages/prop-flow/src/report.ts b/packages/prop-flow/src/report.ts index 940b2b01..67b77fa7 100644 --- a/packages/prop-flow/src/report.ts +++ b/packages/prop-flow/src/report.ts @@ -20,9 +20,25 @@ export function formatJson(report: Report): string { return `${JSON.stringify(report, null, 2)}\n`; } +/** + * The `--json` counterpart of the stderr line. Under `--json` every terminating + * outcome is one object on stdout, so a loop over files can pipe straight into + * `jq`: a bare error line in the middle of the stream would break the parse for + * every other file too. + */ +export function formatJsonError(message: string): string { + return `${JSON.stringify({ error: message }, null, 2)}\n`; +} + export function formatText(report: Report): string { const head = `tsconfig: ${report.configPath} (${report.fileCount} files in Program)\n` + `file: ${report.file}\n\n`; + // Two ways to have nothing to say, and which one it was decides whether there + // is anything to do about it: adding props to a route module is not on the + // table, adding a `?` somewhere might be. + if (report.components === 0) { + return `${head}No exported component with a typed props object.\n`; + } if (report.props.length === 0) { return `${head}No props to report.\n`; } diff --git a/packages/prop-flow/src/types.ts b/packages/prop-flow/src/types.ts index ecc9eb2f..2a548692 100644 --- a/packages/prop-flow/src/types.ts +++ b/packages/prop-flow/src/types.ts @@ -63,6 +63,13 @@ export interface PropReport extends PropAnalysis { } export interface Report { + /** + * Exported components with a typed props object found in `file`. Zero is a + * result, not a failure — a route module or a props-less page has none, and + * that is the answer. It is what tells an empty `props` apart: no component + * to look at, versus a component whose props are all required. + */ + readonly components: number; /** tsconfig the Program was built from, relative to cwd. */ readonly configPath: string; /** Inspected file, relative to cwd. */ From b193be61d03530644a9a5f79ba34c9cafd7421e3 Mon Sep 17 00:00:00 2001 From: Dominic Buetow Date: Tue, 4 Aug 2026 01:09:21 +0200 Subject: [PATCH 2/2] linting --- packages/prop-flow/CHANGELOG.md | 13 ++-- packages/prop-flow/README.md | 13 ++-- packages/prop-flow/fixtures/basic/hooks.tsx | 9 +++ packages/prop-flow/src/analyzer.test.ts | 6 +- packages/prop-flow/src/cli.test.ts | 17 +++++ packages/prop-flow/src/cli.ts | 7 +- packages/prop-flow/src/prop-flow.test.ts | 14 ++-- packages/prop-flow/src/prop-flow.ts | 75 +++++++++++++-------- 8 files changed, 108 insertions(+), 46 deletions(-) diff --git a/packages/prop-flow/CHANGELOG.md b/packages/prop-flow/CHANGELOG.md index e94f4b40..0b68496f 100644 --- a/packages/prop-flow/CHANGELOG.md +++ b/packages/prop-flow/CHANGELOG.md @@ -19,11 +19,14 @@ This project adheres to [Semantic Versioning](http://semver.org/). 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 every invocation 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 + 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 diff --git a/packages/prop-flow/README.md b/packages/prop-flow/README.md index 5e76243d..5dbdee99 100644 --- a/packages/prop-flow/README.md +++ b/packages/prop-flow/README.md @@ -69,10 +69,10 @@ justified Button.title ## Batching over files -Under `--json` every terminating outcome is 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`: +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 ' @@ -92,6 +92,11 @@ 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 | diff --git a/packages/prop-flow/fixtures/basic/hooks.tsx b/packages/prop-flow/fixtures/basic/hooks.tsx index b00f1431..8326e83d 100644 --- a/packages/prop-flow/fixtures/basic/hooks.tsx +++ b/packages/prop-flow/fixtures/basic/hooks.tsx @@ -10,3 +10,12 @@ export function useFilter({ initial }: UseFilterOptions) { export function FilterChip({ label }: { label?: string }) { return {label}; } + +/** + * 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 {tone}; +} diff --git a/packages/prop-flow/src/analyzer.test.ts b/packages/prop-flow/src/analyzer.test.ts index af92939b..eb762ef6 100644 --- a/packages/prop-flow/src/analyzer.test.ts +++ b/packages/prop-flow/src/analyzer.test.ts @@ -263,8 +263,10 @@ describe('createAnalyzer', () => { 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. - expect(analyzer.findComponents(sourceFile('hooks.tsx')).map(({ name }) => name)).toEqual(['FilterChip']); + // `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([ diff --git a/packages/prop-flow/src/cli.test.ts b/packages/prop-flow/src/cli.test.ts index 3ef5bd9c..b30e74ca 100644 --- a/packages/prop-flow/src/cli.test.ts +++ b/packages/prop-flow/src/cli.test.ts @@ -117,6 +117,23 @@ describe('runCli', () => { expect(stdout).toContain('Usage: prop-flow '); }); + 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 '); + expect(help.code).toBe(0); + expect(help.stdout).toContain('Usage: prop-flow '); + expect(() => { + JSON.parse(help.stdout); + }).toThrow(); + }); + it.each([ [['--nope', 'button.tsx'], 'prop-flow: Unknown option: --nope\n'], [['missing.tsx'], 'prop-flow: File not found: '], diff --git a/packages/prop-flow/src/cli.ts b/packages/prop-flow/src/cli.ts index 9a3b8164..7e231b38 100644 --- a/packages/prop-flow/src/cli.ts +++ b/packages/prop-flow/src/cli.ts @@ -35,8 +35,9 @@ export const USAGE = * (usage printed), 2 a handled failure. */ export function runCli(argv: readonly string[], context: CliContext): number { - // Read off the raw argv rather than off the parsed args, so the JSON envelope - // also covers a failure to parse the arguments themselves. + // 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); @@ -52,7 +53,7 @@ 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) { diff --git a/packages/prop-flow/src/prop-flow.test.ts b/packages/prop-flow/src/prop-flow.test.ts index 7ffe033e..a08b9482 100644 --- a/packages/prop-flow/src/prop-flow.test.ts +++ b/packages/prop-flow/src/prop-flow.test.ts @@ -39,13 +39,19 @@ describe('analyseProps', () => { expect(report).toMatchObject({ components: 0, file: 'app.tsx', props: [] }); }); - it('leaves hooks out of discovery', () => { + 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`. + // 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(1); - expect(verdicts(report)).toEqual({ 'FilterChip.label': 'unused-component' }); + 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', () => { diff --git a/packages/prop-flow/src/prop-flow.ts b/packages/prop-flow/src/prop-flow.ts index 4c976a00..33372f82 100644 --- a/packages/prop-flow/src/prop-flow.ts +++ b/packages/prop-flow/src/prop-flow.ts @@ -38,25 +38,7 @@ export interface AnalyseOptions { export function analyseProps(options: AnalyseOptions): Report { const cwd = options.cwd ?? process.cwd(); const ts = options.ts ?? loadTypeScript(cwd); - - const file = isAbsolute(options.file) ? options.file : resolve(cwd, options.file); - if (!existsSync(file)) { - throw new PropFlowError(`File not found: ${file}`); - } - - const configPath = options.tsconfig ? resolve(cwd, options.tsconfig) : discoverTsconfig(ts, file); - if (!configPath || !existsSync(configPath)) { - throw new PropFlowError('No tsconfig found. Pass one explicitly with --tsconfig .'); - } - - const parsed = readTsConfig(ts, configPath, cwd); - const program = ts.createProgram({ options: parsed.options, rootNames: parsed.fileNames }); - const sourceFile = findSourceFile(program, file); - if (!sourceFile) { - throw new PropFlowError( - `The chosen tsconfig (${relativeTo(cwd, configPath)}) does not include ${relativeTo(cwd, file)}.\nPass the solution/root tsconfig with --tsconfig so the file and its call sites are both in scope.`, - ); - } + const { configPath, file, program, sourceFile } = resolveTarget(options, cwd, ts); const analyzer = createAnalyzer({ cwd, program, ts }); // No component is not a failure — it is the answer for a route module or a @@ -80,16 +62,15 @@ export function analyseProps(options: AnalyseOptions): Report { // A prop with no `?` has nothing for the verdicts to say: `justified` and // `caller-dead` are impossible on it and `unnecessary-optional` is a lie. // Its constant value is the whole reason it is here — no value, no row. - if (!prop.optional && analysis.constant === null) { - continue; + if (prop.optional || analysis.constant !== null) { + props.push({ + ...analysis, + component: component.name, + hasDefault: prop.hasDefault, + prop: prop.name, + verdict: prop.optional ? analysis.verdict : 'required', + }); } - props.push({ - ...analysis, - component: component.name, - hasDefault: prop.hasDefault, - prop: prop.name, - verdict: prop.optional ? analysis.verdict : 'required', - }); } } @@ -107,6 +88,44 @@ export function analyseProps(options: AnalyseOptions): Report { }; } +/** The file to analyse, the config it was found under, and the Program both live in. */ +interface Target { + readonly configPath: string; + /** Absolute; the report shortens it against `cwd` on the way out. */ + readonly file: string; + readonly program: TS.Program; + readonly sourceFile: TS.SourceFile; +} + +/** + * Resolve what the analysis runs against. Every way this can fail is a + * `PropFlowError`, and deliberately so: exit 2 is reserved for what actually + * blocked the analysis, and a missing file, an unfindable compiler config and a + * tsconfig that does not span the file are the three things that do. + */ +function resolveTarget(options: AnalyseOptions, cwd: string, ts: TypeScriptApi): Target { + const file = isAbsolute(options.file) ? options.file : resolve(cwd, options.file); + if (!existsSync(file)) { + throw new PropFlowError(`File not found: ${file}`); + } + + const configPath = options.tsconfig ? resolve(cwd, options.tsconfig) : discoverTsconfig(ts, file); + if (!configPath || !existsSync(configPath)) { + throw new PropFlowError('No tsconfig found. Pass one explicitly with --tsconfig .'); + } + + const parsed = readTsConfig(ts, configPath, cwd); + const program = ts.createProgram({ options: parsed.options, rootNames: parsed.fileNames }); + const sourceFile = findSourceFile(program, file); + if (!sourceFile) { + throw new PropFlowError( + `The chosen tsconfig (${relativeTo(cwd, configPath)}) does not include ${relativeTo(cwd, file)}.\nPass the solution/root tsconfig with --tsconfig so the file and its call sites are both in scope.`, + ); + } + + return { configPath, file, program, sourceFile }; +} + /** `getSourceFile` is exact-match; fall back to comparing resolved paths. */ function findSourceFile(program: TS.Program, file: string): TS.SourceFile | undefined { return program.getSourceFile(file) ?? program.getSourceFiles().find((sf) => resolve(sf.fileName) === resolve(file));