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 3, 2025
1 parent b21c5c7 commit 8b1c375
Show file tree
Hide file tree
Showing 29 changed files with 516 additions and 141 deletions.
8 changes: 8 additions & 0 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
});
});

Expand All @@ -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,
});
});

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 @@ -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,44 @@ 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 -> {
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());
Expand Down Expand Up @@ -103,7 +136,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 8b1c375

Please sign in to comment.