ci(lint): a grown warning count names the file and line that grew it

lint:warnings failed with the total and the files holding the most of the rule, which are old ones the baseline allows; the file that tipped the count was never named, so finding it meant linting by hand. When a rule grows, the ratchet now lints the files that differ from the base twice, as they are and as they were there, and lists every one holding more of the rule, line and message. The base is TREK_LINT_BASE (the CI fetches the pushed-over commit on a push and the target branch on a pull request) or origin/dev; without it the report stays as it was.

.github/workflows/test.yml

@@ -257,6 +257,15 @@ jobs:
         # migration that changes a column regenerates it in the same change.
         run: cd server && npm run check:db-types
 
+      - name: Fetch the base the warnings are compared with
+        # lint:warnings names the file behind a grown count by linting it as
+        # it is on the base: the pushed-over commit on a push, the target
+        # branch on a pull request. A miss only drops that hint.
+        run: >-
+          git fetch --no-tags --depth=1 origin
+          "+${{ github.event_name == 'push' && github.event.before || format('refs/heads/{0}', github.base_ref || 'dev') }}:refs/remotes/lint-base"
+          || true
+
       - name: Lint
         # Errors fail, and the warnings per rule may not pass
         # scripts/eslint-baseline.json (src, tests and eslint-disable'd
@@ -264,6 +273,7 @@ jobs:
         # program, which outgrows Node's default ~4 GB heap on the runner.
         env:
           NODE_OPTIONS: --max-old-space-size=8192
+          TREK_LINT_BASE: refs/remotes/lint-base
         run: cd server && npm run lint:warnings
 
       - name: Formatting check
@@ -395,6 +405,15 @@ jobs:
         # one included, has to be clean (scripts/strict-lint.mjs).
         run: cd client && npm run lint:strict
 
+      - name: Fetch the base the warnings are compared with
+        # lint:warnings names the file behind a grown count by linting it as
+        # it is on the base: the pushed-over commit on a push, the target
+        # branch on a pull request. A miss only drops that hint.
+        run: >-
+          git fetch --no-tags --depth=1 origin
+          "+${{ github.event_name == 'push' && github.event.before || format('refs/heads/{0}', github.base_ref || 'dev') }}:refs/remotes/lint-base"
+          || true
+
       - name: Lint
         # Fails on any ESLint error, like lint:check did, and on any rule
         # whose warning count rises above scripts/eslint-baseline.json.
@@ -402,6 +421,7 @@ jobs:
         # whole TypeScript program, so it gets the heap the server lint gets.
         env:
           NODE_OPTIONS: --max-old-space-size=8192
+          TREK_LINT_BASE: refs/remotes/lint-base
         run: cd client && npm run lint:warnings
 
       - name: Page pattern check

client/scripts/lib/eslint-warnings.mjs

@@ -2,6 +2,7 @@
  * The tally and the comparison behind lint:warnings (scripts/eslint-ratchet.mjs).
  */
 import { ESLint } from 'eslint';
+import { execFileSync } from 'node:child_process';
 import { join } from 'node:path';
 import ts from 'typescript';
 import {
@@ -137,6 +138,78 @@ export async function lintClient(root) {
   }));
 }
 
+/** The ref new warnings are measured against: TREK_LINT_BASE (the CI sets it), else origin/dev. */
+const baseRef = () => process.env.TREK_LINT_BASE || 'origin/dev';
+
+/** Git in the workspace: its output, or null when it fails (no repository, no such ref). */
+function gitIn(root, args) {
+  try {
+    return execFileSync('git', args, {
+      cwd: root,
+      encoding: 'utf8',
+      stdio: ['ignore', 'pipe', 'ignore'],
+      maxBuffer: 1 << 28,
+    });
+  } catch {
+    return null;
+  }
+}
+
+/** The messages of one rule in one area of a single file's lint result. */
+function ofRule(results, rule, area) {
+  const result = results[0];
+  if (!result) return [];
+  const pool =
+    area === 'suppressed' ? (result.suppressedMessages ?? []) : result.messages.filter((m) => m.severity === 1);
+  return pool.filter((m) => ruleOf(m) === rule);
+}
+
+/**
+ * Where a grown rule's new warnings sit: every file among `keys` (the files
+ * holding the rule now) that differs from `ref` and holds more of the rule
+ * than it did there, each of its warnings of the rule as `file:line  message`.
+ * The count alone names no file, and the files holding the most are old ones;
+ * this names the file that tipped it. Null when `ref` is not at hand (a clone
+ * without it), and the report then keeps to the files holding the most.
+ */
+export async function newWarningsOf({ root, rule, area, keys, lintText, ref = baseRef(), git = gitIn }) {
+  const changed = git(root, ['diff', '--name-only', '--relative', ref, '--', '.']);
+  if (changed === null) return null;
+  const differs = new Set(changed.split('\n').filter(Boolean));
+  const found = await Promise.all(
+    keys
+      .filter((key) => differs.has(key))
+      .map(async (key) => {
+        const path = join(root, key);
+        const now = ofRule(await lintText(readText(path), path), rule, area);
+        const before = git(root, ['show', `${ref}:./${key}`]);
+        const was = before === null ? [] : ofRule(await lintText(before, path), rule, area);
+        return now.length > was.length ? now.map((m) => `${key}:${m.line ?? 0}  ${m.message}`) : [];
+      })
+  );
+  return found.flat();
+}
+
+/** Prints, under the grown counts, the files and lines behind each one (see newWarningsOf). */
+async function reportNew(grown, { root, files, lintText, error }) {
+  if (!grown.length || !lintText) return;
+  const found = await Promise.all(
+    grown.map(({ area, rule }) => newWarningsOf({ root, rule, area, keys: Object.keys(files[area][rule]), lintText }))
+  );
+  grown.forEach(({ area, rule }, i) => {
+    const lines = found[i];
+    if (!lines?.length) return;
+    error(`New ${rule} warning(s) in ${area}, against ${baseRef()}:`);
+    for (const line of lines) error(`  ${line}`);
+  });
+}
+
+/** Lints one file's text with the client's own config, for newWarningsOf. */
+export function lintTextClient(root) {
+  const eslint = new ESLint({ cwd: root });
+  return (text, filePath) => eslint.lintText(text, { filePath });
+}
+
 const sum = (map) => Object.values(map).reduce((a, b) => a + b, 0);
 
 /**
@@ -148,6 +221,7 @@ export async function check({
   baselinePath = join(root, 'scripts/eslint-baseline.json'),
   update = false,
   lint = lintClient,
+  lintText = lint === lintClient ? lintTextClient : null,
   log = console.log,
   error = console.error,
 }) {
@@ -160,13 +234,13 @@ export async function check({
   }
 
   for (const line of errors) error(`FAIL  ${line}`);
-  let grown = 0;
+  const grown = [];
   let stale = 0;
   for (const area of AREAS) {
     for (const [rule, n] of Object.entries(counts[area])) {
       const allowed = baseline[area][rule] ?? 0;
       if (n <= allowed) continue;
-      grown++;
+      grown.push({ area, rule });
       const top = Object.entries(files[area][rule])
         .sort(([, a], [, b]) => b - a)
         .slice(0, 5)
@@ -182,7 +256,8 @@ export async function check({
     });
     stale += fallen.length;
   }
-  if (grown) {
+  await reportNew(grown, { root, files, lintText: lintText && lintText(root), error });
+  if (grown.length) {
     error(
       'Fix the new warning rather than silencing it: an eslint-disable comment is counted under "suppressed". ' +
         'Run npx eslint <file> to see each one.'
@@ -191,5 +266,5 @@ export async function check({
   log(
     `eslint: ${errors.length} error(s); warnings ${AREAS.map((area) => `${area} ${sum(counts[area])}/${sum(baseline[area])}`).join(', ')} (now/baseline)`
   );
-  return errors.length || grown || stale ? 1 : 0;
+  return errors.length || grown.length || stale ? 1 : 0;
 }

client/tests/unit/scripts/eslint-ratchet.test.ts

@@ -1,7 +1,15 @@
-import { readFileSync } from 'node:fs';
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { afterEach, describe, expect, it } from 'vitest';
-import { areaCounts, check, inlineConfigsOf, lintClient, tally } from '../../../scripts/lib/eslint-warnings.mjs';
+import {
+  areaCounts,
+  check,
+  inlineConfigsOf,
+  lintClient,
+  newWarningsOf,
+  tally,
+} from '../../../scripts/lib/eslint-warnings.mjs';
 import { RatchetError } from '../../../scripts/lib/ratchet.mjs';
 import { ratchetTree, type RatchetTree } from '../../helpers/ratchetFixture';
 
@@ -183,4 +191,33 @@ describe('lint:warnings', () => {
     expect(byKey['b.js'].messages.map((m) => m.ruleId)).toEqual(['no-empty']);
     expect(byKey['b.js'].inlineConfigs).toEqual([]);
   });
+
+  it('names the file and line behind a grown count, measured against the base, and nothing without one', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'lint-new-'));
+    try {
+      mkdirSync(join(root, 'src'));
+      writeFileSync(join(root, 'src/new.ts'), 'two');
+      writeFileSync(join(root, 'src/old.ts'), 'one');
+      // new.ts held none of the rule on the base, old.ts already held its one
+      const git = (_root: string, args: string[]) =>
+        args[0] === 'diff' ? 'src/new.ts\nsrc/old.ts\n' : args[1]?.endsWith('old.ts') ? 'one' : 'none';
+      const counts: Record<string, number> = { two: 2, one: 1, none: 0 };
+      const lintText = async (text: string) => [
+        {
+          messages: Array.from({ length: counts[text] ?? 0 }, (_, i) => ({
+            ruleId: 'no-empty',
+            severity: 1,
+            message: `w${i}`,
+            line: i + 1,
+          })),
+        },
+      ];
+      const keys = ['src/new.ts', 'src/old.ts', 'src/untouched.ts'];
+      const lines = await newWarningsOf({ root, rule: 'no-empty', area: 'src', keys, lintText, ref: 'base', git });
+      expect(lines).toEqual(['src/new.ts:1  w0', 'src/new.ts:2  w1']);
+      expect(await newWarningsOf({ root, rule: 'no-empty', area: 'src', keys, lintText, git: () => null })).toBeNull();
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
 });

scripts/lib/eslint-warnings.mjs

@@ -6,6 +6,7 @@
  * own major and has to lint with the one its eslint.config.mjs was written
  * for. This file never imports eslint, so it resolves no version of its own.
  */
+import { execFileSync } from 'node:child_process';
 import { join } from 'node:path';
 import ts from 'typescript';
 import {
@@ -145,6 +146,77 @@ export function lintWith(ESLint) {
   };
 }
 
+/** The ref new warnings are measured against: TREK_LINT_BASE (the CI sets it), else origin/dev. */
+const baseRef = () => process.env.TREK_LINT_BASE || 'origin/dev';
+
+/** Git in the workspace: its output, or null when it fails (no repository, no such ref). */
+function gitIn(root, args) {
+  try {
+    return execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], maxBuffer: 1 << 28 });
+  } catch {
+    return null;
+  }
+}
+
+/** The messages of one rule in one area of a single file's lint result. */
+function ofRule(results, rule, area) {
+  const result = results[0];
+  if (!result) return [];
+  const pool =
+    area === 'suppressed' ? (result.suppressedMessages ?? []) : result.messages.filter((m) => m.severity === 1);
+  return pool.filter((m) => ruleOf(m) === rule);
+}
+
+/**
+ * Where a grown rule's new warnings sit: every file among `keys` (the files
+ * holding the rule now) that differs from `ref` and holds more of the rule
+ * than it did there, each of its warnings of the rule as `file:line  message`.
+ * The count alone names no file, and the files holding the most are old ones;
+ * this names the file that tipped it. Null when `ref` is not at hand (a clone
+ * without it), and the report then keeps to the files holding the most.
+ */
+export async function newWarningsOf({ root, rule, area, keys, lintText, ref = baseRef(), git = gitIn }) {
+  const changed = git(root, ['diff', '--name-only', '--relative', ref, '--', '.']);
+  if (changed === null) return null;
+  const differs = new Set(changed.split('\n').filter(Boolean));
+  const found = await Promise.all(
+    keys
+      .filter((key) => differs.has(key))
+      .map(async (key) => {
+        const path = join(root, key);
+        const now = ofRule(await lintText(readText(path), path), rule, area);
+        const before = git(root, ['show', `${ref}:./${key}`]);
+        const was = before === null ? [] : ofRule(await lintText(before, path), rule, area);
+        return now.length > was.length ? now.map((m) => `${key}:${m.line ?? 0}  ${m.message}`) : [];
+      })
+  );
+  return found.flat();
+}
+
+/** Prints, under the grown counts, the files and lines behind each one (see newWarningsOf). */
+async function reportNew(grown, { root, files, lintText, error }) {
+  if (!grown.length || !lintText) return;
+  const found = await Promise.all(
+    grown.map(({ area, rule }) =>
+      newWarningsOf({ root, rule, area, keys: Object.keys(files[area][rule]), lintText })
+    )
+  );
+  grown.forEach(({ area, rule }, i) => {
+    const lines = found[i];
+    if (!lines?.length) return;
+    error(`New ${rule} warning(s) in ${area}, against ${baseRef()}:`);
+    for (const line of lines) error(`  ${line}`);
+  });
+}
+
+/** A one-file lint for newWarningsOf, with the ESLint the workspace imports (see lintWith). */
+export function lintTextWith(ESLint) {
+  return (root) => {
+    const eslint = new ESLint({ cwd: root });
+    return (text, filePath) => eslint.lintText(text, { filePath });
+  };
+}
+
 const sum = (map) => Object.values(map).reduce((a, b) => a + b, 0);
 
 /**
@@ -156,6 +228,8 @@ export async function check({
   baselinePath = join(root, 'scripts/eslint-baseline.json'),
   update = false,
   lint,
+  /** lintTextWith(ESLint): names the files behind a grown count; left out, only the counts are told. */
+  lintText = null,
   log = console.log,
   error = console.error,
 }) {
@@ -169,13 +243,13 @@ export async function check({
   }
 
   for (const line of errors) error(`FAIL  ${line}`);
-  let grown = 0;
+  const grown = [];
   let stale = 0;
   for (const area of AREAS) {
     for (const [rule, n] of Object.entries(counts[area])) {
       const allowed = baseline[area][rule] ?? 0;
       if (n <= allowed) continue;
-      grown++;
+      grown.push({ area, rule });
       const top = Object.entries(files[area][rule])
         .sort(([, a], [, b]) => b - a)
         .slice(0, 5)
@@ -191,7 +265,8 @@ export async function check({
     });
     stale += fallen.length;
   }
-  if (grown) {
+  await reportNew(grown, { root, files, lintText: lintText && lintText(root), error });
+  if (grown.length) {
     error(
       'Fix the new warning rather than silencing it: an eslint-disable comment is counted under "suppressed". ' +
         'Run npx eslint <file> to see each one.'
@@ -200,5 +275,5 @@ export async function check({
   log(
     `eslint: ${errors.length} error(s); warnings ${AREAS.map((area) => `${area} ${sum(counts[area])}/${sum(baseline[area])}`).join(', ')} (now/baseline)`
   );
-  return errors.length || grown || stale ? 1 : 0;
+  return errors.length || grown.length || stale ? 1 : 0;
 }

scripts/lib/eslint-warnings.test.mjs

@@ -1,9 +1,10 @@
 import { ESLint } from 'eslint';
 import assert from 'node:assert/strict';
-import { readFileSync } from 'node:fs';
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { afterEach, describe, it } from 'node:test';
-import { areaCounts, check, inlineConfigsOf, lintWith, tally } from './eslint-warnings.mjs';
+import { areaCounts, check, inlineConfigsOf, lintWith, newWarningsOf, tally } from './eslint-warnings.mjs';
 import { RatchetError } from './ratchet.mjs';
 import { ratchetTree } from './tree-fixture.mjs';
 
@@ -92,6 +93,26 @@ describe('scripts/lib/eslint-warnings', () => {
     assert.match(out, /Fix the new warning rather than silencing it/);
   });
 
+  it('names the file and line behind a grown count, measured against the base, and nothing without one', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'lint-new-'));
+    try {
+      mkdirSync(join(root, 'src'));
+      writeFileSync(join(root, 'src/new.ts'), 'two');
+      writeFileSync(join(root, 'src/old.ts'), 'one');
+      // new.ts held none of the rule on the base, old.ts already held its one
+      const git = (_root, args) => (args[0] === 'diff' ? 'src/new.ts\nsrc/old.ts\n' : args[1].endsWith('old.ts') ? 'one' : 'none');
+      const lintText = async (text) => [
+        { messages: Array.from({ length: { two: 2, one: 1, none: 0 }[text] }, (_, i) => ({ ...warn('no-empty', `w${i}`), line: i + 1 })) },
+      ];
+      const keys = ['src/new.ts', 'src/old.ts', 'src/untouched.ts'];
+      const lines = await newWarningsOf({ root, rule: 'no-empty', area: 'src', keys, lintText, ref: 'base', git });
+      assert.deepEqual(lines, ['src/new.ts:1  w0', 'src/new.ts:2  w1']);
+      assert.equal(await newWarningsOf({ root, rule: 'no-empty', area: 'src', keys, lintText, git: () => null }), null);
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+
   it('counts a message an eslint-disable comment silenced', async () => {
     assert.equal(await run(empty, (root) => [result(root, 'src/a.ts', [], [warn('no-console')])]), 1);
     assert.match(tree.error.join('\n'), /suppressed: 1 no-console warning/);

server/scripts/eslint-ratchet.mjs

@@ -23,11 +23,13 @@
  * ESLint comes from this workspace, so the server lints with the major its
  * config was written for.
  */
-import { check, lintWith } from '../../scripts/lib/eslint-warnings.mjs';
+import { check, lintTextWith, lintWith } from '../../scripts/lib/eslint-warnings.mjs';
 import { runCli } from '../../scripts/lib/ratchet.mjs';
 
 import { ESLint } from 'eslint';
 import { fileURLToPath } from 'node:url';
 
 const root = fileURLToPath(new URL('..', import.meta.url));
-await runCli('lint:warnings', (args) => check({ root, update: args.includes('--update'), lint: lintWith(ESLint) }));
+await runCli('lint:warnings', (args) =>
+  check({ root, update: args.includes('--update'), lint: lintWith(ESLint), lintText: lintTextWith(ESLint) }),
+);