From 81abb2e31afad5013f0930fce5afffcdc81fc36f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Rodr=C3=ADguez=20Mart=C3=ADnez?= Date: Thu, 10 Sep 2026 12:52:08 -0400 Subject: [PATCH 1/6] (lint) restore real lint coverage across every source file class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R13. The audit named the Writing Insights exclusion at eslint.config.mjs:12. The gap was wider: `eslint .` reached 131 files and applied exactly 8 rules to each, all eslint-plugin-astro deprecated-API checks. Flat config claims no .ts file unless a block names it, so every TypeScript file in src/ was skipped silently ("File ignored because no matching configuration was supplied"), and .js/.mjs files got the Astro rules and no JavaScript rules at all. A green lint run established almost nothing. The config now covers 214 files with a correctness rule set on .js, .mjs, .cjs, .ts, and .astro, and no source directory is excluded. Dropping the Writing Insights exclusion surfaced one real defect: an unescaped `>` in insights.astro:621 that the Astro compiler tolerates but astro-eslint-parser cannot parse. Rendered output is unchanged. Unused symbols are split by language. Core no-unused-vars cannot read TypeScript type positions and reported all 45 parameter names in function types as unused arguments, with no true positives; it now runs on plain JavaScript only, where TypeScript does not look. noUnusedLocals and noUnusedParameters are enabled in tsconfig.json so astro check covers TypeScript. Both were already clean. Two rules are deliberately omitted and documented in the config: no-undef, which needs a globals list the repo does not depend on and which TypeScript already covers, and no-unmodified-loop-condition, which cannot see a Date mutated through setMonth and reads brainScience/data.ts as an infinite loop. Verified by planting a deliberate violation in each file class — duplicate key in a .ts, unused variable in an .mjs, invalid typeof comparison in public/sw.js, duplicate case label in a Writing Insights .astro, and an unused local plus parameter for the TypeScript half — confirming each was reported, then reverting. Type-aware rules remain uncovered; they need typescript-eslint as a new devDependency and are out of scope here. Local gate: 261 tests, astro check (0/0/0), ESLint, Prettier, and build all pass. Co-Authored-By: Claude Opus 5 --- docs/TECHNICAL-AUDIT.md | 26 +++++- eslint.config.mjs | 106 ++++++++++++++++++++-- src/pages/writing-insights/insights.astro | 4 +- tsconfig.json | 4 +- 4 files changed, 125 insertions(+), 15 deletions(-) diff --git a/docs/TECHNICAL-AUDIT.md b/docs/TECHNICAL-AUDIT.md index 90ee3501..f7d7a274 100644 --- a/docs/TECHNICAL-AUDIT.md +++ b/docs/TECHNICAL-AUDIT.md @@ -122,8 +122,8 @@ No accounts. No server-side reading progress. Constitution principle IV applies. | Gate | Status | | ----------------------------------- | -------------------------------------------------------- | | `pnpm run format:check` | CI | -| `pnpm run check` | CI | -| `pnpm run lint` | CI | +| `pnpm run check` | CI — also owns unused locals/params (see below) | +| `pnpm run lint` | CI — coverage detailed below | | `pnpm run build` | CI (+ social image step) | | `pnpm run validate-feeds` | CI, after the build (needs `dist/`) | | `pnpm run audit-frontmatter` | CI, before the build — walks `src/content/p` | @@ -132,6 +132,28 @@ No accounts. No server-side reading progress. Constitution principle IV applies. | Unit tests (`pnpm test`) | CI — see §9 for coverage | | Browser / e2e tests | **None** — Playwright is installed but unconfigured | +**Lint coverage (corrected 2026-09-10).** Before this pass `eslint .` reached 131 +files and applied exactly 8 rules to each — all `eslint-plugin-astro` deprecated-API +checks. `.ts` files matched no config block at all and were skipped silently, plain +`.js`/`.mjs` got the Astro rules and no JavaScript rules, and +`src/pages/writing-insights/**` was excluded outright. A green `pnpm run lint` +therefore established very little. + +It now reaches 214 files with a correctness rule set applied to `.js`, `.mjs`, +`.cjs`, `.ts`, and `.astro`, and no source directory is excluded. Removing the +Writing Insights exclusion surfaced one real defect: an unescaped `>` in +`insights.astro` that the Astro compiler tolerates but `astro-eslint-parser` +cannot parse. + +Unused symbols are split by language on purpose. Core `no-unused-vars` cannot read +TypeScript type positions — it reports every parameter name in a function type as +an unused argument — so it runs on plain JavaScript only, and `noUnusedLocals` / +`noUnusedParameters` in `tsconfig.json` cover TypeScript through `astro check`. +Both halves were confirmed by planting a deliberate violation in each file class. + +Still uncovered: type-aware lint rules. Adding `typescript-eslint` would bring +them, at the cost of a new devDependency. + --- ## 8. Findings — closed this pass diff --git a/eslint.config.mjs b/eslint.config.mjs index e84bd586..800dd604 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,17 +1,103 @@ import eslintPluginAstro from 'eslint-plugin-astro'; +import tsParser from '@typescript-eslint/parser'; + +/** + * Correctness-focused core rules. These are ESLint built-ins, so they need no + * extra plugin package. + * + * Two rules are deliberately absent from this shared set: + * + * - `no-undef` needs a per-environment globals list to be accurate, and the + * `globals` package is not a direct dependency here. TypeScript already + * reports undefined identifiers across `.ts` and `.astro` via `astro check`. + * - `no-unused-vars` is applied to plain JavaScript only, further down. The core + * rule cannot read TypeScript type positions, so it reports every parameter + * name in a function *type* — `warn: (message: string) => void` — as an unused + * argument: 45 false positives and no true ones in this repo. `noUnusedLocals` + * and `noUnusedParameters` in `tsconfig.json` cover TypeScript correctly and + * run in CI through `astro check`. + */ +const correctnessRules = { + // Default 'except-parens' on purpose: it still catches an accidental + // `if (a = b)` while permitting the parenthesized `while ((m = re.exec(s)))` + // iteration idiom the social-image script uses. + 'no-cond-assign': 'error', + 'no-constant-binary-expression': 'error', + 'no-constant-condition': ['error', { checkLoops: false }], + 'no-dupe-args': 'error', + 'no-dupe-else-if': 'error', + 'no-dupe-keys': 'error', + 'no-duplicate-case': 'error', + 'no-empty': ['error', { allowEmptyCatch: true }], + 'no-fallthrough': 'error', + 'no-func-assign': 'error', + 'no-irregular-whitespace': 'error', + 'no-self-assign': 'error', + 'no-self-compare': 'error', + 'no-sparse-arrays': 'error', + 'no-template-curly-in-string': 'error', + // 'no-unmodified-loop-condition' is left off: it cannot see a binding mutated + // through a method, so `while (d <= end) { d.setMonth(...) }` in + // brainScience/data.ts reads as an infinite loop to it. Not in ESLint's + // recommended set for the same reason. + 'no-unreachable': 'error', + 'no-unsafe-finally': 'error', + 'no-unsafe-negation': 'error', + 'no-unsafe-optional-chaining': 'error', + 'no-async-promise-executor': 'error', + 'no-compare-neg-zero': 'error', + 'use-isnan': 'error', + 'valid-typeof': 'error', +}; export default [ { - // Ignore build output, dependencies, and complex Writing Insights dashboards - ignores: [ - 'dist/**', - 'build/**', - 'coverage/**', - '**/*.min.js', - 'node_modules/**', - 'src/pages/writing-insights/**', - ], + // Build output and dependencies only. Source directories are not excluded: + // an ignored directory makes a green lint run mean less than it appears to. + ignores: ['dist/**', 'build/**', 'coverage/**', '.vercel/**', '**/*.min.js', 'node_modules/**'], }, - // Astro + JavaScript/TypeScript recommended rules for .astro files and scripts + + // Astro's own rules for .astro files. ...eslintPluginAstro.configs.recommended, + + // Plain JavaScript: scripts/, public/, and config files. TypeScript does not + // check these (`checkJs` is off), so ESLint owns unused symbols here. + { + files: ['**/*.js', '**/*.mjs', '**/*.cjs'], + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + }, + rules: { + ...correctnessRules, + 'no-unused-vars': [ + 'error', + { + args: 'after-used', + argsIgnorePattern: '^_', + varsIgnorePattern: '^_', + caughtErrors: 'none', + ignoreRestSiblings: true, + }, + ], + }, + }, + + // TypeScript. Flat config lints no .ts file unless a config block claims it, + // so without this entry every .ts file in src/ was silently skipped. + { + files: ['**/*.ts', '**/*.mts', '**/*.cts'], + languageOptions: { + parser: tsParser, + ecmaVersion: 'latest', + sourceType: 'module', + }, + rules: correctnessRules, + }, + + // The same rules inside .astro frontmatter and