diff --git a/.changeset/check-scope-and-summary.md b/.changeset/check-scope-and-summary.md new file mode 100644 index 00000000..aafd7202 --- /dev/null +++ b/.changeset/check-scope-and-summary.md @@ -0,0 +1,11 @@ +--- +'onsystem': minor +--- + +`check` and `check_ui` read only JS and TS files. A CSS, Markdown or JSON file named outright or matched by a glob, as `check $(git diff --name-only)`, lint-staged's `"*"` or `check "**/*"` passes them, was parsed as TSX and gave hundreds of syntax errors (209 for a shadcn/ui `app/globals.css`), so the run failed on files with no UI code in them. `check` now skips such a file and counts it in the summary line (`3 files that are not JS or TS skipped`). `check_ui` answers with an error for a `path` or `filename` that is not `.tsx`, `.jsx`, `.ts` or `.js` (or `.mts`, `.cts`, `.mjs`, `.cjs`): a `.vue` or `.css` file was checked as TSX before. + +`--require-design-system` also fails (exit code 2) when the folders and globs it is given leave no file to check, so CI cannot pass by checking nothing: a JavaScript app whose pages are `.js` files, a job that runs in a design-system package, whose files are all its own components, or a glob that matches only styles. Files named outright that are not JS or TS still pass with nothing to check. This can fail a job that passed before. + +Folders still stand for their `.tsx` and `.jsx` files, and `check` now names the `.js` files in them that have JSX (Next.js's JavaScript template writes `app/page.js`) with the command that checks them, such as `check . "**/*.js"`, in place of "no files match" for a folder of `.js` pages, and as the first line of the `--require-design-system` failure. In the GitHub Action, `paths: . **/*.js` checks them. + +A run with 50 findings or more ends, in the default output, with the count per rule and the ten Tailwind palette classes it found most, each with the token to use. `--format json`, `sarif` and `github` print the findings alone, as before. diff --git a/action.yml b/action.yml index 79ccad09..9ad4c153 100644 --- a/action.yml +++ b/action.yml @@ -7,7 +7,7 @@ branding: inputs: paths: - description: Files, folders or globs to check, relative to working-directory, separated by spaces or new lines. + description: Files, folders or globs to check, relative to working-directory, separated by spaces or new lines, without quotes. A folder stands for its .tsx and .jsx files, so a JavaScript app lists its .js pages too, as in `. **/*.js`. default: . working-directory: description: Where to run the check, such as one app of a monorepo. The config and the default baseline are read from there. @@ -22,7 +22,7 @@ inputs: description: On pull requests and merge groups, report only findings on the lines the change touches. false reports every finding in the checked files. default: 'true' require-design-system: - description: Fail when no components or no color tokens are found, so the check cannot pass by checking nothing. An app on a component library such as MUI passes with its components alone. + description: Fail when no components or no color tokens are found, or when paths leaves no file to check, so the check cannot pass by checking nothing. An app on a component library such as MUI passes with its components alone. default: 'true' outputs: diff --git a/docs/ci.md b/docs/ci.md index 58caa071..dce7eaab 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -2,7 +2,9 @@ The check that runs after each edit in Claude Code also gates every pull request, whoever or whatever wrote it. `onsystem check` runs the same rules as `check_ui` and exits 1 on errors (or on more than `--max-warnings` warnings). It skips the design system's own component files, the ones `components` matches: they implement the scale and the primitives the rules enforce, so a fresh shadcn/ui project's `p-[3px]` is not a finding. `--include-design-system`, or `"includeDesignSystem": true` in the config, lints them too. -In folders and globs, `check` also leaves out tests and stories (`*.test.tsx`, `*.spec.tsx`, `*.stories.tsx`, and whatever is in `__tests__`, `__stories__` and `__mocks__` folders; `--include-tests` checks them), or what the config's `exclude` lists in their place ([a list replaces them](configuration.md#config-file)), and what git ignores (through `git check-ignore`, so nested `.gitignore` files count); a file named outright is always checked. A file that renders an image or an email is not checked, since tokens and classes don't apply there: one that imports `next/og`, `@vercel/og`, `satori` or `@react-email/*`, a Next.js `opengraph-image`, `twitter-image`, `icon` or `apple-icon` route, or a file in an `og` folder. The summary line counts what was left out. Code installed from a shadcn registry is checked as your own until the config sets it apart: [code from shadcn registries](configuration.md#code-from-shadcn-registries). +In folders and globs, `check` also leaves out tests and stories (`*.test.tsx`, `*.spec.tsx`, `*.stories.tsx`, and whatever is in `__tests__`, `__stories__` and `__mocks__` folders; `--include-tests` checks them), or what the config's `exclude` lists in their place ([a list replaces them](configuration.md#config-file)), and what git ignores (through `git check-ignore`, so nested `.gitignore` files count); a file named outright is always checked. Only JS and TS files are (`.tsx`, `.jsx`, `.ts`, `.js` and their `.mts`, `.cts`, `.mjs` and `.cjs` forms): the CSS, Markdown and JSON that `check $(git diff --name-only)` names or `check "**/*"` matches are skipped, as `check_ui` refuses them. A file that renders an image or an email is not checked, since tokens and classes don't apply there: one that imports `next/og`, `@vercel/og`, `satori` or `@react-email/*`, a Next.js `opengraph-image`, `twitter-image`, `icon` or `apple-icon` route, or a file in an `og` folder. The summary line counts what was left out. Code installed from a shadcn registry is checked as your own until the config sets it apart: [code from shadcn registries](configuration.md#code-from-shadcn-registries). + +A folder stands for the `.tsx` and `.jsx` files in it. A JavaScript app can write its JSX in `.js` files (Next.js's JavaScript template has `app/page.js`): `check` names those it left out and gives the command that checks them, such as `check . "**/*.js"`. A `.js` file over 512 KB or named `*.min.js` is taken for a bundle and not looked at. With the [GitHub Action](required-check.md), list the glob in `paths` without quotes, `paths: . **/*.js`: the action passes each path as it is, with no shell. Pin it as a dev dependency, so CI, the plugin's hook and everyone on the team run the same version: @@ -32,7 +34,7 @@ Or use the GitHub Action, `uses: dgesteves/onsystem@v0`, which runs the same che Install the dependencies first: without them, the props of components that wrap a library (Radix, React Aria) are unknown, and `no-unknown-prop` skips those components ([troubleshooting](troubleshooting.md#components-show-0-props)). `--format github` prints workflow commands, so findings show up as annotations on the pull request; their paths are relative to the repository root, so a job that runs in a folder (`working-directory: apps/web`) annotates the right files. `--format json` prints the raw results, and `--format sarif` a SARIF log for [code scanning](#code-scanning-and-other-sarif-tools). From a [monorepo root](configuration.md#monorepo-roots), `check .` checks each file against its own project's design system and keeps one baseline at the root; there, `--require-design-system` fails only when no workspace package has components and color tokens (or a component library such as MUI), so a job per app (`working-directory: apps/web`) is the stricter gate. -When no components or no color tokens are found, `check` and `check_ui` say which rules could not run and link to the [configuration docs](configuration.md), so a clean result is not mistaken for a checked one. In CI, `--require-design-system` turns that into a failure (exit code 2), for when the design system moves and the globs stop matching. An app whose components come from a [published library](configuration.md#published-libraries-mui) (MUI) has a design system even when its theme's colors cannot be read, as with a palette from Sass variables: the check runs and says that `no-hardcoded-color` did not. +When no components or no color tokens are found, `check` and `check_ui` say which rules could not run and link to the [configuration docs](configuration.md), so a clean result is not mistaken for a checked one. In CI, `--require-design-system` turns that into a failure (exit code 2), for when the design system moves and the globs stop matching. It also fails when the folders and globs it is given leave no file to check, as in a JavaScript app whose pages are `.js` files, in a design-system package whose files are all its own components, or with a glob that matches only styles. Files named outright that are not JS or TS leave nothing to check without failing: a pull request that changes only styles has no UI code to check. An app whose components come from a [published library](configuration.md#published-libraries-mui) (MUI) has a design system even when its theme's colors cannot be read, as with a palette from Sass variables: the check runs and says that `no-hardcoded-color` did not. `check` and `check_ui` also say when the dependencies are not installed (no `node_modules`), since `no-unknown-prop` then skips the components whose props come from them, and plain `inspect` says it on stderr; `--require-design-system` does not fail on it. When a project without components of its own depends on a component library onsystem does not read yet (Chakra UI, Mantine, HeroUI, Radix Themes or Ant Design), they say that the library is not read, and `inspect` lists it on its `library` line. @@ -80,3 +82,16 @@ npx -y onsystem check src --update-baseline From then on, `check` reads `onsystem.baseline.json` from the root whenever it exists and fails only on new findings: `No new problems in 569 files (1,340 in the baseline).` Entries are keyed by file, rule and the offending text with a count, not by line, so edits elsewhere in a file do not invalidate them, while a second `bg-[#f7f7f7]` where the baseline accepts one is reported. When findings get fixed, `check` says so and prints the command that drops them, `check src --prune-baseline`, which locks in the progress: it only removes entries, while `--update-baseline` would also accept any new finding. Entries of a rule you turn off are kept rather than reported as fixed, a malformed baseline (a bad merge, say) is an error rather than something to overwrite, and paths are matched by their real spelling, so `APP/` on macOS or a linked checkout finds the same entries. `--ignore-baseline` shows everything, and `--baseline ` uses another path, creating its folder; the commands `check` prints repeat it, with `--root`, `--config` and the other options that decide what is found. The baseline applies to the CLI only: `check_ui` still shows an agent every finding in the file it is editing. The baseline also records the onsystem version that wrote it and the rules that ran. When a rule that did not run then reports findings, as when an upgrade finds tokens an older version missed, `check` says they are new to the baseline rather than to the code, and how many other new findings `--update-baseline` would accept with them. A baseline written before these fields existed is read as before, without that line, until an update from the root that checks every file it lists, such as `check . --update-baseline`, records them: an update of one folder cannot tell which rules ran over the rest. Give the baseline to code owners ([CODEOWNERS](rollout.md#record-the-baseline)), so that a pull request that accepts findings by rewriting it needs their review. + +Hundreds of findings scroll by faster than they can be read. A run with 50 findings or more ends with what they add up to: the count per rule, and the Tailwind palette classes it found most, each with the token to use, marked `(nearest token)` when that token is another color, which reads as a plan for moving to the tokens: + +```text +By rule: no-hardcoded-color 48, prefer-design-system-component 12. +Tailwind palette classes: 48 in 12 files. The most common, with the token to use: + 12× border-neutral-200 → border-border + 12× hover:border-neutral-200 → hover:border-border + 12× text-neutral-500 → text-muted-foreground + 12× text-neutral-800 → text-primary (nearest token) +``` + +`--format json`, `sarif` and `github` print the findings alone. diff --git a/docs/required-check.md b/docs/required-check.md index ea56be41..ef4c8b23 100644 --- a/docs/required-check.md +++ b/docs/required-check.md @@ -48,16 +48,16 @@ Install the dependencies first, as above: without them the props of components t `@v0` follows the latest 0.x release, and each release of the action runs the CLI of the same version (`npx onsystem@`). To stay on one release, pin a commit SHA. -| Input | Default | What it does | -| ----------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- | -| `paths` | `.` | Files, folders or globs to check, relative to `working-directory`, separated by spaces or new lines | -| `working-directory` | `.` | Where to run the check, such as one app of a monorepo; its config and baseline are used | -| `baseline` | | Baseline file, relative to `working-directory`; by default `onsystem.baseline.json` in the project root, whenever it exists | -| `fail-on` | `error` | `error`, `warning` (errors or warnings) or `none` | -| `only-changed-lines` | `true` | On pull requests and merge groups, report only findings on the changed lines; `false` reports every finding in the checked files | -| `require-design-system` | `true` | Fail when no components or no color tokens are found, so the check cannot pass by checking nothing ([CI and baselines](ci.md)) | - -An app on a [published library](configuration.md#published-libraries-mui) (MUI) passes `require-design-system` with the library's components alone; when its theme's colors cannot be read, the step's log says that `no-hardcoded-color` did not run. The outputs `errors`, `warnings` and `outside-diff` count what was reported and what was left out. In a monorepo, add a job per app with `working-directory: apps/web`, each with its own design system and baseline. +| Input | Default | What it does | +| ----------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `paths` | `.` | Files, folders or globs to check, relative to `working-directory`, separated by spaces or new lines, without quotes | +| `working-directory` | `.` | Where to run the check, such as one app of a monorepo; its config and baseline are used | +| `baseline` | | Baseline file, relative to `working-directory`; by default `onsystem.baseline.json` in the project root, whenever it exists | +| `fail-on` | `error` | `error`, `warning` (errors or warnings) or `none` | +| `only-changed-lines` | `true` | On pull requests and merge groups, report only findings on the changed lines; `false` reports every finding in the checked files | +| `require-design-system` | `true` | Fail when no components or no color tokens are found, or when `paths` leaves no file to check, so the check cannot pass by checking nothing ([CI and baselines](ci.md)) | + +A folder in `paths` stands for its `.tsx` and `.jsx` files. A JavaScript app whose pages are `.js` files (Next.js's JavaScript template) checks them with `paths: . **/*.js`, with no quotes, since the action passes each path as it is. With `paths: .` alone the step's log names them, and `require-design-system` fails when that leaves nothing to check. An app on a [published library](configuration.md#published-libraries-mui) (MUI) passes `require-design-system` with the library's components alone; when its theme's colors cannot be read, the step's log says that `no-hardcoded-color` did not run. The outputs `errors`, `warnings` and `outside-diff` count what was reported and what was left out. In a monorepo, add a job per app with `working-directory: apps/web`, each with its own design system and baseline. ## Make the check required diff --git a/src/cli.ts b/src/cli.ts index 44500535..2c9f9a6a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -7,6 +7,7 @@ import { parseArgs } from 'node:util'; import type { Transport } from '@modelcontextprotocol/server'; import { escapePath, glob, type GlobOptions } from 'tinyglobby'; +import ts from 'typescript'; import { applyBaseline, @@ -39,7 +40,7 @@ import { serveStdio } from './server/stdio.js'; import { toDesignMd, toDtcg } from './tokens/export.js'; import type { CheckResult, DesignSystemModel, Diagnostic } from './types.js'; import { stderrLogger, silentLogger } from './util/log.js'; -import { isInside, relativePath, toPosix } from './util/paths.js'; +import { isInside, relativePath, SCRIPT_FILE, toPosix } from './util/paths.js'; import { plural, unique } from './util/strings.js'; import { NAME, VERSION } from './version.js'; import { loadTarget, type Project, type Workspace } from './workspace.js'; @@ -104,7 +105,8 @@ Options many projects --require-design-system check: exit 2 when no components or no color tokens - are found, so CI cannot pass by checking nothing + are found, or when the folders and globs given leave + no file to check, so CI cannot pass by checking nothing --explain inspect: every candidate zero config looked at and why it was taken or not, the resolved config, unresolved tokens, and components whose props did not resolve @@ -310,6 +312,22 @@ interface Selection { /** Design-system files left out, for the hint when nothing is left. */ skipped: number; testsLeftOut: number; + /** `.js` files under the folders given that have JSX, which folders leave out. */ + jsx: string[]; + /** The globs that name those `.js` files, for the command that checks them. */ + jsxGlobs: string[]; + /** Whether a folder or glob was given, rather than only files named outright. */ + searched: boolean; +} + +/** + * No file matched the patterns. The `.js` files with JSX the folders hold, which folders leave + * out, say why when there are any: `check app` in a JavaScript app. + */ +interface NoMatch { + noMatch: true; + jsx: string[]; + jsxGlobs: string[]; } async function check( @@ -351,17 +369,56 @@ async function check( ? await selectInWorkspace(target.workspace, patterns, values, format, io) : await selectInProject(target.config, patterns, values, format, io); if (typeof selection === 'number') return selection; + // The hints repeat the options that decide the findings and the baseline, so the command they + // suggest rewrites the same file from the same findings. + const command = (flag: string) => checkCommand(patterns, configOptions, values, flag); + if ('noMatch' in selection) { + const jsx = jsxNotice(selection, io.cwd, command); + io.stderr( + jsx + ? `check: nothing to check in ${patterns.join(' ')}. ${jsx}` + : `check: no files match ${patterns.join(' ')}`, + ); + return 2; + } const { files, notes } = selection; + // What the run could not do or left out, after the findings: on stdout, as a workflow warning + // with `--format github`, and on stderr when stdout is JSON or SARIF. + const notify = (text: string) => { + if (format === 'pretty') io.stdout(text); + else if (format === 'github') io.stdout(`::warning title=${NAME}::${text}`); + else io.stderr(text); + }; + const jsx = jsxNotice(selection, io.cwd, command); if (!files.length) { + // `.js` files with JSX are what is left to check: --include-design-system or + // --include-tests would check something else and pass with the pages still unchecked. + const hints = jsx + ? [jsx] + : [ + selection.skipped + ? `Pass --include-design-system to check ${selection.skipped === 1 ? 'it' : 'them'}.` + : '', + selection.testsLeftOut ? 'Pass --include-tests to check tests and stories.' : '', + ].filter(Boolean); + // Folders and globs that leave nothing to check point at the wrong code, or at code `check` + // does not read: CI must not pass on them. Files named outright that are not JS or TS (a + // pull request that changes only styles) have nothing to check. One line, as the GitHub + // Action's error annotation shows only the first. + if (values['require-design-system'] && selection.searched) { + io.stderr( + `check --require-design-system: nothing was checked${notes.length ? ` (${notes.join('; ')})` : ''}. ${hints.join(' ')}`.trimEnd(), + ); + return 2; + } if (format === 'pretty') { - const hints = [ - selection.skipped - ? `Pass --include-design-system to check ${selection.skipped === 1 ? 'it' : 'them'}.` - : '', - selection.testsLeftOut ? 'Pass --include-tests to check tests and stories.' : '', - ].filter(Boolean); - io.stdout(`Nothing to check: ${notes.join('; ')}. ${hints.join(' ')}`.trimEnd()); - } else printNoResults(format, io); + io.stdout( + `Nothing to check${notes.length ? `: ${notes.join('; ')}` : ''}. ${hints.join(' ')}`.trimEnd(), + ); + } else { + printNoResults(format, io); + if (jsx) notify(jsx); + } return 0; } @@ -467,12 +524,10 @@ async function check( else io.stderr(summary); for (const text of selection.notices) io.stderr(text); if (unchecked) io.stderr(unchecked); + if (jsx) io.stderr(jsx); return 0; } - // The hints repeat the options that decide the findings and the baseline, so the command they - // suggest rewrites the same file from the same findings. - const command = (flag: string) => checkCommand(patterns, configOptions, values, flag); const baselineNotes: string[] = []; if (baseline) { const fresh = results.flatMap((r) => r.diagnostics); @@ -525,11 +580,8 @@ async function check( else if (format === 'github') io.stdout(`::notice title=${NAME} baseline::${escapeData(text)}`); else io.stderr(text); } - for (const text of [...selection.notices, unchecked]) { - if (!text) continue; - if (format === 'pretty') io.stdout(text); - else if (format === 'github') io.stdout(`::warning title=${NAME}::${text}`); - else io.stderr(text); + for (const text of [...selection.notices, unchecked, jsx]) { + if (text) notify(text); } const errors = results.reduce((n, r) => n + r.errorCount, 0); @@ -578,12 +630,20 @@ function repositoryPath(cwd: string, env: NonNullable): (file: string }; } -/** Splits the patterns into files named outright and globs (a folder: every TSX/JSX file under it). */ -function expandPatterns(patterns: string[], cwd: string): { named: string[]; globs: string[] } { +/** + * Splits the patterns into files named outright and globs (a folder: every TSX/JSX file under + * it). `scripts` globs the `.js` files under the folders, which are not checked, to say which + * of them have JSX. + */ +function expandPatterns( + patterns: string[], + cwd: string, +): { named: string[]; globs: string[]; scripts: string[] } { // An existing path is taken literally, so `app/(marketing)` and `[slug]` are not // glob syntax. A directory means every TSX/JSX file under it. const named: string[] = []; const globs: string[] = []; + const scripts: string[] = []; for (const pattern of patterns) { const stat = fs.statSync(path.resolve(cwd, pattern), { throwIfNoEntry: false }); if (stat && !stat.isDirectory()) { @@ -591,9 +651,122 @@ function expandPatterns(patterns: string[], cwd: string): { named: string[]; glo continue; } const literal = stat ? escapePath(toPosix(path.join(pattern, '.'))) : toPosix(pattern); - globs.push(!stat ? literal : literal === '.' ? '**/*.{tsx,jsx}' : `${literal}/**/*.{tsx,jsx}`); + if (!stat) { + globs.push(literal); + continue; + } + const under = literal === '.' ? '**/*' : `${literal}/**/*`; + globs.push(`${under}.{tsx,jsx}`); + scripts.push(`${under}.js`); + } + return { named, globs, scripts }; +} + +/** A `.js` file larger than this is a bundle or generated, not a page: not read for JSX. */ +const MAX_PAGE_BYTES = 512 * 1024; + +/** + * `.js` files the folders hold that have JSX, which are not checked: folders expand to TSX and + * JSX files, and a JavaScript app writes its pages as `.js` (Next.js's JavaScript template has + * `app/page.js`). Files the run checks anyway (a glob names them), what git ignores and what + * `leftOut` says (the config's `exclude`, the design system's own files) are not counted, nor + * are minified files and files over 512 KB, which are bundles rather than pages. + */ +async function jsxLeftOut( + scripts: string[], + taken: readonly string[], + cwd: string, + leftOut: (file: string) => boolean | Promise, +): Promise { + if (!scripts.length) return []; + const checked = new Set(taken.map(realPath)); + const found = oncePerFile( + ( + await findFiles(scripts, { + cwd, + absolute: true, + expandDirectories: false, + ignore: ['**/node_modules/**', '**/*.min.js'], + }) + ).filter((file) => !checked.has(realPath(file))), + ).sort(); + const ignored = gitIgnored(cwd, found); + const files: string[] = []; + for (const file of found) { + if (ignored.has(file) || (await leftOut(file))) continue; + // Only a notice depends on it: a file that cannot be read is left out, not a failed run. + let code: string; + try { + if (fs.statSync(file).size > MAX_PAGE_BYTES) continue; + code = fs.readFileSync(file, 'utf8'); + } catch { + continue; + } + if (hasJsx(code)) files.push(file); } - return { named, globs }; + return files; +} + +/** + * Where a JSX element can start: after `(`, `,`, `=`, `=>`, `?` (not a regex's `(?`), `:`, + * `[`, `{`, `&`, `|`, `return`, `yield`, `await` or `default`, with spaces and short comments + * between. Plain JavaScript has a `<` and a name there only in strings and comments. + */ +const JSX_START = + /(?:(?|[(,=:[{&|]|\b(?:return|yield|await|default))(?:\s|\/\/[^\n]{0,200}\n|\/\*(?:[^*]|\*(?!\/)){0,500}\*\/)*<([A-Za-z][\w.:-]*|>(?=[\s<{]))/g; + +/** + * Whether the code may have JSX: an element starts where only JSX can start one, and it is a + * fragment, its closing tag is there, or a `/>` follows it. A library bundle whose strings hold + * HTML rarely passes, so most `.js` files are never parsed. + */ +function mayHaveJsx(code: string): boolean { + for (const match of code.matchAll(JSX_START)) { + const name = match[1] ?? ''; + if (name === '>' || code.includes(``)) return true; + const from = match.index + match[0].length; + const close = code.indexOf('/>', from); + if (close >= 0 && close - from < 2000) return true; + } + return false; +} + +/** Whether the code has a JSX element or fragment. */ +function hasJsx(code: string): boolean { + if (!code.includes('/>') && !code.includes(' + ts.isJsxElement(node) || + ts.isJsxSelfClosingElement(node) || + ts.isJsxFragment(node) || + ts.forEachChild(node, visit) === true; + try { + return visit(source); + } catch { + // Nested too deeply to walk: not a page anyone wrote. + return false; + } +} + +/** Which `.js` files with JSX the folders left out, and the command that checks them. */ +function jsxNotice( + selection: Pick, + cwd: string, + command: (flag: string) => string, +): string | undefined { + const files = selection.jsx.map((file) => relativePath(cwd, file)); + if (!files.length) return undefined; + const check = command(selection.jsxGlobs.map(shellQuote).join(' ')); + return files.length === 1 + ? `${files[0] ?? ''} has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: \`${check}\`.` + : `${files.length.toLocaleString('en-US')} .js files have JSX (${listOf(files)}), but folders are checked for .tsx and .jsx files, so they were not checked. Name them to check them: \`${check}\`.`; } /** One project: the root's config and design system for every file. */ @@ -603,19 +776,29 @@ async function selectInProject( values: CheckValues, format: OutputFormat, io: Io, -): Promise { +): Promise { const ds = await loadDesignSystem(config, { cache: values.cache, logger: silentLogger }); // Folders and globs leave out what the config excludes (tests and stories by default) and - // what git ignores. A file named outright is checked, unless --skip-excluded. - const { named, globs } = expandPatterns(patterns, io.cwd); + // what git ignores. A file named outright is checked, unless --skip-excluded. Only JS and TS + // files are: `check $(git diff --name-only)` and `check "**/*"` name styles and docs too. + const { named: given, globs, scripts } = expandPatterns(patterns, io.cwd); const options = { cwd: io.cwd, absolute: true, expandDirectories: false }; - const all = globs.length + const globbed = globs.length ? await findFiles(globs, { ...options, ignore: ['**/node_modules/**'] }) : []; + const named = given.filter((file) => SCRIPT_FILE.test(file)); + const all = globbed.filter((file) => SCRIPT_FILE.test(file)); const excluded = (file: string) => excludedBy(config, relativePath(config.root, file), { includeTests: values['include-tests'], }) !== undefined; + // The design system's own files implement the scale and the primitives the + // rules enforce (shadcn/ui's `p-[3px]`), so `check .` leaves them alone. + const ownFiles = async () => + new Set(config.includeDesignSystem ? [] : (await componentFiles(config)).map(realPath)); + // `.js` files with JSX in the folders, left out as their `.tsx` files would be. + const jsxIn = (taken: readonly string[], own: Set) => + jsxLeftOut(scripts, taken, io.cwd, (file) => excluded(file) || own.has(realPath(file))); const skip = values['skip-excluded']; const leftOut = unique([...(skip ? named : []), ...all]).filter(excluded); const leftOutSet = new Set(leftOut); @@ -625,14 +808,15 @@ async function selectInProject( tests: leftOut.filter((file) => TEST_FILE.test(file)).length, excluded: leftOut.filter((file) => !TEST_FILE.test(file)).length, gitignored: ignored.size, + notScripts: notScripts(given, globbed), }; let files = oncePerFile([ ...(skip ? named.filter((file) => !leftOutSet.has(file)) : named), ...kept.filter((file) => !ignored.has(file)), ]).sort(); - if (!files.length && !all.length && !leftOut.length) { - io.stderr(`check: no files match ${patterns.join(' ')}`); - return 2; + if (!files.length && !all.length && !leftOut.length && !left.notScripts) { + const jsx = scripts.length ? await jsxIn([], await ownFiles()) : []; + return { noMatch: true, jsx, jsxGlobs: scripts }; } // A hook installed for every project should not lint React apps that have no @@ -654,15 +838,14 @@ async function selectInProject( return 2; } - // The design system's own files implement the scale and the primitives the - // rules enforce (shadcn/ui's `p-[3px]`), so `check .` leaves them alone. let skipped = 0; - if (!config.includeDesignSystem) { - const own = new Set((await componentFiles(config)).map(realPath)); + const own = await ownFiles(); + if (own.size) { const rest = files.filter((file) => !own.has(realPath(file))); skipped = files.length - rest.length; files = rest; } + const jsx = await jsxIn([...named, ...all], own); return { files, designSystemOf: () => ds, @@ -671,6 +854,9 @@ async function selectInProject( notices: [...(notice ? [notice] : []), ...notReadNotices([{ ds }])], skipped, testsLeftOut: left.tests, + jsx, + jsxGlobs: scripts, + searched: globs.length > 0, }; } @@ -685,9 +871,9 @@ async function selectInWorkspace( values: CheckValues, format: OutputFormat, io: Io, -): Promise { - const { named, globs } = expandPatterns(patterns, io.cwd); - const all = globs.length +): Promise { + const { named: given, globs, scripts } = expandPatterns(patterns, io.cwd); + const globbed = globs.length ? await findFiles(globs, { cwd: io.cwd, absolute: true, @@ -695,6 +881,8 @@ async function selectInWorkspace( ignore: ['**/node_modules/**'], }) : []; + const named = given.filter((file) => SCRIPT_FILE.test(file)); + const all = globbed.filter((file) => SCRIPT_FILE.test(file)); const projectOf = new Map(); for (const file of unique([...named, ...all])) { projectOf.set(file, await workspace.projectFor(file)); @@ -730,14 +918,37 @@ async function selectInWorkspace( tests: leftOut.filter((file) => TEST_FILE.test(file)).length, excluded: leftOut.filter((file) => !TEST_FILE.test(file)).length, gitignored: ignored.size, + notScripts: notScripts(given, globbed), }; let files = oncePerFile([ ...(skip ? named.filter((file) => !leftOutSet.has(file)) : named), ...kept.filter((file) => !ignored.has(file)), ]).sort(); - if (!files.length && !all.length && !leftOut.length) { - io.stderr(`check: no files match ${patterns.join(' ')}`); - return 2; + + // A project's design-system files implement its scale and primitives. Only a file's own + // project counts: a package's files are checked as `check` in its folder checks them, + // even when an app uses them as its design system. + const own = new Map>(); + const ownFiles = async (p: Project) => { + let mine = own.get(p.root); + if (!mine) { + mine = new Set( + p.config.includeDesignSystem ? [] : (await componentFiles(p.config)).map(realPath), + ); + own.set(p.root, mine); + } + return mine; + }; + // `.js` files with JSX, each left out as `check` in its own project leaves it out. + const jsxIn = (taken: readonly string[]) => + jsxLeftOut(scripts, taken, io.cwd, async (file) => { + const p = await workspace.projectFor(file); + projectOf.set(file, p); + return excluded(file) || (await ownFiles(p)).has(realPath(file)); + }); + + if (!files.length && !all.length && !leftOut.length && !left.notScripts) { + return { noMatch: true, jsx: await jsxIn([]), jsxGlobs: scripts }; } const loaded = new Map(); @@ -779,18 +990,13 @@ async function selectInWorkspace( } } - // A project's design-system files implement its scale and primitives. Only a file's own - // project counts: a package's files are checked as `check` in its folder checks them, - // even when an app uses them as its design system. - const own = new Map>(); - for (const p of unique(files.map(project))) { - if (p.config.includeDesignSystem) continue; - own.set(p.root, new Set((await componentFiles(p.config)).map(realPath))); - } + for (const p of unique(files.map(project))) await ownFiles(p); const rest = files.filter((file) => !own.get(project(file).root)?.has(realPath(file))); const skipped = files.length - rest.length; files = rest; + const jsx = await jsxIn([...named, ...all]); + // What could not run, once per kind of gap, naming the projects. const gaps = new Map(); for (const p of unique(files.map(project))) { @@ -816,6 +1022,9 @@ async function selectInWorkspace( notices: notices.filter(Boolean), skipped, testsLeftOut: left.tests, + jsx, + jsxGlobs: scripts, + searched: globs.length > 0, }; } @@ -1135,10 +1344,18 @@ function defaultBaseline(root: string, warn: ((text: string) => void) | undefine /** Tests and stories, which `check` leaves out of folders and globs unless asked. */ const TEST_FILE = /\.(?:test|spec|stories)\.[cm]?[jt]sx?$|[\\/]__(?:tests|stories|mocks)__[\\/]/; +/** + * How many of the files named outright or matched by a glob are not JS or TS (styles, docs, + * JSON), which are not checked. + */ +function notScripts(named: readonly string[], globbed: readonly string[]): number { + return oncePerFile([...named, ...globbed].filter((file) => !SCRIPT_FILE.test(file))).length; +} + /** What a run left out, for the summary line: `5 design-system files skipped`. */ function scopeNotes( skipped: number, - left: { tests: number; excluded: number; gitignored: number }, + left: { tests: number; excluded: number; gitignored: number; notScripts: number }, ): string[] { return [ skipped ? `${plural(skipped, 'design-system file')} skipped` : '', @@ -1147,6 +1364,9 @@ function scopeNotes( : '', left.excluded ? `${plural(left.excluded, 'excluded file')} left out` : '', left.gitignored ? `${plural(left.gitignored, 'file')} git ignores left out` : '', + left.notScripts + ? `${plural(left.notScripts, 'file')} that ${left.notScripts === 1 ? 'is' : 'are'} not JS or TS skipped` + : '', ].filter(Boolean); } diff --git a/src/lint/format.ts b/src/lint/format.ts index b3291e63..4fccbe2f 100644 --- a/src/lint/format.ts +++ b/src/lint/format.ts @@ -1,9 +1,15 @@ import type { CheckResult } from '../types.js'; import { plural } from '../util/strings.js'; import { formatSarif } from './sarif.js'; +import { paletteColor, parseUtility, withBase } from './tailwind.js'; export type OutputFormat = 'pretty' | 'json' | 'github' | 'sarif'; +/** From this many findings, `pretty` ends with a summary: more than a screen to scroll back through. */ +const SUMMARY_FROM = 50; +/** The palette classes the summary lists. */ +const TOP_CLASSES = 10; + const color = (code: number) => (text: string, enabled: boolean) => enabled ? `\u001b[${code}m${text}\u001b[39m` : text; const red = color(31); @@ -71,6 +77,7 @@ export function formatDiagnostics( lines.push(''); } const total = errors + warnings; + if (total >= SUMMARY_FROM) lines.push(...summarize(results), ''); const notes = [ options.baselined === undefined ? '' @@ -93,6 +100,59 @@ export function formatDiagnostics( return lines.join('\n'); } +/** + * What a long run's findings add up to: the count per rule, as `init`'s dry run gives it, and + * the Tailwind palette classes found most, each with the token to use. On a palette-heavy app + * (Dub's web app, over 14,000 lines of findings) that reads as a plan for moving to the tokens. + */ +function summarize(results: readonly CheckResult[]): string[] { + const byRule = new Map(); + const pairs = new Map(); + const paletteFiles = new Set(); + let palette = 0; + for (const result of results) { + for (const d of result.diagnostics) { + byRule.set(d.ruleId, (byRule.get(d.ruleId) ?? 0) + 1); + if (d.ruleId !== 'no-hardcoded-color') continue; + const utility = parseUtility(d.source); + if (!paletteColor(utility.base)) continue; + palette++; + paletteFiles.add(result.file); + // The fix's class, with the variants and modifier of the one it replaces; a suggestion + // that is not applied names the bare class (or a variant, `variant="ghost"`). + const fix = d.fix?.length === 1 ? d.fix[0]?.text : undefined; + const to = + fix ?? + (d.suggestion && /^[^\s"'=]+$/.test(d.suggestion) + ? withBase(utility, d.suggestion) + : d.suggestion); + // `Nearest token`, not `Matches token`: another color, to check before replacing. + const nearest = d.message.includes('. Nearest token ') ? ' (nearest token)' : ''; + const text = `${d.source} → ${to ? `${to}${nearest}` : 'no close token'}`; + const pair = pairs.get(text) ?? { text, count: 0 }; + pair.count++; + pairs.set(text, pair); + } + } + const lines = [ + `By rule: ${[...byRule] + .sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])) + .map(([rule, n]) => `${rule} ${n.toLocaleString('en-US')}`) + .join(', ')}.`, + ]; + const top = [...pairs.values()] + .sort((a, b) => b.count - a.count || a.text.localeCompare(b.text)) + .slice(0, TOP_CLASSES); + if (top.length) { + const width = Math.max(...top.map((pair) => pair.count.toLocaleString('en-US').length)); + lines.push( + `Tailwind palette classes: ${palette.toLocaleString('en-US')} in ${plural(paletteFiles.size, 'file')}. The most common, with the token to use:`, + ...top.map((pair) => ` ${pair.count.toLocaleString('en-US').padStart(width)}× ${pair.text}`), + ); + } + return lines; +} + /** Escapes a workflow command's message, so `%` and line breaks reach GitHub as written. */ export function escapeData(value: string): string { return value.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A'); diff --git a/src/server/index.ts b/src/server/index.ts index a53d88e5..9597e7f0 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -10,7 +10,7 @@ import type { DesignSystem } from '../design-system.js'; import { uncheckedNotice } from '../lint/index.js'; import { notReadNotices } from '../not-read.js'; import { TOKEN_CATEGORIES, type ComponentInfo } from '../types.js'; -import { isInside, relativePath } from '../util/paths.js'; +import { isInside, relativePath, SCRIPT_FILE } from '../util/paths.js'; import { NAME, VERSION } from '../version.js'; import { capDiagnostics, @@ -54,6 +54,12 @@ const MAX_CODE = 1_000_000; /** Diagnostics check_ui returns by default: a 3,000-line file can have thousands, more than a context holds. */ const DEFAULT_LIMIT = 50; const MAX_LIMIT = 1000; +/** check_ui's answer for a path or filename that is not JS or TS. */ +function notScript(name: string): CallToolResult { + return errorResult( + `check_ui checks .tsx, .jsx, .ts and .js files; ${name} is not one, so it was not checked.`, + ); +} /** * An integer, without the ±9007199254740991 bounds zod adds to the JSON Schema, @@ -556,7 +562,7 @@ export function createServer({ getDesignSystem, getProjects }: CreateServerOptio .max(MAX_PATH) .optional() .describe( - 'Path of a file to check: relative to the project root (the monorepo root, when served from there), or absolute.', + 'Path of a .tsx, .jsx, .ts or .js file to check: relative to the project root (the monorepo root, when served from there), or absolute.', ), filename: z .string() @@ -610,6 +616,9 @@ export function createServer({ getDesignSystem, getProjects }: CreateServerOptio }, async ({ code, path: filePath, filename, limit }): Promise => { if (code === undefined && !filePath) return errorResult('Pass either `code` or `path`.'); + if (code !== undefined && filename !== undefined && !SCRIPT_FILE.test(filename)) { + return notScript(filename); + } // At a workspace root, the file's own project checks it. const routeBy = code === undefined ? filePath : (filename ?? undefined); const set = await getProjects?.(routeBy); @@ -643,6 +652,7 @@ export function createServer({ getDesignSystem, getProjects }: CreateServerOptio const absolute = path.resolve(base, filePath); const outside = errorResult(`${filePath} is outside the project root (${base}).`); if (!isInside(base, absolute)) return outside; + if (!SCRIPT_FILE.test(absolute)) return notScript(filePath); try { // A symlink inside the root can point anywhere: compare where both really are. const [root, real] = await Promise.all([fs.realpath(base), fs.realpath(absolute)]); diff --git a/src/util/paths.ts b/src/util/paths.ts index 2d274249..3f5ef1d8 100644 --- a/src/util/paths.ts +++ b/src/util/paths.ts @@ -18,6 +18,13 @@ export function slashGlob(pattern: string): string { return pattern.replace(/\\(?![()[\]{}*?!+@])/g, '/'); } +/** + * The files `check` and `check_ui` read: `.tsx`, `.jsx`, `.ts`, `.js` and the `.mts`, `.cts`, + * `.mjs` and `.cjs` forms, in any case. CSS, Markdown or JSON parsed as TSX gives hundreds of + * syntax errors (209 for a shadcn/ui `app/globals.css`). + */ +export const SCRIPT_FILE = /\.(?:[jt]sx?|[cm][jt]s)$/i; + /** Path relative to `root`, always with forward slashes. */ export function relativePath(root: string, file: string): string { return toPosix(path.relative(root, file)); diff --git a/test/check-scope.test.ts b/test/check-scope.test.ts new file mode 100644 index 00000000..315f1dff --- /dev/null +++ b/test/check-scope.test.ts @@ -0,0 +1,429 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import { describe, expect, it } from 'vitest'; + +import { main, type Io } from '../src/cli.js'; +import { createServer } from '../src/server/index.js'; +import { DEMO_ROOT, fixture, load } from './helpers.js'; + +// What `check` and `check_ui` read: JS and TS files. A CSS, Markdown or JSON file named outright +// (`check $(git diff --name-only)`, lint-staged's `"*"`, an agent's check_ui on globals.css) was +// parsed as TSX and gave hundreds of syntax errors. And a JavaScript app, whose pages hold JSX in +// `.js` files (Next.js's JavaScript template writes app/page.js), had nothing checked by +// `check .`, which passed with --require-design-system. + +async function run(args: string[], cwd: string) { + const out: string[] = []; + const err: string[] = []; + const io: Io = { cwd, color: false, stdout: (t) => out.push(t), stderr: (t) => err.push(t) }; + const code = await main(args, io); + return { code, stdout: out.join('\n'), stderr: err.join('\n') }; +} + +const BUTTON_JSX = 'export function Button(props) {\n return + + + ); +} +`; + +/** A JavaScript app: shadcn/ui's tsx:false components (.jsx) and pages in .js files. */ +function jsApp(extra: Record = {}): string { + return fixture({ + 'components/ui/button.jsx': BUTTON_JSX, + 'app/globals.css': GLOBALS, + 'app/page.js': PAGE_JS, + 'next.config.js': 'module.exports = { reactStrictMode: true };\n', + 'jsconfig.json': JSON.stringify({ compilerOptions: { paths: { '@/*': ['./*'] } } }), + ...extra, + }); +} + +describe('check: files that are not JS or TS', () => { + const project = () => + fixture({ + 'components/ui/button.tsx': BUTTON_TSX, + 'app/globals.css': GLOBALS, + 'app/page.tsx': 'export default () => ', + 'README.md': '# App\n\nRun `pnpm dev`, then open .\n', + 'package.json': '{ "name": "app", "private": true }\n', + 'app/Page.module.SCSS': '.page { color: red; }\n', + }); + + it('skips them when named outright, and says how many', async () => { + const root = project(); + const named = [ + 'README.md', + 'app/globals.css', + 'package.json', + 'app/Page.module.SCSS', + 'app/page.tsx', + ]; + const json = await run(['check', ...named, '--format', 'json', '--no-cache'], root); + const results = JSON.parse(json.stdout) as { + file: string; + diagnostics: { ruleId: string }[]; + }[]; + expect(results.map((r) => r.file)).toEqual(['app/page.tsx']); + expect(results.flatMap((r) => r.diagnostics.map((d) => d.ruleId))).not.toContain('syntax'); + + const pretty = await run(['check', ...named, '--no-cache'], root); + expect(pretty.code).toBe(1); + expect(pretty.stdout.split('\n').find((line) => /^\d+ errors?,/.test(line))).toBe( + '2 errors, 0 warnings in 1 file (4 files that are not JS or TS skipped)', + ); + }); + + it('has nothing to check when no file named is JS or TS, and passes', async () => { + const root = project(); + const result = await run(['check', 'README.md', 'app/globals.css', '--no-cache'], root); + expect(result).toEqual({ + code: 0, + stdout: 'Nothing to check: 2 files that are not JS or TS skipped.', + stderr: '', + }); + // A pull request that changes only styles and docs has no UI code to check: the gate is + // about the design system and the folders checked, not about the files a diff names. + const gated = await run( + ['check', 'README.md', '--require-design-system', '--format', 'json', '--no-cache'], + root, + ); + expect(gated).toEqual({ code: 0, stdout: '[]', stderr: '' }); + }); + + it('skips them when a glob matches them, and fails the gate when nothing else matches', async () => { + const root = project(); + // A glob that matches only styles has no UI code to check. + const css = await run(['check', 'app/**/*.css', '--no-cache'], root); + expect(css).toEqual({ + code: 0, + stdout: 'Nothing to check: 1 file that is not JS or TS skipped.', + stderr: '', + }); + // In CI that glob points at the wrong code, as a folder with nothing to check does. + const gated = await run(['check', 'app/**/*.css', '--require-design-system'], root); + expect(gated).toEqual({ + code: 2, + stdout: '', + stderr: + 'check --require-design-system: nothing was checked (1 file that is not JS or TS skipped).', + }); + + // Every file: the page is checked, the rest counted, and no syntax error from the others. + const everything = await run(['check', '**/*', '--format', 'json', '--no-cache'], root); + const results = JSON.parse(everything.stdout) as { + file: string; + diagnostics: { ruleId: string }[]; + }[]; + expect(results.map((r) => r.file)).toEqual(['app/page.tsx']); + expect(results.flatMap((r) => r.diagnostics.map((d) => d.ruleId))).not.toContain('syntax'); + const pretty = await run(['check', '**/*', '--no-cache'], root); + expect(pretty.stdout).not.toContain('syntax'); + expect(pretty.stdout.split('\n').at(-1)).toBe( + '2 errors, 0 warnings in 1 file (1 design-system file skipped; 4 files that are not JS or TS skipped)', + ); + // A file both named and matched by a glob is counted once. + const both = await run(['check', 'app/globals.css', 'app/**/*.css', '--no-cache'], root); + expect(both.stdout).toBe('Nothing to check: 1 file that is not JS or TS skipped.'); + }); + + it('still checks .ts, .js, .mjs and .cjs files named outright', async () => { + const root = fixture({ + 'components/ui/button.tsx': BUTTON_TSX, + 'app/globals.css': GLOBALS, + 'lib/tone.ts': 'export const tone = "bg-[#ef4444]";\n', + 'app/page.js': PAGE_JS, + 'app/page.mjs': PAGE_JS, + 'app/page.cjs': PAGE_JS, + }); + const json = await run( + ['check', 'lib/tone.ts', 'app/page.js', 'app/page.mjs', 'app/page.cjs', '--format', 'json'], + root, + ); + const results = JSON.parse(json.stdout) as { file: string }[]; + expect(results.map((r) => r.file)).toEqual([ + 'app/page.cjs', + 'app/page.js', + 'app/page.mjs', + 'lib/tone.ts', + ]); + }); +}); + +describe('check: JSX in .js files', () => { + it("says that a folder's .js files with JSX were not checked, and how to check them", async () => { + const root = jsApp(); + // It takes the place of --include-design-system's hint, which would check the button and + // pass with the page still unchecked. + expect(await run(['check', '.', '--no-cache'], root)).toEqual({ + code: 0, + stdout: + 'Nothing to check: 1 design-system file skipped. app/page.js has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: `check . "**/*.js"`.', + stderr: '', + }); + // The command the notice gives checks it. + const named = await run(['check', '.', '**/*.js', '--no-cache'], root); + expect(named.code).toBe(1); + expect(named.stdout).toContain('app/page.js'); + expect(named.stdout).toContain('Hardcoded color `bg-[#ef4444]`'); + expect(named.stdout).not.toContain('has JSX'); + }); + + it('counts them next to the files it checked, and leaves out what folders leave out', async () => { + const root = jsApp({ + 'app/about/page.tsx': 'export default () =>

About

', + 'app/blog/page.js': PAGE_JS, + 'app/contact/page.js': PAGE_JS, + 'app/page.test.js': PAGE_JS, + 'app/legacy/page.js': PAGE_JS, + 'onsystem.config.json': JSON.stringify({ + exclude: ['**/*.test.{js,jsx,ts,tsx}', 'app/legacy/**'], + }), + // A .js file without JSX is no page: not counted. + 'app/lib/format.js': 'export const format = (n) => `${n} items`;\n', + }); + const result = await run(['check', 'app', '--no-cache'], root); + expect(result.code).toBe(0); + expect(result.stdout.split('\n')).toEqual([ + 'No problems in 1 file.', + '3 .js files have JSX (app/blog/page.js, app/contact/page.js and app/page.js), but folders are checked for .tsx and .jsx files, so they were not checked. Name them to check them: `check app "app/**/*.js"`.', + ]); + // On stderr with JSON, and as a workflow warning with --format github. + const json = await run(['check', 'app', '--format', 'json', '--no-cache'], root); + expect(JSON.parse(json.stdout)).toHaveLength(1); + expect(json.stderr).toContain('3 .js files have JSX'); + const github = await run(['check', 'app', '--format', 'github', '--no-cache'], root); + expect(github.stdout).toMatch(/^::warning title=onsystem::3 \.js files have JSX/m); + }); + + it('fails --require-design-system when a folder leaves nothing to check', async () => { + const root = jsApp(); + const gated = await run( + ['check', '.', '--require-design-system', '--format', 'github', '--no-cache'], + root, + ); + expect(gated.code).toBe(2); + expect(gated.stdout).toBe(''); + // One line, which is all the GitHub Action's error annotation shows: what to name, not + // --include-design-system, which would pass with the page still unchecked. + expect(gated.stderr).toBe( + 'check --require-design-system: nothing was checked (1 design-system file skipped). app/page.js has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: `check . "**/*.js"`.', + ); + // With the pages named, it checks them and fails on their findings, not on the gate. + const named = await run( + ['check', '.', '**/*.js', '--require-design-system', '--format', 'json', '--no-cache'], + root, + ); + expect(named.code).toBe(1); + + // A design-system package checked on its own: nothing but its own files. + const ui = fixture({ 'components/ui/button.tsx': BUTTON_TSX, 'app/globals.css': GLOBALS }); + const own = await run(['check', '.', '--require-design-system', '--no-cache'], ui); + expect(own).toEqual({ + code: 2, + stdout: '', + stderr: + 'check --require-design-system: nothing was checked (1 design-system file skipped). Pass --include-design-system to check it.', + }); + }); +}); + +describe('check: a folder of .js pages', () => { + const LAYOUT_JS = `export default function RootLayout({ children }) { + return ( + + {children} + + ); +} +`; + + it('names them when the folder has no .tsx or .jsx file, and still exits 2', async () => { + const root = jsApp({ 'app/layout.js': LAYOUT_JS }); + const expected = { + code: 2, + stdout: '', + stderr: + 'check: nothing to check in app. 2 .js files have JSX (app/layout.js and app/page.js), but folders are checked for .tsx and .jsx files, so they were not checked. Name them to check them: `check app "app/**/*.js"`.', + }; + expect(await run(['check', 'app', '--no-cache'], root)).toEqual(expected); + expect(await run(['check', 'app', '--format', 'json', '--no-cache'], root)).toEqual(expected); + // The command it gives checks them. + const named = await run( + ['check', 'app', 'app/**/*.js', '--format', 'json', '--no-cache'], + root, + ); + expect((JSON.parse(named.stdout) as { file: string }[]).map((r) => r.file)).toEqual([ + 'app/layout.js', + 'app/page.js', + ]); + // A folder with no page at all is still a folder that matches nothing. + const empty = jsApp({ 'lib/format.js': 'export const format = (n) => `${n} items`;\n' }); + expect(await run(['check', 'lib', '--no-cache'], empty)).toEqual({ + code: 2, + stdout: '', + stderr: 'check: no files match lib', + }); + }); + + it('finds JSX as pages write it, and not in bundles, minified files or HTML in strings', async () => { + const big = `export const data = ${JSON.stringify('x'.repeat(600 * 1024))};\nexport default () =>

Big

;\n`; + const root = jsApp({ + // Before the element: a comment, an arrow, a generic call in its props, a fragment. + 'app/a/page.js': + 'export default function A() {\n return (\n // A comment first\n
\n );\n}\n', + 'app/b/page.js': 'export const B = ({ children }) =>
{children}
;\n', + 'app/c/page.js': + 'export default function C(props) {\n return ;\n}\n', + 'app/d/page.js': 'export default () => (\n <>\n

D

\n \n);\n', + // Not pages: HTML in strings and comments, a named regex group, a minified file and one + // over 512 KB. + 'app/vendor/escape.js': + '// e.g. abc\nconst re = /(?[a-z]+)/;\nexport const html = "(bold)" + \'
\';\nexport const lt = (a, b) => a < b;\n', + 'app/vendor/lib.min.js': 'export default () =>

min

;\n', + 'app/vendor/big.js': big, + }); + const result = await run(['check', 'app', '--no-cache'], root); + expect(result.stderr).toBe( + 'check: nothing to check in app. 5 .js files have JSX (app/a/page.js, app/b/page.js, app/c/page.js and 2 more), but folders are checked for .tsx and .jsx files, so they were not checked. Name them to check them: `check app "app/**/*.js"`.', + ); + const json = await run(['check', '.', '--format', 'json', '--no-cache'], root); + expect(json.stderr).toContain('5 .js files have JSX'); + }); + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'leaves out a .js file it cannot read', + async () => { + const root = jsApp({ 'app/locked.js': 'export default () =>

Locked

;\n' }); + fs.chmodSync(path.join(root, 'app/locked.js'), 0o000); + try { + expect(await run(['check', '.', '--no-cache'], root)).toEqual({ + code: 0, + stdout: + 'Nothing to check: 1 design-system file skipped. app/page.js has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: `check . "**/*.js"`.', + stderr: '', + }); + } finally { + fs.chmodSync(path.join(root, 'app/locked.js'), 0o644); + } + }, + ); +}); + +describe('check at a monorepo root', () => { + const json = (value: unknown) => JSON.stringify(value); + const monorepo = () => + fixture({ + 'pnpm-workspace.yaml': 'packages:\n - apps/*\n', + 'package.json': json({ name: 'acme', private: true }), + 'apps/web/package.json': json({ name: '@acme/web' }), + 'apps/web/components.json': json({ tsx: false, aliases: { ui: '@/components/ui' } }), + 'apps/web/jsconfig.json': json({ compilerOptions: { paths: { '@/*': ['./*'] } } }), + 'apps/web/components/ui/button.jsx': BUTTON_JSX, + 'apps/web/app/globals.css': GLOBALS, + 'apps/web/app/page.js': PAGE_JS, + 'apps/web/app/page.test.js': PAGE_JS, + 'apps/web/README.md': '# Web\n\nOpen .\n', + }); + + it('names the .js files with JSX of each project, and skips named files that are not JS or TS', async () => { + const root = monorepo(); + expect(await run(['check', '.', '--no-cache'], root)).toMatchObject({ + code: 0, + stdout: + 'Nothing to check: 1 design-system file skipped. apps/web/app/page.js has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: `check . "**/*.js"`.', + }); + // A folder with nothing but .js pages. + expect(await run(['check', 'apps/web/app', '--no-cache'], root)).toEqual({ + code: 2, + stdout: '', + stderr: + 'check: nothing to check in apps/web/app. apps/web/app/page.js has JSX, but folders are checked for .tsx and .jsx files, so it was not checked. Name it to check it: `check apps/web/app "apps/web/app/**/*.js"`.', + }); + expect((await run(['check', '.', '--require-design-system', '--no-cache'], root)).code).toBe(2); + const named = await run( + ['check', 'apps/web/README.md', 'apps/web/app/page.js', '--format', 'json', '--no-cache'], + root, + ); + expect((JSON.parse(named.stdout) as { file: string }[]).map((r) => r.file)).toEqual([ + 'apps/web/app/page.js', + ]); + // A glob that matches every file checks the JS and TS ones, each in its own project. + const everything = await run(['check', '**/*', '--no-cache'], root); + expect(everything.stdout).not.toContain('syntax'); + expect(everything.stdout.split('\n').at(-1)).toBe( + '2 errors, 1 warning in 1 file (1 design-system file skipped; 1 test or story left out; 7 files that are not JS or TS skipped)', + ); + }); +}); + +describe('check_ui: files that are not JS or TS', () => { + async function connect(root: string) { + const ds = await load(root); + const server = createServer({ getDesignSystem: () => Promise.resolve(ds) }); + const client = new Client({ name: 'test-client', version: '1.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + return client; + } + const text = (result: CallToolResult) => + result.content.map((c) => (c.type === 'text' ? c.text : '')).join(''); + + it('returns an error for a path or filename that is not JS or TS', async () => { + const client = await connect(DEMO_ROOT); + const css = (await client.callTool({ + name: 'check_ui', + arguments: { path: 'app/globals.css' }, + })) as CallToolResult; + expect(css.isError).toBe(true); + expect(text(css)).toBe( + 'check_ui checks .tsx, .jsx, .ts and .js files; app/globals.css is not one, so it was not checked.', + ); + const vue = (await client.callTool({ + name: 'check_ui', + arguments: { code: '', filename: 'x.vue' }, + })) as CallToolResult; + expect(vue.isError).toBe(true); + expect(text(vue)).toContain('x.vue is not one'); + // Only real extensions: `.mts` is TypeScript, `.mtsx` and `.cjsx` are not. + for (const filename of ['x.mtsx', 'x.cjsx']) { + const made = (await client.callTool({ + name: 'check_ui', + arguments: { code: '', filename }, + })) as CallToolResult; + expect(text(made)).toContain(`${filename} is not one`); + } + const mts = (await client.callTool({ + name: 'check_ui', + arguments: { code: 'export const tone = "bg-[#ef4444]";', filename: 'lib/tone.mts' }, + })) as CallToolResult; + expect(mts.isError).toBeFalsy(); + expect(mts.structuredContent).toMatchObject({ file: 'lib/tone.mts' }); + // JS and TS files are checked as before. + const page = (await client.callTool({ + name: 'check_ui', + arguments: { path: 'app/settings/danger-zone.tsx' }, + })) as CallToolResult; + expect(page.isError).toBeFalsy(); + expect(page.structuredContent).toMatchObject({ ok: false }); + const js = (await client.callTool({ + name: 'check_ui', + arguments: { code: '', filename: 'app/page.js' }, + })) as CallToolResult; + expect(js.structuredContent).toMatchObject({ ok: false, file: 'app/page.js' }); + await client.close(); + }); +}); diff --git a/test/check-summary.test.ts b/test/check-summary.test.ts new file mode 100644 index 00000000..0552b49a --- /dev/null +++ b/test/check-summary.test.ts @@ -0,0 +1,111 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { main, type Io } from '../src/cli.js'; +import { DEMO_ROOT, fixture } from './helpers.js'; +import { blocks } from './markdown.js'; + +// A first `check .` on a palette-heavy app (Dub's web app) prints over 14,000 lines, one per +// finding, and used to end with one line of totals. A long run now ends with what the findings +// add up to: the count per rule, as `init`'s dry run gives it, and the palette classes found +// most, each with the token to use, which reads as a plan for moving to the tokens. JSON, SARIF +// and GitHub output keep every finding and nothing else. + +async function run(args: string[], cwd: string) { + const out: string[] = []; + const err: string[] = []; + const io: Io = { cwd, color: false, stdout: (t) => out.push(t), stderr: (t) => err.push(t) }; + const code = await main(args, io); + return { code, stdout: out.join('\n'), stderr: err.join('\n') }; +} + +const BUTTON = + 'export function Button(props: React.ComponentProps<"button">) { return +
+ ); +} +`; + } + return fixture(files); +} + +describe('check: the summary of a long run', () => { + it('ends with the findings per rule and the most common palette classes', async () => { + const root = paletteApp(12); + const { code, stdout } = await run(['check', 'app', '--no-cache'], root); + expect(code).toBe(1); + const lines = stdout.split('\n'); + const start = lines.findIndex((line) => line.startsWith('By rule:')); + expect(start).toBeGreaterThan(0); + // After the last file's findings and before the totals. + expect(lines[start - 1]).toBe(''); + expect(lines.slice(start)).toEqual([ + 'By rule: no-hardcoded-color 48, prefer-design-system-component 12.', + 'Tailwind palette classes: 48 in 12 files. The most common, with the token to use:', + ' 12× border-neutral-200 → border-border', + ' 12× hover:border-neutral-200 → hover:border-border', + ' 12× text-neutral-500 → text-muted-foreground', + ' 12× text-neutral-800 → text-primary (nearest token)', + '', + '60 errors, 0 warnings in 12 files', + ]); + // docs/ci.md shows this summary. + const ci = fs.readFileSync(path.join(import.meta.dirname, '../docs/ci.md'), 'utf8'); + const shown = blocks(ci, 'text').find((block) => block.startsWith('By rule:')); + expect(shown?.trimEnd()).toBe(lines.slice(start, start + 6).join('\n')); + }); + + it('lists the ten most common palette classes, most first', async () => { + const files: Record = { + 'components/ui/button.tsx': BUTTON, + 'app/globals.css': GLOBALS, + }; + // Shades 50 to 950 of neutral as borders: shade i appears i + 1 times. + const shades = ['50', '100', '200', '300', '400', '500', '600', '700', '800', '900', '950']; + for (const [i, shade] of shades.entries()) { + const classes = Array.from({ length: i + 1 }, () => `border-neutral-${shade}`).join(' '); + files[`app/border-${shade}.tsx`] = `export default () =>
`; + } + const root = fixture(files); + const { stdout } = await run(['check', 'app', '--no-cache'], root); + const rows = stdout.split('\n').filter((line) => /^\s+\d+× /.test(line)); + expect(rows).toHaveLength(10); + expect(rows[0]).toMatch(/^ {2}11× border-neutral-950 → /); + expect(rows[9]).toMatch(/^ {3}2× border-neutral-100 → /); + // A class with no token close to its color is listed too: the theme lacks one. + expect(rows).toContain(' 8× border-neutral-700 → no close token'); + expect(stdout).not.toContain('border-neutral-50 →'); + expect(stdout).toContain('Tailwind palette classes: 66 in 11 files.'); + }); + + it('is left out of short runs and of JSON, SARIF and GitHub output', async () => { + const short = await run(['check', 'app', '--no-cache'], paletteApp(2)); + expect(short.stdout).not.toContain('By rule:'); + expect(short.stdout).not.toContain('Tailwind palette classes'); + expect(short.stdout.split('\n').at(-1)).toBe('10 errors, 0 warnings in 2 files'); + + const root = paletteApp(12); + for (const format of ['json', 'sarif', 'github']) { + const { stdout } = await run(['check', 'app', '--format', format, '--no-cache'], root); + expect(stdout).not.toContain('By rule:'); + expect(stdout).not.toContain('Tailwind palette classes'); + } + }); +});