JS-542 - Implement the deduplication of issues raised from both SonarJS and ESLint (#5089)

Co-authored-by: Eric Morand <eroc.morand@sonarsource.com>
Co-authored-by: Victor Diez <victor.diez@sonarsource.com>

Author: 3 people

packages/bridge/tests/router.test.ts

@@ -200,6 +200,7 @@ describe('router', () => {
       { key: 'S3923', configurations: [], fileTypeTarget: ['MAIN'] },
     ]);
     const filePath = path.join(fixtures, 'file.yaml');
+    const filePathWithLambda = path.join(fixtures, 'file-SomeLambdaFunction.yaml');
     const data = { filePath };
     const response = (await request(server, '/analyze-yaml', 'POST', data)) as string;
     const {
@@ -215,6 +216,8 @@ describe('router', () => {
         "Remove this conditional structure or edit its code blocks so that they're not all the same.",
       quickFixes: [],
       secondaryLocations: [],
+      ruleESLintKeys: ['no-all-duplicated-branches'],
+      filePath: filePathWithLambda,
     });
   });
 
@@ -238,6 +241,8 @@ describe('router', () => {
         "Remove this conditional structure or edit its code blocks so that they're not all the same.",
       quickFixes: [],
       secondaryLocations: [],
+      ruleESLintKeys: ['no-all-duplicated-branches'],
+      filePath,
     });
   });
 

packages/jsts/src/linter/issues/issue.ts

@@ -43,4 +43,6 @@ export interface Issue {
   cost?: number;
   secondaryLocations: Location[];
   quickFixes?: QuickFix[];
+  ruleESLintKeys: Array<string>;
+  filePath: string;
 }

packages/jsts/src/linter/issues/message.ts

@@ -17,6 +17,22 @@
 import { Linter, SourceCode } from 'eslint';
 import { transformFixes } from '../quickfixes/transform.js';
 import { Issue } from './issue.js';
+import * as ruleMetas from '../../rules/metas.js';
+
+function getESLintKeys(sonarKey: string) {
+  const ruleMeta = ruleMetas[sonarKey as keyof typeof ruleMetas];
+  if (!ruleMeta?.eslintId) {
+    return [];
+  }
+  const keys = new Set<string>();
+  keys.add(ruleMeta.eslintId);
+  if (ruleMeta.implementation === 'decorated') {
+    ruleMeta.externalRules.forEach(externalRule => {
+      keys.add(externalRule.externalRule);
+    });
+  }
+  return Array.from(keys);
+}
 
 /**
  * Converts an ESLint message into a SonarQube issue
@@ -29,9 +45,14 @@ import { Issue } from './issue.js';
  *
  * @param source the source code
  * @param message the ESLint message to convert
+ * @param filePath the path to the file where the issue was found
  * @returns the converted SonarQube issue
  */
-export function convertMessage(source: SourceCode, message: Linter.LintMessage): Issue | null {
+export function convertMessage(
+  source: SourceCode,
+  message: Linter.LintMessage,
+  filePath: string,
+): Issue | null {
   /**
    * The property `ruleId` equals `null` on parsing errors and not applied directives.
    * The first should not happen because we lint ready SourceCode instances and not file contents.
@@ -40,14 +61,17 @@ export function convertMessage(source: SourceCode, message: Linter.LintMessage):
   if (!message.ruleId?.startsWith('sonarjs/')) {
     return null;
   }
+  const ruleId = message.ruleId.slice(8); // remove "sonarjs/" prefix
   return {
-    ruleId: message.ruleId.slice(8), // remove "sonarjs/" prefix
+    ruleId,
     line: message.line,
     column: message.column,
     endLine: message.endLine,
     endColumn: message.endColumn,
     message: message.message,
     quickFixes: transformFixes(source, message),
     secondaryLocations: [],
+    ruleESLintKeys: getESLintKeys(ruleId),
+    filePath,
   };
 }

packages/jsts/src/linter/issues/transform.ts

@@ -67,7 +67,7 @@ export type LintingResult = {
  */
 export function transformMessages(
   messages: Linter.LintMessage[],
-  ctx: { sourceCode: SourceCode; rules: Record<string, Rule.RuleModule> },
+  ctx: { sourceCode: SourceCode; rules: Record<string, Rule.RuleModule>; filePath: string },
 ): LintingResult {
   const issues: Issue[] = [];
   const ucfgPaths: string[] = [];
@@ -76,7 +76,7 @@ export function transformMessages(
     if (message.ruleId === 'sonarjs/ucfg') {
       ucfgPaths.push(message.message);
     } else {
-      let issue = convertMessage(ctx.sourceCode, message);
+      let issue = convertMessage(ctx.sourceCode, message, ctx.filePath);
       if (issue !== null) {
         issue = normalizeLocation(decodeSonarRuntime(ctx.rules[issue.ruleId], issue));
         issues.push(issue);

packages/jsts/src/linter/wrapper.ts

@@ -183,7 +183,7 @@ export class LinterWrapper {
     };
 
     const messages = this.linter.verify(sourceCode, config, createOptions(filePath));
-    return transformMessages(messages, { sourceCode, rules });
+    return transformMessages(messages, { sourceCode, rules, filePath });
   }
 
   /**

packages/jsts/tests/analysis/analyzer.test.ts

@@ -457,6 +457,8 @@ describe('analyzeJSTS', () => {
         message: 'Octal literals should not be used.',
         quickFixes: [],
         secondaryLocations: [],
+        ruleESLintKeys: ['no-octal'],
+        filePath,
       },
     ]);
   });

packages/jsts/tests/linter/issues/extract.test.ts

@@ -36,6 +36,8 @@ describe('extract', () => {
           references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }],
         }),
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
     ];
     expect(extractHighlightedSymbols(issues)).toEqual({
@@ -56,6 +58,8 @@ describe('extract', () => {
         column: 2,
         message: '42',
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
     ];
     expect(extractCognitiveComplexity(issues)).toEqual(42);
@@ -69,6 +73,8 @@ describe('extract', () => {
         column: 2,
         message: 'nan',
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
     ];
     expect(extractCognitiveComplexity(issues)).toEqual(undefined);
@@ -89,20 +95,26 @@ describe('extract', () => {
           references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }],
         }),
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
       {
         ruleId: 'non-extracted-rule',
         line: 1,
         column: 2,
         message: 'non-extract-message',
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
       {
         ruleId: cognitiveComplexityRule.ruleId,
         line: 1,
         column: 2,
         message: '42',
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
     ];
     extractHighlightedSymbols(issues);
@@ -114,6 +126,8 @@ describe('extract', () => {
         column: 2,
         message: 'non-extract-message',
         secondaryLocations: [],
+        ruleESLintKeys: [],
+        filePath: 'foo.js',
       },
     ]);
   });

packages/jsts/tests/linter/issues/message.test.ts

@@ -36,7 +36,7 @@ describe('convertMessage', () => {
       rules: { [`sonarjs/${ruleId}`]: 'error' },
     });
 
-    expect(convertMessage(sourceCode, message)).toEqual({
+    expect(convertMessage(sourceCode, message, 'foo.bar')).toEqual({
       ruleId,
       line: 1,
       column: 9,
@@ -60,10 +60,12 @@ describe('convertMessage', () => {
         },
       ],
       secondaryLocations: [],
+      ruleESLintKeys: ['no-extra-semi'],
+      filePath: 'foo.bar',
     });
   });
 
   it('should return null when an ESLint message is missing a rule id', () => {
-    expect(convertMessage({} as SourceCode, {} as Linter.LintMessage)).toEqual(null);
+    expect(convertMessage({} as SourceCode, {} as Linter.LintMessage, '')).toEqual(null);
   });
 });

packages/jsts/tests/linter/issues/transform.test.ts

@@ -44,6 +44,7 @@ describe('transformMessages', () => {
     const [issue] = transformMessages(messages, {
       sourceCode,
       rules: { [ruleId]: rules[ruleId] },
+      filePath: 'foo.js',
     }).issues;
     expect(issue).toEqual(
       expect.objectContaining({
@@ -73,6 +74,7 @@ describe('transformMessages', () => {
     const [issue] = transformMessages(messages, {
       sourceCode,
       rules: { [ruleId]: rules[ruleId] },
+      filePath: 'foo.js',
     }).issues;
     expect(issue).toEqual(
       expect.objectContaining({
@@ -101,6 +103,7 @@ describe('transformMessages', () => {
     const [issue] = transformMessages(messages, {
       sourceCode,
       rules: { [ruleId]: rules[ruleId] },
+      filePath: 'foo.js',
     }).issues;
     expect(issue).toEqual(
       expect.objectContaining({
@@ -141,6 +144,7 @@ describe('transformMessages', () => {
     const [{ secondaryLocations }] = transformMessages(messages, {
       sourceCode,
       rules: { [ruleId]: rules[ruleId] },
+      filePath: 'foo.js',
     }).issues;
     expect(secondaryLocations).toEqual([
       {
@@ -168,6 +172,7 @@ describe('transformMessages', () => {
     const { issues, ucfgPaths } = transformMessages(messages as Linter.LintMessage[], {
       sourceCode,
       rules: {},
+      filePath: 'foo.js',
     });
     expect(ucfgPaths.length).toEqual(1);
     expect(issues.length).toEqual(0);

sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java

@@ -161,7 +161,9 @@ record Issue(
     String ruleId,
     List<IssueLocation> secondaryLocations,
     Double cost,
-    List<QuickFix> quickFixes
+    List<QuickFix> quickFixes,
+    List<String> ruleESLintKeys,
+    String filePath
   ) {}
 
   record QuickFix(String message, List<QuickFixEdit> edits) {}