fix(eslint): non-blocking refresh, config edits, rule options and suppression warnings

.changeset/eslint-refresh-options.md

@@ -0,0 +1,13 @@
+---
+'onsystem': minor
+---
+
+The ESLint plugin no longer holds up a lint while it looks at the design system again. In an editor, every lint after a pause of two seconds or more waited for that look, which takes longer the larger the design system. Now the lint is answered at once from the design system already loaded, the look runs in the background, and the lints after it get what it found. The lint that starts a look is answered from what was loaded before, so after a change to the design system or the config, the file shows its earlier findings until it is linted again (the next edit, or the next save when the editor lints on save). A one-shot `eslint` run always loads fresh.
+
+Edits to the project's config reach an editor's ESLint server without a restart: a rule turned off in the `rules` or `overrides` of `onsystem.config.json`, `.mjs`, `.js` or `.ts` is no longer reported once the plugin has looked at the project again. Before, the plugin kept the rules it loaded first until the server restarted.
+
+A rule's options can be set in ESLint's config, as in `onsystem.config.json`: `'onsystem/no-hardcoded-color': ['error', { allow: ['#fff'] }]` no longer fails ESLint's config validation ("should NOT have more than 0 items"), and its `allow` takes the place of the one the project's config gives that rule.
+
+A misspelled rule in a suppression comment, such as `// onsystem-disable-next-line no-hardcoded-colour`, is reported by the new `onsystem/suppression` rule, a warning in the recommended config, with the message `check` gives. Through ESLint, nothing said why the comment suppressed nothing. This is a new warning on upgrade: a run with `--max-warnings 0` over code with such a comment now fails, until the comment is fixed or `'onsystem/suppression': 'off'` turns the rule off.
+
+docs/eslint.md says where rule options live, what each ESLint run pays for the plugin (prefer `onsystem check` or the GitHub Action for a CI gate), and that ESLint's `--cache` does not see design-system changes.

docs/eslint.md

@@ -40,7 +40,9 @@ Each file is checked as `onsystem check` checks it, with zero config in a shadcn
 - with the `// onsystem-disable-*` [suppression comments](rules.md#suppressing-findings), next to ESLint's own `eslint-disable` comments,
 - leaving out what `check .` leaves out: the files the config's `exclude` lists, the design system's own components, and files that render an image or an email.
 
-Findings have the same messages, positions and fixes as `check`, and `eslint --fix` applies the fixes. ESLint's config sets each rule's severity (`'onsystem/no-hardcoded-spacing': 'error'`). The [real-world corpus](how-it-works.md#on-real-codebases) also runs through the plugin (`pnpm corpus eslint`): on every one of its runs, it reports exactly the findings `check` reports.
+Findings have the same messages, positions and fixes as `check`, and `eslint --fix` applies the fixes. A misspelled rule in a suppression comment, which suppresses nothing, is reported by `onsystem/suppression` as a warning, as `check` reports it. ESLint's config sets each rule's severity (`'onsystem/no-hardcoded-spacing': 'error'`). The [real-world corpus](how-it-works.md#on-real-codebases) also runs through the plugin (`pnpm corpus eslint`): on every one of its runs, it reports exactly the findings `check` reports.
+
+A rule's options (`allow`) belong in the project's [config](configuration.md#config-file), where `check`, the MCP server and the Claude Code hook read them too. ESLint's config can set them as well, for ESLint only: `'onsystem/no-hardcoded-color': ['error', { allow: ['#fff'] }]` takes the place of the `allow` the project's config gives that rule. A rule the project's config turns off for a file stays off.
 
 When the project root or the config is not where it would be found from ESLint's working directory, say where in the settings, as `--root` and `--config` do:
 
@@ -51,7 +53,11 @@ export default [
 ];
 ```
 
-The plugin loads each design system once, in a worker thread (ESLint rules run synchronously and extraction does not), and looks at its files again at most every two seconds, so an editor's ESLint server picks up a new component or variant without a restart.
+The plugin loads each design system once, in a worker thread (ESLint rules run synchronously and extraction does not), and answers each lint from what it has loaded. When that is more than two seconds old, the worker looks at the project's config and the design system's files again in the background, and the lints after that look get what it found, so an editor's ESLint server picks up a new component or variant, or a rule turned off in the config, without a restart. The lint that starts a look does not wait for it: it is answered from what was loaded before, so after a change to the design system or the config, the file shows its earlier findings until it is linted again (the next edit, or the next save when the editor lints on save). A one-shot `eslint` run always loads fresh.
+
+Each ESLint run pays for the plugin on top of ESLint's own work: every file is parsed a second time, by TypeScript as `check` parses it, and each ESLint thread (`--concurrency`) loads the design system in a worker of its own. For a CI gate, `onsystem check` or the [GitHub Action](ci.md) does the same checks with neither cost; the plugin is for editors, and for a setup that runs only ESLint.
+
+ESLint's `--cache` keys each file on its own content and ESLint's config, not on the design system. After a design-system change, a file that did not change keeps the findings ESLint cached for it: a variant added to a component is still reported as unknown, and a variant taken out passes where the file uses it. Leave `--cache` out of runs with the onsystem rules, or delete `.eslintcache` when the design system changes. `check` and the Action cache no findings.
 
 A few things stay with the CLI. The [baseline](ci.md#adopting-it-in-an-existing-codebase) applies to `check` only. ESLint has its own: on ESLint 9.24 and later, `npx eslint . --suppress-all` records the findings already in the code in `eslint-suppressions.json`, which you commit, and later runs leave them out ([bulk suppressions](https://eslint.org/docs/latest/use/suppressions)). Once a recorded finding is fixed, ESLint fails the run until `npx eslint . --prune-suppressions` takes it out of the file. On older versions, use `eslint-disable` comments, or set the onsystem rules to `warn` and cap them with `--max-warnings` (it counts warnings only, and most onsystem rules default to `error`). ESLint reports syntax errors itself. Which files ESLint lints is up to its config (`ignores`), where `check` leaves out what git ignores.
 

src/eslint/index.ts

@@ -9,7 +9,8 @@
  * working directory), with the project's config: its `rules` and `overrides` turn rules off
  * per file, `// onsystem-disable-*` comments suppress findings, and the files `check .` leaves
  * out (the config's `exclude`, the design system's own components) are not checked. ESLint's
- * config sets each rule's severity.
+ * config sets each rule's severity, and can set a rule's options (`allow`) over the project's.
+ * `onsystem/suppression` reports a misspelled rule in a suppression comment, as check does.
  */
 import path from 'node:path';
 
@@ -19,10 +20,14 @@ import {
   DEFAULT_SEVERITY,
   DEFAULT_TEST_EXCLUDE,
   DEFAULT_TEST_FOLDERS,
+  RULE_IDS,
+  rulesFor,
+  type ResolvedRule,
   type RuleId,
+  type RuleOptions,
 } from '../config.js';
 import { DesignSystem } from '../design-system.js';
-import { RULES } from '../lint/index.js';
+import { checkSource, RULES } from '../lint/index.js';
 import type { CheckResult, TextEdit } from '../types.js';
 import { relativePath } from '../util/paths.js';
 import { NAME, VERSION } from '../version.js';
@@ -43,10 +48,15 @@ const passes = new WeakMap<SourceCode, { outcome: Outcome; reported: boolean }>(
 
 /**
  * A checked file's findings, with `fromSystem`: whether an import specifier in that file leads
- * to the design system.
+ * to the design system, and `checkWith`: the file checked by one rule alone, with options from
+ * ESLint's config over the ones the project's config gives the rule for the file.
  */
 type Outcome =
-  | { result: CheckResult; fromSystem: (specifier: string) => boolean }
+  | {
+      result: CheckResult;
+      fromSystem: (specifier: string) => boolean;
+      checkWith: (id: RuleId, options: RuleOptions) => CheckResult;
+    }
   | { skipped: true }
   | { error: string };
 
@@ -86,7 +96,22 @@ function run(cwd: string, file: string, text: string, settings: Settings): Outco
   const relative = relativePath(ds.root, file);
   const result = ds.check(text, relative);
   if (result.skipped) return { skipped: true };
-  return { result, fromSystem: (specifier) => ds.lint.isDesignSystemImport(specifier, relative) };
+  return {
+    result,
+    fromSystem: (specifier) => ds.lint.isDesignSystemImport(specifier, relative),
+    checkWith(id, options) {
+      // A rule the project's config turns off for the file stays off.
+      const own = rulesFor(ds.config, relative)[id];
+      const off: ResolvedRule = { severity: 'off', options: {} };
+      const rules = Object.fromEntries(
+        RULE_IDS.map((rule) => [
+          rule,
+          rule === id ? { ...own, options: { ...own.options, ...options } } : off,
+        ]),
+      ) as Record<RuleId, ResolvedRule>;
+      return checkSource(text, ds.lint, { filename: relative, rules });
+    },
+  };
 }
 
 /** The part of an ESLint AST node this file reads; JSX nodes are not in ESTree's types. */
@@ -158,34 +183,55 @@ function typeOnly(def: Scope.Definition): boolean {
   return def.type === 'ImportBinding' && (kind(def.node) === 'type' || kind(def.parent) === 'type');
 }
 
-function createRule(id: RuleId, description: string): Rule.RuleModule {
+/** A rule's options in ESLint's config, as in onsystem.config.json: `['error', { allow: ['#fff'] }]`. */
+const OPTIONS_SCHEMA: Rule.RuleMetaData['schema'] = [
+  {
+    type: 'object',
+    properties: { allow: { type: 'array', items: { type: 'string' } } },
+    additionalProperties: false,
+  },
+];
+
+/**
+ * Check's misspelled rule names in `// onsystem-disable-*` comments, which suppress nothing. A rule
+ * of its own, so that it is a warning, as check reports it, and can be turned off like the others.
+ */
+const SUPPRESSION = 'suppression';
+
+function createRule(id: RuleId | typeof SUPPRESSION, description: string): Rule.RuleModule {
+  const suppression = id === SUPPRESSION;
   return {
     meta: {
       type: 'problem',
-      docs: { description, url: `${RULES_PAGE}#${id}` },
-      fixable: 'code',
-      hasSuggestions: true,
-      schema: [],
+      docs: { description, url: `${RULES_PAGE}#${suppression ? 'suppressing-findings' : id}` },
+      ...(suppression ? {} : { fixable: 'code', hasSuggestions: true }),
+      schema: suppression ? [] : OPTIONS_SCHEMA,
     },
     create(context) {
       const file = path.resolve(context.cwd, context.filename);
       const settings = (context.settings.onsystem ?? {}) as Settings;
+      const options = (context.options as RuleOptions[] | undefined)?.[0];
       return {
         'Program:exit'(node) {
           const pass = passFor(context, file, settings);
           const { outcome } = pass;
           if ('skipped' in outcome) return;
           if ('error' in outcome) {
             // Once per file and pass, from the first rule that runs, rather than once per rule.
-            if (pass.reported) return;
+            // Not from `suppression`, a warning about a comment, when a config lists it first.
+            if (suppression || pass.reported) return;
             pass.reported = true;
             context.report({
               node,
               message: `onsystem could not check this file: ${outcome.error}`,
             });
             return;
           }
-          for (const d of outcome.result.diagnostics) {
+          const { diagnostics } =
+            !suppression && options && Object.keys(options).length
+              ? outcome.checkWith(id, options)
+              : outcome.result;
+          for (const d of diagnostics) {
             if (d.ruleId !== id) continue;
             const edits = d.fix ?? [];
             const fix: Rule.ReportFixer = (fixer) =>
@@ -218,6 +264,7 @@ function createRule(id: RuleId, description: string): Rule.RuleModule {
 const rules = Object.fromEntries(
   RULES.map((rule) => [rule.id, createRule(rule.id, rule.description)]),
 );
+rules[SUPPRESSION] = createRule(SUPPRESSION, 'Suppression comments must name rules that exist.');
 
 const plugin = {
   meta: { name: NAME, version: VERSION },
@@ -226,17 +273,18 @@ const plugin = {
 } satisfies ESLint.Plugin;
 
 /**
- * Every rule at its default severity, for `.tsx` and `.jsx` files other than tests and stories,
- * which `check` also leaves out unless asked (`--include-tests`).
+ * Every rule at its default severity, and `suppression` as a warning, for `.tsx` and `.jsx` files
+ * other than tests and stories, which `check` also leaves out unless asked (`--include-tests`).
  */
 plugin.configs.recommended = {
   name: 'onsystem/recommended',
   files: ['**/*.{tsx,jsx}'],
   ignores: [DEFAULT_TEST_EXCLUDE, DEFAULT_TEST_FOLDERS],
   plugins: { onsystem: plugin },
-  rules: Object.fromEntries(
-    RULES.map((rule) => [`onsystem/${rule.id}`, DEFAULT_SEVERITY[rule.id]]),
-  ),
+  rules: {
+    ...Object.fromEntries(RULES.map((rule) => [`onsystem/${rule.id}`, DEFAULT_SEVERITY[rule.id]])),
+    [`onsystem/${SUPPRESSION}`]: 'warn',
+  },
 };
 
 export default plugin;