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 Feb 4, 2025
1 parent b21c5c7 commit f69fae4
Show file tree
Hide file tree
Showing 31 changed files with 540 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ private void assertIssues(String projectKey) {
List<Issue> jsIssuesList = getIssues(projectKey + ":src/file.js");
List<Issue> tsIssuesList = getIssues(projectKey + ":src/file.ts");

jsIssuesList.forEach(i -> {
System.out.println(i.toString());
});

assertThat(jsIssuesList)
.extracting(Issue::getLine, Issue::getRule)
.containsExactlyInAnyOrder(
Expand Down
5 changes: 5 additions & 0 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: ['sonarjs/S3923'],
filePath: filePathWithLambda,
});
});

Expand All @@ -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: ['sonarjs/S3923'],
filePath,
});
});

Expand Down
2 changes: 2 additions & 0 deletions packages/jsts/src/linter/issues/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,6 @@ export interface Issue {
cost?: number;
secondaryLocations: Location[];
quickFixes?: QuickFix[];
ruleESLintKeys: Array<string>;
filePath: string;
}
9 changes: 8 additions & 1 deletion packages/jsts/src/linter/issues/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -50,5 +55,7 @@ export function convertMessage(source: SourceCode, message: Linter.LintMessage):
message: message.message,
quickFixes: transformFixes(source, message),
secondaryLocations: [],
ruleESLintKeys: [message.ruleId],
filePath,
};
}
4 changes: 2 additions & 2 deletions packages/jsts/src/linter/issues/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/src/linter/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/jsts/tests/analysis/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,8 @@ describe('analyzeJSTS', () => {
message: 'Octal literals should not be used.',
quickFixes: [],
secondaryLocations: [],
ruleESLintKeys: ['sonarjs/S1314'],
filePath,
},
]);
});
Expand Down
14 changes: 14 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,8 @@ describe('extract', () => {
references: [{ startLine: 10, startCol: 20, endLine: 30, endCol: 40 }],
}),
secondaryLocations: [],
ruleESLintKeys: [],
filePath: 'foo.js',
},
];
expect(extractHighlightedSymbols(issues)).toEqual({
Expand All @@ -56,6 +58,8 @@ describe('extract', () => {
column: 2,
message: '42',
secondaryLocations: [],
ruleESLintKeys: [],
filePath: 'foo.js',
},
];
expect(extractCognitiveComplexity(issues)).toEqual(42);
Expand All @@ -69,6 +73,8 @@ describe('extract', () => {
column: 2,
message: 'nan',
secondaryLocations: [],
ruleESLintKeys: [],
filePath: 'foo.js',
},
];
expect(extractCognitiveComplexity(issues)).toEqual(undefined);
Expand All @@ -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);
Expand All @@ -114,6 +126,8 @@ describe('extract', () => {
column: 2,
message: 'non-extract-message',
secondaryLocations: [],
ruleESLintKeys: [],
filePath: 'foo.js',
},
]);
});
Expand Down
6 changes: 4 additions & 2 deletions packages/jsts/tests/linter/issues/message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<typeof console.error>).mock.calls.map(
call => call.arguments[0],
);
Expand Down
5 changes: 5 additions & 0 deletions packages/jsts/tests/linter/issues/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('transformMessages', () => {
const [issue] = transformMessages(messages, {
sourceCode,
rules: { [ruleId]: rules[ruleId] },
filePath: 'foo.js',
}).issues;
expect(issue).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -73,6 +74,7 @@ describe('transformMessages', () => {
const [issue] = transformMessages(messages, {
sourceCode,
rules: { [ruleId]: rules[ruleId] },
filePath: 'foo.js',
}).issues;
expect(issue).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -101,6 +103,7 @@ describe('transformMessages', () => {
const [issue] = transformMessages(messages, {
sourceCode,
rules: { [ruleId]: rules[ruleId] },
filePath: 'foo.js',
}).issues;
expect(issue).toEqual(
expect.objectContaining({
Expand Down Expand Up @@ -141,6 +144,7 @@ describe('transformMessages', () => {
const [{ secondaryLocations }] = transformMessages(messages, {
sourceCode,
rules: { [ruleId]: rules[ruleId] },
filePath: 'foo.js',
}).issues;
expect(secondaryLocations).toEqual([
{
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,14 @@ public TsConfigFile loadTsConfig(String filename) {
@Override
public TsProgram createProgram(TsProgramRequest tsProgramRequest) {
var response = request(GSON.toJson(tsProgramRequest), "create-program");
return GSON.fromJson(response.json(), TsProgram.class);

try {
return GSON.fromJson(response.json(), TsProgram.class);
} catch (JsonSyntaxException exception) {
LOG.error("Error while deserializing {}", response.json());

throw exception;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
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,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 {
Expand Down Expand Up @@ -61,14 +64,41 @@ 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<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 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 -> {
var filteredOut =
(issue.ruleESLintKeys().contains(externalIssue.name()) &&
issue.filePath().equals(externalIssue.file().uri().getPath()) &&
issue.line() == externalIssue.location().start().line() &&
issue.column() == externalIssue.location().start().lineOffset() &&
issue.endLine() == externalIssue.location().end().line() &&
issue.endColumn() == externalIssue.location().end().lineOffset());

return filteredOut;
})
.findFirst();

if (persistedIssue.isEmpty()) {
ExternalIssueRepository.save(externalIssue, context);
}
}
} catch (CancellationException e) {
// do not propagate the exception
LOG.info(e.toString());
Expand Down Expand Up @@ -103,7 +133,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, and return the list of persisted issues.
*/
protected abstract List<BridgeServer.Issue> analyzeFiles(List<InputFile> inputFiles)
throws IOException;

protected abstract List<InputFile> getInputFiles();
}
Loading

0 comments on commit f69fae4

Please sign in to comment.