diff --git a/client/CLAUDE.md b/client/CLAUDE.md index 6dbf6927a..f79ee1cef 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -10,7 +10,7 @@ npm run build # prebuild generates PWA icons, then vite build npm run typecheck # tsc --noEmit (CI) npm run lint # eslint . (lint:check in CI) npm run lint:pages # enforce the Page pattern (CI gate) -npm run lint:rtl # physical left/right sides may only shrink per file (CI gate; --list shows them, --update lowers the baseline) +npm run lint:rtl # physical left/right sides and rtl-lint-disable comments may only shrink per file (CI gate; --list shows them, --update lowers both baselines) npm run lint:size # no source file or stylesheet past 1000 lines, no test past 2000, counted wrapped at 120 columns; longer ones may only shrink (CI gate; --update lowers the baseline) npm run test # vitest run (tests/** + co-located src/**/*.test.{ts,tsx}); also test:unit / test:integration / test:coverage npm run e2e # Playwright (CI runs --project=public --project=app; e2e:report opens the last report) diff --git a/client/scripts/lib/rtl.mjs b/client/scripts/lib/rtl.mjs new file mode 100644 index 000000000..e31450548 --- /dev/null +++ b/client/scripts/lib/rtl.mjs @@ -0,0 +1,182 @@ +/* + * The matching and the comparison behind lint:rtl (scripts/rtl-lint.mjs). + */ +import { join } from 'node:path' +import ts from 'typescript' +import { countMap, listFiles, lowerCounts, readBaseline, readText, TEST_FILE, toKey, writeBaseline } from './ratchet.mjs' + +// Tailwind utilities that name a physical side, with any variant prefix +// (sm:, hover:, group-hover:) and the negative form (-ml-2). +const CLASS = new RegExp( + String.raw`(? { + for (let i = pos; i < end; i++) if (chars[i] !== '\n') chars[i] = ' ' + } + if (file.endsWith('.css')) { + for (const m of source.matchAll(/\/\*[\s\S]*?\*\//g)) blank(m.index, m.index + m[0].length) + return chars.join('') + } + const kind = file.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS + const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, kind) + const seen = new Set() + const take = (ranges) => { + for (const r of ranges ?? []) { + if (seen.has(r.pos)) continue + seen.add(r.pos) + blank(r.pos, r.end) + } + } + const visit = (node) => { + take(ts.getLeadingCommentRanges(source, node.pos)) + take(ts.getTrailingCommentRanges(source, node.end)) + // JSX text holds no comments, and asking for them there would read `//` in a URL as one. + if (!ts.isJsxText(node)) for (const child of node.getChildren(sf)) visit(child) + } + visit(sf) + take(ts.getLeadingCommentRanges(source, sf.endOfFileToken.pos)) + return chars.join('') +} + +/** Whether the line carries the marker inside a comment; in a string or in code it is no marker. */ +function markedInComment(line, code) { + for (let at = line.indexOf(DISABLE); at !== -1; at = line.indexOf(DISABLE, at + 1)) { + if (!code.slice(at, at + DISABLE.length).trim()) return true + } + return false +} + +/** + * One file read once: every physical use as `line: match`, skipping the lines + * marked as physical on purpose, and how many lines carry that mark. + */ +export function inspect(source, file) { + const isCss = file.endsWith('.css') + const uses = [] + let markers = 0 + const lines = source.split('\n') + withoutComments(source, file).split('\n').forEach((code, i) => { + if (markedInComment(lines[i], code)) { + markers++ + return + } + const patterns = isCss ? [CSS] : [CLASS, STYLE] + for (const re of patterns) for (const m of code.match(re) ?? []) uses.push(`${i + 1}: ${m.trim()}`) + }) + return { uses, markers } +} + +/** Every physical use in one file, as `line: match`. */ +export function physicalUses(source, file) { + return inspect(source, file).uses +} + +/** The physical uses and the disable markers per file under root/src, keyed by the path from src/. */ +export function scan(root) { + const counts = {} + const listed = {} + const markers = {} + const accept = (key) => /\.(?:tsx?|css)$/.test(key) && !TEST_FILE.test(key) + for (const path of listFiles(root, ['src'], accept)) { + const found = inspect(readText(path), path) + const key = toKey(join(root, 'src'), path) + if (found.uses.length) { + counts[key] = found.uses.length + listed[key] = found.uses + } + if (found.markers) markers[key] = found.markers + } + return { counts, listed, markers } +} + +const sum = (counts) => Object.values(counts).reduce((a, b) => a + b, 0) + +/** + * Runs the check against both baselines: rtl-baseline.json for the physical + * sides, rtl-disable-baseline.json for the lines that opt out of the check. A + * marker hides a side from the count, so the markers are a ratchet as well: + * a file holds no more of them than its entry. Returns the exit code. + */ +export function check({ + root, + baselinePath = join(root, 'scripts/rtl-baseline.json'), + disableBaselinePath = join(root, 'scripts/rtl-disable-baseline.json'), + update = false, + list = false, + log = console.log, + error = console.error, +}) { + const { counts, listed, markers } = scan(root) + let baseline = readBaseline(baselinePath, countMap) + let allowedMarkers = readBaseline(disableBaselinePath, countMap) + + if (list) { + for (const [file, uses] of Object.entries(listed).sort()) { + log(`${file} (${uses.length}, baseline ${baseline[file] ?? 0})`) + for (const use of uses) log(` ${use}`) + } + } + + if (update) { + baseline = lowerCounts(baseline, counts) + allowedMarkers = lowerCounts(allowedMarkers, markers) + writeBaseline(baselinePath, baseline) + writeBaseline(disableBaselinePath, allowedMarkers) + } + + const grown = Object.entries(counts).filter(([file, n]) => n > (baseline[file] ?? 0)) + const marked = Object.entries(markers).filter(([file, n]) => n > (allowedMarkers[file] ?? 0)) + const lowerable = + Object.entries(baseline).filter(([file, n]) => (counts[file] ?? 0) < n).length + + Object.entries(allowedMarkers).filter(([file, n]) => (markers[file] ?? 0) < n).length + + for (const [file, n] of grown) { + error( + `FAIL ${file}: ${n} physical side(s), baseline ${baseline[file] ?? 0}. ` + + 'Use ms-/me-/ps-/pe-/start-/end-/text-start/text-end/border-s/border-e/rounded-s/rounded-e ' + + 'or marginInlineStart/paddingInlineEnd/insetInlineStart/textAlign: start. ' + + `If the side is geometry, mark the line with ${DISABLE} in a comment.`, + ) + } + for (const [file, n] of marked) { + error( + `FAIL ${file}: ${n} line(s) marked ${DISABLE}, scripts/rtl-disable-baseline.json allows ${allowedMarkers[file] ?? 0}. ` + + 'Use the logical form where the side follows the text. A side that is geometry on purpose ' + + '(a map, a measured position, a time axis) needs a reviewer to raise the entry by hand.', + ) + } + if (lowerable && !update) { + log(`${lowerable} entr(ies) are above what the files hold now: run with --update to lower them.`) + } + log( + `rtl: ${sum(counts)} physical side(s) in ${Object.keys(counts).length} file(s), baseline allows ${sum(baseline)}; ` + + `${sum(markers)} line(s) marked ${DISABLE}, baseline allows ${sum(allowedMarkers)}`, + ) + return grown.length || marked.length ? 1 : 0 +} diff --git a/client/scripts/rtl-disable-baseline.json b/client/scripts/rtl-disable-baseline.json new file mode 100644 index 000000000..f26793f81 --- /dev/null +++ b/client/scripts/rtl-disable-baseline.json @@ -0,0 +1,3 @@ +{ + "pages/tripPlanner/PlanMapStage.tsx": 4 +} diff --git a/client/scripts/rtl-lint.mjs b/client/scripts/rtl-lint.mjs index bf74034c7..28bd4046a 100644 --- a/client/scripts/rtl-lint.mjs +++ b/client/scripts/rtl-lint.mjs @@ -14,161 +14,21 @@ * uses are counted against scripts/rtl-baseline.json, and the check fails when * a file holds more than its entry (a file without an entry holds none), so * new code reaches for the logical forms. A line that is physical on purpose - * in new code carries `rtl-lint-disable` in a comment. + * carries `rtl-lint-disable` in a comment (in a string it is no marker). The + * marked lines are counted too, against scripts/rtl-disable-baseline.json, so + * the marker cannot quietly take the place of the logical forms: a new one + * raises its file's entry by hand, where review sees it. * - * npm run lint:rtl check against the baseline (CI) + * npm run lint:rtl check against the baselines (CI) * npm run lint:rtl -- --list print every counted use, file by file - * npm run lint:rtl -- --update lower the baseline to what the files hold now; + * npm run lint:rtl -- --update lower both baselines to what the files hold now; * it never raises an entry + * + * The matching lives in scripts/lib/rtl.mjs. */ -import { readdirSync, readFileSync, statSync, writeFileSync } from 'node:fs' -import { join, relative } from 'node:path' import { fileURLToPath } from 'node:url' -import ts from 'typescript' +import { runCli } from './lib/ratchet.mjs' +import { check } from './lib/rtl.mjs' -const SRC = fileURLToPath(new URL('../src', import.meta.url)) -const BASELINE = fileURLToPath(new URL('./rtl-baseline.json', import.meta.url)) - -// Tailwind utilities that name a physical side, with any variant prefix -// (sm:, hover:, group-hover:) and the negative form (-ml-2). -const CLASS = new RegExp( - String.raw`(? { - for (let i = pos; i < end; i++) if (chars[i] !== '\n') chars[i] = ' ' - } - if (file.endsWith('.css')) { - for (const m of source.matchAll(/\/\*[\s\S]*?\*\//g)) blank(m.index, m.index + m[0].length) - return chars.join('') - } - const kind = file.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS - const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, kind) - const seen = new Set() - const take = (ranges) => { - for (const r of ranges ?? []) { - if (seen.has(r.pos)) continue - seen.add(r.pos) - blank(r.pos, r.end) - } - } - const visit = (node) => { - take(ts.getLeadingCommentRanges(source, node.pos)) - take(ts.getTrailingCommentRanges(source, node.end)) - // JSX text holds no comments, and asking for them there would read `//` in a URL as one. - if (!ts.isJsxText(node)) for (const child of node.getChildren(sf)) visit(child) - } - visit(sf) - take(ts.getLeadingCommentRanges(source, sf.endOfFileToken.pos)) - return chars.join('') -} - -/** Every physical use in one file, as `line: match`. */ -export function physicalUses(source, file) { - const isCss = file.endsWith('.css') - const uses = [] - const lines = source.split('\n') - withoutComments(source, file).split('\n').forEach((code, i) => { - if (lines[i].includes(DISABLE)) return - const patterns = isCss ? [CSS] : [CLASS, STYLE] - for (const re of patterns) for (const m of code.match(re) ?? []) uses.push(`${i + 1}: ${m.trim()}`) - }) - return uses -} - -function scan() { - const counts = {} - const listed = {} - for (const file of walk(SRC)) { - const uses = physicalUses(readFileSync(file, 'utf8'), file) - if (!uses.length) continue - const key = relative(SRC, file).replace(/\\/g, '/') - counts[key] = uses.length - listed[key] = uses - } - return { counts, listed } -} - -function readBaseline() { - try { - return JSON.parse(readFileSync(BASELINE, 'utf8')) - } catch { - return {} - } -} - -const isCli = process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1] -if (isCli) { - const { counts, listed } = scan() - const baseline = readBaseline() - - if (process.argv.includes('--list')) { - for (const [file, uses] of Object.entries(listed).sort()) { - console.log(`${file} (${uses.length}, baseline ${baseline[file] ?? 0})`) - for (const use of uses) console.log(` ${use}`) - } - } - - if (process.argv.includes('--update')) { - const lowered = {} - for (const [file, allowed] of Object.entries(baseline)) { - const now = counts[file] ?? 0 - if (now > 0) lowered[file] = Math.min(allowed, now) - } - const sorted = Object.fromEntries(Object.entries(lowered).sort(([a], [b]) => a.localeCompare(b))) - writeFileSync(BASELINE, JSON.stringify(sorted, null, 2) + '\n') - } - - const grown = Object.entries(counts).filter(([file, n]) => n > (baseline[file] ?? 0)) - const total = Object.values(counts).reduce((a, b) => a + b, 0) - const allowed = Object.values(baseline).reduce((a, b) => a + b, 0) - const lowerable = Object.entries(baseline).filter(([file, n]) => (counts[file] ?? 0) < n) - - for (const [file, n] of grown) { - console.error( - `FAIL ${file}: ${n} physical side(s), baseline ${baseline[file] ?? 0}. ` + - 'Use ms-/me-/ps-/pe-/start-/end-/text-start/text-end/border-s/border-e/rounded-s/rounded-e ' + - 'or marginInlineStart/paddingInlineEnd/insetInlineStart/textAlign: start. ' + - `If the side is geometry, mark the line with ${DISABLE}.`, - ) - } - if (lowerable.length && !process.argv.includes('--update')) { - console.log(`${lowerable.length} file(s) now hold fewer physical sides than the baseline: run with --update to lower it.`) - } - console.log(`rtl: ${total} physical side(s) in ${Object.keys(counts).length} file(s), baseline allows ${allowed}`) - if (grown.length) process.exit(1) -} +const root = fileURLToPath(new URL('..', import.meta.url)) +await runCli('lint:rtl', (args) => check({ root, update: args.includes('--update'), list: args.includes('--list') })) diff --git a/client/tests/unit/scripts/rtl-lint.test.ts b/client/tests/unit/scripts/rtl-lint.test.ts new file mode 100644 index 000000000..12452aa2b --- /dev/null +++ b/client/tests/unit/scripts/rtl-lint.test.ts @@ -0,0 +1,102 @@ +import { spawnSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { RatchetError } from '../../../scripts/lib/ratchet.mjs'; +import { check, inspect, physicalUses } from '../../../scripts/lib/rtl.mjs'; +import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture'; + +let tree: RatchetTree; +afterEach(() => tree?.remove()); + +const USES = 'scripts/rtl-baseline.json'; +const MARKERS = 'scripts/rtl-disable-baseline.json'; + +function run( + uses: Record, + markers: Record, + files: Record, + update = false +) { + tree = ratchetTree({ [USES]: JSON.stringify(uses), [MARKERS]: JSON.stringify(markers), ...files }); + return check({ + root: tree.root, + baselinePath: tree.path(USES), + disableBaselinePath: tree.path(MARKERS), + update, + ...tree.out, + }); +} + +describe('lint:rtl', () => { + it('RTL-001: counts physical classes and styles, not the logical ones or prose in comments', () => { + const source = [ + '// the top right corner', + 'const a =
', + ].join('\n'); + expect(physicalUses(source, 'a.tsx')).toEqual(['2: ml-2', '2: text-left', '2: paddingRight:']); + expect(physicalUses('.a { margin-left: 2px; margin-inline-end: 1px }', 'a.css')).toEqual(['1: margin-left:']); + }); + + it('RTL-002: honours the marker in a comment and counts the line', () => { + const source = + "const s = { left: '50%', /* rtl-lint-disable: centred */ top: 0 }\nconst t = { right: 1 } // rtl-lint-disable\n"; + expect(inspect(source, 'a.tsx')).toEqual({ uses: [], markers: 2 }); + }); + + it('RTL-003: a marker inside a string is no marker', () => { + const source = "const s = { left: 0, note: 'rtl-lint-disable' }\n"; + expect(inspect(source, 'a.tsx')).toEqual({ uses: ['1: left:'], markers: 0 }); + }); + + it('RTL-004: fails a file that gains a physical side', () => { + expect(run({ 'a.tsx': 1 }, {}, { 'src/a.tsx': 'const a =
\n' })).toBe(1); + expect(tree.error.join('\n')).toMatch(/a\.tsx: 2 physical side\(s\), baseline 1/); + }); + + it('RTL-005: fails a file that gains a disable marker, and passes one within its entry', () => { + const marked = 'const s = { left: 0 } // rtl-lint-disable\n'; + expect(run({}, {}, { 'src/a.tsx': marked })).toBe(1); + expect(tree.error.join('\n')).toMatch(/a\.tsx: 1 line\(s\) marked rtl-lint-disable/); + tree.remove(); + expect(run({}, { 'a.tsx': 1 }, { 'src/a.tsx': marked })).toBe(0); + }); + + it('RTL-006: --update lowers both baselines and never raises them', () => { + run( + { 'a.tsx': 3, 'b.tsx': 1 }, + { 'a.tsx': 2 }, + { + 'src/a.tsx': 'const a =
\nconst s = { left: 0 } // rtl-lint-disable\n', + 'src/b.tsx': 'const b =
\n', + }, + true + ); + expect(JSON.parse(readFileSync(tree.path(USES), 'utf8'))).toEqual({ 'a.tsx': 1, 'b.tsx': 1 }); + expect(JSON.parse(readFileSync(tree.path(MARKERS), 'utf8'))).toEqual({ 'a.tsx': 1 }); + }); + + it('RTL-007: a missing or broken baseline stops the check instead of reading as empty', () => { + tree = ratchetTree({ [USES]: '{}', 'src/a.tsx': 'const a = 1\n' }); + expect(() => + check({ root: tree.root, baselinePath: tree.path(USES), disableBaselinePath: tree.path(MARKERS), ...tree.out }) + ).toThrow(RatchetError); + tree.write({ [USES]: '{ "a.tsx": ', [MARKERS]: '{}' }); + expect(() => + check({ root: tree.root, baselinePath: tree.path(USES), disableBaselinePath: tree.path(MARKERS), ...tree.out }) + ).toThrow(/not valid JSON/); + }); + + it('RTL-008: the command runs its check whichever way it is started', { timeout: 60_000 }, () => { + // It used to compare its own path with argv[1] and do nothing, exit 0, when they differed. + const scripts = resolve('scripts'); + for (const [cwd, arg] of [ + [process.cwd(), join(scripts, 'rtl-lint.mjs')], + [scripts, 'rtl-lint.mjs'], + ]) { + const result = spawnSync(process.execPath, [arg], { cwd, encoding: 'utf8' }); + expect(result.stdout).toMatch(/^rtl: \d+ physical side\(s\)/m); + expect(result.status).toBe(0); + } + }); +});