perf: lint what webpack rebuilt, while it is still building

.changeset/lint-only-what-changed.md

@@ -0,0 +1,5 @@
+---
+"diagnostics-webpack-plugin": patch
+---
+
+Lint only the files webpack rebuilt, reporting the rest from the previous compilation, and start linting while the module graph is still being built.

README.md

@@ -308,7 +308,9 @@ new DiagnosticsPlugin({
 
 Run with `{ use: "eslint" }`. It lints the files webpack builds, so only the modules that end up in the bundle are checked.
 
-Alongside the shared options you can pass any [ESLint Node.js API option](https://eslint.org/docs/latest/integrate/nodejs-api#-new-eslintoptions) — they are handed to the `ESLint` class as they are.
+Alongside the shared options you can pass any [ESLint Node.js API option](https://eslint.org/docs/latest/integrate/nodejs-api#-new-eslintoptions) — they are handed to the `ESLint` class as they are. `concurrency` is worth knowing about: it spreads a lint across worker threads, and ESLint warns on the runs where doing so costs more than it saves, so measure your own project rather than turning it on by default.
+
+A rebuild lints only the files webpack rebuilt and reports the rest from the previous run, so `lintDirtyModulesOnly` is only worth setting to skip the first lint entirely.
 
 ### `configType`
 

src/check.js

@@ -2,14 +2,45 @@ import { isAbsolute, join } from "node:path";
 
 import DiagnosticError from "./DiagnosticError.js";
 import { reportedAs } from "./options.js";
+import { toPosixPath } from "./utils.js";
 
 /** @typedef {import("webpack").Compilation} Compilation */
 /** @typedef {import("./checks/index.js").CheckResult} CheckResult */
 /** @typedef {import("./checks/index.js").CheckInstance} CheckInstance */
 /** @typedef {import("./options.js").EnabledCheck} EnabledCheck */
 /** @typedef {{ filePath: string, content: string }} OutputReportContent */
 /** @typedef {{ errors?: DiagnosticError, warnings?: DiagnosticError, outputReport?: OutputReportContent }} Report */
-/** @typedef {{ lint: (files: string[]) => void, report: () => Promise<Report> }} Runner */
+/** @typedef {{ lint: (files: string[]) => void, keep: (files: string[]) => void, keepKnown: (removed: ReadonlySet<string>) => void, report: () => Promise<Report> }} Runner */
+/** @typedef {Map<string, CheckResult | undefined>} ResultStore */
+
+/** @type {WeakMap<Compilation["compiler"], Map<string, ResultStore>>} */
+const resultStores = new WeakMap();
+
+/**
+ * The results of the last compilation, so a rebuild lints what webpack rebuilt
+ * and reports the rest from here.
+ * @param {Compilation} compilation compilation
+ * @param {string} check a key unique to the check within the compiler
+ * @returns {ResultStore} what the check last found in every file it covered
+ */
+function getResultStore(compilation, check) {
+  const { compiler } = compilation;
+  let stores = resultStores.get(compiler);
+
+  if (!stores) {
+    stores = new Map();
+    resultStores.set(compiler, stores);
+  }
+
+  let store = stores.get(check);
+
+  if (!store) {
+    store = new Map();
+    stores.set(check, store);
+  }
+
+  return store;
+}
 
 /**
  * @param {Promise<CheckResult[]>[]} results results
@@ -42,21 +73,125 @@ function createCheckRunner(key, { name, adapter, options }, compilation) {
 
   /** @type {Promise<CheckResult[]>[]} */
   const rawResults = [];
+  // Every path the store is keyed by goes through `toPosixPath`: webpack hands
+  // over a module's resource with the separators the platform uses, and a
+  // check answers with whatever its own tool wrote.
+  const store = getResultStore(compilation, `${key}:${name}`);
+  // A check that cannot say which file a result came from is linted whole.
+  const { resultPath } = adapter;
+  /** @type {Set<string>} */
+  const covered = new Set();
+  /** @type {Set<string>} */
+  const linted = new Set();
+  // A check that cannot lint fails the same way for every batch it is given.
+  let failed = false;
 
   /**
    * @param {string[]} files files
    */
   function lint(files) {
+    for (const file of files) {
+      const known = toPosixPath(file);
+
+      covered.add(known);
+      linted.add(known);
+    }
+
     rawResults.push(
       pending
         .then((instance) => (instance ? instance.lintFiles(files) : []))
         .catch((err) => {
-          compilation.errors.push(new DiagnosticError(name, err.message));
+          if (!failed) {
+            failed = true;
+            compilation.errors.push(new DiagnosticError(name, err.message));
+          }
+
           return [];
         }),
     );
   }
 
+  /**
+   * Reports a file from the last compilation rather than linting it again.
+   * A file the store does not hold is linted: webpack restores a module from
+   * its own cache without building it, and the first run of a compiler that
+   * does so has nothing to report it from.
+   * @param {string[]} files the files webpack did not rebuild
+   */
+  function keep(files) {
+    if (!resultPath) {
+      lint(files);
+      return;
+    }
+
+    /** @type {string[]} */
+    const unknown = [];
+
+    for (const file of files) {
+      const known = toPosixPath(file);
+
+      if (store.has(known)) covered.add(known);
+      else unknown.push(file);
+    }
+
+    if (unknown.length > 0) lint(unknown);
+  }
+
+  /**
+   * Keeps every file the last compilation covered bar the ones webpack says
+   * are gone — what a check that walks the file system knows about the files
+   * it is not being told changed, without walking it again.
+   * @param {ReadonlySet<string>} removed the files webpack no longer sees
+   */
+  function keepKnown(removed) {
+    if (!resultPath) return;
+
+    const gone = new Set([...removed].map((file) => toPosixPath(file)));
+
+    for (const file of store.keys()) {
+      if (!gone.has(file)) covered.add(file);
+    }
+  }
+
+  /**
+   * Puts what was just linted into the store, drops what webpack no longer
+   * builds, and answers with the results for every file this compilation
+   * covers — the fresh ones and the ones kept from the last.
+   * @param {CheckResult[]} results what the check produced this time
+   * @returns {CheckResult[]} the results to report
+   */
+  function remember(results) {
+    if (!resultPath) return results;
+
+    // A check reports nothing for a file it found nothing in, so what was
+    // linted is forgotten first and only what came back is put back.
+    for (const file of linted) store.delete(file);
+
+    // A result the check cannot put a file to is reported as it is: it is
+    // this compilation's, and there is nothing to remember it under.
+    /** @type {CheckResult[]} */
+    const loose = [];
+
+    for (const result of results) {
+      const file = resultPath(result);
+
+      if (file) store.set(toPosixPath(file), result);
+      else loose.push(result);
+    }
+
+    // A file a check found nothing in is remembered as nothing found, which is
+    // what tells a rebuild it has been linted at all.
+    for (const file of linted) {
+      if (!store.has(file)) store.set(file, undefined);
+    }
+
+    for (const file of store.keys()) {
+      if (!covered.has(file)) store.delete(file);
+    }
+
+    return [...store.values(), ...loose].filter(Boolean);
+  }
+
   /**
    * @returns {Promise<Report>} report
    */
@@ -70,7 +205,7 @@ function createCheckRunner(key, { name, adapter, options }, compilation) {
 
     await instance.cleanup();
 
-    const results = await instance.getResults(raw);
+    const results = remember(await instance.getResults(raw));
 
     // Do not analyze when the check reported nothing.
     if (!results || results.length === 0) {
@@ -111,7 +246,7 @@ function createCheckRunner(key, { name, adapter, options }, compilation) {
     return report;
   }
 
-  return { lint, report };
+  return { keep, keepKnown, lint, report };
 }
 
 export default createCheckRunner;

src/checks/eslint.js

@@ -267,6 +267,8 @@ export default {
     extensions: "js",
   },
   defaultExclude: () => "**/node_modules/**",
+  resultPath: (/** @type {EXPECTED_ANY} */ result) =>
+    /** @type {LintResult} */ (result).filePath,
   create,
   getESLintOptions,
 };

src/checks/index.js

@@ -31,6 +31,7 @@ import stylelint from "./stylelint.js";
  * @property {(files: string[]) => Promise<CheckResult[]>} lintFiles lints the given files
  * @property {(results: CheckResult[]) => Promise<CheckResult[]>} getResults turns the raw results of every `lintFiles` call into the results to report
  * @property {(results: CheckResult[]) => { errors: CheckResult[], warnings: CheckResult[] }} splitResults splits the results by their own severity, leaving `reportAs` to the plugin
+ * @property {((result: CheckResult) => string | undefined)=} resultPath the file a result came from, without which a rebuild re-lints everything
  * @property {(formatter?: FormatterOption) => Promise<Format>} getFormatter loads a formatter, falling back to the tool's default one
  * @property {() => Promise<void>} cleanup releases whatever the tool holds after a run
  */
@@ -57,6 +58,7 @@ import stylelint from "./stylelint.js";
  * @property {{ [key: string]: EXPECTED_ANY }} defaults default options for this check
  * @property {(compiler: Compiler) => string | string[]} defaultExclude the globs excluded when the user specifies none
  * @property {(context: CheckContext) => Promise<CheckInstance>} create creates a check for one compilation
+ * @property {((result: CheckResult) => string | undefined)=} resultPath the file a result came from, without which a rebuild re-lints everything
  */
 
 /** @type {Map<string, CheckAdapter>} */

src/checks/stylelint.js

@@ -65,26 +65,6 @@ const KEPT_OPTIONS = ["cache", "cacheLocation", "files", "fix", "formatter"];
 /** @type {{ [key: string]: Loaded }} */
 const cache = {};
 
-/** @type {WeakMap<Compiler, LintResultMap>} */
-const resultStorage = new WeakMap();
-
-/**
- * Stylelint only lints the files webpack reports as modified, so results of
- * files left untouched by a watch rebuild are carried over from the last run.
- * @param {Compiler} compiler compiler
- * @returns {LintResultMap} lint result map
- */
-function getResultStorage(compiler) {
-  let storage = resultStorage.get(compiler);
-
-  if (!storage) {
-    storage = {};
-    resultStorage.set(compiler, storage);
-  }
-
-  return storage;
-}
-
 /**
  * @param {Options} options options
  * @returns {Partial<StylelintOptions>} stylelint options
@@ -228,9 +208,8 @@ function getLoadedStylelint(key, options) {
  * @param {CheckContext} context check context
  * @returns {Promise<CheckInstance>} stylelint check
  */
-async function create({ key, options, compilation }) {
+async function create({ key, options }) {
   const loaded = getLoadedStylelint(key, options);
-  const storage = getResultStorage(compilation.compiler);
 
   /** @type {LintResult[]} */
   let lastResults = [];
@@ -239,10 +218,6 @@ async function create({ key, options, compilation }) {
     async lintFiles(files) {
       const resolved = parseFiles(files, String(options.context));
 
-      for (const file of resolved) {
-        delete storage[file];
-      }
-
       // One task per file keeps every worker of the pool busy.
       if (loaded.threads > 1) {
         const results = await Promise.all(
@@ -255,13 +230,9 @@ async function create({ key, options, compilation }) {
       return loaded.lintFiles(resolved);
     },
     async getResults(results) {
-      for (const result of /** @type {LintResult[]} */ (results)) {
-        if (result.ignored) continue;
-
-        storage[String(result.source)] = result;
-      }
-
-      lastResults = Object.values(storage);
+      lastResults = /** @type {LintResult[]} */ (results).filter(
+        (result) => !result.ignored,
+      );
 
       return lastResults;
     },
@@ -332,6 +303,8 @@ export default {
   },
   getLoadedStylelint,
   getStylelintOptions,
+  resultPath: (/** @type {EXPECTED_ANY} */ result) =>
+    /** @type {LintResult} */ (result).source || undefined,
   /**
    * @param {Compiler} compiler compiler
    * @returns {string[]} default excluded globs