[eslint-bulk-suppressions] Use suppressedMessages

Defer detection of bulk suppressions until after inline suppressions.
Simplify patching.

Author: dmichon-msft

common/changes/@rushstack/eslint-patch/eslint-bulk-overhaul_2024-12-19-00-10.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/eslint-patch",
+      "comment": "Ensure that lint problems suppressed by eslint-bulk-suppressions are available in the `getSuppressedMessages()` function on the linter. Defer evaluation of bulk suppressions until after inline suppressions.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/eslint-patch"
+}

eslint/eslint-patch/src/eslint-bulk-suppressions/bulk-suppressions-file.ts

@@ -2,7 +2,7 @@
 // See LICENSE in the project root for license information.
 
 import fs from 'fs';
-import { VSCODE_PID_ENV_VAR_NAME } from './constants';
+import { VSCODE_PID_ENV_VAR_NAME, SUPPRESSIONS_JSON_FILENAME } from './constants';
 
 export interface ISuppression {
   file: string;
@@ -23,7 +23,6 @@ export interface IBulkSuppressionsJson {
 
 const IS_RUNNING_IN_VSCODE: boolean = process.env[VSCODE_PID_ENV_VAR_NAME] !== undefined;
 const TEN_SECONDS_MS: number = 10 * 1000;
-const SUPPRESSIONS_JSON_FILENAME: string = '.eslint-bulk-suppressions.json';
 
 function throwIfAnythingOtherThanNotExistError(e: NodeJS.ErrnoException): void | never {
   if (e?.code !== 'ENOENT') {
@@ -56,7 +55,8 @@ export function getSuppressionsConfigForEslintrcFolderPath(
     const suppressionsPath: string = `${eslintrcFolderPath}/${SUPPRESSIONS_JSON_FILENAME}`;
     let rawJsonFile: string | undefined;
     try {
-      rawJsonFile = fs.readFileSync(suppressionsPath).toString();
+      // Decoding during read hits an optimized fast path in NodeJS.
+      rawJsonFile = fs.readFileSync(suppressionsPath, 'utf8');
     } catch (e) {
       throwIfAnythingOtherThanNotExistError(e);
     }

eslint/eslint-patch/src/eslint-bulk-suppressions/bulk-suppressions-patch.ts

@@ -8,9 +8,8 @@ import * as Guards from './ast-guards';
 
 import { eslintFolder } from '../_patch-base';
 import {
-  ESLINT_BULK_ENABLE_ENV_VAR_NAME,
-  ESLINT_BULK_PRUNE_ENV_VAR_NAME,
-  ESLINT_BULK_SUPPRESS_ENV_VAR_NAME
+  ESLINT_BULK_SUPPRESS_ENV_VAR_NAME,
+  ESLINT_BULK_ESLINTRC_FOLDER_PATH_ENV_VAR_NAME
 } from './constants';
 import {
   getSuppressionsConfigForEslintrcFolderPath,
@@ -27,21 +26,49 @@ const ESLINTRC_FILENAMES: string[] = [
   // Several other filenames are allowed, but this patch requires that it be loaded via a JS config file,
   // so we only need to check for the JS-based filenames
 ];
-const SUPPRESSION_SYMBOL: unique symbol = Symbol('suppression');
 const ESLINT_BULK_SUPPRESS_ENV_VAR_VALUE: string | undefined = process.env[ESLINT_BULK_SUPPRESS_ENV_VAR_NAME];
 const SUPPRESS_ALL_RULES: boolean = ESLINT_BULK_SUPPRESS_ENV_VAR_VALUE === '*';
 const RULES_TO_SUPPRESS: Set<string> | undefined = ESLINT_BULK_SUPPRESS_ENV_VAR_VALUE
   ? new Set(ESLINT_BULK_SUPPRESS_ENV_VAR_VALUE.split(','))
   : undefined;
 
+interface IBulkSuppression {
+  suppression: ISuppression;
+  serializedSuppression: string;
+}
+
 interface IProblem {
-  [SUPPRESSION_SYMBOL]?: {
-    config: IBulkSuppressionsConfig;
-    suppression: ISuppression;
-    serializedSuppression: string;
+  line: number;
+  column: number;
+  ruleId: string;
+  suppressions?: {
+    kind: string;
+    justification: string;
+  }[];
+}
+
+export type VerifyMethod = (
+  textOrSourceCode: string,
+  config: unknown,
+  filename: string
+) => IProblem[] | undefined;
+
+export interface ILinterClass {
+  prototype: {
+    verify: VerifyMethod;
   };
 }
 
+const astNodeForProblem: Map<IProblem, TSESTree.Node> = new Map();
+
+export function setAstNodeForProblem(problem: IProblem, node: TSESTree.Node): void {
+  astNodeForProblem.set(problem, node);
+}
+
+interface ILinterInternalSlots {
+  lastSuppressedMessages: IProblem[] | undefined;
+}
+
 function getNodeName(node: TSESTree.Node): string | undefined {
   if (Guards.isClassDeclarationWithName(node)) {
     return node.id.name;
@@ -91,6 +118,12 @@ function calculateScopeId(node: NodeWithParent | undefined): string {
 const eslintrcPathByFileOrFolderPath: Map<string, string> = new Map();
 
 function findEslintrcFolderPathForNormalizedFileAbsolutePath(normalizedFilePath: string): string {
+  // Heft, for example, suppresses nested eslintrc files, so it can pass this environment variable to suppress
+  // searching for the eslintrc file completely.
+  let eslintrcFolderPath: string | undefined = process.env[ESLINT_BULK_ESLINTRC_FOLDER_PATH_ENV_VAR_NAME];
+  if (eslintrcFolderPath) {
+    return eslintrcFolderPath;
+  }
   const cachedFolderPathForFilePath: string | undefined =
     eslintrcPathByFileOrFolderPath.get(normalizedFilePath);
   if (cachedFolderPathForFilePath) {
@@ -102,7 +135,6 @@ function findEslintrcFolderPathForNormalizedFileAbsolutePath(normalizedFilePath:
   );
 
   const pathsToCache: string[] = [normalizedFilePath];
-  let eslintrcFolderPath: string | undefined;
   findEslintrcFileLoop: for (
     let currentFolder: string = normalizedFileFolderPath;
     currentFolder; // 'something'.substring(0, -1) is ''
@@ -133,39 +165,46 @@ function findEslintrcFolderPathForNormalizedFileAbsolutePath(normalizedFilePath:
   }
 }
 
-// One-line insert into the ruleContext report method to prematurely exit if the ESLint problem has been suppressed
-export function shouldBulkSuppress(params: {
-  filename: string;
-  currentNode: TSESTree.Node;
-  ruleId: string;
-  problem: IProblem;
-}): boolean {
-  // Use this ENV variable to turn off eslint-bulk-suppressions functionality, default behavior is on
-  if (process.env[ESLINT_BULK_ENABLE_ENV_VAR_NAME] === 'false') {
-    return false;
+let rawGetLinterInternalSlots: ((linter: unknown) => ILinterInternalSlots) | undefined;
+
+export function getLinterInternalSlots(linter: unknown): ILinterInternalSlots {
+  if (!rawGetLinterInternalSlots) {
+    throw new Error('getLinterInternalSlots has not been set');
   }
 
-  const { filename: fileAbsolutePath, currentNode, ruleId: rule, problem } = params;
-  const normalizedFileAbsolutePath: string = fileAbsolutePath.replace(/\\/g, '/');
-  const eslintrcDirectory: string =
-    findEslintrcFolderPathForNormalizedFileAbsolutePath(normalizedFileAbsolutePath);
-  const fileRelativePath: string = normalizedFileAbsolutePath.substring(eslintrcDirectory.length + 1);
+  return rawGetLinterInternalSlots(linter);
+}
+
+export function getBulkSuppression(params: {
+  serializedSuppressions: Set<string>;
+  fileRelativePath: string;
+  problem: IProblem;
+}): IBulkSuppression | undefined {
+  const { fileRelativePath, serializedSuppressions, problem } = params;
+  const { ruleId: rule } = problem;
+
+  const currentNode: TSESTree.Node | undefined = astNodeForProblem.get(problem);
+
   const scopeId: string = calculateScopeId(currentNode);
   const suppression: ISuppression = { file: fileRelativePath, scopeId, rule };
 
-  const config: IBulkSuppressionsConfig = getSuppressionsConfigForEslintrcFolderPath(eslintrcDirectory);
   const serializedSuppression: string = serializeSuppression(suppression);
-  const currentNodeIsSuppressed: boolean = config.serializedSuppressions.has(serializedSuppression);
+  const currentNodeIsSuppressed: boolean = serializedSuppressions.has(serializedSuppression);
 
   if (currentNodeIsSuppressed || SUPPRESS_ALL_RULES || RULES_TO_SUPPRESS?.has(suppression.rule)) {
-    problem[SUPPRESSION_SYMBOL] = {
+    // The suppressions object should already be empty, otherwise we shouldn't see this problem
+    problem.suppressions = [
+      {
+        kind: 'bulk',
+        justification: serializedSuppression
+      }
+    ];
+
+    return {
       suppression,
-      serializedSuppression,
-      config
+      serializedSuppression
     };
   }
-
-  return process.env[ESLINT_BULK_PRUNE_ENV_VAR_NAME] !== '1' && currentNodeIsSuppressed;
 }
 
 export function prune(): void {
@@ -187,15 +226,11 @@ export function prune(): void {
   }
 }
 
+/**
+ * @deprecated Use "prune" instead.
+ */
 export function write(): void {
-  for (const [
-    eslintrcFolderPath,
-    suppressionsConfig
-  ] of getAllBulkSuppressionsConfigsByEslintrcFolderPath()) {
-    if (suppressionsConfig) {
-      writeSuppressionsJsonToFile(eslintrcFolderPath, suppressionsConfig);
-    }
-  }
+  return prune();
 }
 
 // utility function for linter-patch.js to make require statements that use relative paths in linter.js work in linter-patch.js
@@ -209,56 +244,94 @@ export function requireFromPathToLinterJS(importPath: string): import('eslint').
   return require(moduleAbsolutePath);
 }
 
-export function patchClass<T, U extends T>(originalClass: new () => T, patchedClass: new () => U): void {
-  // Get all the property names of the patched class prototype
-  const patchedProperties: string[] = Object.getOwnPropertyNames(patchedClass.prototype);
-
-  // Loop through all the properties
-  for (const prop of patchedProperties) {
-    // Override the property in the original class
-    originalClass.prototype[prop] = patchedClass.prototype[prop];
-  }
+/**
+ * Patches ESLint's Linter class to support bulk suppressions
+ * @param originalClass - The original Linter class from ESLint
+ * @param patchedClass - The patched Linter class from the generated file
+ * @param originalGetLinterInternalSlots - The original getLinterInternalSlots function from ESLint
+ */
+export function patchLinter(
+  originalClass: ILinterClass,
+  patchedClass: ILinterClass,
+  originalGetLinterInternalSlots: typeof getLinterInternalSlots
+): void {
+  // Ensure we use the correct internal slots map
+  rawGetLinterInternalSlots = originalGetLinterInternalSlots;
 
-  // Handle getters and setters
+  // Transfer all properties
   for (const [prop, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(patchedClass.prototype))) {
-    if (descriptor.get || descriptor.set) {
-      Object.defineProperty(originalClass.prototype, prop, descriptor);
-    }
+    Object.defineProperty(originalClass.prototype, prop, descriptor);
   }
-}
 
-/**
- * This returns a wrapped version of the "verify" function from ESLint's Linter class
- * that postprocesses rule violations that weren't suppressed by comments. This postprocessing
- * records suppressions that weren't otherwise suppressed by comments to be used
- * by the "suppress" and "prune" commands.
- */
-export function extendVerifyFunction(
-  originalFn: (this: unknown, ...args: unknown[]) => IProblem[] | undefined
-): (this: unknown, ...args: unknown[]) => IProblem[] | undefined {
-  return function (this: unknown, ...args: unknown[]): IProblem[] | undefined {
-    const problems: IProblem[] | undefined = originalFn.apply(this, args);
-    if (problems) {
+  const originalVerify: (...args: unknown[]) => IProblem[] | undefined = originalClass.prototype.verify as (
+    ...args: unknown[]
+  ) => IProblem[] | undefined;
+  originalClass.prototype.verify = verify;
+
+  function verify(this: unknown, ...args: unknown[]): IProblem[] | undefined {
+    try {
+      const problems: IProblem[] | undefined = originalVerify.apply(this, args);
+      if (!problems) {
+        return problems;
+      }
+
+      const internalSlots: ILinterInternalSlots = getLinterInternalSlots(this);
+
+      if (args.length < 3) {
+        throw new Error('Expected at least 3 arguments to Linter.prototype.verify');
+      }
+
+      const fileNameOrOptions: string | { filename: string } = args[2] as string | { filename: string };
+      const filename: string =
+        typeof fileNameOrOptions === 'string' ? fileNameOrOptions : fileNameOrOptions.filename;
+
+      let { lastSuppressedMessages } = internalSlots;
+
+      const normalizedFileAbsolutePath: string = filename.replace(/\\/g, '/');
+      const eslintrcDirectory: string =
+        findEslintrcFolderPathForNormalizedFileAbsolutePath(normalizedFileAbsolutePath);
+      const fileRelativePath: string = normalizedFileAbsolutePath.substring(eslintrcDirectory.length + 1);
+      const config: IBulkSuppressionsConfig = getSuppressionsConfigForEslintrcFolderPath(eslintrcDirectory);
+      const {
+        newSerializedSuppressions,
+        serializedSuppressions,
+        jsonObject: { suppressions },
+        newJsonObject: { suppressions: newSuppressions }
+      } = config;
+
+      const filteredProblems: IProblem[] = [];
+
       for (const problem of problems) {
-        if (problem[SUPPRESSION_SYMBOL]) {
-          const {
-            serializedSuppression,
-            suppression,
-            config: {
-              newSerializedSuppressions,
-              jsonObject: { suppressions },
-              newJsonObject: { suppressions: newSuppressions }
-            }
-          } = problem[SUPPRESSION_SYMBOL];
-          if (!newSerializedSuppressions.has(serializedSuppression)) {
-            newSerializedSuppressions.add(serializedSuppression);
-            newSuppressions.push(suppression);
-            suppressions.push(suppression);
+        const bulkSuppression: IBulkSuppression | undefined = getBulkSuppression({
+          fileRelativePath,
+          serializedSuppressions,
+          problem
+        });
+
+        if (!bulkSuppression) {
+          filteredProblems.push(problem);
+          continue;
+        }
+
+        const { serializedSuppression, suppression } = bulkSuppression;
+
+        if (!newSerializedSuppressions.has(serializedSuppression)) {
+          newSerializedSuppressions.add(serializedSuppression);
+          newSuppressions.push(suppression);
+          suppressions.push(suppression);
+
+          if (!lastSuppressedMessages) {
+            lastSuppressedMessages = [];
+            internalSlots.lastSuppressedMessages = lastSuppressedMessages;
           }
+
+          lastSuppressedMessages.push(problem);
         }
       }
-    }
 
-    return problems;
-  };
+      return filteredProblems;
+    } finally {
+      astNodeForProblem.clear();
+    }
+  }
 }

eslint/eslint-patch/src/eslint-bulk-suppressions/constants.ts

@@ -7,6 +7,8 @@ export const ESLINT_BULK_SUPPRESS_ENV_VAR_NAME: 'RUSHSTACK_ESLINT_BULK_SUPPRESS'
   'RUSHSTACK_ESLINT_BULK_SUPPRESS';
 export const ESLINT_BULK_ENABLE_ENV_VAR_NAME: 'ESLINT_BULK_ENABLE' = 'ESLINT_BULK_ENABLE';
 export const ESLINT_BULK_PRUNE_ENV_VAR_NAME: 'ESLINT_BULK_PRUNE' = 'ESLINT_BULK_PRUNE';
+export const ESLINT_BULK_ESLINTRC_FOLDER_PATH_ENV_VAR_NAME: 'ESLINT_BULK_ESLINTRC_FOLDER_PATH' =
+  'ESLINT_BULK_ESLINTRC_FOLDER_PATH';
 export const ESLINT_BULK_DETECT_ENV_VAR_NAME: '_RUSHSTACK_ESLINT_BULK_DETECT' =
   '_RUSHSTACK_ESLINT_BULK_DETECT';
 export const ESLINT_BULK_FORCE_REGENERATE_PATCH_ENV_VAR_NAME: 'RUSHSTACK_ESLINT_BULK_FORCE_REGENERATE_PATCH' =
@@ -18,3 +20,5 @@ export const ESLINT_PACKAGE_NAME_ENV_VAR_NAME: '_RUSHSTACK_ESLINT_PACKAGE_NAME'
 
 export const BULK_SUPPRESSIONS_CLI_ESLINT_PACKAGE_NAME: string =
   process.env[ESLINT_PACKAGE_NAME_ENV_VAR_NAME] ?? 'eslint';
+
+export const SUPPRESSIONS_JSON_FILENAME: '.eslint-bulk-suppressions.json' = '.eslint-bulk-suppressions.json';