mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
Add code quality lint for type assertions (#19462)
* Add casting code quality lint scan Enforce type assertion style by adding a new oxlint scan with `typescript/consistent-type-assertions` rule. Requires using `as const`, type annotations, or `satisfies` instead of raw type casts, with documented `SAFETY:` exceptions for unavoidable cases. * fix minor issue
This commit is contained in:
@@ -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`)
|
||||
|
||||
@@ -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"]
|
||||
}
|
||||
@@ -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 = <string>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 })
|
||||
}
|
||||
})
|
||||
@@ -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)}.`
|
||||
|
||||
Reference in New Issue
Block a user