mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
ci(client): lint:warnings fails an inline rule config comment
A block comment such as /* eslint @typescript-eslint/no-explicit-any: off */ turns a rule off for its file without leaving a message behind, neither among the warnings nor among the suppressed ones, so a new file could switch any warning rule off and the ratchet stayed green. eslint-disable comments were already counted under "suppressed"; this was the way around them. lintClient now reads the block comments of every file ESLint linted from the TypeScript parser, so the same text in a string, a template or a regex does not count, and the tally reports each config comment as an error. Nothing in the client uses one today, so the check is zero tolerance. The tests baseline drops by one no-unused-vars warning that is gone.
This commit is contained in:
+1
-1
@@ -9,7 +9,7 @@ npm run dev # Vite dev server; proxies the API/ws/uploads/MCP/OAut
|
||||
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:warnings # eslint, failing on any error and on any warning count per rule above scripts/eslint-baseline.json (app code, tests and eslint-disable'd messages counted apart; --update lowers it)
|
||||
npm run lint:warnings # eslint, failing on any error and on any warning count per rule above scripts/eslint-baseline.json (app code, tests and eslint-disable'd messages counted apart; a `/* eslint <rule>: ... */` config comment fails outright; --update lowers it)
|
||||
npm run lint:pages # enforce the Page pattern (CI gate)
|
||||
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 (Playwright specs and fixtures under e2e/ included) past 2000, counted wrapped at 120 columns; longer ones may only shrink (CI gate; --update lowers the baseline)
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
"@typescript-eslint/no-explicit-any": 494,
|
||||
"@typescript-eslint/no-this-alias": 1,
|
||||
"@typescript-eslint/no-unsafe-function-type": 2,
|
||||
"@typescript-eslint/no-unused-vars": 54,
|
||||
"@typescript-eslint/no-unused-vars": 53,
|
||||
"react-refresh/only-export-components": 1
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,7 +9,10 @@
|
||||
* client and counts the warnings per rule, separately for app code and
|
||||
* tests, and the messages an eslint-disable comment silenced as a third
|
||||
* group. Each count may not pass its entry in scripts/eslint-baseline.json.
|
||||
* Any error fails as well, so the check can stand in for `npm run lint:check`.
|
||||
* Any error fails as well, so the check can stand in for `npm run lint:check`,
|
||||
* and so does a block comment that configures a rule inline (such as one that
|
||||
* turns no-explicit-any off): it leaves no message to count, so it would
|
||||
* otherwise be the free way past the ratchet.
|
||||
*
|
||||
* npm run lint:warnings check against the baseline (CI)
|
||||
* npm run lint:warnings -- --update lower the baseline to today's counts;
|
||||
|
||||
@@ -3,12 +3,14 @@
|
||||
*/
|
||||
import { ESLint } from 'eslint';
|
||||
import { join } from 'node:path';
|
||||
import { countMap, lowerCounts, readBaseline, TEST_FILE, toKey, writeBaseline } from './ratchet.mjs';
|
||||
import ts from 'typescript';
|
||||
import { countMap, lowerCounts, readBaseline, readText, TEST_FILE, toKey, writeBaseline } from './ratchet.mjs';
|
||||
|
||||
/**
|
||||
* Where a warning is counted: in the app code, in the tests, or, for a
|
||||
* message an eslint-disable comment silenced, among the suppressed ones. An
|
||||
* inline disable must not be the cheap way under the ratchet.
|
||||
* inline disable must not be the cheap way under the ratchet, and an inline
|
||||
* rule config, which silences without a trace, fails outright (see tally).
|
||||
*/
|
||||
export const AREAS = ['src', 'tests', 'suppressed'];
|
||||
|
||||
@@ -30,9 +32,55 @@ export function areaCounts(value) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const scriptKind = (file) =>
|
||||
file.endsWith('.tsx')
|
||||
? ts.ScriptKind.TSX
|
||||
: /\.[cm]?ts$/.test(file)
|
||||
? ts.ScriptKind.TS
|
||||
: file.endsWith('.jsx')
|
||||
? ts.ScriptKind.JSX
|
||||
: ts.ScriptKind.JS;
|
||||
|
||||
/** ESLint's label for a block comment that configures rules, as opposed to eslint-disable and the like. */
|
||||
const INLINE_CONFIG = /^\s*eslint(?:\s|$)/;
|
||||
|
||||
/**
|
||||
* The block comments in a file that configure ESLint rules, such as
|
||||
* `/* eslint no-empty: off *\/`, as `line:column comment`. Such a comment
|
||||
* switches a rule off for the whole file without a single message reaching
|
||||
* either the warnings or the suppressed ones, so it would walk past the
|
||||
* ratchet unseen. The comments come from the TypeScript parser, so the same
|
||||
* text inside a string or a template does not count.
|
||||
*/
|
||||
export function inlineConfigsOf(source, file) {
|
||||
const sf = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, scriptKind(file));
|
||||
const seen = new Set();
|
||||
const found = [];
|
||||
const collect = (ranges) => {
|
||||
for (const range of ranges ?? []) {
|
||||
if (seen.has(range.pos)) continue;
|
||||
seen.add(range.pos);
|
||||
if (range.kind !== ts.SyntaxKind.MultiLineCommentTrivia) continue;
|
||||
const text = source.slice(range.pos, range.end);
|
||||
if (!INLINE_CONFIG.test(text.slice(2, -2))) continue;
|
||||
const { line, character } = sf.getLineAndCharacterOfPosition(range.pos);
|
||||
found.push(`${line + 1}:${character + 1} ${text.replace(/\s+/g, ' ').slice(0, 80)}`);
|
||||
}
|
||||
};
|
||||
const visit = (node) => {
|
||||
collect(ts.getLeadingCommentRanges(source, node.pos));
|
||||
collect(ts.getTrailingCommentRanges(source, node.pos));
|
||||
for (const child of node.getChildren(sf)) visit(child);
|
||||
};
|
||||
visit(sf);
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* The warnings per area and rule in ESLint's results, the files behind each
|
||||
* count, and every error (an error, a parse failure included, always fails).
|
||||
* An inline rule config comment (see inlineConfigsOf) is an error too: the
|
||||
* place to change a rule is eslint.config.mjs.
|
||||
*/
|
||||
export function tally(results, root) {
|
||||
const counts = Object.fromEntries(AREAS.map((area) => [area, {}]));
|
||||
@@ -45,6 +93,8 @@ export function tally(results, root) {
|
||||
};
|
||||
for (const result of results) {
|
||||
const key = toKey(root, result.filePath);
|
||||
for (const comment of result.inlineConfigs ?? [])
|
||||
errors.push(`${key}:${comment} configures a rule inline; set it in eslint.config.mjs (inline-config)`);
|
||||
for (const message of result.messages) {
|
||||
if (message.fatal || message.severity === 2)
|
||||
errors.push(`${key}:${message.line ?? 0}:${message.column ?? 0} ${message.message} (${ruleOf(message)})`);
|
||||
@@ -59,17 +109,22 @@ export function tally(results, root) {
|
||||
* The part of an ESLint result the tally reads.
|
||||
*
|
||||
* @typedef {{ ruleId: string | null, severity: number, message: string, fatal?: boolean, line?: number, column?: number }} LintMessage
|
||||
* @typedef {{ filePath: string, messages: LintMessage[], suppressedMessages?: LintMessage[] }} LintResult
|
||||
* @typedef {{ filePath: string, messages: LintMessage[], suppressedMessages?: LintMessage[], inlineConfigs?: string[] }} LintResult
|
||||
*/
|
||||
|
||||
/**
|
||||
* ESLint over the whole client, with its own config, as `npm run lint:check` runs it.
|
||||
* ESLint over the whole client, with its own config, as `npm run lint:check`
|
||||
* runs it, each result with the inline rule configs of its file.
|
||||
*
|
||||
* @param {string} root
|
||||
* @returns {Promise<LintResult[]>}
|
||||
*/
|
||||
export async function lintClient(root) {
|
||||
return new ESLint({ cwd: root }).lintFiles(['.']);
|
||||
const results = await new ESLint({ cwd: root }).lintFiles(['.']);
|
||||
return results.map((result) => ({
|
||||
...result,
|
||||
inlineConfigs: inlineConfigsOf(readText(result.filePath), result.filePath),
|
||||
}));
|
||||
}
|
||||
|
||||
const sum = (map) => Object.values(map).reduce((a, b) => a + b, 0);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { areaCounts, check, tally } from '../../../scripts/lib/eslint-warnings.mjs';
|
||||
import { areaCounts, check, inlineConfigsOf, lintClient, tally } from '../../../scripts/lib/eslint-warnings.mjs';
|
||||
import { RatchetError } from '../../../scripts/lib/ratchet.mjs';
|
||||
import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture';
|
||||
|
||||
@@ -128,4 +128,48 @@ describe('lint:warnings', () => {
|
||||
expect(areaCounts({ ...empty, src: { a: -1 } })).toMatch(/in "src"/);
|
||||
await expect(run({ src: {} }, () => [])).rejects.toThrow(RatchetError);
|
||||
});
|
||||
|
||||
it('ESLINT-009: finds the block comments that configure a rule, and nothing in strings, templates or regexes', () => {
|
||||
const source = [
|
||||
'/* eslint @typescript-eslint/no-explicit-any: off */',
|
||||
'const a = "/* eslint no-empty: off */";',
|
||||
'const b = `${a} /* eslint no-empty: off */`;',
|
||||
'/* eslint-disable no-empty */',
|
||||
'/*eslint no-empty:0*/ const c = <div>{/* eslint no-empty: 1 */}</div>;',
|
||||
'const r = /\\/\\* eslint no-empty/;',
|
||||
'// eslint no-empty: off',
|
||||
'/** eslint is the linter */',
|
||||
].join('\n');
|
||||
expect(inlineConfigsOf(source, 'src/a.tsx')).toEqual([
|
||||
'1:1 /* eslint @typescript-eslint/no-explicit-any: off */',
|
||||
'5:1 /*eslint no-empty:0*/',
|
||||
'5:39 /* eslint no-empty: 1 */',
|
||||
]);
|
||||
expect(inlineConfigsOf('export const a = 1 /* eslint no-empty: off */\n', 'src/a.ts')).toHaveLength(1);
|
||||
expect(inlineConfigsOf('export const a = 1\n', 'src/a.ts')).toEqual([]);
|
||||
});
|
||||
|
||||
it('ESLINT-010: an inline rule config fails the check although it leaves no message behind', async () => {
|
||||
const code = await run(empty, (root) => [
|
||||
{ ...result(root, 'src/a.ts', []), inlineConfigs: ['1:1 /* eslint no-empty: off */'] },
|
||||
]);
|
||||
expect(code).toBe(1);
|
||||
expect(tree.error.join('\n')).toMatch(
|
||||
/src\/a\.ts:1:1 {2}\/\* eslint no-empty: off \*\/ {2}configures a rule inline/
|
||||
);
|
||||
});
|
||||
|
||||
it('ESLINT-011: lintClient reads the inline rule configs of every file it lints', async () => {
|
||||
tree = ratchetTree({
|
||||
'eslint.config.mjs': "export default [{ files: ['**/*.js'], rules: { 'no-empty': 'warn' } }];\n",
|
||||
'src/a.js': '/* eslint no-empty: off */\nif (globalThis.x) {}\n',
|
||||
'src/b.js': 'if (globalThis.x) {}\n',
|
||||
});
|
||||
const results = await lintClient(tree.root);
|
||||
const byKey = Object.fromEntries(results.map((r) => [r.filePath.split('\\').join('/').split('/src/')[1], r]));
|
||||
expect(byKey['a.js'].messages).toEqual([]);
|
||||
expect(byKey['a.js'].inlineConfigs).toEqual(['1:1 /* eslint no-empty: off */']);
|
||||
expect(byKey['b.js'].messages.map((m) => m.ruleId)).toEqual(['no-empty']);
|
||||
expect(byKey['b.js'].inlineConfigs).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user