Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CHANGE (CodeAnalyzer): @W-14009441@: create output file when no violations are found #1170

Merged
merged 4 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion messages/RunOutputProcessor.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module.exports = {
"output": {
"engineSummaryTemplate": "Executed %s, found %s violation(s) across %s file(s).",
"noViolationsDetected": "Executed engines: %s. No rule violations found.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message is now no longer used, anywhere. Instead, the above message is used. If there are no violations, a message similar to Executed sfge, found 0 violation(s) across 0 file(s) is printed.

"sevThresholdSummary": "Rule violations of severity %s or more severe were detected.",
"writtenToConsole": "Rule violations were logged to the console.",
"writtenToOutFile": "Rule violations were written to %s."
Expand Down
32 changes: 3 additions & 29 deletions src/lib/formatter/RuleResultRecombinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,7 @@ export class RuleResultRecombinator {
private static constructXml(results: RuleResult[]): string {
let resultXml = ``;

// If the results were just an empty string, we can return it.
if (results.length === 0) {
return resultXml;
}

const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined)
const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined)

let problemCount = 0;

Expand Down Expand Up @@ -188,10 +183,6 @@ export class RuleResultRecombinator {
}

private static constructJunit(results: RuleResult[]): string {
// If there are no results, we can just return an empty string.
if (!results || results.length === 0) {
return '';
}

// Otherwise, we'll need to start constructing our JUnit XML. To do that, we'll need a map from file names to
// lists of the <failure> tags generated from violations found in the corresponding file.
Expand Down Expand Up @@ -279,10 +270,6 @@ URL: ${url}`;
}

private static constructTable(results: RuleResult[]): RecombinedData {
// If the results were just an empty string, we can return it.
if (results.length === 0) {
return '';
}
const columns = this.violationsAreDfa(results)
? ['Source Location', 'Sink Location', 'Description', 'Category', 'URL']
: ['Location', 'Description', 'Category', 'URL'];
Expand Down Expand Up @@ -341,10 +328,6 @@ URL: ${url}`;
}

private static constructJson(results: RuleResult[], verboseViolations = false): string {
if (results.length === 0) {
return '';
}

if (verboseViolations) {
const resultsVerbose = JSON.parse(JSON.stringify(results)) as RuleResult[];
for (const result of resultsVerbose) {
Expand All @@ -363,12 +346,7 @@ URL: ${url}`;
}

private static async constructHtml(results: RuleResult[], verboseViolations = false): Promise<string> {
// If the results were just an empty string, we can return it.
if (results.length === 0) {
return '';
}

const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined);
const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined);
const isDfa = this.violationsAreDfa(results);


Expand Down Expand Up @@ -434,12 +412,8 @@ URL: ${url}`;
}

private static async constructCsv(results: RuleResult[]): Promise<string> {
// If the results were just an empty list, we can return an empty string
if (results.length === 0) {
return '';
}
const isDfa: boolean = this.violationsAreDfa(results);
const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined)
const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined)

const csvRows = [];
// There will always be columns for the problem counter and the severity.
Expand Down
16 changes: 3 additions & 13 deletions src/lib/util/RunOutputProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,11 @@ export class RunOutputProcessor {


public processRunOutput(rrr: RecombinedRuleResults): AnyJson {
const {minSev, summaryMap, results} = rrr;
// If the results are an empty string, it means no violations were found.
if (results === '') {
// Build an appropriate message...
const msg = messages.getMessage('output.noViolationsDetected', [[...summaryMap.keys()].join(', ')]);
// ...log it to the console...
this.ux.log(msg);
// ...and return it for use with the --json flag.
return msg;
}
Comment on lines -31 to -40
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this shortcut return is the main change here. The logic below works fine with 0 violations, it now returns [] when there are no violations, instead of an empty string.

Most of the other changes in this PR are a result of removing these lines. The other changes involve updating the automated tests.

const {minSev, results} = rrr;

// If we actually have violations, there's some stuff we need to do with them. We'll build an array of message parts,
// and then log them all at the end.
// With the list of violations, we'll build an array of message parts, and then log them all at the end.
let msgComponents: string[] = [];
// We need a summary of the information we were provided.
// We need a summary of the information we were provided (blank/empty if no violations).
msgComponents = [...msgComponents, ...this.buildRunSummaryMsgParts(rrr)];
// We need to surface the results directly to the user, then add a message describing what we did.
msgComponents.push(this.opts.outfile ? this.writeToOutfile(results) : this.writeToConsole(results));
Expand Down
11 changes: 10 additions & 1 deletion test/commands/scanner/run.filters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,16 @@ describe('scanner:run tests that result in the use of RuleFilters', function ()
'--engine', 'eslint-lwc'
])
.it('LWC Engine Successfully parses LWC code', ctx => {
expect(ctx.stdout).to.contain('No rule violations found.');
// If there's a summary, then it'll be separated from the CSV by an empty line. Throw it away.
const [csv, _] = ctx.stdout.trim().split(/\n\r?\n/);

// Since it's a CSV, the rows themselves are separated by newline characters, and there's a header row we
// need to discard.
const rows = csv.trim().split('\n');
rows.shift();

// There should be no rows (besides the header) because there are no violations.
expect(rows.length).to.equal(0, 'Should be no violations detected');
MrEminent42 marked this conversation as resolved.
Show resolved Hide resolved
});

setupCommandTest
Expand Down
4 changes: 3 additions & 1 deletion test/commands/scanner/run.severity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ describe('scanner:run', function () {
'--severity-threshold', '3'
])
.it('When no violations are found, no error is thrown', ctx => {
expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
const output = JSON.parse(ctx.stdout);
// There should be no violations (nor any exceptions).
expect(output.length).to.equal(0, 'Should be no violations from one engine');
expect(ctx.stderr).to.not.contain(processorMessages.getMessage('output.sevThresholdSummary', ['3']), 'Error should not be present');
});

Expand Down
Loading