From 9b0cb388976cb994b4da7c9d9437616b5e141bee Mon Sep 17 00:00:00 2001 From: Lukas Date: Fri, 10 Apr 2026 16:17:30 +0200 Subject: [PATCH] Fix oxlint --deny-warnings and eliminate all biome-ignores --deny warnings was a no-op (not a valid category); the correct flag is --deny-warnings. Fixed all 8 pre-existing warnings and removed every biome-ignore from source and test files. Simplified the check script to zero-tolerance: any biome-ignore now fails the build. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/app-integration.test.tsx | 3 +- .../web/src/__tests__/confirm-button.test.tsx | 30 ++++--- .../__tests__/source-manager.test.tsx | 10 +-- apps/web/src/hooks/use-encounter.ts | 5 +- package.json | 2 +- .../domain/src/__tests__/conditions.test.ts | 4 +- packages/domain/src/auto-number.ts | 49 ++++++----- scripts/check-lint-ignores.mjs | 87 +++---------------- 8 files changed, 65 insertions(+), 125 deletions(-) diff --git a/apps/web/src/__tests__/app-integration.test.tsx b/apps/web/src/__tests__/app-integration.test.tsx index b921a5c..d61bad5 100644 --- a/apps/web/src/__tests__/app-integration.test.tsx +++ b/apps/web/src/__tests__/app-integration.test.tsx @@ -33,8 +33,7 @@ async function addCombatant( opts?: { maxHp?: string }, ) { const inputs = screen.getAllByPlaceholderText("+ Add combatants"); - // biome-ignore lint/style/noNonNullAssertion: getAllBy always returns at least one - const input = inputs.at(-1)!; + const input = inputs.at(-1) ?? inputs[0]; await user.type(input, name); if (opts?.maxHp) { diff --git a/apps/web/src/__tests__/confirm-button.test.tsx b/apps/web/src/__tests__/confirm-button.test.tsx index 452bd7b..92b31d7 100644 --- a/apps/web/src/__tests__/confirm-button.test.tsx +++ b/apps/web/src/__tests__/confirm-button.test.tsx @@ -198,21 +198,23 @@ describe("ConfirmButton", () => { it("Enter/Space keydown stops propagation to prevent parent handlers", () => { const parentHandler = vi.fn(); - render( - // biome-ignore lint/a11y/noStaticElementInteractions: test wrapper - // biome-ignore lint/a11y/noNoninteractiveElementInteractions: test wrapper -
- } - label="Remove combatant" - onConfirm={vi.fn()} - /> -
, - ); - const button = screen.getByRole("button"); + function Wrapper() { + return ( + + ); + } + render(); + const buttons = screen.getAllByRole("button"); + const confirmButton = buttons.at(-1) ?? buttons[0]; - fireEvent.keyDown(button, { key: "Enter" }); - fireEvent.keyDown(button, { key: " " }); + fireEvent.keyDown(confirmButton, { key: "Enter" }); + fireEvent.keyDown(confirmButton, { key: " " }); expect(parentHandler).not.toHaveBeenCalled(); }); diff --git a/apps/web/src/components/__tests__/source-manager.test.tsx b/apps/web/src/components/__tests__/source-manager.test.tsx index 2c85a49..d5315eb 100644 --- a/apps/web/src/components/__tests__/source-manager.test.tsx +++ b/apps/web/src/components/__tests__/source-manager.test.tsx @@ -28,7 +28,7 @@ beforeAll(() => { afterEach(cleanup); -function renderWithSources(sources: CachedSourceInfo[] = []) { +function renderWithSources(sources: CachedSourceInfo[] = []): void { const adapters = createTestAdapters(); // Wire getCachedSources to return the provided sources initially, // then empty after clear operations @@ -57,14 +57,14 @@ function renderWithSources(sources: CachedSourceInfo[] = []) { describe("SourceManager", () => { it("shows 'No cached sources' empty state when no sources", async () => { - void renderWithSources([]); + renderWithSources([]); await waitFor(() => { expect(screen.getByText("No cached sources")).toBeInTheDocument(); }); }); it("lists cached sources with display name and creature count", async () => { - void renderWithSources([ + renderWithSources([ { sourceCode: "mm", displayName: "Monster Manual", @@ -88,7 +88,7 @@ describe("SourceManager", () => { it("Clear All button removes all sources", async () => { const user = userEvent.setup(); - void renderWithSources([ + renderWithSources([ { sourceCode: "mm", displayName: "Monster Manual", @@ -110,7 +110,7 @@ describe("SourceManager", () => { it("individual source delete button removes that source", async () => { const user = userEvent.setup(); - void renderWithSources([ + renderWithSources([ { sourceCode: "mm", displayName: "Monster Manual", diff --git a/apps/web/src/hooks/use-encounter.ts b/apps/web/src/hooks/use-encounter.ts index 1ed0749..3c84b7c 100644 --- a/apps/web/src/hooks/use-encounter.ts +++ b/apps/web/src/hooks/use-encounter.ts @@ -421,7 +421,10 @@ function dispatchEncounterAction( export function useEncounter() { const { encounterPersistence, undoRedoPersistence } = useAdapters(); const [state, dispatch] = useReducer(encounterReducer, null, () => - initializeState(encounterPersistence.load, undoRedoPersistence.load), + initializeState( + () => encounterPersistence.load(), + () => undoRedoPersistence.load(), + ), ); const { encounter, undoRedoState, events } = state; diff --git a/package.json b/package.json index cc224fe..1499f74 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "knip": "knip", "jscpd": "jscpd", "jsinspect": "jsinspect -c .jsinspectrc apps/web/src packages/domain/src packages/application/src", - "oxlint": "oxlint --tsconfig apps/web/tsconfig.json --type-aware --deny warnings", + "oxlint": "oxlint --tsconfig apps/web/tsconfig.json --type-aware --deny-warnings", "check:ignores": "node scripts/check-lint-ignores.mjs", "check:classnames": "node scripts/check-cn-classnames.mjs", "check:props": "node scripts/check-component-props.mjs", diff --git a/packages/domain/src/__tests__/conditions.test.ts b/packages/domain/src/__tests__/conditions.test.ts index e2d27ee..ef86872 100644 --- a/packages/domain/src/__tests__/conditions.test.ts +++ b/packages/domain/src/__tests__/conditions.test.ts @@ -47,8 +47,8 @@ describe("getConditionDescription", () => { (d.systems.includes("5e") && d.systems.includes("5.5e")), ); for (const def of sharedDndConditions) { - expect(def.description, `${def.id} missing description`).toBeTruthy(); - expect(def.description5e, `${def.id} missing description5e`).toBeTruthy(); + expect(def.description).toBeTruthy(); + expect(def.description5e).toBeTruthy(); } }); diff --git a/packages/domain/src/auto-number.ts b/packages/domain/src/auto-number.ts index 17c2c18..1ff1d66 100644 --- a/packages/domain/src/auto-number.ts +++ b/packages/domain/src/auto-number.ts @@ -1,3 +1,28 @@ +const DIGITS_ONLY = /^\d+$/; + +function scanExisting( + baseName: string, + existingNames: readonly string[], +): { exactMatches: number[]; maxNumber: number } { + const exactMatches: number[] = []; + let maxNumber = 0; + const prefix = `${baseName} `; + + for (let i = 0; i < existingNames.length; i++) { + const name = existingNames[i]; + if (name === baseName) { + exactMatches.push(i); + } else if (name.startsWith(prefix)) { + const suffix = name.slice(prefix.length); + if (DIGITS_ONLY.test(suffix)) { + const num = Number.parseInt(suffix, 10); + if (num > maxNumber) maxNumber = num; + } + } + } + return { exactMatches, maxNumber }; +} + /** * Resolves a creature name against existing combatant names, * handling auto-numbering for duplicates. @@ -14,25 +39,7 @@ export function resolveCreatureName( newName: string; renames: ReadonlyArray<{ from: string; to: string }>; } { - // Find exact matches and numbered matches (e.g., "Goblin 1", "Goblin 2") - const exactMatches: number[] = []; - let maxNumber = 0; - - for (let i = 0; i < existingNames.length; i++) { - const name = existingNames[i]; - if (name === baseName) { - exactMatches.push(i); - } else { - const match = new RegExp( - String.raw`^${escapeRegExp(baseName)} (\d+)$`, - ).exec(name); - // biome-ignore lint/nursery/noUnnecessaryConditions: RegExp.exec() returns null on no match — false positive - if (match) { - const num = Number.parseInt(match[1], 10); - if (num > maxNumber) maxNumber = num; - } - } - } + const { exactMatches, maxNumber } = scanExisting(baseName, existingNames); // No conflict at all if (exactMatches.length === 0 && maxNumber === 0) { @@ -51,7 +58,3 @@ export function resolveCreatureName( const nextNumber = Math.max(maxNumber, exactMatches.length) + 1; return { newName: `${baseName} ${nextNumber}`, renames: [] }; } - -function escapeRegExp(s: string): string { - return s.replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`); -} diff --git a/scripts/check-lint-ignores.mjs b/scripts/check-lint-ignores.mjs index 5755cf7..b6c95c4 100644 --- a/scripts/check-lint-ignores.mjs +++ b/scripts/check-lint-ignores.mjs @@ -1,29 +1,14 @@ /** - * Backpressure check for biome-ignore comments. + * Zero-tolerance check for biome-ignore comments. * - * 1. Ratcheting cap — source and test files have separate max counts. - * Lower these numbers as you fix ignores; they can never go up silently. - * 2. Banned rules — ignoring certain rule categories is never allowed. - * 3. Justification — every ignore must have a non-empty explanation after - * the rule name. + * Any `biome-ignore` in tracked .ts/.tsx files fails the build. + * Fix the underlying issue instead of suppressing the rule. */ import { execSync } from "node:child_process"; import { readFileSync } from "node:fs"; -// ── Configuration ────────────────────────────────────────────────────── -const MAX_SOURCE_IGNORES = 2; -const MAX_TEST_IGNORES = 3; - -/** Rule prefixes that must never be suppressed. */ -const BANNED_PREFIXES = [ - "lint/security/", - "lint/correctness/noGlobalObjectCalls", - "lint/correctness/noUnsafeFinally", -]; -// ─────────────────────────────────────────────────────────────────────── - -const IGNORE_PATTERN = /biome-ignore\s+([\w/]+)(?::\s*(.*))?/; +const IGNORE_PATTERN = /biome-ignore\s+([\w/]+)/; function findFiles() { return execSync("git ls-files -- '*.ts' '*.tsx'", { encoding: "utf-8" }) @@ -32,17 +17,7 @@ function findFiles() { .filter(Boolean); } -function isTestFile(path) { - return ( - path.includes("__tests__/") || - path.endsWith(".test.ts") || - path.endsWith(".test.tsx") - ); -} - -let errors = 0; -let sourceCount = 0; -let testCount = 0; +let count = 0; for (const file of findFiles()) { const lines = readFileSync(file, "utf-8").split("\n"); @@ -51,58 +26,16 @@ for (const file of findFiles()) { const match = lines[i].match(IGNORE_PATTERN); if (!match) continue; - const rule = match[1]; - const justification = (match[2] ?? "").trim(); - const loc = `${file}:${i + 1}`; - - // Count by category - if (isTestFile(file)) { - testCount++; - } else { - sourceCount++; - } - - // Banned rules - for (const prefix of BANNED_PREFIXES) { - if (rule.startsWith(prefix)) { - console.error(`BANNED: ${loc} — ${rule} must not be suppressed`); - errors++; - } - } - - // Justification required - if (!justification) { - console.error( - `MISSING JUSTIFICATION: ${loc} — biome-ignore ${rule} needs an explanation after the colon`, - ); - errors++; - } + count++; + console.error(`FORBIDDEN: ${file}:${i + 1} — biome-ignore ${match[1]}`); } } -// Ratcheting caps -if (sourceCount > MAX_SOURCE_IGNORES) { +if (count > 0) { console.error( - `SOURCE CAP EXCEEDED: ${sourceCount} biome-ignore comments in source (max ${MAX_SOURCE_IGNORES}). Fix issues and lower the cap.`, + `\n${count} biome-ignore comment(s) found. Fix the issue or restructure the code.`, ); - errors++; -} - -if (testCount > MAX_TEST_IGNORES) { - console.error( - `TEST CAP EXCEEDED: ${testCount} biome-ignore comments in tests (max ${MAX_TEST_IGNORES}). Fix issues and lower the cap.`, - ); - errors++; -} - -// Summary -console.log( - `biome-ignore: ${sourceCount} source (max ${MAX_SOURCE_IGNORES}), ${testCount} test (max ${MAX_TEST_IGNORES})`, -); - -if (errors > 0) { - console.error(`\n${errors} problem(s) found.`); process.exit(1); } else { - console.log("All checks passed."); + console.log("biome-ignore: 0 — all clear."); }