Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
Basically, executing the tests using the CI platform.
  • Loading branch information
Eric Morand committed Jan 31, 2025
1 parent b21c5c7 commit 8e0e629
Show file tree
Hide file tree
Showing 29 changed files with 506 additions and 198 deletions.
2 changes: 2 additions & 0 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ describe('router', () => {
"Remove this conditional structure or edit its code blocks so that they're not all the same.",
quickFixes: [],
secondaryLocations: [],
ruleESLintKey: 'sonarjs/S3923',
});
});

Expand All @@ -238,6 +239,7 @@ describe('router', () => {
"Remove this conditional structure or edit its code blocks so that they're not all the same.",
quickFixes: [],
secondaryLocations: [],
ruleESLintKey: 'sonarjs/S3923',
});
});

Expand Down
1 change: 1 addition & 0 deletions packages/jsts/src/linter/issues/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,5 @@ export interface Issue {
cost?: number;
secondaryLocations: Location[];
quickFixes?: QuickFix[];
ruleESLintKeys: Array<string>;
}
1 change: 1 addition & 0 deletions packages/jsts/src/linter/issues/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ export function convertMessage(source: SourceCode, message: Linter.LintMessage):
message: message.message,
quickFixes: transformFixes(source, message),
secondaryLocations: [],
ruleESLintKeys: [message.ruleId],
};
}
1 change: 1 addition & 0 deletions packages/jsts/tests/analysis/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ describe('analyzeJSTS', () => {
message: 'Octal literals should not be used.',
quickFixes: [],
secondaryLocations: [],
ruleESLintKeys: ['sonarjs/S1314'],
},
]);
});
Expand Down
7 changes: 7 additions & 0 deletions packages/jsts/tests/linter/issues/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('extract', () => {
references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }],
}),
secondaryLocations: [],
ruleESLintKeys: [],
},
];
expect(extractHighlightedSymbols(issues)).toEqual({
Expand All @@ -56,6 +57,7 @@ describe('extract', () => {
column: 2,
message: '42',
secondaryLocations: [],
ruleESLintKeys: [],
},
];
expect(extractCognitiveComplexity(issues)).toEqual(42);
Expand All @@ -69,6 +71,7 @@ describe('extract', () => {
column: 2,
message: 'nan',
secondaryLocations: [],
ruleESLintKeys: [],
},
];
expect(extractCognitiveComplexity(issues)).toEqual(undefined);
Expand All @@ -89,20 +92,23 @@ describe('extract', () => {
references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }],
}),
secondaryLocations: [],
ruleESLintKeys: [],
},
{
ruleId: 'non-extracted-rule',
line: 1,
column: 2,
message: 'non-extract-message',
secondaryLocations: [],
ruleESLintKeys: [],
},
{
ruleId: cognitiveComplexityRule.ruleId,
line: 1,
column: 2,
message: '42',
secondaryLocations: [],
ruleESLintKeys: [],
},
];
extractHighlightedSymbols(issues);
Expand All @@ -114,6 +120,7 @@ describe('extract', () => {
column: 2,
message: 'non-extract-message',
secondaryLocations: [],
ruleESLintKeys: [],
},
]);
});
Expand Down
1 change: 1 addition & 0 deletions packages/jsts/tests/linter/issues/message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ describe('convertMessage', () => {
},
],
secondaryLocations: [],
ruleESLintKeys: ['sonarjs/S1116'],
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ record Issue(
String ruleId,
List<IssueLocation> secondaryLocations,
Double cost,
List<QuickFix> quickFixes
List<QuickFix> quickFixes,
List<String> ruleESLintKeys
) {}

record QuickFix(String message, List<QuickFixEdit> edits) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +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;
import org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl;
Expand Down Expand Up @@ -277,9 +275,7 @@ public void define(Context context) {
if (!context.getRuntime().getProduct().equals(SonarProduct.SONARLINT)) {
context.addExtensions(
CoverageSensor.class,
EslintReportSensor.class,
EslintRulesDefinition.class,
TslintReportSensor.class,
TslintRulesDefinition.class,
AnalysisWithProgram.class
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,14 +91,16 @@ protected boolean isJavaScript(InputFile file) {
return inputFileLanguage(file).equals(JavaScriptLanguage.KEY);
}

abstract void analyzeFiles(List<InputFile> inputFiles) throws IOException;
abstract List<BridgeServer.Issue> analyzeFiles(List<InputFile> inputFiles) throws IOException;

protected void analyzeFile(
protected List<BridgeServer.Issue> analyzeFile(
InputFile file,
@Nullable List<String> tsConfigs,
@Nullable TsProgram tsProgram,
boolean dirtyPackageJSONCache
) throws IOException {
List<BridgeServer.Issue> issues = new ArrayList<>();

if (context.isCancelled()) {
throw new CancellationException(
"Analysis interrupted because the SensorContext is in cancelled state"
Expand All @@ -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
Expand All @@ -137,6 +140,8 @@ protected void analyzeFile(
var cacheAnalysis = cacheStrategy.readAnalysisFromCache();
analysisProcessor.processCacheAnalysis(context, file, cacheAnalysis);
}

return issues;
}

private boolean shouldSkipAstProduction() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
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.EslintReportSensor;
import org.sonar.plugins.javascript.external.ExternalIssueRepository;
import org.sonar.plugins.javascript.nodejs.NodeCommandException;
import org.sonar.plugins.javascript.utils.Exclusions;

Expand Down Expand Up @@ -61,14 +63,34 @@ 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 esLintReporter = new EslintReportSensor();
var externalIssues = esLintReporter.execute(context);

try {
List<InputFile> inputFiles = getInputFiles();
if (inputFiles.isEmpty()) {
LOG.info("No input files found for analysis");
return;
}
bridgeServer.startServerLazily(BridgeServerConfig.fromSensorContext(context));
analyzeFiles(inputFiles);
var issues = analyzeFiles(inputFiles);

// at that point, we have the list of issues that were registered
// we can register the ESLint issues that have not already been registered
for (var externalIssue : externalIssues) {
var registeredIssue = issues
.stream()
.filter(issue -> {
// todo: assert location and filename
return issue.ruleESLintKeys().contains(externalIssue.name());
})
.findFirst();

if (registeredIssue.isEmpty()) {
ExternalIssueRepository.saveIssue(externalIssue, context);
}
}
} catch (CancellationException e) {
// do not propagate the exception
LOG.info(e.toString());
Expand Down Expand Up @@ -103,7 +125,11 @@ protected void logErrorOrWarn(String msg, Throwable e) {
LOG.error(msg, e);
}

protected abstract void analyzeFiles(List<InputFile> inputFiles) throws IOException;
/**
* Analyze the passed input files, register issues to the instance context, and return the list of registered issues
*/
protected abstract List<BridgeServer.Issue> analyzeFiles(List<InputFile> inputFiles)
throws IOException;

protected abstract List<InputFile> getInputFiles();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,39 +84,48 @@ public AnalysisProcessor(
this.uniqueParsingErrors = new HashSet<>();
}

void processResponse(
/**
* todo: To call AnalysisProcessor::saveIssue, we first need to call AnalysisProcessor::processResponse
*/
List<Issue> processResponse(
SensorContext context,
JsTsChecks checks,
InputFile file,
AnalysisResponse response
) {
List<Issue> issues;

this.context = context;
contextUtils = new ContextUtils(context);
this.checks = checks;
this.file = file;
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())
) {
// SonarQube expects that there is a single analyzer that saves analysis data like metrics, highlighting,
// 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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -48,7 +44,9 @@ public AnalysisWithProgram(
}

@Override
void analyzeFiles(List<InputFile> inputFiles) throws IOException {
List<BridgeServer.Issue> analyzeFiles(List<InputFile> inputFiles) throws IOException {
var issues = new ArrayList<BridgeServer.Issue>();

var tsConfigs = TsConfigProvider.getTsConfigs(contextUtils, this::createTsConfigFile);
progressReport = new ProgressReport(PROGRESS_REPORT_TITLE, PROGRESS_REPORT_PERIOD);
progressReport.start(inputFiles.size(), inputFiles.iterator().next().toString());
Expand Down Expand Up @@ -99,7 +97,7 @@ void analyzeFiles(List<InputFile> 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;
Expand All @@ -119,6 +117,8 @@ void analyzeFiles(List<InputFile> inputFiles) throws IOException {
progressReport.cancel();
}
}

return issues;
}

private void analyzeProgram(TsProgram program, Set<InputFile> analyzedFiles) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,19 +42,23 @@ public AnalysisWithWatchProgram(
}

@Override
public void analyzeFiles(List<InputFile> inputFiles) throws IOException {
public List<BridgeServer.Issue> analyzeFiles(List<InputFile> inputFiles) throws IOException {
var issues = new ArrayList<BridgeServer.Issue>();

TsConfigProvider.initializeTsConfigCache(contextUtils, this::createTsConfigFile, tsConfigCache);
boolean success = false;
progressReport = new ProgressReport(PROGRESS_REPORT_TITLE, PROGRESS_REPORT_PERIOD);
try {
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;
Expand All @@ -72,5 +77,7 @@ public void analyzeFiles(List<InputFile> inputFiles) throws IOException {
progressReport.cancel();
}
}

return issues;
}
}
Loading

0 comments on commit 8e0e629

Please sign in to comment.