NEW @W-20485780@ - Add Fixes and Suggestions support in eslint (#441)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: nikhil-mittal-165

packages/code-analyzer-eslint-engine/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-eslint-engine",
   "description": "Plugin package that adds 'eslint' as an engine into Salesforce Code Analyzer",
-  "version": "0.41.0",
+  "version": "0.42.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",

packages/code-analyzer-eslint-engine/src/engine.ts

@@ -6,14 +6,16 @@ import {
     Engine,
     EngineRunResults,
     EventType,
+    Fix,
     LogLevel,
     RuleDescription,
     RunOptions,
+    Suggestion,
     SeverityLevel,
     Violation,
     Workspace,
 } from '@salesforce/code-analyzer-engine-api'
-import {ESLint, Linter} from "eslint";
+import {ESLint, Linter, Rule} from "eslint";
 import {RulesMeta} from "@eslint/core";
 import {ESLintEngineConfig} from "./config";
 import {UserConfigInfo, UserConfigState} from "./user-config-info";
@@ -136,26 +138,40 @@ export class ESLintEngine extends Engine {
             rulesToRun: ruleNames,
             engineConfig: this.engineConfig,
             eslintContext: context,
-            progressRange: [30, 95] // 30% to 95%
+            progressRange: [30, 95], // 30% to 95%
+            includeFixes: runOptions.includeFixes,
+            includeSuggestions: runOptions.includeSuggestions
         }
         const lintResults: ESLint.LintResult[] = await this._runESLintWorkerTask.run(runTaskInput, runOptions.workingFolder);
 
         const engineResults: EngineRunResults = {
-            violations: this.toViolations(lintResults, new Set(ruleNames))
+            violations: await this.toViolations(lintResults, new Set(ruleNames),
+                runOptions.includeFixes ?? false, runOptions.includeSuggestions ?? false)
         };
         this.emitRunRulesProgressEvent(100);
         return engineResults;
     }
 
-    private toViolations(eslintResults: ESLint.LintResult[], specifiedRules: Set<string>): Violation[] {
+    private async toViolations(eslintResults: ESLint.LintResult[], specifiedRules: Set<string>,
+            includeFixes: boolean, includeSuggestions: boolean): Promise<Violation[]> {
         const violations: Violation[] = [];
         for (const eslintResult of eslintResults) {
+            let lineStartOffsets: number[] | undefined;
             for (const resultMsg of eslintResult.messages) {
                 if (!resultMsg.ruleId) { // If there is no ruleName, this is how ESLint indicates something else went wrong (like a parse error).
                     this.handleEslintErrorOrWarning(eslintResult.filePath, resultMsg);
                     continue;
                 }
-                const violation: Violation = toViolation(eslintResult.filePath, resultMsg);
+
+                const needsFileContent = (includeFixes && resultMsg.fix) ||
+                    (includeSuggestions && resultMsg.suggestions?.length);
+                if (needsFileContent && !lineStartOffsets) {
+                    const source = eslintResult.source ?? await fs.readFile(eslintResult.filePath, 'utf8');
+                    lineStartOffsets = computeLineStartOffsets(source);
+                }
+
+                const violation: Violation = toViolation(eslintResult.filePath, resultMsg,
+                    includeFixes, includeSuggestions, lineStartOffsets);
 
                 if (specifiedRules.has(violation.ruleName)) {
                     violations.push(violation);
@@ -219,6 +235,10 @@ function toRuleDescription(ruleName: string, metadata: RulesMeta, status: ESLint
     if (ruleUrl && ruleUrl.includes("://git.soma")) {
         ruleUrl = undefined;
     }
+    if (metadata.fixable) {
+        tags = [...tags, 'Fixable'];
+    }
+
     return {
         name: ruleName,
         severityLevel: severityLevel,
@@ -260,11 +280,10 @@ function toTagsForCustomRule(metadata: RulesMeta): string[] {
 }
 
 
-function toViolation(file: string, resultMsg: Linter.LintMessage): Violation {
-    // Note: If in the future we add in some sort of suggestion or fix field on Violation, then we might want to
-    // leverage the fix and/or suggestions field on the LintMessage object.
-    // See: https://eslint.org/docs/v8.x/integrate/nodejs-api#-lintmessage-type
-    return {
+function toViolation(file: string, resultMsg: Linter.LintMessage,
+        includeFixes: boolean, includeSuggestions: boolean,
+        lineStartOffsets?: number[]): Violation {
+    const violation: Violation = {
         ruleName: resultMsg.ruleId as string,
         message: resultMsg.message,
         codeLocations: [{
@@ -276,6 +295,77 @@ function toViolation(file: string, resultMsg: Linter.LintMessage): Violation {
         }],
         primaryLocationIndex: 0
     };
+
+    if (!lineStartOffsets) {
+        return violation;
+    }
+    if (includeFixes && resultMsg.fix) {
+        violation.fixes = [convertEslintFix(file, resultMsg.fix, lineStartOffsets)];
+    }
+    if (includeSuggestions && resultMsg.suggestions?.length) {
+        violation.suggestions = resultMsg.suggestions.map(s =>
+            convertEslintSuggestion(file, s, lineStartOffsets));
+    }
+
+    return violation;
+}
+
+function computeLineStartOffsets(fileContent: string): number[] {
+    const offsets: number[] = [0];
+    for (let i = 0; i < fileContent.length; i++) {
+        if (fileContent[i] === '\n') {
+            offsets.push(i + 1);
+        }
+    }
+    return offsets;
+}
+
+function indexToLineColumn(index: number, lineStartOffsets: number[]): { line: number, column: number } {
+    // Binary search for the line containing this index
+    let low = 0;
+    let high = lineStartOffsets.length - 1;
+    while (low < high) {
+        const mid = Math.ceil((low + high + 1) / 2);
+        if (mid >= lineStartOffsets.length || lineStartOffsets[mid] > index) {
+            high = mid - 1;
+        } else {
+            low = mid;
+        }
+    }
+    return {
+        line: low + 1, // 1-based
+        column: index - lineStartOffsets[low] + 1 // 1-based
+    };
+}
+
+function convertEslintFix(file: string, eslintFix: Rule.Fix, lineStartOffsets: number[]): Fix {
+    const start = indexToLineColumn(eslintFix.range[0], lineStartOffsets);
+    const end = indexToLineColumn(eslintFix.range[1], lineStartOffsets);
+    return {
+        location: {
+            file: file,
+            startLine: start.line,
+            startColumn: start.column,
+            endLine: end.line,
+            endColumn: end.column
+        },
+        fixedCode: eslintFix.text
+    };
+}
+
+function convertEslintSuggestion(file: string, eslintSuggestion: Linter.LintSuggestion, lineStartOffsets: number[]): Suggestion {
+    const start = indexToLineColumn(eslintSuggestion.fix.range[0], lineStartOffsets);
+    const end = indexToLineColumn(eslintSuggestion.fix.range[1], lineStartOffsets);
+    return {
+        location: {
+            file: file,
+            startLine: start.line,
+            startColumn: start.column,
+            endLine: end.line,
+            endColumn: end.column
+        },
+        message: eslintSuggestion.desc
+    };
 }
 
 function normalizeStartValue(startValue: number): number {

packages/code-analyzer-eslint-engine/src/run-eslint-worker-task.ts

@@ -10,7 +10,9 @@ export type RunESLintWorkerTaskInput = {
     rulesToRun: string[]
     engineConfig: ESLintEngineConfig,
     eslintContext: ESLintContext,
-    progressRange: [number, number]
+    progressRange: [number, number],
+    includeFixes?: boolean,
+    includeSuggestions?: boolean
 }
 
 export class RunESLintWorkerTask extends WorkerTask<RunESLintWorkerTaskInput, ESLint.LintResult[]> {

packages/code-analyzer-eslint-engine/test/engine.test.ts

@@ -34,6 +34,7 @@ const workspaceWithNoCustomConfig: string = path.join(testDataFolder, 'workspace
 const workspaceThatHasCustomConfigModifyingExistingRules: string = path.join(testDataFolder, 'workspaceWithFlatConfigCjs');
 const workspaceThatHasCustomConfigWithNewRules: string = path.join(testDataFolder, 'workspaceWithFlatConfigWithNewRules');
 const workspaceWithReactFiles: string = path.join(testDataFolder, 'workspaceWithReactFiles');
+const workspaceWithFixableViolations: string = path.join(testDataFolder, 'workspace_FixableViolations');
 
 let original_working_directory: string;
 beforeAll(() => {
@@ -1009,6 +1010,180 @@ describe('Tests for emitting events', () => {
     });
 });
 
+describe('Tests for Fixable tag on rule descriptions', () => {
+    it('When a rule has fixable metadata, the Fixable tag is included in its description', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const describeOptions = createDescribeOptions(new Workspace('id', [workspaceWithNoCustomConfig]));
+        const ruleDescriptions: RuleDescription[] = await engine.describeRules(describeOptions);
+
+        const preferConstRule = ruleDescriptions.find(r => r.name === 'prefer-const');
+        expect(preferConstRule).toBeDefined();
+        expect(preferConstRule!.tags).toContain('Fixable');
+    });
+
+    it('When a rule does not have fixable metadata, the Fixable tag is not present', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const describeOptions = createDescribeOptions(new Workspace('id', [workspaceWithNoCustomConfig]));
+        const ruleDescriptions: RuleDescription[] = await engine.describeRules(describeOptions);
+
+        const noInvalidRegexpRule = ruleDescriptions.find(r => r.name === 'no-invalid-regexp');
+        expect(noInvalidRegexpRule).toBeDefined();
+        expect(noInvalidRegexpRule!.tags).not.toContain('Fixable');
+    });
+});
+
+describe('Tests for fixes and suggestions in runRules', () => {
+    const fixableFile: string = path.join(workspaceWithFixableViolations, 'fixable.js');
+
+    it('When includeFixes is true, violations from fixable rules include fixes', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeFixes: true
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const'], runOptions);
+
+        expect(results.violations.length).toBeGreaterThanOrEqual(1);
+        const violationWithFix = results.violations.find(v => v.fixes && v.fixes.length > 0);
+        expect(violationWithFix).toBeDefined();
+
+        const fix = violationWithFix!.fixes![0];
+        expect(fix.fixedCode).toBeDefined();
+        expect(fix.location.file).toEqual(fixableFile);
+        expect(fix.location.startLine).toBeGreaterThanOrEqual(1);
+        expect(fix.location.startColumn).toBeGreaterThanOrEqual(1);
+        expect(fix.location.endLine).toBeGreaterThanOrEqual(fix.location.startLine);
+    });
+
+    it('When includeFixes is false, violations do not include fixes even for fixable rules', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeFixes: false
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const'], runOptions);
+
+        expect(results.violations.length).toBeGreaterThanOrEqual(1);
+        for (const violation of results.violations) {
+            expect(violation.fixes).toBeUndefined();
+        }
+    });
+
+    it('When includeFixes is not specified, violations do not include fixes', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile]));
+        const results: EngineRunResults = await engine.runRules(['prefer-const'], runOptions);
+
+        expect(results.violations.length).toBeGreaterThanOrEqual(1);
+        for (const violation of results.violations) {
+            expect(violation.fixes).toBeUndefined();
+        }
+    });
+
+    it('When includeSuggestions is true, violations with suggestions include them', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeSuggestions: true
+        };
+        const results: EngineRunResults = await engine.runRules(['no-unused-vars'], runOptions);
+
+        const violationsWithSuggestions = results.violations.filter(v => v.suggestions && v.suggestions.length > 0);
+        for (const violation of violationsWithSuggestions) {
+            for (const suggestion of violation.suggestions!) {
+                expect(suggestion.message).toBeDefined();
+                expect(suggestion.location.file).toEqual(fixableFile);
+                expect(suggestion.location.startLine).toBeGreaterThanOrEqual(1);
+                expect(suggestion.location.startColumn).toBeGreaterThanOrEqual(1);
+            }
+        }
+    });
+
+    it('When includeSuggestions is false, violations do not include suggestions', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeSuggestions: false
+        };
+        const results: EngineRunResults = await engine.runRules(['no-unused-vars'], runOptions);
+
+        for (const violation of results.violations) {
+            expect(violation.suggestions).toBeUndefined();
+        }
+    });
+
+    it('When both includeFixes and includeSuggestions are true, both are populated where applicable', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeFixes: true,
+            includeSuggestions: true
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const', 'no-unused-vars'], runOptions);
+
+        expect(results.violations.length).toBeGreaterThanOrEqual(1);
+        const hasAnyFixes = results.violations.some(v => v.fixes && v.fixes.length > 0);
+        expect(hasAnyFixes).toBe(true);
+    });
+
+    it('Fix locations have correct line and column values converted from byte offsets', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeFixes: true
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const'], runOptions);
+
+        const preferConstViolation = results.violations.find(v => v.ruleName === 'prefer-const');
+        expect(preferConstViolation).toBeDefined();
+        expect(preferConstViolation!.fixes).toBeDefined();
+
+        const fix = preferConstViolation!.fixes![0];
+        expect(fix.location.startLine).toEqual(1);
+        expect(fix.location.startColumn).toBeGreaterThanOrEqual(1);
+        expect(fix.fixedCode).toContain('const');
+    });
+
+    it('Multiple violations in the same file produce fixes without errors', async () => {
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [fixableFile])),
+            includeFixes: true
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const', 'no-var'], runOptions);
+
+        const fixableViolations = results.violations.filter(v => v.fixes && v.fixes.length > 0);
+        expect(fixableViolations.length).toBeGreaterThanOrEqual(1);
+        for (const violation of fixableViolations) {
+            expect(violation.fixes![0].location.file).toEqual(fixableFile);
+        }
+    });
+
+    it('Fix locations are correct for files with multi-byte characters', async () => {
+        const multibyteFile: string = path.join(workspaceWithFixableViolations, 'fixable_multibyte.js');
+        const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING);
+        const runOptions: RunOptions = {
+            ...createRunOptions(new Workspace('id', [workspaceWithFixableViolations], [multibyteFile])),
+            includeFixes: true
+        };
+        const results: EngineRunResults = await engine.runRules(['prefer-const'], runOptions);
+
+        expect(results.violations.length).toBeGreaterThanOrEqual(1);
+        const violationWithFix = results.violations.find(v => v.fixes && v.fixes.length > 0);
+        expect(violationWithFix).toBeDefined();
+
+        const fix = violationWithFix!.fixes![0];
+        // Fix should target line 1 (let greeting) and replace with const
+        expect(fix.location.file).toEqual(multibyteFile);
+        expect(fix.location.startLine).toEqual(1);
+        expect(fix.fixedCode).toContain('const');
+        // Verify the line/column values are valid positive integers
+        expect(fix.location.startColumn).toBeGreaterThanOrEqual(1);
+        expect(fix.location.endLine).toBeGreaterThanOrEqual(1);
+        expect(fix.location.endColumn).toBeGreaterThanOrEqual(1);
+    });
+});
+
 function loadRuleDescriptions(fileNameFromTestDataFolder: string): RuleDescription[] {
     return JSON.parse(fs.readFileSync(path.join(testDataFolder,
         fileNameFromTestDataFolder), 'utf8')) as RuleDescription[];