[heft-lint-plugin] Add ESLint and TSLint fixer support (#4886)

* Nit: docs and cleanup

* Add fix functionality to tslint

* Add support for --fix in ESLint

* Rush change

* Update heft-plugins/heft-lint-plugin/src/Eslint.ts

Co-authored-by: David Michon <[email protected]>

* Update heft-plugins/heft-lint-plugin/src/Eslint.ts

---------

Co-authored-by: David Michon <[email protected]>

Author: D4N14L

common/changes/@rushstack/heft-lint-plugin/user-danade-LintFixer_2024-08-14-00-32.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/heft-lint-plugin",
+      "comment": "Add autofix functionality for ESLint and TSLint. Fixes can now be applied by providing the \"--fix\" command-line argument, or setting the \"alwaysFix\" plugin option to \"true\"",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/heft-lint-plugin"
+}

heft-plugins/heft-lint-plugin/heft-plugin.json

@@ -4,7 +4,17 @@
   "taskPlugins": [
     {
       "pluginName": "lint-plugin",
-      "entryPoint": "./lib/LintPlugin"
+      "entryPoint": "./lib/LintPlugin",
+      "optionsSchema": "./lib/schemas/heft-lint-plugin.schema.json",
+
+      "parameterScope": "lint",
+      "parameters": [
+        {
+          "longName": "--fix",
+          "parameterKind": "flag",
+          "description": "Fix all encountered rule violations where the violated rule provides a fixer."
+        }
+      ]
     }
   ]
 }

heft-plugins/heft-lint-plugin/src/Eslint.ts

@@ -1,7 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
-import * as path from 'path';
 import * as crypto from 'crypto';
 import * as semver from 'semver';
 import type * as TTypescript from 'typescript';
@@ -26,8 +25,10 @@ enum EslintMessageSeverity {
   error = 2
 }
 
+// Patch the timer used to track rule execution time. This allows us to get access to the detailed information
+// about how long each rule took to execute, which we provide on the CLI when running in verbose mode.
 async function patchTimerAsync(eslintPackagePath: string, timingsMap: Map<string, number>): Promise<void> {
-  const timingModulePath: string = path.join(eslintPackagePath, 'lib', 'linter', 'timing');
+  const timingModulePath: string = `${eslintPackagePath}/lib/linter/timing`;
   const timing: IEslintTiming = (await import(timingModulePath)).default;
   timing.enabled = true;
   const patchedTime: (key: string, fn: (...args: unknown[]) => unknown) => (...args: unknown[]) => unknown = (
@@ -46,26 +47,55 @@ async function patchTimerAsync(eslintPackagePath: string, timingsMap: Map<string
   timing.time = patchedTime;
 }
 
+function getFormattedErrorMessage(lintMessage: TEslint.Linter.LintMessage): string {
+  // https://eslint.org/docs/developer-guide/nodejs-api#◆-lintmessage-type
+  return lintMessage.ruleId ? `(${lintMessage.ruleId}) ${lintMessage.message}` : lintMessage.message;
+}
+
 export class Eslint extends LinterBase<TEslint.ESLint.LintResult> {
   private readonly _eslintPackage: typeof TEslint;
   private readonly _linter: TEslint.ESLint;
   private readonly _eslintTimings: Map<string, number> = new Map();
+  private readonly _currentFixMessages: TEslint.Linter.LintMessage[] = [];
+  private readonly _fixMessagesByResult: Map<TEslint.ESLint.LintResult, TEslint.Linter.LintMessage[]> =
+    new Map();
 
   protected constructor(options: IEslintOptions) {
     super('eslint', options);
 
-    const { buildFolderPath, eslintPackage, linterConfigFilePath, tsProgram, eslintTimings } = options;
+    const { buildFolderPath, eslintPackage, linterConfigFilePath, tsProgram, eslintTimings, fix } = options;
     this._eslintPackage = eslintPackage;
-    this._linter = new eslintPackage.ESLint({
-      cwd: buildFolderPath,
-      overrideConfigFile: linterConfigFilePath,
-      // Override config takes precedence over overrideConfigFile, which allows us to provide
-      // the source TypeScript program to ESLint
-      overrideConfig: {
+
+    let overrideConfig: TEslint.Linter.Config | undefined;
+    let fixFn: Exclude<TEslint.ESLint.Options['fix'], boolean>;
+    if (fix) {
+      // We do not recieve the messages for the issues that were fixed, so we need to track them ourselves
+      // so that we can log them after the fix is applied. This array will be populated by the fix function,
+      // and subsequently mapped to the results in the ESLint.lintFileAsync method below. After the messages
+      // are mapped, the array will be cleared so that it is ready for the next fix operation.
+      fixFn = (message: TEslint.Linter.LintMessage) => {
+        this._currentFixMessages.push(message);
+        return true;
+      };
+    } else {
+      // The @typescript-eslint/parser package allows providing an existing TypeScript program to avoid needing
+      // to reparse. However, fixers in ESLint run in multiple passes against the underlying code until the
+      // fix fully succeeds. This conflicts with providing an existing program as the code no longer maps to
+      // the provided program, producing garbage fix output. To avoid this, only provide the existing program
+      // if we're not fixing.
+      overrideConfig = {
         parserOptions: {
           programs: [tsProgram]
         }
-      }
+      };
+    }
+
+    this._linter = new eslintPackage.ESLint({
+      cwd: buildFolderPath,
+      overrideConfigFile: linterConfigFilePath,
+      // Override config takes precedence over overrideConfigFile
+      overrideConfig,
+      fix: fixFn
     });
     this._eslintTimings = eslintTimings;
   }
@@ -121,17 +151,32 @@ export class Eslint extends LinterBase<TEslint.ESLint.LintResult> {
       filePath: sourceFile.fileName
     });
 
+    // Map the fix messages to the results. This API should only return one result per file, so we can be sure
+    // that the fix messages belong to the returned result. If we somehow receive multiple results, we will
+    // drop the messages on the floor, but since they are only used for logging, this should not be a problem.
+    const fixMessages: TEslint.Linter.LintMessage[] = this._currentFixMessages.splice(0);
+    if (lintResults.length === 1) {
+      this._fixMessagesByResult.set(lintResults[0], fixMessages);
+    }
+
+    this._fixesPossible =
+      this._fixesPossible ||
+      (!this._fix &&
+        lintResults.some((lintResult: TEslint.ESLint.LintResult) => {
+          return lintResult.fixableErrorCount + lintResult.fixableWarningCount > 0;
+        }));
+
     const failures: TEslint.ESLint.LintResult[] = [];
     for (const lintResult of lintResults) {
-      if (lintResult.messages.length > 0) {
+      if (lintResult.messages.length > 0 || lintResult.output) {
         failures.push(lintResult);
       }
     }
 
     return failures;
   }
 
-  protected lintingFinished(lintFailures: TEslint.ESLint.LintResult[]): void {
+  protected async lintingFinishedAsync(lintFailures: TEslint.ESLint.LintResult[]): Promise<void> {
     let omittedRuleCount: number = 0;
     const timings: [string, number][] = Array.from(this._eslintTimings).sort(
       (x: [string, number], y: [string, number]) => {
@@ -150,45 +195,58 @@ export class Eslint extends LinterBase<TEslint.ESLint.LintResult> {
       this._terminal.writeVerboseLine(`${omittedRuleCount} rules took 0ms`);
     }
 
-    const errors: Error[] = [];
-    const warnings: Error[] = [];
-
-    for (const eslintFailure of lintFailures) {
-      for (const message of eslintFailure.messages) {
-        // https://eslint.org/docs/developer-guide/nodejs-api#◆-lintmessage-type
-        const formattedMessage: string = message.ruleId
-          ? `(${message.ruleId}) ${message.message}`
-          : message.message;
-        const errorObject: FileError = new FileError(formattedMessage, {
-          absolutePath: eslintFailure.filePath,
-          projectFolder: this._buildFolderPath,
-          line: message.line,
-          column: message.column
-        });
-        switch (message.severity) {
+    if (this._fix && this._fixMessagesByResult.size > 0) {
+      await this._eslintPackage.ESLint.outputFixes(lintFailures);
+    }
+
+    for (const lintFailure of lintFailures) {
+      // Report linter fixes to the logger. These will only be returned when the underlying failure was fixed
+      const fixMessages: TEslint.Linter.LintMessage[] | undefined =
+        this._fixMessagesByResult.get(lintFailure);
+      if (fixMessages) {
+        for (const fixMessage of fixMessages) {
+          const formattedMessage: string = `[FIXED] ${getFormattedErrorMessage(fixMessage)}`;
+          const errorObject: FileError = this._getLintFileError(lintFailure, fixMessage, formattedMessage);
+          this._scopedLogger.emitWarning(errorObject);
+        }
+      }
+
+      // Report linter errors and warnings to the logger
+      for (const lintMessage of lintFailure.messages) {
+        const errorObject: FileError = this._getLintFileError(lintFailure, lintMessage);
+        switch (lintMessage.severity) {
           case EslintMessageSeverity.error: {
-            errors.push(errorObject);
+            this._scopedLogger.emitError(errorObject);
             break;
           }
 
           case EslintMessageSeverity.warning: {
-            warnings.push(errorObject);
+            this._scopedLogger.emitWarning(errorObject);
             break;
           }
         }
       }
     }
-
-    for (const error of errors) {
-      this._scopedLogger.emitError(error);
-    }
-
-    for (const warning of warnings) {
-      this._scopedLogger.emitWarning(warning);
-    }
   }
 
   protected async isFileExcludedAsync(filePath: string): Promise<boolean> {
     return await this._linter.isPathIgnored(filePath);
   }
+
+  private _getLintFileError(
+    lintResult: TEslint.ESLint.LintResult,
+    lintMessage: TEslint.Linter.LintMessage,
+    message?: string
+  ): FileError {
+    if (!message) {
+      message = getFormattedErrorMessage(lintMessage);
+    }
+
+    return new FileError(message, {
+      absolutePath: lintResult.filePath,
+      projectFolder: this._buildFolderPath,
+      line: lintMessage.line,
+      column: lintMessage.column
+    });
+  }
 }

heft-plugins/heft-lint-plugin/src/LintPlugin.ts

@@ -22,10 +22,23 @@ import type { IExtendedProgram, IExtendedSourceFile } from './internalTypings/Ty
 
 const PLUGIN_NAME: 'lint-plugin' = 'lint-plugin';
 const TYPESCRIPT_PLUGIN_NAME: typeof TypeScriptPluginName = 'typescript-plugin';
+const FIX_PARAMETER_NAME: string = '--fix';
 const ESLINTRC_JS_FILENAME: string = '.eslintrc.js';
 const ESLINTRC_CJS_FILENAME: string = '.eslintrc.cjs';
 
-export default class LintPlugin implements IHeftTaskPlugin {
+interface ILintPluginOptions {
+  alwaysFix?: boolean;
+}
+
+interface ILintOptions {
+  taskSession: IHeftTaskSession;
+  heftConfiguration: HeftConfiguration;
+  tsProgram: IExtendedProgram;
+  fix?: boolean;
+  changedFiles?: ReadonlySet<IExtendedSourceFile>;
+}
+
+export default class LintPlugin implements IHeftTaskPlugin<ILintPluginOptions> {
   private readonly _lintingPromises: Promise<void>[] = [];
 
   // These are initliazed by _initAsync
@@ -35,7 +48,11 @@ export default class LintPlugin implements IHeftTaskPlugin {
   private _tslintToolPath: string | undefined;
   private _tslintConfigFilePath: string | undefined;
 
-  public apply(taskSession: IHeftTaskSession, heftConfiguration: HeftConfiguration): void {
+  public apply(
+    taskSession: IHeftTaskSession,
+    heftConfiguration: HeftConfiguration,
+    pluginOptions?: ILintPluginOptions
+  ): void {
     // Disable linting in watch mode. Some lint rules require the context of multiple files, which
     // may not be available in watch mode.
     if (!taskSession.parameters.watch) {
@@ -48,12 +65,15 @@ export default class LintPlugin implements IHeftTaskPlugin {
           accessor.onChangedFilesHook.tap(
             PLUGIN_NAME,
             (changedFilesHookOptions: IChangedFilesHookOptions) => {
-              const lintingPromise: Promise<void> = this._lintAsync(
+              const lintingPromise: Promise<void> = this._lintAsync({
                 taskSession,
                 heftConfiguration,
-                changedFilesHookOptions.program as IExtendedProgram,
-                changedFilesHookOptions.changedFiles as ReadonlySet<IExtendedSourceFile>
-              );
+                fix:
+                  pluginOptions?.alwaysFix ||
+                  taskSession.parameters.getFlagParameter(FIX_PARAMETER_NAME).value,
+                tsProgram: changedFilesHookOptions.program as IExtendedProgram,
+                changedFiles: changedFilesHookOptions.changedFiles as ReadonlySet<IExtendedSourceFile>
+              });
               lintingPromise.catch(() => {
                 // Suppress unhandled promise rejection error
               });
@@ -114,12 +134,9 @@ export default class LintPlugin implements IHeftTaskPlugin {
     }
   }
 
-  private async _lintAsync(
-    taskSession: IHeftTaskSession,
-    heftConfiguration: HeftConfiguration,
-    tsProgram: IExtendedProgram,
-    changedFiles?: ReadonlySet<IExtendedSourceFile>
-  ): Promise<void> {
+  private async _lintAsync(options: ILintOptions): Promise<void> {
+    const { taskSession, heftConfiguration, tsProgram, changedFiles, fix } = options;
+
     // Ensure that we have initialized. This promise is cached, so calling init
     // multiple times will only init once.
     await this._ensureInitializedAsync(taskSession, heftConfiguration);
@@ -128,6 +145,7 @@ export default class LintPlugin implements IHeftTaskPlugin {
     if (this._eslintConfigFilePath && this._eslintToolPath) {
       const eslintLinter: Eslint = await Eslint.initializeAsync({
         tsProgram,
+        fix,
         scopedLogger: taskSession.logger,
         linterToolPath: this._eslintToolPath,
         linterConfigFilePath: this._eslintConfigFilePath,
@@ -140,6 +158,7 @@ export default class LintPlugin implements IHeftTaskPlugin {
     if (this._tslintConfigFilePath && this._tslintToolPath) {
       const tslintLinter: Tslint = await Tslint.initializeAsync({
         tsProgram,
+        fix,
         scopedLogger: taskSession.logger,
         linterToolPath: this._tslintToolPath,
         linterConfigFilePath: this._tslintConfigFilePath,

heft-plugins/heft-lint-plugin/src/LinterBase.ts

@@ -20,6 +20,7 @@ export interface ILinterBaseOptions {
   linterToolPath: string;
   linterConfigFilePath: string;
   tsProgram: IExtendedProgram;
+  fix?: boolean;
 }
 
 export interface IRunLinterOptions {
@@ -56,6 +57,9 @@ export abstract class LinterBase<TLintResult> {
   protected readonly _buildFolderPath: string;
   protected readonly _buildMetadataFolderPath: string;
   protected readonly _linterConfigFilePath: string;
+  protected readonly _fix: boolean;
+
+  protected _fixesPossible: boolean = false;
 
   private readonly _linterName: string;
 
@@ -66,6 +70,7 @@ export abstract class LinterBase<TLintResult> {
     this._buildMetadataFolderPath = options.buildMetadataFolderPath;
     this._linterConfigFilePath = options.linterConfigFilePath;
     this._linterName = linterName;
+    this._fix = options.fix || false;
   }
 
   public abstract printVersionHeader(): void;
@@ -156,7 +161,13 @@ export abstract class LinterBase<TLintResult> {
     }
     //#endregion
 
-    this.lintingFinished(lintFailures);
+    await this.lintingFinishedAsync(lintFailures);
+
+    if (!this._fix && this._fixesPossible) {
+      this._terminal.writeWarningLine(
+        'The linter reported that fixes are possible. To apply fixes, run Heft with the "--fix" option.'
+      );
+    }
 
     const updatedTslintCacheData: ILinterCacheData = {
       cacheVersion: linterCacheVersion,
@@ -173,7 +184,7 @@ export abstract class LinterBase<TLintResult> {
 
   protected abstract lintFileAsync(sourceFile: IExtendedSourceFile): Promise<TLintResult[]>;
 
-  protected abstract lintingFinished(lintFailures: TLintResult[]): void;
+  protected abstract lintingFinishedAsync(lintFailures: TLintResult[]): Promise<void>;
 
   protected abstract isFileExcludedAsync(filePath: string): Promise<boolean>;
 }