From 21867fb7d21e919afa1912bf79563f586a2e5559 Mon Sep 17 00:00:00 2001 From: Joey Farina Date: Sat, 17 Oct 2026 20:34:18 -0700 Subject: [PATCH 1/2] feat(cli): doctor integration components judges documented prop types against the code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `astryx doctor integration components` now also judges each documented prop's type against the code, for props the package declares, a documented field (`status.type`) and a name the package's own index signature takes included, and warns when they differ (`type_mismatch`). A type is judged only where both sides read exactly, with the project's own TypeScript: the same text as the source once whitespace, quotes and union order are set aside (the text of the prop's own declaration, of the generic type's prop it instantiates when the result prints in the component's file as the declared type, as when a generic component passes its own type parameters on, or of one a mapping copies unchanged, such as `Pick`; a prop a mapping rewrites, such as `{[K in keyof T]: T[K][]}` or `Required`, or another type instantiates, such as `Box` or `ListProps`, is judged by its type), or primitives, literals, string enum members (each only its own enum's, as TypeScript takes no other enum's member of the same value; two enums of one name, which TypeScript relates by their members, are listed), arrays and unions of those, and names for them, on both sides. `Name = …` and `Name (…)` must match in both parts, and a doc may describe one member of a union whose members give a prop different types; a field the prop's variants give different types is listed when the doc gives only some variants' types (`string` for `{value: string} | {value: number}`), as those are neither the field's type nor wrong, and judged when it gives them all. Every other type (a callback, an object, a generic with arguments, an intersection, `keyof`, a type that does not resolve, a doc type that does not parse, prose in parentheses after a type, a prop a mapped type builds from keys the check can't trace) is listed as not compared, never reported. A prop declared outside the package is not judged either, and is listed with its reason: a standard prop (`className`, `style`, …) or another package's prop (a field of one once the field is found), a name only another package's index signature takes included; a `className` the package declares itself is judged like any other prop. A name a mapped type's index (`Record`) takes is listed, as nothing says whose it is. `React.X` in a doc resolves through `React` where the prop is declared, so it is never read as a bare name that means another type there. The code's type is printed where the prop (or its index signature) is declared, where the doc's names resolve, so a type a file imports under another name is never read as the file's own type of the same name. With a project TypeScript older than 4.8 no type is judged: the text prints `[skip] Prop types not judged` with the reason, and JSON adds `typesSkipped`. A field of a name only an index signature takes, and an `a / b` entry whose one type does not split across the names (a field once it is found), are listed too, by component and then reason. With `exactOptionalPropertyTypes`, an optional prop takes `undefined` only when its type says so, and is judged that way. A name an index signature takes, and a field a `Record` takes, take `undefined` only when the index's type does. A readonly array documented for a mutable array is a mismatch, while a mutable array documented for a readonly one matches, as TypeScript passes it. Text that writes `readonly` matches only as written, the same text with `readonly` kept on both sides; otherwise it is judged by its parts, so `readonly` that sits elsewhere (another depth, a callback's parameter, an object's field) is listed as not compared. JSON adds `typesCompared` and `typesNotCompared` ({component, doc, reason, props}) to `propDrift`; the text says how many types were judged and lists the rest with the reason. The type judge itself (type-match.mjs) comes from the PR below. --- .changeset/doctor-prop-types.md | 7 + packages/cli/README.md | 2 +- .../integrationComponentConflicts.doc.mjs | 23 +- packages/cli/assets/docs/tree/checks.doc.mjs | 2 +- .../doctor-integration-components.doc.mjs | 11 +- .../cli/commands/doctor-integration.test.mjs | 30 +- packages/cli/clients/cli/commands/doctor.mjs | 19 + .../cli/foundation/discovery/prop-drift.mjs | 620 +++++++++- .../foundation/discovery/prop-drift.test.mjs | 1042 ++++++++++++++++- .../response/response-types.doc.mjs | 2 +- .../cli/test/core-prop-docs.known-drift.json | 4 + 11 files changed, 1679 insertions(+), 83 deletions(-) create mode 100644 .changeset/doctor-prop-types.md diff --git a/.changeset/doctor-prop-types.md b/.changeset/doctor-prop-types.md new file mode 100644 index 0000000000000..098e87450e237 --- /dev/null +++ b/.changeset/doctor-prop-types.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/cli': patch +--- + +[feat] `astryx doctor integration components` also judges each documented prop's type against the code, for props the package declares, a documented field such as `status.type` and a name the package's own index signature takes included, and warns when they differ (`type_mismatch`). A type is judged only where both sides read exactly, with the project's own `typescript`: the same text once whitespace, quotes and union order are set aside, or primitives, literals, arrays and unions of those, and names for them. Every other type (a callback, an object, a generic with arguments, an intersection, `keyof`, a type that does not resolve, a name two index signatures take) is listed as not compared, never reported. The comparison needs TypeScript 4.8 or later: with an older one, no type is judged and the text says so in one `[skip]` line. JSON adds `typesCompared`, `typesNotCompared` and, when the types were not judged, `typesSkipped` to `propDrift`. The findings are warnings, so the exit code does not change. + +@josephfarina diff --git a/packages/cli/README.md b/packages/cli/README.md index 7595b2a55cf70..bef4460b9602f 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -478,7 +478,7 @@ Every response has a `type` discriminant. The full set is below (generated from | `integration.pack-check` | The packed-package check: name, version, packable, tarball {filename, fileCount, size, unpackedSize} or null, inventory {manifest, roots [{kind, path, expectedFiles, missingFiles, complete}], expectedFiles, packedFiles}, contributions {local, packed}, each null or {themes [{slug, exportName}], components, templates [{id, type, name}], codemods [{version, id}], docs, agentDocsAppend}, and issues [{code, severity, message}]. | | `integration.validate` | The validation result: validated (false when nothing was validated: with no package.json the issues list carries one no_package error so the command exits 1, and with a package.json but no integration manifest the issues list is empty and proves nothing), the package name and version (both null when validated is false) plus issues, an AstryxIntegrationIssue[] of {code, severity: warning \| error, message}. | | `integration.template-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking Core template-id conflicts as {id, severity: warning, integrationPackage, integrationType, integrationName, coreMatches, message, command}. | -| `integration.component-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked \| skipped), reason, typescript, components, findings (all warnings), unchecked, and propsNotChecked. | +| `integration.component-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked \| skipped), reason, typescript, components, typesCompared, typesSkipped, findings (all warnings), unchecked, typesNotCompared, and propsNotChecked. | | `integration.doc-conflicts` | validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and Core doc overlaps. Each finding includes `severity` (`info` \| `error`) and `relationship` (`replaces` \| `extends` \| `accidental`). | diff --git a/packages/cli/api/integration/integrationComponentConflicts.doc.mjs b/packages/cli/api/integration/integrationComponentConflicts.doc.mjs index add38b2f98ef4..7260289493642 100644 --- a/packages/cli/api/integration/integrationComponentConflicts.doc.mjs +++ b/packages/cli/api/integration/integrationComponentConflicts.doc.mjs @@ -19,9 +19,24 @@ export const doc = { 'declares the CLI range that turns replacement on and warnings otherwise. ' + "Each component doc's props are compared with the component's TypeScript props, " + "read with the project's own `typescript`: a documented prop the code lacks, a " + - 'prop the package declares that the doc omits, or a wrong `required`. Those ' + - 'findings are warnings ' + - 'in `propDrift`; without TypeScript it is `skipped` with a reason.', + 'prop the package declares that the doc omits, a wrong `required`, or a type ' + + 'that does not match. A type is judged only when it reads exactly: the same ' + + 'text as the source, once whitespace, quotes and union order are set aside (exactly, if it contains `readonly`), ' + + 'or primitives, literals, arrays and unions of those, or a name for one, on ' + + 'both sides, a field such as `status.type` included, and for a union of ' + + "props types, the type of any one member. A field the prop's variants give " + + "different types is listed when the doc gives only some variants' types. A " + + 'mutable array type matches a readonly one, because callers can pass it; a ' + + 'readonly one documented for a mutable array is a mismatch. ' + + '`Name = …` with a union after it ' + + "must match in both parts; one value after it (`Size = 'md'`) is read as a " + + "default. A name only TypeScript's own libraries declare is not resolved, and " + + '`React.X` resolves through React where the prop is declared. Every ' + + 'other type is listed as not compared, never reported, and so is a name an ' + + 'index signature takes unless exactly one the package declares takes it; ' + + 'with a TypeScript older than 4.8 no type is judged. ' + + 'Those findings are warnings in `propDrift`; without TypeScript it is ' + + '`skipped` with a reason.', importPath: '@astryxdesign/cli/api', signature: 'integrationComponentConflicts(pkg?: string, options?: IntegrationAuthoringOptions): Promise', @@ -42,7 +57,7 @@ export const doc = { { type: 'integration.component-conflicts', description: - '`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).', + '`validated` (false when no integration manifest was found, so nothing was inspected), integration identity, structural and replaces issues, and Core component-name conflicts with package-qualified commands. An active replacement that keeps the Core name it replaces is not a conflict. When the manifest loads and declares a components root, `propDrift` holds the props comparison: `status` (`checked` | `skipped`), `reason` when skipped, `typescript`, `components` compared, `typesCompared` (documented prop types judged), `typesSkipped` (why none was, with a TypeScript older than 4.8), `findings` ({component, prop, kind: `not_in_code` | `not_in_doc` | `required_mismatch` | `type_mismatch`, severity: `warning`, message, doc, source, docType?, codeType?, docRequired?, codeRequired?}), `unchecked` ({component, doc, reason}), `typesNotCompared` ({component, doc, reason, props}: every other documented prop type, grouped by why it was not judged: not a form the check judges exactly (a doc type that does not parse, or a prop a mapped type builds from keys the check cannot trace, included), a field the prop\'s variants give different types, a standard prop such as `className`, or a prop declared outside the package), and `propsNotChecked` when a mapped type builds props from keys the check cannot trace to the package ({component, doc, reason, props}: no warning is drawn for those props).', }, ], examples: [ diff --git a/packages/cli/assets/docs/tree/checks.doc.mjs b/packages/cli/assets/docs/tree/checks.doc.mjs index 4e89188c0dffe..caf053b5f74b0 100644 --- a/packages/cli/assets/docs/tree/checks.doc.mjs +++ b/packages/cli/assets/docs/tree/checks.doc.mjs @@ -38,7 +38,7 @@ export const docs = { '`npx astryx doctor integration components`', "No component name clashes with a Core component, each `replaces` names one Core component, and each component doc's props match the component's TypeScript props", 'Core is not installed (`core_not_found`). In a package that declares the CLI range that turns replacement on, a `replaces` target is missing (`missing_component_replacement_target`) or invalid (`invalid_component_replacement`), or two components replace one Core component (`ambiguous_component_replacement`)', - 'A name clashes with Core, a component sets `replaces` in a package without that range (`inactive_component_replacement`, plus any missing, invalid, or duplicate `replaces` reported as a warning), or a documented prop differs from the code: it is missing there (`not_in_code`), the doc omits a prop the package declares (`not_in_doc`), or its `required` disagrees (`required_mismatch`). It reads props with your installed `typescript`; without it, it prints `[skip] Props not compared`', + 'A name clashes with Core, a component sets `replaces` in a package without that range (`inactive_component_replacement`, plus any missing, invalid, or duplicate `replaces` reported as a warning), or a documented prop differs from the code: it is missing there (`not_in_code`), the doc omits a prop the package declares (`not_in_doc`), or its `required` (`required_mismatch`) or type (`type_mismatch`) disagrees. It reads props with your installed `typescript`; without it, it prints `[skip] Props not compared`', ], [ '`npx astryx doctor integration templates`', diff --git a/packages/cli/clients/cli/commands/doctor-integration-components.doc.mjs b/packages/cli/clients/cli/commands/doctor-integration-components.doc.mjs index f80dd88ab835c..2433912e81d24 100644 --- a/packages/cli/clients/cli/commands/doctor-integration-components.doc.mjs +++ b/packages/cli/clients/cli/commands/doctor-integration-components.doc.mjs @@ -21,8 +21,10 @@ export const doc = { 'with a warning for each such component, naming the range to declare. ' + "Then it compares each component doc's props with the component's TypeScript " + "props, using the project's own `typescript`, and warns about a documented prop " + - 'the code lacks, a prop the package declares that the doc omits, and a wrong ' + - '`required`. Without TypeScript it prints one ' + + 'the code lacks, a prop the package declares that the doc omits, a wrong ' + + '`required`, and a type that does not match where both sides read exactly ' + + '(it lists every other documented type as not compared, with the reason). ' + + 'Without TypeScript it prints one ' + 'line saying the comparison was skipped.', fn: 'integrationComponentConflicts', args: [ @@ -35,7 +37,10 @@ export const doc = { }, ], examples: [ - {label: 'Check the local integration', cli: 'astryx doctor integration components'}, + { + label: 'Check the local integration', + cli: 'astryx doctor integration components', + }, { label: 'Check an installed integration', cli: 'astryx doctor integration components @acme/widgets --json', diff --git a/packages/cli/clients/cli/commands/doctor-integration.test.mjs b/packages/cli/clients/cli/commands/doctor-integration.test.mjs index 65ef2d45014f0..799b03939c214 100644 --- a/packages/cli/clients/cli/commands/doctor-integration.test.mjs +++ b/packages/cli/clients/cli/commands/doctor-integration.test.mjs @@ -639,12 +639,14 @@ describe('doctor integration — command', () => { `export default {type: 'component', name: 'AcmeBadge', displayName: 'AcmeBadge', description: 'Fixture component.', props: [${[ "{name: 'label', type: 'string', description: 'Text.'}", "{name: 'elevation', type: 'number', description: 'Shadow.'}", + "{name: 'size', type: 'string', description: 'Size.'}", + "{name: 'meta', type: '{id: string; extra?: number}', description: 'Data.'}", ].join(', ')}]};\n`, ); // `key` is a prop a doc never needs, though the component declares it. fs.writeFileSync( path.join(tmpDir, 'components', 'AcmeBadge.tsx'), - "export function AcmeBadge(props: {label: string; tone?: 'neutral' | 'info'; key?: string}) { return null; }\n", + "export function AcmeBadge(props: {label: string; tone?: 'neutral' | 'info'; size?: number; meta?: {id: string}; key?: string}) { return null; }\n", ); fs.writeFileSync( path.join(tmpDir, 'components', 'AcmeLonely.doc.mjs'), @@ -674,10 +676,30 @@ describe('doctor integration — command', () => { expect(report.findings.map(f => `${f.prop} ${f.kind}`)).toEqual([ 'elevation not_in_code', 'label required_mismatch', + 'size type_mismatch', 'tone not_in_doc', ]); expectRecordsCarry(output, report.findings, ['component', 'prop', 'kind']); expectRecordsCarry(output, report.unchecked, ['component', 'reason']); + expectRecordsCarry(output, report.typesNotCompared, [ + 'component', + 'reason', + 'props', + ]); + expect(report.typesNotCompared).toHaveLength(1); + expect(output).toContain( + `Documented prop types judged: ${report.typesCompared}; not compared: 1.`, + ); + // The response-type reference, which the README's JSON table comes from, + // names every field too. + const {doc: responseTypes} = + await import('../../../foundation/response/response-types.doc.mjs'); + const member = responseTypes.members.find( + m => m.value === 'integration.component-conflicts', + ); + for (const field of Object.keys(report)) { + expect(member?.description).toMatch(new RegExp(`\\b${field}\\b`)); + } expect(output).toContain( `in 1 of ${report.components} component(s) compared (TypeScript ${report.typescript}).`, ); @@ -695,9 +717,13 @@ describe('doctor integration — command', () => { 'components', ]); - expect(logCalls.join('\n')).toMatch( + const output = logCalls.join('\n'); + expect(output).toMatch( /\[ok\] Documented prop names and required flags match the code in 1 component\(s\) \(TypeScript \d/, ); + expect(output).toMatch( + /\[info\] Documented prop types judged: \d+; not compared: 0\./, + ); }); it('components prints no [ok] for props it could not compare', async () => { diff --git a/packages/cli/clients/cli/commands/doctor.mjs b/packages/cli/clients/cli/commands/doctor.mjs index b23ac37b72c92..37aa98084ccde 100644 --- a/packages/cli/clients/cli/commands/doctor.mjs +++ b/packages/cli/clients/cli/commands/doctor.mjs @@ -178,6 +178,7 @@ function printTemplateConflicts(data) { * visible line when it was skipped, the findings (all warnings), the * components it could not read, and how many were compared with which * TypeScript, as an [ok] when nothing differs (never after a failed check). + * Then how many documented prop types were judged and which were not. * @param {import('../../../api/integration/authoring-checks.type.mjs').PropDriftReport | undefined} report * @param {boolean} failed */ @@ -243,6 +244,24 @@ function propDriftBlocks(report, failed) { ), ); } + if (report.typesSkipped) { + blocks.push(text(`[skip] Prop types not judged: ${report.typesSkipped}.`)); + return blocks; + } + const notCompared = report.typesNotCompared.flatMap(entry => entry.props); + blocks.push( + text( + `[info] Documented prop types judged: ${report.typesCompared}; not compared: ${notCompared.length}. ` + + 'A type is judged when the doc writes it the same as the source (exactly, if it contains `readonly`), or when both are primitives, literals, arrays and unions of those, or names for them.', + ), + ); + if (notCompared.length > 0) { + blocks.push( + records(report.typesNotCompared, { + fields: ['component', 'doc', 'reason', 'props'], + }), + ); + } return blocks; } diff --git a/packages/cli/foundation/discovery/prop-drift.mjs b/packages/cli/foundation/discovery/prop-drift.mjs index 07ae713f284e7..206238fda6a93 100644 --- a/packages/cli/foundation/discovery/prop-drift.mjs +++ b/packages/cli/foundation/discovery/prop-drift.mjs @@ -28,7 +28,10 @@ * pass the prop (one only some members of the props type require, or that * the package's `defaultProps` fills, is optional, as it is to JSX * callers; an entry that names several props claims its `required` for - * each). + * each); + * - `type_mismatch`: for a prop declared in the package, the documented type + * differs from the code's where both sides read exactly (type-match.mjs + * states the rules); every other type is listed as not compared. * * @input Component records (name, docPath, sourcePath), or prop tables, the * package root, and the directories to resolve `typescript` from. @@ -46,13 +49,15 @@ import { resolveProjectTypeScript, unusableTypeScriptReason, } from './project-typescript.mjs'; +import {judgeType} from './type-match.mjs'; +import {typeTextTooOld} from './type-text.mjs'; import { unresolvedGlobals, unresolvedReference, } from './unresolved-references.mjs'; /** - * @typedef {'not_in_code' | 'not_in_doc' | 'required_mismatch'} PropDriftKind + * @typedef {'not_in_code' | 'not_in_doc' | 'required_mismatch' | 'type_mismatch'} PropDriftKind */ /** @@ -77,6 +82,16 @@ import { * @property {string} reason why its props could not be read */ +/** + * @typedef {object} PropDriftTypesNotCompared + * @property {string} component + * @property {string} doc the doc file, relative to the package root + * @property {string} reason why: not a form the check judges exactly, a + * field the prop's variants give different types, a standard prop + * (`className`, `style`, …), or a prop declared outside the package + * @property {string[]} props documented props whose types were not compared + */ + /** * @typedef {object} PropDriftReport * @property {'checked' | 'skipped'} status `skipped` means nothing was @@ -84,12 +99,16 @@ import { * @property {string} [reason] * @property {string} [typescript] the version of the compiler that read the props * @property {number} components how many documented prop tables were compared + * @property {number} typesCompared how many documented prop types were judged + * @property {string} [typesSkipped] why no prop type was judged * @property {PropDriftFinding[]} findings * @property {PropDriftUnchecked[]} unchecked documented components whose props * could not be read from the source * @property {Array} [propsNotChecked] * props a mapped type builds from keys the check can't trace, which a doc * leaving out draws no warning for + * @property {PropDriftTypesNotCompared[]} typesNotCompared every other + * documented prop type, grouped by why it was not judged */ /** @@ -125,6 +144,17 @@ export const PROPS_NOT_DOCUMENTED = Object.freeze([ /** The guidance's internal props: a declaration tagged `@internal`. */ const SKIP_TAGS = new Set(['internal']); +/** Why a documented prop type was not judged (`typesNotCompared`). */ +const NOT_EXACT = 'not a form the check judges exactly'; +/** A field documented with one variant's type, where they differ. */ +const VARIANTS = "the prop's variants give this field different types"; +/** @type {(name: string, code?: CodeProp) => string} why a prop is not judged */ +const outside = (name, code) => + code?.untraced + ? NOT_EXACT + : PROPS_NOT_DOCUMENTED.includes(name) + ? 'standard prop, not compared' + : 'declared outside the package, not compared'; const HOOK_NAME = /^use[A-Z0-9]/; /** The line both surfaces print when the compiler is missing. */ @@ -201,8 +231,14 @@ function ownFileTest(root) { * @property {boolean} [untraced] built from keys the check can't trace * @property {boolean} skipTagged its declaration carries an internal JSDoc tag * @property {string[]} declared the type as written, per own declaration - * @property {() => string[]} renderings the compiler's renderings, computed on demand + * @property {() => string[][]} renderings the compiler's renderings, computed on demand * @property {() => any[]} types the prop's type in each member of the props type + * @property {boolean} intersected one member declares it more than once (an + * intersection), so no one declaration's text is its type + * @property {() => any[] | null} indexed the type an index signature gives + * the name in each member that lacks the prop; null when two signatures + * in one member take it + * @property {any} location where names in the prop's type resolve */ /** @@ -216,7 +252,7 @@ function ownFileTest(root) { * @param {import('./unresolved-references.mjs').UnresolvedReference | null} ambient * what the program's other roots cannot resolve, which any component may * use - * @returns {{ok: true, props: Map, accepts: (name: string) => boolean} | {ok: false, reason: string}} + * @returns {{ok: true, props: Map, accepts: (name: string) => any} | {ok: false, reason: string}} */ function readCodeProps( ts, @@ -328,7 +364,7 @@ function readCodeProps( } /** @type {Map} */ const props = new Map(); - if (propsTypes.length === 0) return {ok: true, props, accepts: () => false}; + if (propsTypes.length === 0) return {ok: true, props, accepts: () => []}; // Props that are `any`, written so or made from it (`FC`, or // `forwardRef`'s `Omit`). @@ -475,42 +511,75 @@ function readCodeProps( ), ), // A property's `type` is the prop's type; a method's is its return - // type, so a method prop reads as the compiler renders it. - declared: ownDeclarations - .filter( - /** @param {any} d */ d => - d.type && !ts.isMethodSignature(d) && !ts.isMethodDeclaration(d), - ) - .map(/** @param {any} d */ d => d.type.getText(d.getSourceFile())), + // type, so a method prop reads as the compiler renders it. A + // declaration's text counts only for its own symbol, or where a mapping + // copies its type unchanged (`Pick`, `Omit`): one that rewrites it + // (`{[K in keyof T]: T[K][]}`, `Required`) is judged by its type. + declared: symbols.flatMap( + /** @param {any} s */ s => + (s.declarations ?? []) + .filter( + /** @param {any} d */ d => + ownDeclarations.includes(d) && + d.type && + !ts.isMethodSignature(d) && + !ts.isMethodDeclaration(d) && + copiesType(ts, checker, s, d, sourceFile), + ) + .map(/** @param {any} d */ d => d.type.getText(d.getSourceFile())), + ), types: () => symbols.map( /** @param {any} s */ s => checker.getTypeOfSymbolAtLocation(s, sourceFile), ), + intersected: symbols.some( + /** @param {any} s */ s => + (s.declarations ?? []).filter(/** @param {any} d */ d => d.type) + .length > 1, + ), + indexed: memo(() => { + const lacking = members.filter( + /** @param {any} m */ m => !checker.getPropertyOfType(m, name), + ); + const each = lacking.map( + /** @param {any} m */ m => indexesFor(ts, checker, [m], name), + ); + return each.some(indexes => indexes.length > 1) + ? null + : each.flatMap(indexes => indexes.map(info => info.type)); + }), + location: ownDeclarations[0] ?? sourceFile, + // Printed where the prop is declared, where a documented type's names + // resolve: a type another file imports under another name prints as + // that file names it. renderings: memo(() => { + const at = ownDeclarations[0] ?? sourceFile; /** @type {string[]} */ const printed = []; + /** @type {string[]} */ + const expanded = []; for (const s of symbols) { const propType = checker.getTypeOfSymbolAtLocation(s, sourceFile); for (const part of inDeclaredOrder(ts, checker, propType, s)) { - printed.push(checker.typeToString(part, sourceFile, flags)); + printed.push(checker.typeToString(part, at, flags)); + expanded.push( + checker.typeToString( + part, + at, + flags | ts.TypeFormatFlags.InTypeAlias, + ), + ); } } - return printed; + return [printed, expanded]; }), }); } - /** Does an index signature of the props type accept this name? */ + /** The index signatures of the props type that take this name. */ const accepts = (/** @type {string} */ name) => - members.some( - /** @param {any} member */ member => - (checker.getIndexInfosOfType?.(member) ?? []).some( - /** @param {any} info */ info => - !holdsNothing(ts, info.type) && - keyAccepts(ts, checker, info.keyType, name), - ), - ); + indexesFor(ts, checker, members, name); return {ok: true, props, accepts}; } @@ -567,30 +636,140 @@ function inDeclaredOrder(ts, checker, type, symbol) { ); } +/** + * Whether a declaration's text reads as the prop's type: it is the prop's own + * declaration, or the generic type's prop it instantiates when the result + * prints as the declared type in the component's file (a generic component + * passing its own type parameters on), or a mapping copies its type + * unchanged. Instantiated with another type (`Box`, `ListProps`, + * or an imported `Item` the file names another way), its text names a type + * the prop no longer has. + * @param {any} ts + * @param {any} checker + * @param {any} symbol the prop + * @param {any} declaration one of its declarations + * @param {any} location + */ +function copiesType(ts, checker, symbol, declaration, location) { + const own = declaration.name && checker.getSymbolAtLocation(declaration.name); + if (!own) return false; + if (own === symbol) return true; + const type = checker.getTypeOfSymbolAtLocation(symbol, location); + const declared = checker.getTypeOfSymbolAtLocation(own, location); + /** @param {any} t */ + const print = t => + checker.typeToString(t, location, ts.TypeFormatFlags.NoTruncation); + return ( + declared === type || + (own === (symbol.links ?? symbol).target && print(type) === print(declared)) + ); +} + +/** + * Does the code take `undefined` itself, written or through an alias? With + * `exactOptionalPropertyTypes` that is part of the type a message shows; + * otherwise an optional prop takes `undefined` anyway. + * @param {any} checker + * @param {CodeProp} code + * @param {boolean} exactOptional + */ +function ownUndefined(checker, code, exactOptional) { + const undefinedType = checker.getUndefinedType?.(); + return ( + exactOptional && + (code.declared.some(text => /\bundefined\b/.test(text)) || + code + .types() + .some(t => (t.isUnion() ? t.types : [t]).includes(undefinedType))) + ); +} + +/** + * One of a prop's renderings for a message, without the `undefined` an + * optional prop adds, unless the code takes it itself. + * @param {CodeProp} code + * @param {number} which 0 as printed, 1 expanded + * @param {boolean} keepUndefined + */ +function rendered(code, which, keepUndefined) { + const parts = code.renderings()[which]; + return [ + ...new Set( + parts.filter( + part => + !(code.required !== true && part === 'undefined' && !keepUndefined), + ), + ), + ].join(' | '); +} + /** * The prop's type for a message: as the source writes it when its * declarations agree, else (a method or accessor, or declarations that differ - * or narrow each other in an intersection) as the compiler prints it, without - * the `undefined` an optional prop adds. + * or narrow each other in an intersection) as the compiler prints it. * @param {CodeProp} code + * @param {boolean} keepUndefined */ -function codeTypeText(code) { +function codeTypeText(code, keepUndefined) { const written = [...new Set(code.declared)]; const text = - written.length === 1 - ? written[0] - : [ - ...new Set( - code - .renderings() - .filter( - part => !(code.required !== true && part === 'undefined'), - ), - ), - ].join(' | '); + written.length === 1 ? written[0] : rendered(code, 0, keepUndefined); return text.replace(/\s+/g, ' ').trim(); } +/** + * Judge a documented type against a prop's code type (type-match.mjs). + * @param {any} ts + * @param {any} checker + * @param {string} docType + * @param {CodeProp} code + * @param {{nullable: boolean, exactOptional: boolean, isDefaultLibrary: (file: any) => boolean}} reading + * how the project checks null, and which files are TypeScript's own + */ +function judgeProp(ts, checker, docType, code, reading) { + // A member that lacks the prop may take its name through an index + // signature, which a doc may describe too. + const indexed = code.indexed(); + if (indexed == null) return 'not_compared'; + return judgeMembers(docType, [...code.types(), ...indexed], { + ts, + checker, + location: code.location, + optional: code.required !== true && !reading.exactOptional, + ...reading, + // Declared text reads as the type only when each declaration is one + // member's; intersected declarations narrow each other. + written: code.intersected + ? code.renderings() + : [ + code.declared, + ...code.declared.map(text => [text]), + ...code.renderings(), + ], + }); +} + +/** + * Judge a documented type against the types a prop has in the members of a + * union of props types: a doc may describe one member, so any member's type, + * or their union, matches, and it is a mismatch only against every one. + * @param {string} docType + * @param {any[]} types the type in each member + * @param {import('./type-match.mjs').JudgeContext} ctx + * @returns {import('./type-match.mjs').TypeVerdict} + */ +function judgeMembers(docType, types, ctx) { + const candidates = + types.length > 1 ? [types, ...types.map(type => [type])] : [types]; + const verdicts = candidates.map(candidate => + judgeType(docType, candidate, ctx), + ); + if (verdicts.includes('match')) return 'match'; + return verdicts.every(verdict => verdict === 'mismatch') + ? 'mismatch' + : 'not_compared'; +} + /** * The props one doc entry names. `a / b` documents each of `a` and `b`, with * the entry's `required` (and the type split the same way when it splits @@ -656,37 +835,101 @@ function holders(ts, checker, type, depth = 0) { } /** - * Does a field path exist in a prop's type, in at least one member of a - * union (through arrays)? + * Whether a type takes `undefined` (an index signature's value type, which + * TypeScript holds an explicit `undefined` to). + * @param {any} ts + * @param {any} type + */ +function takesUndefined(ts, type) { + return (type.isUnion() ? type.types : [type]).some( + /** @param {any} t */ t => t.flags & ts.TypeFlags.Undefined, + ); +} + +/** + * The types a field path reaches in a prop's type (in each member of a union + * that has it, through arrays at any depth), and whether its last field is + * optional: null when no member has the path, `any` when a type on the way + * is `any` or `unknown`, which holds any field. * @param {any} ts * @param {any} checker * @param {CodeProp} code * @param {string[]} fieldPath + * @returns {{types: any[], optional: boolean, ambiguous: boolean, indexed: boolean} | 'any' | null} + * `ambiguous` when two index signatures take a field on the path, and + * `indexed` when only index signatures take the last field */ -function fieldExists(ts, checker, code, fieldPath) { +function fieldTypes(ts, checker, code, fieldPath) { /** @type {any[]} */ let current = code.types(); + let optional = false; + let ambiguous = false; + let indexed = false; for (const field of fieldPath) { /** @type {any[]} */ const next = []; + optional = false; + let byName = false; for (const holder of current.flatMap(t => holders(ts, checker, t))) { - if (holder.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) return true; + if (holder.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) { + return 'any'; + } const prop = checker.getPropertyOfType(holder, field); - // Or a field an index signature takes (`Record`). - const index = prop - ? null - : (checker.getIndexInfosOfType?.(holder) ?? []).find( - /** @param {any} info */ info => - !holdsNothing(ts, info.type) && - keyAccepts(ts, checker, info.keyType, field), - ); - if (prop) next.push(checker.getTypeOfSymbol(prop)); - else if (index) next.push(index.type); + if (prop) { + next.push(checker.getTypeOfSymbol(prop)); + if (prop.flags & ts.SymbolFlags.Optional) optional = true; + byName = true; + continue; + } + // A field an index signature takes (`Record`) is of the + // index's type, which takes `undefined` only when it says so; two that + // take it hold it to both. + const indexes = indexesFor(ts, checker, [holder], field); + if (indexes.length > 1) ambiguous = true; + if (indexes.length > 0) { + next.push(indexes[0].type); + if (takesUndefined(ts, indexes[0].type)) optional = true; + } } - if (next.length === 0) return false; + if (next.length === 0) return null; current = next; + indexed = !byName; } - return true; + return {types: current, optional, ambiguous, indexed}; +} + +/** + * The types a field path reaches, printed where the prop is declared, where + * a documented type's names resolve. + * @param {any} ts + * @param {any} checker + * @param {{types: any[], optional: boolean}} reached + * @param {boolean} exactOptional the project sets `exactOptionalPropertyTypes` + * @param {any} location where the prop is declared + * @returns {string[]} + */ +function fieldTypeText(ts, checker, reached, exactOptional, location) { + // An optional field's `undefined` is the compiler's own; with + // `exactOptionalPropertyTypes` it is a marker of its own, and one the code + // declares stays. + const own = /** @param {any} t */ t => + reached.optional && + t.flags & ts.TypeFlags.Undefined && + (!exactOptional || t !== checker.getUndefinedType?.()); + return [ + ...new Set( + reached.types + .flatMap(t => (t.isUnion() ? t.types : [t])) + .filter(t => !own(t)) + .map(t => + checker.typeToString( + t, + location, + ts.TypeFormatFlags.UseSingleQuotesForStringLiteralType, + ), + ), + ), + ]; } /** @@ -902,6 +1145,33 @@ function keyAccepts(ts, checker, key, name) { return new RegExp(`^${pattern}$`).test(name); } +/** + * The index signatures of these types that take a name, as TypeScript + * matches keys. TypeScript holds a value to every one of them. + * @param {any} ts + * @param {any} checker + * @param {any[]} types + * @param {string} name + * @returns {any[]} + */ +function indexesFor(ts, checker, types, name) { + // One signature seen through several union members is one signature. + const seen = new Set(); + return types + .flatMap(type => checker.getIndexInfosOfType?.(type) ?? []) + .filter( + /** @param {any} info */ info => { + const key = `${checker.typeToString(info.keyType)}\0${checker.typeToString(info.type)}`; + if (seen.has(key)) return false; + seen.add(key); + return ( + !holdsNothing(ts, info.type) && + keyAccepts(ts, checker, info.keyType, name) + ); + }, + ); +} + /** * Can a value of this type be passed at all? `never`, or only `undefined` * (`Record`, the usual way to write "no props"), cannot. @@ -1348,8 +1618,10 @@ export function comparePropTables(tables, options) { ) : TYPESCRIPT_MISSING_REASON, components: 0, + typesCompared: 0, findings: [], unchecked: [], + typesNotCompared: [], }; } const {ts, version} = compiler; @@ -1369,8 +1641,10 @@ export function comparePropTables(tables, options) { status: 'checked', typescript: version, components: 0, + typesCompared: 0, findings: [], unchecked, + typesNotCompared: [], }; } @@ -1393,8 +1667,10 @@ export function comparePropTables(tables, options) { status: 'checked', typescript: version, components: 0, + typesCompared: 0, findings: [], unchecked, + typesNotCompared: [], }; } const program = ts.createProgram({ @@ -1416,12 +1692,30 @@ export function comparePropTables(tables, options) { project.options, isOwnFile, ); + // Without strict null checks the compiler drops `null` and `undefined`. + // Whether they are on by default depends on the TypeScript version. + const compilerOptions = program.getCompilerOptions(); + const nullable = !(typeof ts.getStrictOptionValue === 'function' + ? ts.getStrictOptionValue(compilerOptions, 'strictNullChecks') + : (compilerOptions.strictNullChecks ?? compilerOptions.strict ?? false)); + const reading = { + nullable, + // An optional prop takes `undefined` only when the project lets it. + exactOptional: + !nullable && compilerOptions.exactOptionalPropertyTypes === true, + isDefaultLibrary: (/** @type {any} */ file) => + program.isSourceFileDefaultLibrary(file), + }; /** @type {PropDriftFinding[]} */ const findings = []; /** @type {Array} */ const propsNotChecked = []; let compared = 0; + let typesCompared = 0; + /** @type {PropDriftTypesNotCompared[]} */ + const typesNotCompared = []; + const typesSkipped = typeTextTooOld(ts); for (const table of tables) { /** @type {ReturnType | null} */ let read = null; @@ -1456,14 +1750,41 @@ export function comparePropTables(tables, options) { compared++; const source = rel(sourcePath); const documented = new Set(); + /** @type {Map>} */ + const notCompared = new Map(); + /** @type {(reason: string, name: string) => void} */ + const skip = (reason, name) => { + notCompared.set(reason, (notCompared.get(reason) ?? new Set()).add(name)); + }; for (const raw of table.props) { if (!raw || typeof raw.name !== 'string') continue; const entries = documentedProps(raw); for (const entry of entries) { const propName = entry.name; if (entry.spread) continue; + const docType = entry.type; + if (typesSkipped) entry.type = undefined; documented.add(propName); const code = read.props.get(propName); + const reached = + code && entry.path.length > 0 + ? fieldTypes(ts, checker, code, entry.path) + : null; + if ( + entry.type === undefined && + !typesSkipped && + typeof raw.type === 'string' && + (code && entry.path.length > 0 + ? reached + : code || read.accepts(propName).length > 0) + ) { + // `a / b` with one type that does not split across the names (a + // field only once it is found). + skip( + code && !code.own ? outside(propName, code) : NOT_EXACT, + entry.written, + ); + } const base = { component: table.name, prop: entry.written, @@ -1472,11 +1793,74 @@ export function comparePropTables(tables, options) { source, }; if (entry.path.length > 0) { - if ( - code - ? fieldExists(ts, checker, code, entry.path) - : read.accepts(propName) - ) { + // Listed only once the field is found; a missing one is reported. + if (code && !code.own && reached && typeof entry.type === 'string') { + skip(outside(propName, code), entry.written); + } + if (reached === 'any') { + // A field of an `any` or `unknown` prop has no type to judge. + if (code?.own && typeof entry.type === 'string') { + skip(NOT_EXACT, entry.written); + } + continue; + } + if (code && reached) { + const fieldText = fieldTypeText( + ts, + checker, + reached, + reading.exactOptional, + code.location, + ); + const ctx = { + ts, + checker, + location: code.location, + // A name an index signature takes sets the index's + // `undefined` aside, as a prop of that name does. + optional: + reached.optional && (!reading.exactOptional || reached.indexed), + ...reading, + written: [fieldText], + }; + const verdict = + code.own && typeof entry.type === 'string' + ? reached.ambiguous + ? 'not_compared' + : judgeType(entry.type, reached.types, ctx) + : null; + // The field takes every variant's type: the types of only some + // variants are neither its type nor wrong. Past six, the sets of + // them are not tried, and the type is never reported. + const variants = [...new Set(reached.types)]; + const someVariants = + (verdict === 'mismatch' || verdict === 'not_compared') && + !reached.ambiguous && + variants.length > 1 && + (variants.length > 6 || + Array.from({length: 2 ** variants.length - 2}, (_, i) => + variants.filter((__, j) => ((i + 1) >> j) & 1), + ).some( + some => judgeType(entry.type ?? '', some, ctx) === 'match', + )); + if (someVariants) skip(VARIANTS, entry.written); + else if (verdict === 'not_compared') skip(NOT_EXACT, entry.written); + else if (verdict) typesCompared++; + if (verdict === 'mismatch' && !someVariants) { + const codeType = fieldText.join(' | '); + findings.push({ + ...base, + kind: 'type_mismatch', + message: `"${entry.written}" is documented as ${entry.type}, but the code declares ${codeType}.`, + docType: entry.type, + codeType, + }); + } + continue; + } + if (!code && read.accepts(propName).length > 0) { + // A field of a name only an index signature takes. + if (typeof entry.type === 'string') skip(NOT_EXACT, entry.written); continue; } findings.push({ @@ -1489,12 +1873,70 @@ export function comparePropTables(tables, options) { continue; } if (!code) { - if (read.accepts(propName)) continue; + // A name an index signature takes is a prop of the index's type; + // TypeScript holds it to every signature that takes it, so with + // several its type is not compared. Another package's signature is + // not the package's to judge, and a mapped type's (`Record`) has no + // declaration to say whose it is. + const indexes = read.accepts(propName); + const own = indexes.filter( + (/** @type {any} */ i) => + i.declaration && + isOwnFile(i.declaration.getSourceFile().fileName), + ); + const index = indexes.length === 1 ? own[0] : null; + if (indexes.length > 0 && !index && typeof entry.type === 'string') { + const declared = indexes.every( + (/** @type {any} */ i) => i.declaration, + ); + skip( + own.length || !declared ? NOT_EXACT : outside(propName), + propName, + ); + } + if (index && typeof entry.type === 'string') { + const verdict = judgeType(entry.type, [index.type], { + ts, + checker, + location: index.declaration, + // An index signature takes `undefined` only when its type does. + optional: takesUndefined(ts, index.type), + ...reading, + // Printed where the index is declared, as the doc's names + // resolve there. + written: [ + [ + checker.typeToString( + index.type, + index.declaration, + ts.TypeFormatFlags.UseSingleQuotesForStringLiteralType, + ), + ], + ], + }); + if (verdict === 'not_compared') skip(NOT_EXACT, propName); + else typesCompared++; + if (verdict === 'mismatch') { + const codeType = checker.typeToString( + index.type, + index.declaration, + ts.TypeFormatFlags.UseSingleQuotesForStringLiteralType, + ); + findings.push({ + ...base, + kind: 'type_mismatch', + message: `"${propName}" is documented as ${entry.type}, but ${table.name}'s props take it as ${codeType}.`, + docType: entry.type, + codeType, + }); + } + } + if (indexes.length > 0) continue; findings.push({ ...base, kind: 'not_in_code', message: `"${propName}" is documented, but ${table.name}'s props have no "${propName}".`, - ...(typeof entry.type === 'string' ? {docType: entry.type} : {}), + ...(typeof docType === 'string' ? {docType} : {}), }); continue; } @@ -1510,6 +1952,42 @@ export function comparePropTables(tables, options) { codeRequired: code.required, }); } + if (!code.own && typeof entry.type === 'string') { + skip(outside(propName, code), propName); + } + if (code.own && typeof entry.type === 'string') { + const verdict = judgeProp( + ts, + checker, + String(entry.type), + code, + reading, + ); + if (verdict === 'not_compared') skip(NOT_EXACT, propName); + else typesCompared++; + if (verdict === 'mismatch') { + const keep = ownUndefined(checker, code, reading.exactOptional); + const codeType = codeTypeText(code, keep); + const expanded = rendered(code, 1, keep); + // The optional prop's `undefined` the expansion sets aside is no + // difference to show. + const bare = codeType + .split(' | ') + .filter(part => part !== 'undefined') + .join(' | '); + const shown = + expanded && expanded !== codeType && expanded !== bare + ? `${codeType} (${expanded.length <= 400 ? expanded : `${expanded.slice(0, 400)}…`})` + : codeType; + findings.push({ + ...base, + kind: 'type_mismatch', + message: `"${propName}" is documented as ${entry.type}, but the code declares ${shown}.`, + docType: entry.type, + codeType, + }); + } + } } } /** @type {string[]} */ @@ -1526,6 +2004,10 @@ export function comparePropTables(tables, options) { if (code.untraced) untraced.push(code.name); continue; } + const codeType = codeTypeText( + code, + ownUndefined(checker, code, reading.exactOptional), + ); findings.push({ component: table.name, prop: code.name, @@ -1534,7 +2016,7 @@ export function comparePropTables(tables, options) { message: `${table.name} declares "${code.name}", but the doc does not list it.`, doc: table.doc, source, - codeType: codeTypeText(code), + codeType, codeRequired: code.required, }); } @@ -1546,6 +2028,14 @@ export function comparePropTables(tables, options) { props: untraced.sort(), }); } + for (const [reason, props] of notCompared) { + typesNotCompared.push({ + component: table.name, + doc: table.doc, + reason, + props: [...props].sort(), + }); + } } // Field by field: collation ignores a separator such as NUL, which would @@ -1557,12 +2047,20 @@ export function comparePropTables(tables, options) { a.kind.localeCompare(b.kind), ); unchecked.sort((a, b) => a.component.localeCompare(b.component)); + typesNotCompared.sort( + (a, b) => + a.component.localeCompare(b.component) || + a.reason.localeCompare(b.reason), + ); return { status: 'checked', typescript: version, components: compared, + typesCompared, findings, unchecked, + typesNotCompared, + ...(typesSkipped ? {typesSkipped} : {}), ...(propsNotChecked.length > 0 ? {propsNotChecked} : {}), }; } diff --git a/packages/cli/foundation/discovery/prop-drift.test.mjs b/packages/cli/foundation/discovery/prop-drift.test.mjs index 1091c1e63c3cf..5ef1d19505fff 100644 --- a/packages/cli/foundation/discovery/prop-drift.test.mjs +++ b/packages/cli/foundation/discovery/prop-drift.test.mjs @@ -2,8 +2,8 @@ /** * @file Documented props against a component's TypeScript props: each kind - * of finding, each component it cannot read, and the visible skip when the - * project has no TypeScript. + * of finding, the loose type rules, the props a doc never needs, and the + * visible skip when the project has no TypeScript. */ import * as fs from 'node:fs'; @@ -14,6 +14,7 @@ import {afterEach, beforeEach, describe, expect, it} from 'vitest'; import { comparePropDocs, comparePropTables, + PROPS_NOT_DOCUMENTED, TYPESCRIPT_MISSING_REASON, } from './prop-drift.mjs'; import {unusableTypeScriptReason} from './project-typescript.mjs'; @@ -112,7 +113,173 @@ const kinds = report => report.findings.map(f => `${f.component}.${f.prop} ${f.kind}`); describe('comparePropTables', () => { - it("reports a prop missing from the code, one missing from the doc, and a wrong required flag, each component's together", async () => { + it('judges the type of a prop a mapped type of the package builds, and says why one another package builds is not judged', async () => { + withTypeScript(); + const sizes = path.join(pkg, 'node_modules', '@acme', 'sizes'); + fs.mkdirSync(sizes, {recursive: true}); + fs.writeFileSync( + path.join(sizes, 'package.json'), + JSON.stringify({name: '@acme/sizes', types: 'index.d.ts'}), + ); + fs.writeFileSync( + path.join(sizes, 'index.d.ts'), + "export type Sizes = Partial>;\n", + ); + const tables = [ + component( + 'Mapped', + "export function Mapped(props: Partial>) { return null; }", + {props: [prop('a', "'x' | 'z'"), prop('b', "'x' | 'y'")]}, + ), + component( + 'HandledCell', + "import type {Sizes} from '@acme/sizes';\nexport function HandledCell(props: Sizes) { return null; }", + {props: [prop('sm', 'number')]}, + ), + // Keys the check can't trace, and a doc type that does not parse; and + // a name that starts with another's sorts after it. + component( + 'Handled', + "type A = string;\nexport function Handled(props: {[K in `on${'Open' | 'Close'}`]?: () => void} & {items?: ((items: A[]) => A)[]}) { return null; }", + {props: [prop('onOpen', '() => A'), prop('items', '((A[]) => A)[]')]}, + ), + ]; + + const report = await compare(tables); + + expect(kinds(report)).toEqual(['Mapped.a type_mismatch']); + expect(report.typesNotCompared).toEqual( + [ + ['Handled', 'not a form the check judges exactly', ['items', 'onOpen']], + ['HandledCell', 'declared outside the package, not compared', ['sm']], + ].map(([component, reason, props]) => ({ + component, + doc: `components/${component}.doc.mjs`, + reason, + props, + })), + ); + }); + + it('judges no prop type, and says why, with a TypeScript older than 4.8', async () => { + const dir = path.join(pkg, 'node_modules', 'typescript'); + fs.mkdirSync(dir, {recursive: true}); + fs.writeFileSync(path.join(dir, 'package.json'), '{"version": "4.7.4"}'); + const real = JSON.stringify(TYPESCRIPT_DIR); + fs.writeFileSync( + path.join(dir, 'index.js'), + `module.exports = {...require(${real}), version: '4.7.4'};`, + ); + const source = 'export function Badge(p: {size?: string}) { return null; }'; + const doc = {props: [prop('size', 'number')]}; + + expect(await compare([component('Badge', source, doc)])).toMatchObject({ + findings: [], + typesCompared: 0, + typesSkipped: `TypeScript 4.7.4 is older than 4.8, the oldest the type comparison reads`, + }); + }); + + it("judges a prop a package's mapping rewrites by its type, and keeps the text of one a mapping copies", async () => { + withTypeScript(); + const types = `interface Base { size: 'sm' | 'md' } +interface Opt { tone?: 'info' | undefined } +type Many = {[K in keyof T]: T[K][]}; +type Nullable = {[K in keyof T]: T[K] | null}; +interface Pickable { pick?: (value: T) => void } +type ChooserProps = {pick?: string} & { + change?: (value: readonly T[]) => void | Promise; +}; +interface ListProps { value?: T; onChange?: (v: T) => void } +type MultiProps = ListProps; +import type {Item as Imported} from './item'; +import type * as NS from './item'; +interface Item { a: string } +interface ItemList { items?: Item[] }`; + const size = {props: [prop('size', "'sm' | 'md'", {required: true})]}; + const pick = {props: [prop('pick', '(value: T) => void')]}; + const tables = [ + ['Many', 'Many', size], + ['Nulls', 'Nullable', size], + ['Picked', "Pick", size], + [ + 'Strict', + 'Required', + {props: [prop('tone', "'info' | undefined", {required: true})]}, + ], + // A generic component's prop keeps its declaration's text; one a + // concrete type instantiates does not. + ['Boxed', 'Pickable', pick], + [ + 'Chooser', + 'ChooserProps', + { + props: [ + prop('pick', 'string'), + prop('change', '(value: readonly T[]) => void | Promise'), + ], + }, + '', + ], + // Instantiated with a type built from the component's own parameter, + // the declaration's `T` is no longer the prop's type. + // A type argument that prints with the parameter's name elsewhere is + // still another type here. + ['Renamed', 'ItemList', {props: [prop('items', 'Item[]')]}], + ['Spaced', 'ItemList', {props: [prop('items', 'Item[]')]}], + ['Owned', 'ItemList', {props: [prop('items', 'Item[]')]}], + ...[ + ['Listed', 'ListProps', 'T', '(v: T) => void'], + ['Maybe', 'ListProps', 'T', '(v: T) => void'], + ['Multi', 'MultiProps', 'T[]', '(v: T[]) => void'], + ].map(([name, props, value, onChange]) => [ + name, + props, + {props: [prop('value', value), prop('onChange', onChange)]}, + '', + ]), + ].map(([name, props, doc, generics = '']) => + component( + name, + `${types}\nexport function ${name}${generics}(props: ${props}) { return null; }`, + doc, + ), + ); + + fs.writeFileSync( + path.join(pkg, 'components', 'item.ts'), + 'export interface Item { b: number }\n', + ); + const report = await compare(tables); + + expect(kinds(report)).toEqual([ + 'Many.size type_mismatch', + 'Nulls.size type_mismatch', + 'Strict.tone type_mismatch', + ]); + // A message shows the type the mapping makes, not the source's text. + expect(report.findings.map(f => f.codeType)).toEqual([ + "('sm' | 'md')[]", + "'sm' | 'md' | null", + "'info'", + ]); + expect(report.typesNotCompared).toEqual( + [ + ['Boxed', ['pick']], + ['Listed', ['onChange', 'value']], + ['Maybe', ['onChange', 'value']], + ['Renamed', ['items']], + ['Spaced', ['items']], + ].map(([component, props]) => ({ + component, + doc: `components/${component}.doc.mjs`, + reason: 'not a form the check judges exactly', + props, + })), + ); + }); + + it("reports a prop missing from the code, one missing from the doc, a wrong required flag, and a wrong type, each component's together", async () => { withTypeScript(); const badge = component( 'Badge', @@ -147,20 +314,873 @@ export function Badge(props: BadgeProps) { return null; }`, 'Badge.count not_in_doc', 'Badge.label required_mismatch', 'Badge.size not_in_code', + 'Badge.tone type_mismatch', 'BadgeCount.amount not_in_doc', ]); expect(report.findings.every(f => f.severity === 'warning')).toBe(true); + expect(report.findings.find(f => f.kind === 'type_mismatch')).toMatchObject( + { + docType: "'neutral' | 'info'", + codeType: 'Tone', + doc: 'components/Badge.doc.mjs', + source: 'components/Badge.tsx', + message: expect.stringContaining( + "Tone ('neutral' | 'info' | 'danger')", + ), + }, + ); expect( report.findings.find(f => f.kind === 'required_mismatch'), - ).toMatchObject({ - docRequired: false, - codeRequired: true, - doc: 'components/Badge.doc.mjs', - source: 'components/Badge.tsx', - }); + ).toMatchObject({docRequired: false, codeRequired: true}); + }); + + it("judges a field, a name an index signature takes, and a prop declared in another file by the type's name where the prop is declared, not by the name the type was declared with", async () => { + withTypeScript(); + const dir = path.join(pkg, 'components'); + fs.mkdirSync(dir, {recursive: true}); + fs.writeFileSync( + path.join(dir, 'base.ts'), + "export interface Item { id: string }\nexport type Size = 'sm' | 'md';\n", + ); + // Another file imports the same types under other names, and declares + // its own `Item` and `Size`. + fs.writeFileSync( + path.join(dir, 'types.ts'), + [ + "import type {Item as BaseItem, Size as BaseSize} from './base';", + 'interface Item { label: string }', + "type Size = 'lg';", + 'export type ElsewhereProps = { size?: BaseSize; item?: BaseItem; onPick?: (item: BaseItem) => void; cfg?: {item: BaseItem}; [key: `size-${string}`]: BaseSize | undefined } & Required>;', + ].join('\n'), + ); + const here = component( + 'Renamed', + [ + "import type {Item as BaseItem, Size as BaseSize} from './base';", + 'interface Item { label: string }', + "type Size = 'lg';", + 'export function Renamed(props: {cfg?: {item: BaseItem}; [key: `size-${string}`]: BaseSize | undefined}) { return null; }', + ].join('\n'), + {props: [prop('cfg.item', 'Item'), prop('size-a', 'Size')]}, + ); + const elsewhere = component( + 'Elsewhere', + "import type {ElsewhereProps} from './types';\nexport function Elsewhere(props: ElsewhereProps) { return null; }", + { + props: [ + prop('size', 'Size'), + prop('tone', 'Size', {required: true}), + prop('entry', 'Item', {required: true}), + prop('item', 'Item'), + prop('onPick', '(item: Item) => void'), + prop('cfg.item', 'Item'), + prop('size-a', 'Size'), + ], + }, + ); + // The same docs, naming the types as each file does, are judged matches. + const named = component( + 'Named', + [ + "import type {Item as BaseItem, Size as BaseSize} from './base';", + 'export function Named(props: {cfg?: {item: BaseItem}; [key: `size-${string}`]: BaseSize | undefined}) { return null; }', + ].join('\n'), + {props: [prop('cfg.item', 'BaseItem'), prop('size-a', 'BaseSize')]}, + ); + + const report = await compare([here, elsewhere, named]); + + // `Size` is `'lg'` where each prop is declared, which TypeScript rejects + // for `'sm' | 'md'`; `Item` is an object type there, not compared. + expect(kinds(report)).toEqual([ + 'Elsewhere.size type_mismatch', + 'Elsewhere.size-a type_mismatch', + 'Elsewhere.tone type_mismatch', + 'Renamed.size-a type_mismatch', + ]); + expect(report.typesNotCompared).toEqual([ + expect.objectContaining({ + component: 'Elsewhere', + props: ['cfg.item', 'entry', 'item', 'onPick'], + }), + expect.objectContaining({component: 'Renamed', props: ['cfg.item']}), + ]); + expect(report.typesCompared).toBe(6); + }); + + it("reads `React.X` as React's own where the prop is declared, never as a bare name that means another type there", async () => { + withTypeScript(); + fs.mkdirSync(path.join(pkg, 'node_modules', '@types'), {recursive: true}); + fs.symlinkSync( + path.dirname(require.resolve('@types/react/package.json')), + path.join(pkg, 'node_modules', '@types', 'react'), + 'dir', + ); + const keys = component( + 'Keys', + [ + "import * as React from 'react';", + "type Key = 'a' | 'b';", + 'export function Keys(props: {onKey?: (event: KeyboardEvent) => void; rowKey?: string | number | bigint; pick?: Key; native?: React.Key}) { return null; }', + ].join('\n'), + { + props: [ + prop('onKey', '(event: React.KeyboardEvent) => void'), + prop('rowKey', 'React.Key'), + prop('pick', 'React.Key'), + prop('native', 'React.Key'), + ], + }, + ); + // React's global namespace, with no import. + const global = component( + 'GlobalKeys', + 'export function GlobalKeys(props: {rowKey?: string | number | bigint}) { return null; }', + {props: [prop('rowKey', 'React.Key')]}, + ); + + const report = await compare([keys, global]); + + // The DOM's `KeyboardEvent` is not React's, and the file's own `Key` is + // not `React.Key`. + expect(kinds(report)).toEqual(['Keys.pick type_mismatch']); + expect(report.typesNotCompared).toEqual([ + expect.objectContaining({component: 'Keys', props: ['onKey']}), + ]); + expect(report.typesCompared).toBe(4); + }); + + it('judges primitives, literals, arrays, string enums, and names for them exactly, in any order', async () => { + withTypeScript(); + const card = component( + 'Card', + [ + "type Tone = 'neutral' | 'info';", + "export enum Color { Red = 'red', Blue = 'blue' } export enum Hue { Red = 'red', Blue = 'blue' }", + 'export interface CardProps {', + ' label: string;', + ' tone?: Tone;', + ' items?: ReadonlyArray;', + ' isOpen?: boolean;', + " size?: 'sm' | 'md' | number;", + ' color?: Color;', + ' count?: number;', + ' ids?: string[];', + ' hue?: Color;', + " shade?: Color; shades?: Color[]; pick?: 'red' | 'blue';", + '}', + 'export function Card(props: CardProps) { return null; }', + ].join('\n'), + { + props: [ + prop('label', 'string', {required: true}), + prop('tone', '\'info\' | "neutral"'), + prop('items', 'Array'), + prop('isOpen', 'true | false'), + prop('size', "number | 'md' | 'sm'"), + prop('color', 'Color'), + prop('count', 'string'), + prop('ids', 'number[]'), + prop('hue', "'red' | 'blue'"), + prop('shade', 'Hue'), + prop('shades', 'Hue[]'), + prop('pick', 'Color'), + ], + }, + ); + + const report = await compare([card]); + + // TypeScript takes neither `'red'` nor another enum's `Hue.Red` where the + // enum `Color` is expected, and `Color` does not take the `'red'` `pick` does. + expect(kinds(report)).toEqual([ + 'Card.count type_mismatch', + 'Card.hue type_mismatch', + 'Card.ids type_mismatch', + 'Card.pick type_mismatch', + 'Card.shade type_mismatch', + 'Card.shades type_mismatch', + ]); + expect(report.typesCompared).toBe(12); + expect(report.typesNotCompared).toEqual([]); + }); + + it('reads a union the way TypeScript reduces it, and a default after `=` as no alias', async () => { + withTypeScript(); + const icon = component( + 'Icon', + [ + "type IconName = 'add' | 'close';", + "export enum Color { Red = 'red', Blue = 'blue' }", + 'type Step = 1 | 2;', + "type Size = 'sm' | 'md' | 'lg';", + 'export interface IconProps {', + ' icon?: IconName | string;', + ' color?: Color | string;', + ' step?: Step | number;', + ' isOpen?: boolean;', + ' icons?: string[];', + ' size?: Size;', + ' limit?: 1000 | 2000;', + ' width?: number | string;', + '}', + 'export function Icon(props: IconProps) { return null; }', + ].join('\n'), + { + props: [ + prop('icon', 'IconName | string'), + prop('color', 'Color | string'), + prop('step', 'Step | number'), + prop('isOpen', 'boolean = false'), + prop('icons', '(IconName | string)[]'), + prop('size', "Size = 'md'"), + prop('limit', '1_000 | 2_000'), + prop('width', 'number (or a CSS length)'), + ], + }, + ); + + const report = await compare([icon]); + + expect(report.findings).toEqual([]); + expect(report.typesCompared).toBe(7); + }); + + it('sets aside null and undefined when the project has no strict null checks', async () => { + withTypeScript(); + fs.writeFileSync( + path.join(pkg, 'tsconfig.json'), + JSON.stringify({compilerOptions: {strict: false, jsx: 'react-jsx'}}), + ); + const loose = component( + 'Loose', + [ + 'type MaybeNum = number | null;', + 'export function Loose(props: {count?: MaybeNum; counts?: MaybeNum[]}) { return null; }', + ].join('\n'), + { + props: [ + prop('count', 'number | null'), + prop('counts', '(number | null)[]'), + ], + }, + ); + + const report = await compare([loose]); + + expect(report.findings).toEqual([]); + expect(report.typesCompared).toBe(2); + }); + + it('respects exactOptionalPropertyTypes: an optional prop takes undefined only when its type says so', async () => { + withTypeScript(); + const tsconfig = path.join(pkg, 'tsconfig.json'); + const config = JSON.parse(fs.readFileSync(tsconfig, 'utf8')); + config.compilerOptions.exactOptionalPropertyTypes = true; + fs.writeFileSync(tsconfig, JSON.stringify(config)); + const exact = component( + 'Exact', + "type MaybeTone = 'a' | 'b' | 'c' | undefined;\nexport function Exact(props: {size?: 'sm' | 'md'; tone?: 'a' | undefined; mode?: 'x'; status?: {tone?: 'a' | 'b' | undefined; mode?: 'x'}; labels?: Record; hue?: 'a' | 'b' | undefined; t6?: MaybeTone; [key: `x${string}`]: string | undefined}) { return null; }\n", + { + props: [ + prop('size', "'sm' | 'md' | undefined"), + prop('tone', "'a' | undefined"), + prop('mode', "'x'"), + prop('status.tone', "'a' | 'b'"), + prop('status.mode', "'x'"), + prop('labels.close', 'string'), + prop('xTone', 'string'), + prop('hue', "'a' | 'b'"), + prop('t6', "'a' | 'b' | 'c'"), + ], + }, + ); + const variant = component( + 'Variant', + "export function Variant(props: {kind: 'a'; size?: 'sm' | undefined} | {kind: 'b'; size?: 'md' | undefined}) { return null; }\n", + { + props: [ + prop('kind', "'a' | 'b'", {required: true}), + prop('size', "'sm' | 'md'"), + ], + }, + ); + + const report = await compare([exact, variant]); + + // TypeScript rejects `size={undefined}` here, and takes the others; a + // field that declares `undefined` takes it too, unlike its doc. A name an + // index signature takes is held to the index's type the same way as a + // field or as a prop. + expect(kinds(report)).toEqual([ + 'Exact.hue type_mismatch', + 'Exact.size type_mismatch', + 'Exact.status.tone type_mismatch', + 'Exact.t6 type_mismatch', + 'Variant.size type_mismatch', + ]); + expect(report.typesCompared).toBe(11); + // The message shows the `undefined` the code declares, written once or by + // each member of a union, in the type and in its expansion. + for (const finding of report.findings.filter( + f => f.component === 'Variant' || f.prop === 'hue', + )) { + expect(finding.codeType).toMatch(/undefined/); + const declared = finding.message.split('but the code declares ')[1]; + expect(declared).toMatch(/undefined/); + expect(declared.match(/\((.*)\)\.$/)?.[1] ?? 'undefined').toMatch( + /undefined/, + ); + } + // Through an alias, the expansion shows it. + expect(report.findings.find(f => f.prop === 't6')?.message).toMatch( + /declares MaybeTone \(.*undefined.*\)\.$/, + ); + }); + + it('holds a name an index signature takes, a field a Record takes, and a readonly array to the type TypeScript checks, and lists a field only an index takes and an `a / b` entry whose one type does not split', async () => { + withTypeScript(); + const held = component( + 'Held', + [ + 'export function Held(props: {', + ' labels?: Record;', + ' tags?: string[];', + ' ids?: ReadonlyArray;', + ' keys?: readonly string[];', + ' grid?: readonly string[][];', + ' rows?: (readonly string[])[];', + ' onPick?: (values: readonly string[]) => void;', + ' onDrop?: (values: readonly string[]) => void;', + ' pinned?: {readonly ids: readonly string[]};', + ' item?: {tags: readonly string[]};', + ' mix?: string[] | readonly number[];', + ' min?: number; max?: number; status?: {a: number};', + ' [key: `x${string}`]: string; [key: `y${string}`]: {color: string};', + '}) { return null; }', + ].join('\n'), + { + props: [ + prop('labels.close', 'string | undefined'), + prop('xTone', 'string | undefined'), + prop('tags', 'readonly string[]'), + prop('ids', 'readonly string[]'), + prop('keys', 'string[]'), + prop('grid', '(readonly string[])[]'), + prop('rows', 'readonly string[][]'), + prop('onPick', '(values: string[]) => void'), + prop('onDrop', '(values: readonly string[])=>void'), + prop('pinned', '{readonly ids: readonly string[]}'), + prop('item', '{tags: string[]}'), + prop('mix', 'readonly string[] | number[]'), + prop('min / max', 'number'), + prop('status.a / status.b', 'number'), + prop('yTone.color', 'string'), + ], + }, + ); + + const report = await compare([held]); + + // TypeScript passes no `undefined` through an index that does not take + // it, and no readonly array where a mutable one is taken; a mutable + // array passes where a readonly one is. + expect(kinds(report)).toEqual([ + 'Held.labels.close type_mismatch', + 'Held.status.b not_in_code', + 'Held.tags type_mismatch', + 'Held.xTone type_mismatch', + ]); + expect(report.typesCompared).toBe(7); + // `readonly` written as the code writes it matches; written somewhere + // else, at another depth, in a callback's parameter, in an object's + // field, or on another union member, it is not placed: listed, never a + // match, as are a field only an index takes and `a / b` with one type. + expect( + report.typesNotCompared.map(e => `${e.reason}: ${e.props.join(' ')}`), + ).toEqual([ + 'not a form the check judges exactly: grid item max min mix onPick rows status.a yTone.color', + ]); + }); + + it('reads strict null checks as the TypeScript version defaults them', async () => { + withTypeScript(); + fs.writeFileSync( + path.join(pkg, 'tsconfig.json'), + JSON.stringify({compilerOptions: {jsx: 'react-jsx'}}), + ); + // The TypeScript the test links in: 6 checks strict null by default. + const ts = require(TYPESCRIPT_DIR); + const strictByDefault = ts.getStrictOptionValue({}, 'strictNullChecks'); + const maybe = component( + 'Maybe', + 'export function Maybe(props: {count?: number | null}) { return null; }\n', + {props: [prop('count', 'number')]}, + ); + + const report = await compare([maybe]); + + expect(kinds(report)).toEqual( + strictByDefault ? ['Maybe.count type_mismatch'] : [], + ); + }); + + it("lists a name two index signatures take and a field documented with one variant's type, and reads a default for one union member", async () => { + withTypeScript(); + const pick = component( + 'Pick', + [ + "export enum SizeE { Sm = 'sm', Md = 'md' }", + 'type Sel = {kind: "single"; value: string} | {kind: "multi"; value: string[]};', + 'export type PickProps = {', + ' sizeE?: SizeE;', + ' sel?: Sel;', + ' tri?: {v: string} | {v: number} | {v: boolean};', + ' sev?: {v: 1} | {v: 2} | {v: 3} | {v: 4} | {v: 5} | {v: 6} | {v: 7};', + ' labels?: {close: string} | Record;', + ' [key: `data-${string}`]: string | number | undefined;', + ' [key: `data-track${string}`]: string | undefined;', + '} & ({mode: "str"; [key: `x${string}`]: string} | {mode: "num"; [key: `x${string}`]: number});', + 'export function Pick(props: PickProps) { return null; }', + ].join('\n'), + { + props: [ + prop('sizeE', 'SizeE = SizeE.Md'), + prop('sel.value', 'string'), + prop('tri.v', 'string | number'), + prop('sev.v', '1 | 2'), + prop('labels.close', 'string | number'), + prop('data-tracker', 'string'), + prop('mode', "'str' | 'num'", {required: true}), + prop('xValue', 'number'), + ], + }, + ); + + const report = await compare([pick]); + + // TypeScript holds `data-tracker` to both signatures that take it, and + // `xValue` to the one of the member it is passed with; `sel.value`, + // `tri.v` and `sev.v` (past six variants, never reported) take every + // variant's type, and `labels.close` is documented with both. + expect(report.findings).toEqual([]); + expect(report.typesNotCompared).toEqual([ + expect.objectContaining({ + props: ['data-tracker', 'xValue'], + reason: 'not a form the check judges exactly', + }), + expect.objectContaining({ + props: ['sel.value', 'sev.v', 'tri.v'], + reason: "the prop's variants give this field different types", + }), + ]); + expect(report.typesCompared).toBe(3); + }); + + it('reads fields in arrays of unions, a name one member indexes, and names only where the code can mean them', async () => { + withTypeScript(); + const rows = component( + 'Rows', + [ + 'type Items = number[];', + 'export type RowsProps = {', + " items?: Array<{kind: 'a'; value: string} | {kind: 'b'; value: number}>;", + ' list?: Items;', + " behavior?: 'auto' | 'smooth';", + ' [key: `data-${string}`]: string | undefined;', + "} & ({mode: 'fixed'; xSize: number} | ({mode: 'free'} & {[k: `x${string}`]: string}));", + 'export function Rows(props: RowsProps) { return null; }', + ].join('\n'), + { + props: [ + prop('items[].value', 'string | number'), + prop('list', 'Items = string[]'), + prop('behavior', 'ScrollBehavior'), + prop('data-section', 'string'), + prop('mode', "'fixed' | 'free'", {required: true}), + prop('xSize', 'number | string'), + ], + }, + ); + + const report = await compare([rows]); + + // `ScrollBehavior` is the DOM's, which the code never names. + expect(kinds(report)).toEqual(['Rows.list type_mismatch']); + expect(report.typesNotCompared).toEqual([ + expect.objectContaining({props: ['behavior']}), + ]); + }); + + it('judges a prop two intersected types declare by the type they narrow it to, and undefined on a required prop', async () => { + withTypeScript(); + const narrow = component( + 'Narrow', + [ + "type A = {size?: 'sm'; tone?: 'a' | 'b'};", + "type B = {size?: 'sm' | 'md'; tone?: 'a' | 'b'};", + 'export type NarrowProps = B & A & {isOpen: boolean; label: string | undefined};', + 'export function Narrow(props: NarrowProps) { return null; }', + ].join('\n'), + { + props: [ + prop('size', "'sm' | 'md'"), + prop('tone', "'b' | 'a'"), + prop('isOpen', 'boolean | undefined', {required: true}), + prop('label', 'string', {required: true}), + ], + }, + ); + + const report = await compare([narrow]); + + // TypeScript takes only 'sm' for `size`, rejects `isOpen={undefined}`, + // and accepts `label={undefined}`. + expect(kinds(report)).toEqual([ + 'Narrow.isOpen type_mismatch', + 'Narrow.label type_mismatch', + 'Narrow.size type_mismatch', + ]); + expect(report.findings.find(f => f.prop === 'size')?.codeType).toBe("'sm'"); + }); + + it('lists every other form of type as not compared, never as a mismatch', async () => { + withTypeScript(); + const thread = component( + 'Thread', + [ + 'interface IconShape { path: string }', + 'type MaybePromise = T | Promise;', + 'type Parts = {a?: string; b?: number};', + "type A = {a: string}; type B = {b: number}; enum Color { Red = 'red' } namespace Theme { export enum Color { Red = 'red' } }", + 'export interface ThreadProps {', + ' onSubmit?: (text: string, files?: File[]) => MaybePromise;', + ' onPick?: (value: string, index: number) => void;', + ' item?: A & B;', + ' parts?: Partial;', + ' key2?: keyof (A & B);', + ' key3?: keyof (A & B);', + " target?: '_self' | '_blank' | (string & {});", + ' range?: [number, number?];', + ' labels?: Record;', + ' icon?: IconShape;', + ' data?: any;', + ' extra?: unknown; tint?: Theme.Color;', + '}', + 'export function Thread(props: ThreadProps) { return null; }', + ].join('\n'), + { + props: [ + prop('onSubmit', '(text: string) => Promise'), + prop('onPick', '(value: string, index: number) => void'), + prop('item', '{a: string; b: number}'), + prop('parts', 'Parts'), + prop('key2', 'keyof (A & B)'), + prop('key3', "'a' | 'b'"), + prop('target', "'_self' | '_blank'"), + prop('range', '[number]'), + prop('labels', '{[key: string]: number}'), + prop('icon', 'IconName'), + prop('data', 'string'), + prop('data.id', 'string'), + prop('extra.kind', 'string'), + prop('tint', 'Color'), + ], + }, + ); + + const report = await compare([thread]); + + expect(report.findings).toEqual([]); + // Written exactly as the source writes them, or a literal union the + // code's type resolves to (`keyof (A & B)` is `'a' | 'b'`). + expect(report.typesCompared).toBe(3); + expect( + report.typesNotCompared.map(e => `${e.reason}: ${e.props.join(' ')}`), + ).toEqual([ + 'not a form the check judges exactly: data data.id extra.kind icon item labels onSubmit parts range target tint', + ]); + }); + + it("compares a documented field's type, and a name an index signature takes, with the type the code gives it", async () => { + withTypeScript(); + const note = component( + 'Note', + [ + 'export interface NoteProps {', + " status?: {type: 'error' | 'warning'; message?: string};", + ' items?: Array<{id: string; label: string}>;', + ' labels?: Record;', + ' [key: `data-${string}`]: string | undefined;', + ' [key: `aria-${string}`]: string | number | undefined;', + '}', + 'export function Note(props: NoteProps) { return null; }', + ].join('\n'), + { + props: [ + prop('status.type', "'error' | 'info'"), + prop('items[].label', 'number'), + prop('labels.open', 'string'), + prop('labels.close', 'number'), + prop('data-section', 'string'), + prop('data-count', 'number'), + prop('aria-busy', 'boolean'), + ], + }, + ); + + const report = await compare([note]); + + expect(kinds(report)).toEqual([ + 'Note.aria-busy type_mismatch', + 'Note.data-count type_mismatch', + 'Note.items[].label type_mismatch', + 'Note.labels.close type_mismatch', + 'Note.status.type type_mismatch', + ]); + expect(report.typesCompared).toBe(7); + }); + + it('accepts an alias spelled out beside its name, and reports one whose spelled-out values are stale', async () => { + withTypeScript(); + const links = ["'link' | 'token'", "'link' | 'token' | 'button'"].map( + (values, i) => + component( + `Link${i}`, + `export type LinkVariant = ${values};\nexport function Link${i}(props: {variant?: LinkVariant; tone?: LinkVariant}) { return null; }`, + { + props: [ + prop('variant', "LinkVariant = 'link' | 'token'"), + prop('tone', "LinkVariant ('link' | 'token')"), + ], + }, + ), + ); + + expect(kinds(await compare(links))).toEqual([ + 'Link1.tone type_mismatch', + 'Link1.variant type_mismatch', + ]); + }); + + it("reads an optional prop documented with `| undefined` as the code reads it, and names an alias's members in the source's order", async () => { + withTypeScript(); + const badge = component( + 'Badge', + [ + "type Tone = 'neutral' | 'info';", + 'export function Badge(props: {tone?: Tone}) { return null; }', + ].join('\n'), + {props: [prop('tone', "'neutral' | 'info' | undefined")]}, + ); + const chip = component( + 'Chip', + "export function Chip(props: {tone?: 'a' | 'b' | undefined}) { return null; }\n", + {props: [prop('tone', "'a' | 'c'")]}, + ); + + // The same literals, made first in another order, would set the + // compiler's own order. + const status = component( + 'Status', + [ + "type Other = 'detached' | 'attached' | 'tooltip';", + 'export type Unrelated = Other;', + "type StatusVariant = 'tooltip' | 'attached' | 'detached';", + 'export function Status(props: {variant?: StatusVariant}) { return null; }', + ].join('\n'), + {props: [prop('variant', "'attached' | 'detached'")]}, + ); + + expect(kinds(await compare([badge]))).toEqual([]); + // The optional prop's `undefined` the expansion sets aside is shown once. + const [finding] = (await compare([chip])).findings; + expect(finding.message).toMatch(/declares 'a' \| 'b' \| undefined\.$/); + expect((await compare([status])).findings.map(f => f.message)).toEqual([ + `"variant" is documented as 'attached' | 'detached', but the code declares StatusVariant ('tooltip' | 'attached' | 'detached').`, + ]); + }); + + it('judges a className the package declares, and lists standard props and props from other packages as not compared', async () => { + withTypeScript(); + const base = path.join(pkg, 'node_modules', '@acme', 'base'); + fs.mkdirSync(base, {recursive: true}); + fs.writeFileSync( + path.join(base, 'package.json'), + JSON.stringify({name: '@acme/base', types: 'index.d.ts'}), + ); + fs.writeFileSync( + path.join(base, 'index.d.ts'), + 'export interface BaseProps { style?: object; id?: string; tone?: {level: number}; [key: `data-${string}`]: string | undefined }\n', + ); + // A name only another package's index signature, or a mapped type's, + // takes is not judged either. + const card = component( + 'Card', + "import type {BaseProps} from '@acme/base';\nexport function Card(props: BaseProps & {className?: string} & Record<`aria-${string}`, string>) { return null; }\n", + { + props: [ + prop('className', 'number'), + prop('style', 'string'), + prop('id', 'number'), + prop('tone.level', 'string'), + prop('tone.missing', 'string'), + prop('data-variant', "'solid' | 'outline'"), + prop('aria-note', 'number'), + ], + }, + ); + + const report = await compare([card]); + + expect(kinds(report)).toEqual([ + 'Card.className type_mismatch', + 'Card.tone.missing not_in_code', + ]); + expect(report.typesCompared).toBe(1); + expect( + report.typesNotCompared.map(e => `${e.reason}: ${e.props.join(' ')}`), + ).toEqual([ + 'declared outside the package, not compared: data-variant id tone.level', + 'not a form the check judges exactly: aria-note', + 'standard prop, not compared: style', + ]); + }); + + it('does not ask a doc for props from other packages, or for the props the authoring guidance skips', async () => { + expect(PROPS_NOT_DOCUMENTED).toEqual([ + 'className', + 'style', + 'xstyle', + 'data-testid', + 'ref', + 'key', + ]); + withTypeScript(); + const base = path.join(pkg, 'node_modules', '@acme', 'base'); + fs.mkdirSync(base, {recursive: true}); + fs.writeFileSync( + path.join(base, 'package.json'), + JSON.stringify({name: '@acme/base', types: 'index.d.ts'}), + ); + fs.writeFileSync( + path.join(base, 'index.d.ts'), + [ + 'export interface BaseProps { id?: string; onBlur?: () => void }', + "export type Sizes = Partial>;", + "export interface Spacing extends Partial> {}", + "export type Tone = 'info' | 'danger';", + 'export type Flags = {[P in K]?: boolean};', + 'export type FC

= (props: P) => null;', + "export type Responsive = {[K in 'sm' | 'lg']?: T};", + "export type Breakpoint = 'sm' | 'lg';", + 'export declare const BREAKPOINTS: readonly Breakpoint[];', + 'export declare function memo null>(c: T): {(p: Parameters[0]): null; $$typeof: symbol; type: T};', + 'export declare function forwardRef

(render: (props: P, ref: unknown) => null): (props: P) => null;', + ].join('\n'), + ); + const panel = component( + 'Panel', + [ + "import type {BaseProps} from '@acme/base';", + 'export interface PanelProps extends BaseProps {', + ' title?: string;', + ' className?: string;', + ' style?: {color?: string};', + ' xstyle?: unknown;', + " 'data-testid'?: string;", + ' ref?: unknown;', + ' key?: string;', + ' /** @deprecated Use `title`. */', + ' heading?: string;', + ' /** @internal */', + ' debugName?: string;', + '}', + 'export function Panel(props: PanelProps) { return null; }', + ].join('\n'), + {props: [prop('title', 'string'), prop('id', 'number')]}, + ); + + // A prop a mapped type builds is the package's own when its key traces + // to the package's source: through aliases, `Exclude`, a generic's + // arguments, and a component `memo` or `forwardRef` wraps. A key another + // package defines, behind an alias or not, is that package's, and one + // the check can't trace is listed, not asked for. + const mapped = Object.entries({ + Styled: + '(props: {[K in StyleKey]?: BaseProps[K]} & Partial>)', + Inlined: '(props: {[K in keyof BaseProps]?: BaseProps[K]})', + Spread: '(props: Responsive)', + Memoed: ' = memo(Inner)', + Forwarded: ' = forwardRef(Impl)', + Excluded: "(props: {[K in Exclude<'a' | 'b' | 'c', 'c'>]?: string})", + Templated: "(props: {[K in `on${Capitalize<'open'>}`]?: () => void})", + // Another package's keys through an own interface's property, an + // `infer`, or a constant bound to its value stay that package's. + Indexed: "(props: {[K in Opts['size']]?: number})", + Inferred: '(props: {[K in KeysOf>]?: number})', + Rebound: '(props: {[K in (typeof SIZES)[number]]?: number})', + Inline: "(props: {[K in 'a' | 'b']?: string})", + Recorded: "(props: Partial>)", + Remapped: "(props: {[K in 'open' as `on${Capitalize}`]?: () => void})", + Toned: + "(props: Partial> & {[K in Tone]?: string} & Partial> & {[K in keyof BaseProps as `x${Capitalize}`]?: boolean})", + Flagged: "(props: Flags<'bold'>)", + Extended: '(props: ExtendedProps)', + Sized: '(props: Spaced & Partial)', + }).map(([name, props]) => + component( + name, + [ + "import {type BaseProps, type Sizes, type Spacing, type Tone, type Flags, type FC, type Responsive, type Breakpoint, BREAKPOINTS, memo, forwardRef} from '@acme/base';", + "interface ExtendedProps extends Partial> {}", + 'interface Spaced extends Spacing {}', + 'type StyleKey = keyof BaseProps;', + 'interface Opts { size: Breakpoint }', + 'type KeysOf = T extends Record ? K : never;', + 'const SIZES = BREAKPOINTS;', + "function Inner(props: {[K in 'top' | 'bottom']?: number}) { return null; }", + "function Impl(props: Partial>, ref: unknown) { return null; }", + props.startsWith(' =') + ? `export const ${name}${props};` + : `export function ${name}${props} { return null; }`, + `export const Typed: FC>> = () => null;`, + ].join('\n'), + {props: []}, + ), + ); + + // A deprecated prop is still a prop the component accepts. + const typed = {...mapped[0], name: 'Typed'}; + const report = await compare([panel, ...mapped, typed]); + expect(kinds(report)).toEqual([ + 'Excluded.a not_in_doc', + 'Excluded.b not_in_doc', + 'Extended.a not_in_doc', + 'Flagged.bold not_in_doc', + 'Forwarded.left not_in_doc', + 'Inline.a not_in_doc', + 'Inline.b not_in_doc', + 'Memoed.bottom not_in_doc', + 'Memoed.top not_in_doc', + 'Panel.heading not_in_doc', + 'Recorded.a not_in_doc', + 'Recorded.b not_in_doc', + 'Remapped.onOpen not_in_doc', + 'Toned.xl not_in_doc', + 'Typed.a not_in_doc', + ]); + expect(report.propsNotChecked).toEqual([ + expect.objectContaining({component: 'Templated', props: ['onOpen']}), + expect.objectContaining({component: 'Inferred', props: ['lg', 'sm']}), + ]); + // The alias and the inline form give the same answer: nothing asked. + for (const name of ['Styled', 'Inlined']) { + expect(kinds(report).filter(kind => kind.startsWith(name))).toEqual([]); + } }); - it('reads every overload, one without props included, reads a prop only some members of a union require as optional, and holds every prop one entry names to its `required`', async () => { + it('reads every overload, one without props included, matches a doc that describes one member of a discriminated union, reads a prop only some members require as optional, and holds every prop one entry names to its `required`', async () => { withTypeScript(); const picker = component( 'Picker', @@ -1169,8 +2189,10 @@ export const Hoc = withName((props: {label?: string}) => null);`; status: 'skipped', reason, components: 0, + typesCompared: 0, findings: [], unchecked: [], + typesNotCompared: [], }); expect(await compare([badge])).toEqual(skipped(TYPESCRIPT_MISSING_REASON)); diff --git a/packages/cli/foundation/response/response-types.doc.mjs b/packages/cli/foundation/response/response-types.doc.mjs index 6b0b5816bb56e..edfab0dff70e1 100644 --- a/packages/cli/foundation/response/response-types.doc.mjs +++ b/packages/cli/foundation/response/response-types.doc.mjs @@ -342,7 +342,7 @@ export const doc = { { value: 'integration.component-conflicts', description: - 'validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked | skipped), reason, typescript, components, findings (all warnings), unchecked, and propsNotChecked.', + 'validated (false when no integration manifest was found, so nothing was inspected), the integration identity, structural issues, and non-blocking conflicts where an integration component name is also owned by Core; each conflict includes the exact package-qualified command. When the manifest loads and declares a components root, propDrift compares documented props with the TypeScript props: status (checked | skipped), reason, typescript, components, typesCompared, typesSkipped, findings (all warnings), unchecked, typesNotCompared, and propsNotChecked.', }, { value: 'integration.doc-conflicts', diff --git a/packages/cli/test/core-prop-docs.known-drift.json b/packages/cli/test/core-prop-docs.known-drift.json index 7c583c324035f..48cbe1ccb472d 100644 --- a/packages/cli/test/core-prop-docs.known-drift.json +++ b/packages/cli/test/core-prop-docs.known-drift.json @@ -35,8 +35,10 @@ "src/Field/FieldLabel.doc.mjs FieldLabel.descriptionID not_in_doc", "src/Field/FieldLabel.doc.mjs FieldLabel.isGroupLabel not_in_doc", "src/Field/FieldLabel.doc.mjs FieldLabel.labelID not_in_doc", + "src/FieldStatus/FieldStatus.doc.mjs FieldStatus.variant type_mismatch", "src/Grid/Grid.doc.mjs Grid.rowHeight not_in_doc", "src/HoverCard/HoverCard.doc.mjs HoverCard.children required_mismatch", + "src/Icon/Icon.doc.mjs Icon.color type_mismatch", "src/IconButton/IconButton.doc.mjs IconButton.as not_in_doc", "src/IconButton/IconButton.doc.mjs IconButton.form not_in_doc", "src/IconButton/IconButton.doc.mjs IconButton.href not_in_doc", @@ -56,6 +58,7 @@ "src/MoreMenu/MoreMenu.doc.mjs MoreMenu.isMenuOpen not_in_doc", "src/MultiSelector/MultiSelector.doc.mjs MultiSelector.labelTooltip not_in_doc", "src/PowerSearch/PowerSearch.doc.mjs PowerSearch.components not_in_doc", + "src/PowerSearch/PowerSearch.doc.mjs PowerSearch.statusVariant type_mismatch", "src/Resizable/Resizable.doc.mjs ResizeHandle.position not_in_doc", "src/Resizable/Resizable.doc.mjs ResizeHandle.resizable required_mismatch", "src/Selector/Selector.doc.mjs Selector.changeAction not_in_doc", @@ -79,6 +82,7 @@ "src/Table/TableSelectionToolbar.doc.mjs TableSelectionToolbar.dividers not_in_doc", "src/Table/TableSelectionToolbar.doc.mjs TableSelectionToolbar.gap not_in_doc", "src/Table/TableSelectionToolbar.doc.mjs TableSelectionToolbar.orientation not_in_doc", + "src/Text/Text.doc.mjs Text.as type_mismatch", "src/TimeInput/TimeInput.doc.mjs TimeInput.containerClassName not_in_doc", "src/TimeInput/TimeInput.doc.mjs TimeInput.containerStyle not_in_doc", "src/TimeInput/TimeInput.doc.mjs TimeInput.controlClassName not_in_doc", From a566ea323a516ceab1164df37e6ff844f238f966 Mon Sep 17 00:00:00 2001 From: Joey Farina Date: Sun, 18 Oct 2026 12:33:32 -0700 Subject: [PATCH 2/2] docs(cli): say which documented prop types the props check judges The props-table guide adds `type_mismatch` to its findings, and a section on writing a `type` the check can judge: as the source writes it (exactly, if it writes `readonly`), or built from exact parts; a mutable array type for a readonly one, which matches because callers can pass it; every variant's type for a field the prop's variants type differently; `React.X` as React's own `X`; a type named as the file that declares the prop names it; a union member another covers; an alias with its values; `undefined` on an optional prop; and what is listed as not compared, by reason. --- .../assets/docs/tree/check-the-props.doc.mjs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/packages/cli/assets/docs/tree/check-the-props.doc.mjs b/packages/cli/assets/docs/tree/check-the-props.doc.mjs index eba3f819220d9..35fa7c4937417 100644 --- a/packages/cli/assets/docs/tree/check-the-props.doc.mjs +++ b/packages/cli/assets/docs/tree/check-the-props.doc.mjs @@ -91,8 +91,59 @@ docType: boolean '`required` disagrees with the code', 'Set `required: true` exactly when every caller must pass the prop. One your `defaultProps` fills counts as optional, as it is to JSX callers, and so does one only some members of a union require, since a caller can leave it out: say in its description which variants need it. An entry that names several props (`value / onChange`) makes the same claim about each: give them separate entries when they differ', ], + [ + '`type_mismatch`', + "The documented `type` differs from the code's, where both read exactly", + 'Copy the type from the source, or fix the code. The next section says which types are judged', + ], + ], + }, + ], + }, + { + id: 'write-types-the-check-can-judge', + title: 'Write types the check can judge', + content: [ + { + type: 'prose', + text: 'For a prop your package declares, the check also compares the documented `type`, but only where both sides read exactly. It never guesses, so a type it cannot judge is listed, not reported. It needs TypeScript 4.8 or later: with an older one it prints `[skip] Prop types not judged` and judges none.', + }, + { + type: 'code', + lang: 'text', + code: `severity: [warn] +component: AcmeCarousel +prop: interval +kind: type_mismatch +message: "interval" is documented as string, but the code declares number. +doc: components/AcmeCarousel.doc.mjs +source: components/AcmeCarousel.tsx +docType: string +codeType: number`, + }, + { + type: 'list', + style: 'unordered', + items: [ + "Write the type as the source writes it, such as `ReactNode[]` or `(index: number) => void`. Parameter names, member order, quotes and whitespace may differ. Text that writes `readonly` must match exactly, parameter names included; only whitespace may differ. `React.X` names React's own `X` where the prop is declared, so it matches a bare `X` only where that name is React's: `React.KeyboardEvent` is not the DOM's `KeyboardEvent`.", + "Name a type as the file that declares the prop names it, so an import renamed there (`Item as BaseItem`) is written as that file writes it. A field (`cfg.level`) takes its prop's file, even when the field's own type is declared in another file.", + 'A type written another way is judged only when both sides are built from exact parts: primitives such as `string` and `number`, string, number and boolean literals, string enum members, arrays and unions of these, and type names that stand for them. Union members may come in any order, and a member another covers adds nothing, as TypeScript passes it there: `undefined` beside `void`, or `string[]` beside `(string | number)[]`.', + 'A mutable array type matches a readonly one, because callers can pass it. A readonly one documented for a mutable array is a mismatch.', + "When a prop's variants give a field different types, as `{value: string} | {value: number}` does, document every variant's type (`string | number`). The types of only some variants are listed: the prop's variants give this field different types. With more than six variant types, any other type is listed too.", + "To name an alias and show its values, write `AcmeSize = 'sm' | 'md'` or `AcmeSize ('sm' | 'md')`. Both parts must match.", + "An optional prop's `undefined` is set aside on both sides. With `exactOptionalPropertyTypes`, the doc shows `undefined` exactly when the code declares it.", + 'Anything else, such as an object, a tuple, a generic with arguments, `keyof`, an intersection, or a mapped or conditional type written another way, is not compared.', ], }, + { + type: 'prose', + text: "The check prints how many documented types it judged, then lists the rest by reason: not a form the check judges exactly, a field the prop's variants give different types, a standard prop, or a prop declared outside the package. With `--json`, they are in `data.propDrift.typesNotCompared`.", + }, + { + type: 'code', + lang: 'text', + code: '[info] Documented prop types judged: 2; not compared: 0. A type is judged when the doc writes it the same as the source (exactly, if it contains `readonly`), or when both are primitives, literals, arrays and unions of those, or names for them.', + }, ], }, {