Fix ESLint script (#5150)

There are a few problems with the ESLint script.

First, the `--quiet` option (which is supposed to only show errors)
doesn't work. That's not a big deal, we can fix this easily.

Second, the thresholds file is updated if there are regressions (i.e.
any rules that have more violations than a previous run of `eslint`).
That's not right; we should update the thresholds file if we've seen
improvements (all rules have the same or a fewer number of violations).

More importantly, however, the script doesn't exit with 1 if there are
any lint violations. This is a big problem because it means that even if
a PR introduces violations, CI will pass. In fact, some violations have
slipped into the codebase in a recent commit. So that we can get this PR
approved without requiring other teams, this commit disables those rules
globally, and we will re-enable them in another commit.

eslint-warning-thresholds.json

@@ -6,24 +6,26 @@
   "@typescript-eslint/no-unsafe-enum-comparison": 59,
   "@typescript-eslint/no-unused-vars": 36,
   "@typescript-eslint/prefer-promise-reject-errors": 13,
-  "@typescript-eslint/prefer-readonly": 152,
+  "@typescript-eslint/prefer-readonly": 145,
   "@typescript-eslint/switch-exhaustiveness-check": 10,
   "import-x/namespace": 189,
   "import-x/no-named-as-default": 1,
   "import-x/no-named-as-default-member": 8,
   "import-x/order": 209,
-  "jest/no-conditional-in-test": 104,
-  "jsdoc/check-tag-names": 372,
+  "jest/no-conditional-in-test": 129,
+  "jest/prefer-lowercase-title": 2,
+  "jest/prefer-strict-equal": 2,
+  "jsdoc/check-tag-names": 375,
   "jsdoc/require-returns": 22,
-  "jsdoc/tag-lines": 329,
+  "jsdoc/tag-lines": 328,
   "n/no-unsupported-features/node-builtins": 18,
   "n/prefer-global/text-encoder": 4,
   "n/prefer-global/text-decoder": 4,
-  "prettier/prettier": 116,
+  "prettier/prettier": 115,
   "promise/always-return": 3,
   "promise/catch-or-return": 2,
   "promise/param-names": 8,
   "no-empty-function": 2,
   "no-shadow": 8,
-  "no-unused-private-class-members": 6
+  "no-unused-private-class-members": 5
 }

eslint.config.mjs

@@ -80,6 +80,8 @@ const config = createConfig([
       // TODO: These rules created more errors after the upgrade to ESLint 9.
       // Re-enable these rules and address any lint violations.
       'jest/no-conditional-in-test': 'warn',
+      'jest/prefer-lowercase-title': 'warn',
+      'jest/prefer-strict-equal': 'warn',
     },
   },
   {

scripts/run-eslint.ts

@@ -94,15 +94,20 @@ async function runESLint(
   options: { quiet: boolean; fix: boolean },
 ): Promise<ESLint.LintResult[]> {
   let results = await eslint.lintFiles(['.']);
+  const errorResults = ESLint.getErrorResults(results);
 
-  const formatter = await eslint.loadFormatter('stylish');
-  const resultText = formatter.format(results);
-  console.log(resultText);
+  if (errorResults.length > 0) {
+    process.exitCode = 1;
+  }
 
   if (options.quiet) {
-    results = ESLint.getErrorResults(results);
+    results = errorResults;
   }
 
+  const formatter = await eslint.loadFormatter('stylish');
+  const resultText = formatter.format(results);
+  console.log(resultText);
+
   if (options.fix) {
     await ESLint.outputFixes(results);
   }
@@ -156,20 +161,19 @@ function evaluateWarnings(results: ESLint.LintResult[]) {
         console.log(
           'The overall number of ESLint warnings have decreased, good work! ❤️ \n',
         );
+        // We are still seeing differences on CI when it comes to linting
+        // results. Never write the thresholds file in that case.
+        // eslint-disable-next-line n/no-process-env
+        if (!process.env.CI) {
+          saveWarningThresholds(warningCounts);
+        }
       }
+
       for (const { ruleId, threshold, count, difference } of changes) {
         console.log(
           `- ${ruleId}: ${threshold} -> ${count} (${difference > 0 ? '+' : ''}${difference})`,
         );
       }
-      // We are still seeing differences on CI when it comes to linting results.
-      // Only write the thresholds file if there are regressions in that case
-      // to prevent the "working directory clean" step in CI from failing.
-      // Also, it's okay to use `process.env` as this is a script.
-      // eslint-disable-next-line n/no-process-env
-      if (regressions.length > 0 || !process.env.CI) {
-        saveWarningThresholds(warningCounts);
-      }
     }
   }
 }