From 8b1c375bd4a106e5fa6e92c16fd7ffc1da7493fc Mon Sep 17 00:00:00 2001 From: Eric Morand Date: Thu, 30 Jan 2025 15:54:08 +0100 Subject: [PATCH] WIP Basically, executing the tests using the CI platform. --- packages/bridge/tests/router.test.ts | 8 ++ packages/jsts/src/linter/issues/issue.ts | 2 + packages/jsts/src/linter/issues/message.ts | 9 +- packages/jsts/src/linter/issues/transform.ts | 4 +- packages/jsts/src/linter/wrapper.ts | 2 +- packages/jsts/tests/analysis/analyzer.test.ts | 2 + .../jsts/tests/linter/issues/extract.test.ts | 14 +++ .../jsts/tests/linter/issues/message.test.ts | 6 +- .../tests/linter/issues/transform.test.ts | 5 + .../javascript/bridge/BridgeServer.java | 4 +- .../plugins/javascript/JavaScriptPlugin.java | 2 - .../javascript/analysis/AbstractAnalysis.java | 11 +- .../analysis/AbstractBridgeSensor.java | 41 ++++++- .../analysis/AnalysisProcessor.java | 18 ++- .../analysis/AnalysisWithProgram.java | 14 +-- .../analysis/AnalysisWithWatchProgram.java | 19 ++- .../javascript/analysis/CssRuleSensor.java | 27 ++++- .../javascript/analysis/HtmlSensor.java | 18 ++- .../javascript/analysis/JsTsSensor.java | 6 +- .../javascript/analysis/YamlSensor.java | 17 ++- ...tSensor.java => EslintReportImporter.java} | 112 ++++++++++-------- .../external/ExternalIssueRepository.java | 56 +++++++++ .../plugins/javascript/external/Issue.java | 32 +++++ .../javascript/JavaScriptPluginTest.java | 2 +- .../analysis/AnalysisProcessorTest.java | 14 ++- .../javascript/analysis/JsTsSensorTest.java | 94 ++++++++++++++- .../analysis/QuickFixSupportTest.java | 28 ++++- ...est.java => EslintReportImporterTest.java} | 61 ++++------ .../de-duplicate-issues/eslint-report.json | 29 +++++ 29 files changed, 516 insertions(+), 141 deletions(-) rename sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/{EslintReportSensor.java => EslintReportImporter.java} (61%) create mode 100644 sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/ExternalIssueRepository.java create mode 100644 sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/Issue.java rename sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/{EslintReportSensorTest.java => EslintReportImporterTest.java} (62%) create mode 100644 sonar-plugin/sonar-javascript-plugin/src/test/resources/de-duplicate-issues/eslint-report.json diff --git a/packages/bridge/tests/router.test.ts b/packages/bridge/tests/router.test.ts index 721dfd67943..dc0f1e9a044 100644 --- a/packages/bridge/tests/router.test.ts +++ b/packages/bridge/tests/router.test.ts @@ -200,6 +200,10 @@ describe('router', () => { { key: 'S3923', configurations: [], fileTypeTarget: ['MAIN'] }, ]); const filePath = path.join(fixtures, 'file.yaml'); + /** + * At one point we decided that it would be healthier to put the AWS lambda filename in the file path instead of adding a dedicated `lambdaName` attribute to satisfy UCFG's requirement. 🤷‍♂️ + */ + 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 +219,8 @@ describe('router', () => { "Remove this conditional structure or edit its code blocks so that they're not all the same.", quickFixes: [], secondaryLocations: [], + ruleESLintKeys: ['sonarjs/S3923'], + filePath: filePathWithLambda, }); }); @@ -238,6 +244,8 @@ describe('router', () => { "Remove this conditional structure or edit its code blocks so that they're not all the same.", quickFixes: [], secondaryLocations: [], + ruleESLintKeys: ['sonarjs/S3923'], + filePath, }); }); diff --git a/packages/jsts/src/linter/issues/issue.ts b/packages/jsts/src/linter/issues/issue.ts index 2b81e28d411..473a70ff973 100644 --- a/packages/jsts/src/linter/issues/issue.ts +++ b/packages/jsts/src/linter/issues/issue.ts @@ -43,4 +43,6 @@ export interface Issue { cost?: number; secondaryLocations: Location[]; quickFixes?: QuickFix[]; + ruleESLintKeys: Array; + filePath: string; } diff --git a/packages/jsts/src/linter/issues/message.ts b/packages/jsts/src/linter/issues/message.ts index 92aa842086c..9fdd1b9861e 100644 --- a/packages/jsts/src/linter/issues/message.ts +++ b/packages/jsts/src/linter/issues/message.ts @@ -30,9 +30,14 @@ import { error } from '../../../../shared/src/helpers/logging.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, but it should not * happen because we lint ready SourceCode instances and not file contents. @@ -50,5 +55,7 @@ export function convertMessage(source: SourceCode, message: Linter.LintMessage): message: message.message, quickFixes: transformFixes(source, message), secondaryLocations: [], + ruleESLintKeys: [message.ruleId], + filePath, }; } diff --git a/packages/jsts/src/linter/issues/transform.ts b/packages/jsts/src/linter/issues/transform.ts index ca7061fe7dc..7392616f114 100644 --- a/packages/jsts/src/linter/issues/transform.ts +++ b/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 }, + ctx: { sourceCode: SourceCode; rules: Record; 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); diff --git a/packages/jsts/src/linter/wrapper.ts b/packages/jsts/src/linter/wrapper.ts index 7f9fa88dd6c..28be4adb470 100644 --- a/packages/jsts/src/linter/wrapper.ts +++ b/packages/jsts/src/linter/wrapper.ts @@ -176,7 +176,7 @@ export class LinterWrapper { }; const options = { filename: filePath, allowInlineConfig: false }; const messages = this.linter.verify(sourceCode, config, options); - return transformMessages(messages, { sourceCode, rules }); + return transformMessages(messages, { sourceCode, rules, filePath }); } /** diff --git a/packages/jsts/tests/analysis/analyzer.test.ts b/packages/jsts/tests/analysis/analyzer.test.ts index 3bc6dcff0e6..ee77eeb4442 100644 --- a/packages/jsts/tests/analysis/analyzer.test.ts +++ b/packages/jsts/tests/analysis/analyzer.test.ts @@ -457,6 +457,8 @@ describe('analyzeJSTS', () => { message: 'Octal literals should not be used.', quickFixes: [], secondaryLocations: [], + ruleESLintKeys: ['sonarjs/S1314'], + filePath, }, ]); }); diff --git a/packages/jsts/tests/linter/issues/extract.test.ts b/packages/jsts/tests/linter/issues/extract.test.ts index 370794958e0..1b3017da7f8 100644 --- a/packages/jsts/tests/linter/issues/extract.test.ts +++ b/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,6 +95,8 @@ describe('extract', () => { references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }], }), secondaryLocations: [], + ruleESLintKeys: [], + filePath: 'foo.js', }, { ruleId: 'non-extracted-rule', @@ -96,6 +104,8 @@ describe('extract', () => { column: 2, message: 'non-extract-message', secondaryLocations: [], + ruleESLintKeys: [], + filePath: 'foo.js', }, { ruleId: cognitiveComplexityRule.ruleId, @@ -103,6 +113,8 @@ describe('extract', () => { 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', }, ]); }); diff --git a/packages/jsts/tests/linter/issues/message.test.ts b/packages/jsts/tests/linter/issues/message.test.ts index 35c3a9ea69f..53b9e98b0bf 100644 --- a/packages/jsts/tests/linter/issues/message.test.ts +++ b/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,12 +60,14 @@ describe('convertMessage', () => { }, ], secondaryLocations: [], + ruleESLintKeys: ['sonarjs/S1116'], + filePath: 'foo.bar', }); }); it('should return null when an ESLint message is missing a rule id', () => { console.error = mock.fn(); - expect(convertMessage({} as SourceCode, {} as Linter.LintMessage)).toEqual(null); + expect(convertMessage({} as SourceCode, {} as Linter.LintMessage, '')).toEqual(null); const logs = (console.error as Mock).mock.calls.map( call => call.arguments[0], ); diff --git a/packages/jsts/tests/linter/issues/transform.test.ts b/packages/jsts/tests/linter/issues/transform.test.ts index c6c2503c7cc..12ef31dab34 100644 --- a/packages/jsts/tests/linter/issues/transform.test.ts +++ b/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); diff --git a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java index cb619b8e4b8..a0f9794395e 100644 --- a/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java +++ b/sonar-plugin/bridge/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServer.java @@ -161,7 +161,9 @@ record Issue( String ruleId, List secondaryLocations, Double cost, - List quickFixes + List quickFixes, + List ruleESLintKeys, + String filePath ) {} record QuickFix(String message, List edits) {} diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java index 25d379e2aa7..7df766b16ef 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/JavaScriptPlugin.java @@ -45,7 +45,6 @@ import org.sonar.plugins.javascript.bridge.Environment; import org.sonar.plugins.javascript.bridge.NodeDeprecationWarning; import org.sonar.plugins.javascript.bridge.RulesBundles; -import org.sonar.plugins.javascript.external.EslintReportSensor; import org.sonar.plugins.javascript.external.TslintReportSensor; import org.sonar.plugins.javascript.filter.JavaScriptExclusionsFileFilter; import org.sonar.plugins.javascript.lcov.CoverageSensor; @@ -277,7 +276,6 @@ public void define(Context context) { if (!context.getRuntime().getProduct().equals(SonarProduct.SONARLINT)) { context.addExtensions( CoverageSensor.class, - EslintReportSensor.class, EslintRulesDefinition.class, TslintReportSensor.class, TslintRulesDefinition.class, diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java index 3d5ec45283c..b0c3967e223 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractAnalysis.java @@ -17,6 +17,7 @@ package org.sonar.plugins.javascript.analysis; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -90,14 +91,16 @@ protected boolean isJavaScript(InputFile file) { return inputFileLanguage(file).equals(JavaScriptLanguage.KEY); } - abstract void analyzeFiles(List inputFiles) throws IOException; + abstract List analyzeFiles(List inputFiles) throws IOException; - protected void analyzeFile( + protected List analyzeFile( InputFile file, @Nullable List tsConfigs, @Nullable TsProgram tsProgram, boolean dirtyPackageJSONCache ) throws IOException { + List issues = new ArrayList<>(); + if (context.isCancelled()) { throw new CancellationException( "Analysis interrupted because the SensorContext is in cancelled state" @@ -122,7 +125,7 @@ protected void analyzeFile( ? bridgeServer.analyzeJavaScript(request) : bridgeServer.analyzeTypeScript(request); - analysisProcessor.processResponse(context, checks, file, response); + issues = analysisProcessor.processResponse(context, checks, file, response); cacheStrategy.writeAnalysisToCache( CacheAnalysis.fromResponse(response.ucfgPaths(), response.cpdTokens()), file @@ -137,6 +140,8 @@ protected void analyzeFile( var cacheAnalysis = cacheStrategy.readAnalysisFromCache(); analysisProcessor.processCacheAnalysis(context, file, cacheAnalysis); } + + return issues; } private boolean shouldSkipAstProduction() { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractBridgeSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractBridgeSensor.java index 36e032e9eb4..2dde160a5b6 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractBridgeSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AbstractBridgeSensor.java @@ -32,7 +32,10 @@ import org.sonar.plugins.javascript.bridge.BridgeServer; import org.sonar.plugins.javascript.bridge.BridgeServerConfig; import org.sonar.plugins.javascript.bridge.ServerAlreadyFailedException; +import org.sonar.plugins.javascript.external.EslintReportImporter; +import org.sonar.plugins.javascript.external.ExternalIssueRepository; import org.sonar.plugins.javascript.nodejs.NodeCommandException; +import org.sonar.plugins.javascript.rules.EslintRulesDefinition; import org.sonar.plugins.javascript.utils.Exclusions; public abstract class AbstractBridgeSensor implements Sensor { @@ -61,6 +64,10 @@ public void execute(SensorContext context) { this.contextUtils = new ContextUtils(context); environments = Arrays.asList(context.config().getStringArray(JavaScriptPlugin.ENVIRONMENTS)); globals = Arrays.asList(context.config().getStringArray(JavaScriptPlugin.GLOBALS)); + + var eslintReportImporter = new EslintReportImporter(); + var esLintIssues = eslintReportImporter.execute(context); + try { List inputFiles = getInputFiles(); if (inputFiles.isEmpty()) { @@ -68,7 +75,33 @@ public void execute(SensorContext context) { return; } bridgeServer.startServerLazily(BridgeServerConfig.fromSensorContext(context)); - analyzeFiles(inputFiles); + var issues = analyzeFiles(inputFiles); + + // at that point, we have the list of issues that were persisted + // we can now persist the ESLint issues that match none of the persisted issues + for (var externalIssue : esLintIssues) { + var persistedIssue = issues + .stream() + .filter(issue -> { + return ( + issue.ruleESLintKeys().contains(externalIssue.name()) && + issue.filePath().equals(externalIssue.file().toString()) && + issue.line() == externalIssue.location().start().line() && + issue.column() == externalIssue.location().start().lineOffset() && + issue.endLine() == externalIssue.location().end().line() && + issue.endColumn() == externalIssue.location().end().lineOffset() + ); + }) + .findFirst(); + + if (persistedIssue.isEmpty()) { + ExternalIssueRepository.save( + externalIssue, + context, + EslintRulesDefinition.REPOSITORY_KEY + ); + } + } } catch (CancellationException e) { // do not propagate the exception LOG.info(e.toString()); @@ -103,7 +136,11 @@ protected void logErrorOrWarn(String msg, Throwable e) { LOG.error(msg, e); } - protected abstract void analyzeFiles(List inputFiles) throws IOException; + /** + * Analyze the passed input files, and return the list of persisted issues. + */ + protected abstract List analyzeFiles(List inputFiles) + throws IOException; protected abstract List getInputFiles(); } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java index 4d6f71f378e..4b7d770d61f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java @@ -25,6 +25,7 @@ import static org.sonar.plugins.javascript.utils.UnicodeEscape.unicodeEscape; import java.io.Serializable; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -83,12 +84,17 @@ public AnalysisProcessor( this.uniqueParsingErrors = new HashSet<>(); } - void processResponse( + /** + * todo: To call AnalysisProcessor::saveIssue, we first need to call AnalysisProcessor::processResponse + */ + List processResponse( SensorContext context, JsTsChecks checks, InputFile file, AnalysisResponse response ) { + List issues; + this.context = context; contextUtils = new ContextUtils(context); this.checks = checks; @@ -96,9 +102,11 @@ void processResponse( if (response.parsingError() != null) { uniqueParsingErrors.add(file.absolutePath()); processParsingError(response.parsingError()); - return; + return new ArrayList<>(); } + issues = response.issues(); + if ( YamlSensor.LANGUAGE.equals(file.language()) || HtmlSensor.LANGUAGE.equals(file.language()) ) { @@ -106,16 +114,18 @@ void processResponse( // and symbols. There is an exception for issues, though. Since sonar-iac saves such data for YAML files // from Cloudformation configurations, we can only save issues for these files. Same applies for HTML and // sonar-html plugin. - saveIssues(response.issues()); + saveIssues(issues); } else { // it's important to have an order here: // saving metrics should be done before saving issues so that NO SONAR lines with issues are indeed ignored saveMetrics(response.metrics()); - saveIssues(response.issues()); + saveIssues(issues); saveHighlights(response.highlights()); saveHighlightedSymbols(response.highlightedSymbols()); saveCpd(response.cpdTokens()); } + + return issues; } public int parsingErrorFilesCount() { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithProgram.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithProgram.java index 95d1cde0cb1..2ef89795e67 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithProgram.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithProgram.java @@ -18,11 +18,7 @@ import java.io.IOException; import java.nio.file.Path; -import java.util.ArrayDeque; -import java.util.Deque; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import java.util.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.batch.fs.InputFile; @@ -48,7 +44,9 @@ public AnalysisWithProgram( } @Override - void analyzeFiles(List inputFiles) throws IOException { + List analyzeFiles(List inputFiles) throws IOException { + var issues = new ArrayList(); + var tsConfigs = TsConfigProvider.getTsConfigs(contextUtils, this::createTsConfigFile); progressReport = new ProgressReport(PROGRESS_REPORT_TITLE, PROGRESS_REPORT_PERIOD); progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString()); @@ -99,7 +97,7 @@ void analyzeFiles(List inputFiles) throws IOException { ); for (var f : skippedFiles) { LOG.debug("File not part of any tsconfig.json: {}", f); - analyzeFile(f, null, null, false); + issues.addAll(analyzeFile(f, null, null, false)); } } success = true; @@ -119,6 +117,8 @@ void analyzeFiles(List inputFiles) throws IOException { progressReport.cancel(); } } + + return issues; } private void analyzeProgram(TsProgram program, Set analyzedFiles) throws IOException { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithWatchProgram.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithWatchProgram.java index 5b160c67c7c..e7db049a267 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithWatchProgram.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisWithWatchProgram.java @@ -17,6 +17,7 @@ package org.sonar.plugins.javascript.analysis; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import org.sonar.api.batch.fs.InputFile; import org.sonar.plugins.javascript.bridge.AnalysisWarningsWrapper; @@ -41,7 +42,9 @@ public AnalysisWithWatchProgram( } @Override - public void analyzeFiles(List inputFiles) throws IOException { + public List analyzeFiles(List inputFiles) throws IOException { + var issues = new ArrayList(); + TsConfigProvider.initializeTsConfigCache(contextUtils, this::createTsConfigFile, tsConfigCache); boolean success = false; progressReport = new ProgressReport(PROGRESS_REPORT_TITLE, PROGRESS_REPORT_PERIOD); @@ -49,11 +52,13 @@ public void analyzeFiles(List inputFiles) throws IOException { progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString()); for (InputFile inputFile : inputFiles) { var tsConfigFile = tsConfigCache.getTsConfigForInputFile(inputFile); - analyzeFile( - inputFile, - tsConfigFile == null ? List.of() : List.of(tsConfigFile.getFilename()), - null, - this.tsConfigCache.getAndResetShouldClearDependenciesCache() + issues.addAll( + analyzeFile( + inputFile, + tsConfigFile == null ? List.of() : List.of(tsConfigFile.getFilename()), + null, + this.tsConfigCache.getAndResetShouldClearDependenciesCache() + ) ); } success = true; @@ -72,5 +77,7 @@ public void analyzeFiles(List inputFiles) throws IOException { progressReport.cancel(); } } + + return issues; } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/CssRuleSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/CssRuleSensor.java index e915ed74e89..01ee48c5796 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/CssRuleSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/CssRuleSensor.java @@ -19,6 +19,7 @@ import java.io.File; import java.io.IOException; import java.net.URI; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; @@ -94,7 +95,9 @@ public void execute(SensorContext context) { } @Override - protected void analyzeFiles(List inputFiles) throws IOException { + protected List analyzeFiles(List inputFiles) throws IOException { + var issues = new ArrayList(); + ProgressReport progressReport = new ProgressReport( "Analysis progress", TimeUnit.SECONDS.toMillis(10) @@ -110,7 +113,10 @@ protected void analyzeFiles(List inputFiles) throws IOException { "Analysis interrupted because the SensorContext is in cancelled state" ); } - analyzeFile(inputFile, context, rules); + var fileIssues = analyzeFile(inputFile, context, rules); + + issues.addAll(fileIssues); + progressReport.nextFile(inputFile.toString()); } success = true; @@ -121,14 +127,18 @@ protected void analyzeFiles(List inputFiles) throws IOException { progressReport.cancel(); } } + + return issues; } - void analyzeFile(InputFile inputFile, SensorContext context, List rules) { + List analyzeFile(InputFile inputFile, SensorContext context, List rules) { + List issues; + try { URI uri = inputFile.uri(); if (!"file".equalsIgnoreCase(uri.getScheme())) { LOG.debug("Skipping {} as it has not 'file' scheme", uri); - return; + return new ArrayList<>(); } LOG.debug("Analyzing file: {}", uri); String fileContent = contextUtils.shouldSendFileContent(inputFile) @@ -140,11 +150,16 @@ void analyzeFile(InputFile inputFile, SensorContext context, List rules ); var analysisResponse = bridgeServer.analyzeCss(request); - LOG.debug("Found {} issue(s)", analysisResponse.issues().size()); - saveIssues(context, inputFile, analysisResponse.issues()); + + issues = analysisResponse.issues(); + + LOG.debug("Found {} issue(s)", issues.size()); + saveIssues(context, inputFile, issues); } catch (IOException | RuntimeException e) { throw new IllegalStateException("Failure during analysis of " + inputFile.uri(), e); } + + return issues; } private void saveIssues(SensorContext context, InputFile inputFile, List issues) { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java index d8d954b4550..f8b97da9909 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/HtmlSensor.java @@ -17,6 +17,7 @@ package org.sonar.plugins.javascript.analysis; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.stream.StreamSupport; @@ -63,7 +64,9 @@ public void describe(SensorDescriptor descriptor) { } @Override - protected void analyzeFiles(List inputFiles) throws IOException { + protected List analyzeFiles(List inputFiles) throws IOException { + var issues = new ArrayList(); + var progressReport = new ProgressReport("Analysis progress", TimeUnit.SECONDS.toMillis(10)); analysisMode = AnalysisMode.getMode(context); var success = false; @@ -86,7 +89,7 @@ protected void analyzeFiles(List inputFiles) throws IOException { progressReport.nextFile(inputFile.toString()); var cacheStrategy = CacheStrategies.getStrategyFor(context, inputFile); if (cacheStrategy.isAnalysisRequired()) { - analyze(inputFile, cacheStrategy); + issues.addAll(analyze(inputFile, cacheStrategy)); } } success = true; @@ -97,6 +100,8 @@ protected void analyzeFiles(List inputFiles) throws IOException { progressReport.cancel(); } } + + return issues; } @Override @@ -116,7 +121,10 @@ protected List getInputFiles() { return StreamSupport.stream(inputFiles.spliterator(), false).toList(); } - private void analyze(InputFile file, CacheStrategy cacheStrategy) throws IOException { + private List analyze(InputFile file, CacheStrategy cacheStrategy) + throws IOException { + List issues; + try { LOG.debug("Analyzing file: {}", file.uri()); var fileContent = contextUtils.shouldSendFileContent(file) ? file.contents() : null; @@ -133,7 +141,7 @@ private void analyze(InputFile file, CacheStrategy cacheStrategy) throws IOExcep false ); var response = bridgeServer.analyzeHtml(jsAnalysisRequest); - analysisProcessor.processResponse(context, checks, file, response); + issues = analysisProcessor.processResponse(context, checks, file, response); cacheStrategy.writeAnalysisToCache( CacheAnalysis.fromResponse(response.ucfgPaths(), response.cpdTokens()), file @@ -142,5 +150,7 @@ private void analyze(InputFile file, CacheStrategy cacheStrategy) throws IOExcep LOG.error("Failed to get response while analyzing " + file.uri(), e); throw e; } + + return issues; } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/JsTsSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/JsTsSensor.java index 8d3556fdc39..78375238dbf 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/JsTsSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/JsTsSensor.java @@ -67,7 +67,7 @@ protected List getInputFiles() { } @Override - protected void analyzeFiles(List inputFiles) throws IOException { + protected List analyzeFiles(List inputFiles) throws IOException { var analysisMode = AnalysisMode.getMode(context); bridgeServer.initLinter( checks.eslintRules(), @@ -79,7 +79,9 @@ protected void analyzeFiles(List inputFiles) throws IOException { ); analysis.initialize(context, checks, analysisMode, consumers); - analysis.analyzeFiles(inputFiles); + var issues = analysis.analyzeFiles(inputFiles); consumers.doneAnalysis(); + + return issues; } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java index 1c975d37242..ed644b987c6 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/YamlSensor.java @@ -19,6 +19,7 @@ import static org.sonar.plugins.javascript.JavaScriptFilePredicate.getYamlPredicate; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Scanner; import java.util.concurrent.TimeUnit; @@ -70,7 +71,9 @@ public void describe(SensorDescriptor descriptor) { } @Override - protected void analyzeFiles(List inputFiles) throws IOException { + protected List analyzeFiles(List inputFiles) throws IOException { + var issues = new ArrayList(); + analysisMode = AnalysisMode.getMode(context); var progressReport = new ProgressReport("Analysis progress", TimeUnit.SECONDS.toMillis(10)); var success = false; @@ -91,7 +94,7 @@ protected void analyzeFiles(List inputFiles) throws IOException { ); } progressReport.nextFile(inputFile.toString()); - analyze(inputFile); + issues.addAll(analyze(inputFile)); } success = true; } finally { @@ -101,6 +104,8 @@ protected void analyzeFiles(List inputFiles) throws IOException { progressReport.cancel(); } } + + return issues; } @Override @@ -145,7 +150,9 @@ private static boolean isSamTemplate(InputFile inputFile, Logger logger) { return false; } - private void analyze(InputFile file) throws IOException { + private List analyze(InputFile file) throws IOException { + List issues = new ArrayList<>(); + var cacheStrategy = CacheStrategies.getStrategyFor(context, file); // When there is no analysis required, the sensor doesn't need to do anything as the CPD tokens are handled by the sonar-iac plugin. // See AnalysisProcessor for more details. @@ -166,7 +173,7 @@ private void analyze(InputFile file) throws IOException { false ); var response = bridgeServer.analyzeYaml(jsAnalysisRequest); - analysisProcessor.processResponse(context, checks, file, response); + issues = analysisProcessor.processResponse(context, checks, file, response); cacheStrategy.writeAnalysisToCache( CacheAnalysis.fromResponse(response.ucfgPaths(), response.cpdTokens()), file @@ -176,5 +183,7 @@ private void analyze(InputFile file) throws IOException { throw e; } } + + return issues; } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportSensor.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportImporter.java similarity index 61% rename from sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportSensor.java rename to sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportImporter.java index 8b607c713e2..edb3e015345 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportSensor.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/EslintReportImporter.java @@ -18,50 +18,83 @@ import static org.sonar.plugins.javascript.JavaScriptPlugin.ESLINT_REPORT_PATHS; +import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.api.batch.fs.FilePredicates; import org.sonar.api.batch.fs.InputFile; -import org.sonar.api.batch.fs.TextPointer; import org.sonar.api.batch.fs.TextRange; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.SensorContext; -import org.sonar.api.batch.sensor.issue.NewExternalIssue; -import org.sonar.api.batch.sensor.issue.NewIssueLocation; import org.sonar.api.rules.RuleType; import org.sonar.plugins.javascript.rules.EslintRulesDefinition; +import org.sonarsource.analyzer.commons.ExternalReportProvider; import org.sonarsource.analyzer.commons.ExternalRuleLoader; -public class EslintReportSensor extends AbstractExternalIssuesSensor { +public class EslintReportImporter { - private static final Logger LOG = LoggerFactory.getLogger(EslintReportSensor.class); + private static final Logger LOG = LoggerFactory.getLogger(EslintReportImporter.class); - @Override String linterName() { return EslintRulesDefinition.LINTER_NAME; } - @Override String reportsPropertyName() { return ESLINT_REPORT_PATHS; } - @Override - void importReport(File report, SensorContext context) { + InputFile getInputFile(SensorContext context, String fileName) { + FilePredicates predicates = context.fileSystem().predicates(); + InputFile inputFile = context.fileSystem().inputFile(predicates.hasPath(fileName)); + if (inputFile == null) { + LOG.warn( + "No input file found for {}. No {} issues will be imported on this file.", + fileName, + linterName() + ); + return null; + } + return inputFile; + } + + /** + * Execute the importer, and return the list of external issues found. + */ + public List execute(SensorContext context) { + var results = new ArrayList(); + + List reportFiles = ExternalReportProvider.getReportFiles(context, reportsPropertyName()); + reportFiles.forEach(report -> { + results.addAll(importReport(report, context)); + }); + + return results; + } + + /** + * Import the passed report, and return the list of external issues found. + */ + List importReport(File report, SensorContext context) { LOG.info("Importing {}", report.getAbsoluteFile()); + var results = new ArrayList(); + var serializer = new Gson(); + try ( InputStreamReader inputStreamReader = new InputStreamReader( new FileInputStream(report), StandardCharsets.UTF_8 ) ) { - FileWithMessages[] filesWithMessages = gson.fromJson( + FileWithMessages[] filesWithMessages = serializer.fromJson( inputStreamReader, FileWithMessages[].class ); @@ -70,62 +103,43 @@ void importReport(File report, SensorContext context) { InputFile inputFile = getInputFile(context, fileWithMessages.filePath); if (inputFile != null) { for (EslintError eslintError : fileWithMessages.messages) { - saveEslintError(context, eslintError, inputFile, fileWithMessages.filePath); + if (eslintError.ruleId == null) { + LOG.warn( + "Parse error issue from ESLint will not be imported, file {}", + inputFile.uri() + ); + } else { + results.add(createIssue(eslintError, inputFile)); + } } } } } catch (IOException | JsonSyntaxException e) { - LOG.warn(FILE_EXCEPTION_MESSAGE, e); + LOG.warn("No issues information will be saved as the report file can't be read.", e); } + + return results; } - private static void saveEslintError( - SensorContext context, - EslintError eslintError, - InputFile inputFile, - String originalFilePath - ) { + private static Issue createIssue(EslintError eslintError, InputFile inputFile) { String eslintKey = eslintError.ruleId; - if (eslintKey == null) { - LOG.warn("Parse error issue from ESLint will not be imported, file {}", inputFile.uri()); - return; - } - TextRange location = getLocation(eslintError, inputFile); - TextPointer start = location.start(); ExternalRuleLoader ruleLoader = EslintRulesDefinition.loader(eslintKey); RuleType ruleType = ruleLoader.ruleType(eslintKey); Severity severity = ruleLoader.ruleSeverity(eslintKey); Long effortInMinutes = ruleLoader.ruleConstantDebtMinutes(eslintKey); - LOG.debug( - "Saving external ESLint issue { file:\"{}\", id:{}, message:\"{}\", line:{}, offset:{}, type: {}, severity:{}, remediation:{} }", - originalFilePath, + var issue = new Issue( eslintKey, - eslintError.message, - start.line(), - start.lineOffset(), + inputFile, + location, ruleType, + eslintError.message, severity, - effortInMinutes + effortInMinutes.doubleValue() ); - NewExternalIssue newExternalIssue = context.newExternalIssue(); - - NewIssueLocation primaryLocation = newExternalIssue - .newLocation() - .message(eslintError.message) - .on(inputFile) - .at(location); - - newExternalIssue - .at(primaryLocation) - .engineId(EslintRulesDefinition.REPOSITORY_KEY) - .ruleId(eslintKey) - .type(ruleType) - .severity(severity) - .remediationEffortMinutes(effortInMinutes) - .save(); + return issue; } private static TextRange getLocation(EslintError eslintError, InputFile inputFile) { @@ -135,9 +149,9 @@ private static TextRange getLocation(EslintError eslintError, InputFile inputFil } else { return inputFile.newRange( eslintError.line, - eslintError.column - 1, + eslintError.column - 1, // ESLint location column is 1-based eslintError.endLine, - eslintError.endColumn - 1 + eslintError.endColumn - 1 // ESLint location column is 1-based ); } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/ExternalIssueRepository.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/ExternalIssueRepository.java new file mode 100644 index 00000000000..8e41d8ca77d --- /dev/null +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/ExternalIssueRepository.java @@ -0,0 +1,56 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011-2025 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.plugins.javascript.external; + +import static org.sonar.plugins.javascript.utils.UnicodeEscape.unicodeEscape; + +import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.rule.RuleKey; + +/** + * This is the application of the Repository Pattern. + * It is functional (i.e. exposing static methods) because there is no need to maintain state in a repository. + */ +public class ExternalIssueRepository { + + /** + * Persist the passed issue into the passed context, using the passed rule repository key to resolve the belonging rule. + */ + public static void save(Issue issue, SensorContext context, String ruleRepositoryKey) { + var file = issue.file(); + var newIssue = context.newIssue(); + var location = newIssue.newLocation().on(file); + + if (issue.message() != null) { + var escapedMsg = unicodeEscape(issue.message()); + location.message(escapedMsg); + } + + location.at( + file.newRange( + issue.location().start().line(), + issue.location().start().lineOffset(), + issue.location().end().line(), + issue.location().end().lineOffset() + ) + ); + + newIssue.gap(issue.effort()); + + newIssue.at(location).forRule(RuleKey.of(ruleRepositoryKey, issue.name())).save(); + } +} diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/Issue.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/Issue.java new file mode 100644 index 00000000000..184fc38d9a6 --- /dev/null +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/external/Issue.java @@ -0,0 +1,32 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011-2025 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.plugins.javascript.external; + +import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.TextRange; +import org.sonar.api.batch.rule.Severity; +import org.sonar.api.rules.RuleType; + +public record Issue( + String name, + InputFile file, + TextRange location, + RuleType type, + String message, + Severity severity, + Double effort +) {} diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptPluginTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptPluginTest.java index b252e6134de..f21af3ddaba 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptPluginTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptPluginTest.java @@ -37,7 +37,7 @@ class JavaScriptPluginTest { private static final int BASE_EXTENSIONS = 35; - private static final int SCANNER_EXTENSIONS = 11; + private static final int SCANNER_EXTENSIONS = 10; private static final int SONARLINT_ADDITIONAL_EXTENSIONS = 2; public static final Version LTS_VERSION = Version.create(7, 9); diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/AnalysisProcessorTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/AnalysisProcessorTest.java index b4cc4856579..387e1e76a7a 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/AnalysisProcessorTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/AnalysisProcessorTest.java @@ -155,7 +155,19 @@ void should_not_fail_when_invalid_issue() { var file = TestInputFileBuilder.create("moduleKey", "file.js") .setContents("var x = 1;") .build(); - var issue = new Issue(2, 1, 1, 2, "message", "ruleId", List.of(), 3.14, List.of()); // invalid location startLine > endLine + var issue = new Issue( + 2, + 1, + 1, + 2, + "message", + "ruleId", + List.of(), + 3.14, + List.of(), + List.of("foo"), + "file.js" + ); // invalid location startLine > endLine var response = new AnalysisResponse( null, List.of(issue), diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java index 8aec4d662b2..6a984cf5c77 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/JsTsSensorTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.sonar.plugins.javascript.TestUtils.createInputFile; import com.google.gson.Gson; import java.io.File; @@ -192,6 +193,76 @@ void should_have_descriptor() throws Exception { assertThat(descriptor.languages()).containsOnly("js", "ts"); } + @Test + void should_de_duplicate_issues() throws Exception { + JsTsSensor sensor = createSensor(); + + Path baseDir = Paths.get("src/test/resources/de-duplicate-issues"); + + var context = createSensorContext(baseDir); + + context.settings().setProperty(JavaScriptPlugin.ESLINT_REPORT_PATHS, "eslint-report.json"); + + var content = + "function addOne(i) {\n" + + " if (i != NaN) {\n" + + " return i ++\n" + + " } else {\n" + + " return\n" + + " }\n" + + "};"; + + var inputFile = createInputFile( + context, + "file.js", + StandardCharsets.ISO_8859_1, + baseDir, + content + ); + + var program = new TsProgram("1", List.of(inputFile.absolutePath()), List.of(), false, null); + + AnalysisResponse expectedResponse = createResponse( + List.of( + new BridgeServer.Issue( + 1, + 1, + 2, + 1, + "foo", + "S3923", + List.of(), + 1.0, + List.of(), + List.of("foo-bar"), + "file.js" + ), + new BridgeServer.Issue( + 2, + 8, + 2, + 16, + "foo", + "S3923", + List.of(), + 1.0, + List.of(), + List.of("use-isnan"), + "file.js" + ) + ) + ); + + when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(expectedResponse); + when(bridgeServerMock.createProgram(any())).thenReturn(program); + + sensor.execute(context); + + var issues = context.allIssues(); + + assertThat(issues).hasSize(4); + } + @Test void should_analyse() throws Exception { JsTsSensor sensor = createSensor(); @@ -999,6 +1070,21 @@ private AnalysisWithWatchProgram analysisWithWatchProgram() { ); } + private AnalysisResponse createResponse(List issues) { + var analysisResponse = new AnalysisResponse( + null, + issues, + List.of(), + List.of(), + new BridgeServer.Metrics(), + List.of(), + List.of(), + null + ); + + return analysisResponse; + } + private AnalysisResponse createResponse() { return new Gson() .fromJson( @@ -1019,10 +1105,10 @@ private AnalysisResponse createResponse() { private String createIssues() { return ( - "issues: [{" + - "\"line\":1,\"column\":2,\"endLine\":3,\"endColumn\":4,\"ruleId\":\"S3923\",\"message\":\"Issue message\", \"secondaryLocations\": []}," + - "{\"line\":1,\"column\":1,\"ruleId\":\"S3923\",\"message\":\"Line issue message\", \"secondaryLocations\": []" + - "}]" + "issues: [" + + "{\"line\":1,\"column\":2,\"endLine\":3,\"endColumn\":4,\"ruleId\":\"S3923\",\"message\":\"Issue message\", \"secondaryLocations\": [], \"ruleESLintKeys\": []}," + + "{\"line\":1,\"column\":1,\"ruleId\":\"S3923\",\"message\":\"Line issue message\", \"secondaryLocations\": [], \"ruleESLintKeys\": []}" + + "]" ); } diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/QuickFixSupportTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/QuickFixSupportTest.java index 6a5ee914ec4..0481eb026d8 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/QuickFixSupportTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/analysis/QuickFixSupportTest.java @@ -136,7 +136,19 @@ void test() { static Issue issueWithQuickFix() { var quickFixEdit = new QuickFixEdit(";", new IssueLocation(1, 2, 3, 4, "")); var quickFix = new QuickFix("QuickFix message", List.of(quickFixEdit)); - var issue = new Issue(1, 1, 1, 1, "", "S1116", List.of(), 1.0, List.of(quickFix)); + var issue = new Issue( + 1, + 1, + 1, + 1, + "", + "S1116", + List.of(), + 1.0, + List.of(quickFix), + List.of("foo"), + "index.js" + ); return issue; } @@ -164,7 +176,19 @@ void test_old_version() { @Test void test_null() { var context = createContext(Version.create(6, 3)); - var issue = new Issue(1, 1, 1, 1, "", "S1116", List.of(), 1.0, List.of()); + var issue = new Issue( + 1, + 1, + 1, + 1, + "", + "S1116", + List.of(), + 1.0, + List.of(), + List.of("foo"), + "index.js" + ); var response = new AnalysisResponse( null, List.of(issue), diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportSensorTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportImporterTest.java similarity index 62% rename from sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportSensorTest.java rename to sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportImporterTest.java index 80aadeec6ed..db093bab2d3 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportSensorTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/external/EslintReportImporterTest.java @@ -21,21 +21,19 @@ import java.io.File; import java.util.Collection; -import java.util.Iterator; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.slf4j.event.Level; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.DefaultTextPointer; import org.sonar.api.batch.fs.internal.DefaultTextRange; -import org.sonar.api.batch.sensor.internal.DefaultSensorDescriptor; import org.sonar.api.batch.sensor.internal.SensorContextTester; import org.sonar.api.batch.sensor.issue.ExternalIssue; import org.sonar.api.rules.RuleType; import org.sonar.api.testfixtures.log.LogTesterJUnit5; import org.sonar.plugins.javascript.JavaScriptPlugin; -class EslintReportSensorTest { +class EslintReportImporterTest { @RegisterExtension public final LogTesterJUnit5 logTester = new LogTesterJUnit5(); @@ -54,40 +52,37 @@ class EslintReportSensorTest { private SensorContextTester context = SensorContextTester.create(BASE_DIR); - private EslintReportSensor eslintReportSensor = new EslintReportSensor(); + private EslintReportImporter eslintReportImporter = new EslintReportImporter(); private DefaultInputFile jsInputFile = createInputFile(context, CONTENT, "file.js"); private DefaultInputFile tsInputFile = createInputFile(context, CONTENT, "file-ts.ts"); private DefaultInputFile parseErrorInputFile = createInputFile(context, CONTENT, "parseError.js"); @Test - void should_add_issues_from_report() throws Exception { + void should_create_issues_from_report() throws Exception { logTester.setLevel(Level.DEBUG); setEslintReport("eslint-report.json"); - eslintReportSensor.execute(context); + var issues = eslintReportImporter.execute(context); - Collection externalIssues = context.allExternalIssues(); - assertThat(externalIssues).hasSize(4); - Iterator iterator = externalIssues.iterator(); - ExternalIssue first = iterator.next(); - ExternalIssue second = iterator.next(); - ExternalIssue third = iterator.next(); - ExternalIssue fourth = iterator.next(); + assertThat(issues).hasSize(4); + var iterator = issues.iterator(); + var first = iterator.next(); + var second = iterator.next(); + var third = iterator.next(); + var fourth = iterator.next(); assertThat(first.type()).isEqualTo(RuleType.BUG); assertThat(second.type()).isEqualTo(RuleType.CODE_SMELL); - assertThat(first.primaryLocation().message()).isEqualTo( - "Use the isNaN function to compare with NaN." - ); - assertThat(first.primaryLocation().textRange().start().line()).isEqualTo(2); - assertThat(first.primaryLocation().inputComponent()).isEqualTo(jsInputFile); + assertThat(first.message()).isEqualTo("Use the isNaN function to compare with NaN."); + assertThat(first.location().start().line()).isEqualTo(2); + assertThat(first.file()).isEqualTo(jsInputFile); assertThat(jsInputFile.language()).isEqualTo("js"); - assertThat(third.primaryLocation().textRange()).isEqualTo( + assertThat(third.location()).isEqualTo( new DefaultTextRange(new DefaultTextPointer(2, 0), new DefaultTextPointer(2, 19)) ); - assertThat(fourth.primaryLocation().inputComponent()).isEqualTo(tsInputFile); + assertThat(fourth.file()).isEqualTo(tsInputFile); assertThat(tsInputFile.language()).isEqualTo("ts"); assertThat(logTester.logs(Level.WARN)).contains( @@ -96,19 +91,19 @@ void should_add_issues_from_report() throws Exception { assertThat(logTester.logs(Level.WARN)).contains( "Parse error issue from ESLint will not be imported, file " + parseErrorInputFile.uri() ); - - assertThat(logTester.logs(Level.DEBUG)).containsExactlyInAnyOrder( - "Saving external ESLint issue { file:\"file.js\", id:use-isnan, message:\"Use the isNaN function to compare with NaN.\", line:2, offset:8, type: BUG, severity:MAJOR, remediation:5 }", - "Saving external ESLint issue { file:\"file.js\", id:semi, message:\"Use the isNaN function to compare with NaN.\", line:3, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }", - "Saving external ESLint issue { file:\"file.js\", id:indent, message:\"Expected indentation of 4 spaces but found 0.\", line:2, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }", - "Saving external ESLint issue { file:\"file-ts.ts\", id:semi, message:\"Use the isNaN function to compare with NaN.\", line:3, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }" - ); + // todo: move to the Analysis sensor test + // assertThat(logTester.logs(Level.DEBUG)).containsExactlyInAnyOrder( + // "Saving external ESLint issue { file:\"file.js\", id:use-isnan, message:\"Use the isNaN function to compare with NaN.\", line:2, offset:8, type: BUG, severity:MAJOR, remediation:5 }", + // "Saving external ESLint issue { file:\"file.js\", id:semi, message:\"Use the isNaN function to compare with NaN.\", line:3, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }", + // "Saving external ESLint issue { file:\"file.js\", id:indent, message:\"Expected indentation of 4 spaces but found 0.\", line:2, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }", + // "Saving external ESLint issue { file:\"file-ts.ts\", id:semi, message:\"Use the isNaN function to compare with NaN.\", line:3, offset:0, type: CODE_SMELL, severity:MAJOR, remediation:5 }" + // ); } @Test void should_log_invalid_report() throws Exception { setEslintReport("invalid-eslint-report.json"); - eslintReportSensor.execute(context); + eslintReportImporter.execute(context); Collection externalIssues = context.allExternalIssues(); assertThat(externalIssues).hasSize(0); @@ -121,7 +116,7 @@ void should_log_invalid_report() throws Exception { @Test void should_log_not_existing_report() throws Exception { setEslintReport("not-existing-eslint-report.json"); - eslintReportSensor.execute(context); + eslintReportImporter.execute(context); Collection externalIssues = context.allExternalIssues(); assertThat(externalIssues).hasSize(0); @@ -131,14 +126,6 @@ void should_log_not_existing_report() throws Exception { ); } - @Test - void test_descriptor() throws Exception { - DefaultSensorDescriptor sensorDescriptor = new DefaultSensorDescriptor(); - eslintReportSensor.describe(sensorDescriptor); - assertThat(sensorDescriptor.name()).isEqualTo("Import of ESLint issues"); - assertThat(sensorDescriptor.languages()).isEmpty(); - } - private void setEslintReport(String reportFileName) { context.settings().setProperty(JavaScriptPlugin.ESLINT_REPORT_PATHS, reportFileName); } diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/resources/de-duplicate-issues/eslint-report.json b/sonar-plugin/sonar-javascript-plugin/src/test/resources/de-duplicate-issues/eslint-report.json new file mode 100644 index 00000000000..1db4d8e4375 --- /dev/null +++ b/sonar-plugin/sonar-javascript-plugin/src/test/resources/de-duplicate-issues/eslint-report.json @@ -0,0 +1,29 @@ +[ + { + "filePath": "file.js", + "messages": [ + { + "ruleId": "use-isnan", + "message": "Use the isNaN function to compare with NaN.", + "line": 2, + "column": 9, + "endLine": 2, + "endColumn": 17 + }, + { + "ruleId": "semi", + "message": "Use the isNaN function to compare with NaN.", + "line": 3, + "column": 20 + }, + { + "ruleId": "indent", + "message": "Expected indentation of 4 spaces but found 0.", + "line": 2, + "column": 9, + "endLine": 2, + "endColumn": 9 + } + ] + } +]