diff --git a/AGENTS.md b/AGENTS.md index f1ce31e404b..0a226af20be 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,14 @@ Never use vague names like `helpers`, `utils`, `common`, `misc`, or `shared-stuf ## Type Declarations: Prefer `.ts` Over `.d.ts` +## Type Assertions: Prefer Checked Types + +Avoid type assertions except `as const`. Unavoidable casts need a line-specific `SAFETY:` explanation: + +```ts +// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Explain the verified invariant here. +``` + # Verifying Changes - **Typecheck**: `pnpm tc` (or `tc:node` / `tc:cli` / `tc:web`) diff --git a/config/oxlint-code-quality-casting.json b/config/oxlint-code-quality-casting.json new file mode 100644 index 00000000000..3db01c3a04d --- /dev/null +++ b/config/oxlint-code-quality-casting.json @@ -0,0 +1,17 @@ +{ + "$schema": "../node_modules/oxlint/configuration_schema.json", + "plugins": ["typescript"], + "categories": { + "correctness": "off", + "suspicious": "off", + "pedantic": "off", + "perf": "off", + "style": "off", + "restriction": "off", + "nursery": "off" + }, + "rules": { + "typescript/consistent-type-assertions": ["error", { "assertionStyle": "never" }] + }, + "ignorePatterns": ["**/node_modules", "**/dist", "**/out"] +} diff --git a/config/scripts/casting-code-quality.test.mjs b/config/scripts/casting-code-quality.test.mjs new file mode 100644 index 00000000000..a9be7bf7c80 --- /dev/null +++ b/config/scripts/casting-code-quality.test.mjs @@ -0,0 +1,153 @@ +import { spawnSync } from 'node:child_process' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { expect, it } from 'vitest' +import { + OXLINT_SCANS, + diagnosticTouchesAddedLines, + findCastingDirectivesMissingSafety, + isCastingDirectiveUnusedWarning +} from './check-changed-code-quality.mjs' +import { resolveOxlintInvocation } from './oxlint-cli-invocation.mjs' + +const root = path.resolve(import.meta.dirname, '..', '..') +const oxlint = resolveOxlintInvocation(root) +const rule = 'typescript(consistent-type-assertions)' +const ruleName = 'typescript/consistent-type-assertions' +// Built rather than written out so no line here is itself a casting directive the gate would scan. +const directive = (reason) => `// oxlint-disable-next-line ${ruleName} -- ${reason}` +const trailingDirective = (reason) => `// oxlint-disable-line ${ruleName} -- ${reason}` + +function lint(file, args = []) { + const result = spawnSync( + oxlint.command, + [...oxlint.prefixArgs, ...args, '--format', 'json', file], + { cwd: root, encoding: 'utf8', windowsHide: true } + ) + expect(result.error).toBeUndefined() + return { status: result.status, diagnostics: JSON.parse(result.stdout).diagnostics } +} + +it.each(['config', 'mobile'])('enforces new casts without changing full lint in %s', (parent) => { + const directory = mkdtempSync(path.join(root, parent, 'casting-lint-test-')) + const file = path.join(directory, 'fixture.test.ts') + try { + writeFileSync( + file, + [ + "export const oldCast = { current: '⌘N' as string | null }", + 'export const doubleCast = undefined as unknown as string', + "export const annotated: { current: string | null } = { current: '⌘N' }", + "export const constant = { current: '⌘N' } as const", + "export const checked = { current: '⌘N' } satisfies { current: string | null }", + directive('SAFETY: Exercise the explicit exception.'), + 'export const justified = undefined as unknown' + ].join('\n') + ) + + const full = lint(file) + expect(full.status).toBe(0) + expect(full.diagnostics.filter((diagnostic) => diagnostic.code === rule)).toEqual([]) + + const scan = OXLINT_SCANS.find((candidate) => candidate.label === 'casting code quality') + expect(scan).toBeDefined() + const casting = lint(file, scan.args) + expect(casting.status).toBe(1) + expect(casting.diagnostics).toHaveLength(3) + expect(casting.diagnostics.every((diagnostic) => diagnostic.code === rule)).toBe(true) + + const relative = path.relative(root, file).split(path.sep).join('/') + const changed = new Map([[relative, [{ start: 2, end: 2 }]]]) + const findings = casting.diagnostics.filter((diagnostic) => + diagnosticTouchesAddedLines(diagnostic, changed, root) + ) + expect(findings).toHaveLength(2) + expect(findings.every((diagnostic) => diagnostic.severity === 'error')).toBe(true) + + writeFileSync(file, 'export const angle = undefined\n') + expect(lint(file).status).toBe(1) + expect(lint(file, scan.args).diagnostics.map((diagnostic) => diagnostic.code)).toEqual([rule]) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +}) + +it("exempts the SAFETY: directive from the untyped scan's unused-directive warning", () => { + const directory = mkdtempSync(path.join(root, 'config', 'casting-lint-test-')) + const file = path.join(directory, 'fixture.test.ts') + try { + writeFileSync( + file, + [ + directive('SAFETY: Verified invariant.'), + 'export const justified = undefined as unknown', + '' + ].join('\n') + ) + + const scan = OXLINT_SCANS.find((candidate) => candidate.label === 'code quality') + const untyped = lint(file, scan.args) + const unused = untyped.diagnostics.filter((diagnostic) => + diagnostic.message.startsWith('Unused oxlint-disable directive') + ) + + expect(unused).toHaveLength(1) + expect(unused.every((diagnostic) => isCastingDirectiveUnusedWarning(diagnostic, root))).toBe( + true + ) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +}) + +it('rejects a casting suppression on an added line that omits the SAFETY: rationale', () => { + const directory = mkdtempSync(path.join(root, 'config', 'casting-lint-test-')) + const file = path.join(directory, 'fixture.test.ts') + try { + writeFileSync( + file, + [ + directive('no required prefix'), + 'export const unchecked = undefined as unknown', + directive('SAFETY: Verified invariant.'), + 'export const justified = undefined as unknown', + '' + ].join('\n') + ) + + const relative = path.relative(root, file).split(path.sep).join('/') + const findings = findCastingDirectivesMissingSafety( + root, + new Map([[relative, [{ start: 1, end: 4 }]]]) + ) + + expect(findings.map((finding) => finding.labels[0].span.line)).toEqual([1]) + + // Unchanged lines stay out of the gate. + expect( + findCastingDirectivesMissingSafety(root, new Map([[relative, [{ start: 3, end: 4 }]]])) + ).toEqual([]) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +}) + +// Why: an earlier pattern skipped a directive whose `//` sat right after a quote, which let an +// unjustified cast through the gate -- the wrong failure direction for a gate. +it('catches a trailing casting suppression that abuts a string literal', () => { + const directory = mkdtempSync(path.join(root, 'config', 'casting-lint-test-')) + const file = path.join(directory, 'fixture.test.ts') + try { + writeFileSync(file, `export const abutted = 'a'${trailingDirective('no required prefix')}\n`) + + const relative = path.relative(root, file).split(path.sep).join('/') + const findings = findCastingDirectivesMissingSafety( + root, + new Map([[relative, [{ start: 1, end: 1 }]]]) + ) + + expect(findings.map((finding) => finding.labels[0].span.line)).toEqual([1]) + } finally { + rmSync(directory, { recursive: true, force: true }) + } +}) diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index af4e9e82776..a1b5b2fc88a 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -8,6 +8,9 @@ import { resolveOxlintInvocation } from './oxlint-cli-invocation.mjs' const SOURCE_FILE_PATTERN = /\.(?:[cm]?[jt]sx?)$/ const ROOT_CODE_QUALITY_IGNORED_PREFIXES = ['cloud/'] +const CASTING_RULE = 'typescript/consistent-type-assertions' +const CASTING_DISABLE_PATTERN = + /\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*typescript\/consistent-type-assertions/ export const OXLINT_SCANS = [ { // Why: no --config, so Oxlint keeps discovering nested configs. Pinning the root @@ -15,6 +18,10 @@ export const OXLINT_SCANS = [ label: 'code quality', args: ['--report-unused-disable-directives-severity', 'warn'] }, + { + label: 'casting code quality', + args: ['--config', 'config/oxlint-code-quality-casting.json'] + }, { label: 'type-aware code quality', args: ['--type-aware', '--config', 'config/oxlint-code-quality-type-aware.json'] @@ -303,6 +310,50 @@ function printDiagnostic(diagnostic, root) { console.error(`${file}:${line} ${code}: ${diagnostic.message}`) } +// Why: only the casting scan enforces `assertionStyle: never`, so under the root config an +// `as` cast is legal and the SAFETY: directive AGENTS.md mandates reads as unused. The untyped +// scan reports that as a warning, which the gate counts, so exempt exactly those directives. +export function isCastingDirectiveUnusedWarning(diagnostic, root) { + if (!/^Unused (?:oxlint|eslint)-disable/.test(diagnostic.message ?? '')) { + return false + } + return (diagnostic.labels ?? []).some((label) => + diagnosticHighlightedLines(root, diagnostic.filename, label.span).some((line) => + CASTING_DISABLE_PATTERN.test(line) + ) + ) +} + +// Why: oxlint cannot see the AGENTS.md requirement that every casting suppression carry a +// line-specific SAFETY: rationale, so the directive text itself is checked over added lines. +export function findCastingDirectivesMissingSafety(root, rangesByFile) { + const findings = [] + for (const [file, ranges] of rangesByFile) { + const absolutePath = path.join(root, file) + if (!existsSync(absolutePath)) { + continue + } + readFileSync(absolutePath, 'utf8') + .split(/\r?\n/) + .forEach((text, index) => { + const line = index + 1 + if ( + CASTING_DISABLE_PATTERN.test(text) && + !text.includes('SAFETY:') && + overlapsAddedLines(line, line, ranges) + ) { + findings.push({ + filename: file, + code: `${CASTING_RULE} (missing SAFETY:)`, + message: `Suppressing ${CASTING_RULE} requires a line-specific "SAFETY:" explanation.`, + labels: [{ span: { line } }] + }) + } + }) + } + return findings +} + function isSuppressedDiagnostic(diagnostic, root) { const files = SUPPRESSED_REACT_DOCTOR_DIAGNOSTICS.get(diagnostic.code) return files?.has(normalizedDiagnosticPath(root, diagnostic.filename)) ?? false @@ -344,6 +395,7 @@ export function main( const diagnostics = runOxlintScan(root, scan, files).filter( (diagnostic) => !isSuppressedDiagnostic(diagnostic, root) && + !isCastingDirectiveUnusedWarning(diagnostic, root) && diagnosticTouchesAddedLines(diagnostic, rangesByFile, root, baseBlocks) ) for (const diagnostic of diagnostics) { @@ -355,6 +407,15 @@ export function main( ) } + const missingSafety = findCastingDirectivesMissingSafety(root, rangesByFile) + for (const diagnostic of missingSafety) { + printDiagnostic(diagnostic, root) + } + failures += missingSafety.length + console.log( + `casting SAFETY: rationale: ${missingSafety.length} new finding(s) across ${files.length} changed file(s).` + ) + if (failures > 0) { console.error( `Changed-code quality gate failed with ${failures} finding(s) since ${comparisonBase.slice(0, 12)}.`