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 @W-17053004@ Polished run command output #1669

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
16 changes: 16 additions & 0 deletions messages/action-summary-viewer.md
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New messages are the still-relevant ones from the now-deleted run-summary-viewer.md.

Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,19 @@ Found %d rule(s) from %d engine(s):
# rules-action.rules-item

%d %s rule(s) found.

# run-action.found-no-violations

Found 0 violations.

# run-action.violations-total

Found %d violation(s) across %d file(s):

# run-action.violations-item

%d %s severity violation(s) found.

# run-action.outfiles-total

Results written to:
2 changes: 1 addition & 1 deletion messages/progress-event-listener.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ done.
Executing rules

# execution-spinner.progress-summary
%d of %d engines still executing after %ds.
%d of %d engines finished after %ds.

# execution-spinner.engine-status
- %s at %d% completion.
Expand Down
8 changes: 4 additions & 4 deletions messages/results-viewer.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# summary.detail.found-results

Found %d violation(s) across %d file(s):

# summary.detail.violation-header

%d. %s

# summary.table.results-relative-to

Violation file paths relative to '%s'.

# summary.table.found-results

Found %d violation(s) across %d file(s) relative to '%s':
Expand Down
27 changes: 0 additions & 27 deletions messages/run-summary-viewer.md

This file was deleted.

2 changes: 1 addition & 1 deletion src/commands/code-analyzer/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default class ConfigCommand extends SfCommand<void> implements Displayabl

protected createDependencies(outputFile?: string): ConfigDependencies {
const uxDisplay: UxDisplay = new UxDisplay(this, this.spinner);
const modelGeneratorFunction = (relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) => {
const modelGeneratorFunction = /* istanbul ignore next - Model tested separately */ (relevantEngines: Set<string>, userContext: ConfigContext, defaultContext: ConfigContext) => {
return AnnotatedConfigModel.fromSelection(relevantEngines, userContext, defaultContext);
};
const dependencies: ConfigDependencies = {
Expand Down
4 changes: 2 additions & 2 deletions src/commands/code-analyzer/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {CodeAnalyzerConfigFactoryImpl} from '../../lib/factories/CodeAnalyzerCon
import {EnginePluginsFactoryImpl} from '../../lib/factories/EnginePluginsFactory';
import {CompositeResultsWriter} from '../../lib/writers/ResultsWriter';
import {ResultsDetailDisplayer, ResultsTableDisplayer} from '../../lib/viewers/ResultsViewer';
import {RunSummaryDisplayer} from '../../lib/viewers/RunSummaryViewer';
import {RunActionSummaryViewer} from '../../lib/viewers/ActionSummaryViewer';
import {BundleName, getMessage, getMessages} from '../../lib/messages';
import {LogEventDisplayer} from '../../lib/listeners/LogEventListener';
import {EngineRunProgressSpinner, RuleSelectionProgressSpinner} from '../../lib/listeners/ProgressEventListener';
Expand Down Expand Up @@ -108,7 +108,7 @@ export default class RunCommand extends SfCommand<void> implements Displayable {
resultsViewer: view === View.TABLE
? new ResultsTableDisplayer(uxDisplay)
: new ResultsDetailDisplayer(uxDisplay),
runSummaryViewer: new RunSummaryDisplayer(uxDisplay)
actionSummaryViewer: new RunActionSummaryViewer(uxDisplay)
};
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/lib/actions/RunAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {EnginePluginsFactory} from '../factories/EnginePluginsFactory';
import {createPathStarts} from '../utils/PathStartUtil';
import {createWorkspace} from '../utils/WorkspaceUtil';
import {ResultsViewer} from '../viewers/ResultsViewer';
import {RunSummaryViewer} from '../viewers/RunSummaryViewer';
import {RunActionSummaryViewer} from '../viewers/ActionSummaryViewer';
import {ResultsWriter} from '../writers/ResultsWriter';
import {LogFileWriter} from '../writers/LogWriter';
import {LogEventListener, LogEventLogger} from '../listeners/LogEventListener';
Expand All @@ -27,7 +27,7 @@ export type RunDependencies = {
progressListeners: ProgressEventListener[];
writer: ResultsWriter;
resultsViewer: ResultsViewer;
runSummaryViewer: RunSummaryViewer;
actionSummaryViewer: RunActionSummaryViewer;
}

export type RunInput = {
Expand Down Expand Up @@ -80,7 +80,7 @@ export class RunAction {
this.dependencies.logEventListeners.forEach(listener => listener.stopListening());
this.dependencies.writer.write(results);
this.dependencies.resultsViewer.view(results);
this.dependencies.runSummaryViewer.view(results, logWriter.getLogDestination(), input['output-file']);
this.dependencies.actionSummaryViewer.view(results, logWriter.getLogDestination(), input['output-file']);

const thresholdValue = input['severity-threshold'];
if (thresholdValue) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/listeners/ProgressEventListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export class EngineRunProgressSpinner extends ProgressSpinner implements Progres
engineLines.push(getMessage(BundleName.ProgressEventListener, 'execution-spinner.engine-status', [name, progress]));
}
return [
getMessage(BundleName.ProgressEventListener, 'execution-spinner.progress-summary', [unfinishedEngines, totalEngines, secondsRunning]),
getMessage(BundleName.ProgressEventListener, 'execution-spinner.progress-summary', [totalEngines - unfinishedEngines, totalEngines, secondsRunning]),
...engineLines
].join('\n');
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ export function getMessage(bundle: BundleName, messageKey: string, tokens?: Toke
}

export function getMessages(bundle: BundleName, messageKey: string, tokens?: Tokens): string[] {
INSTANCE = INSTANCE || new MessageCatalog();
INSTANCE = INSTANCE || /* istanbul ignore next */ new MessageCatalog();
return INSTANCE.getMessages(bundle, messageKey, tokens);
}
60 changes: 59 additions & 1 deletion src/lib/viewers/ActionSummaryViewer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Display} from '../Display';
import {RuleSelection} from '@salesforce/code-analyzer-core';
import {RuleSelection, RunResults, SeverityLevel, Violation} from '@salesforce/code-analyzer-core';
import {toStyledHeader, indent} from '../utils/StylingUtil';
import {BundleName, getMessage} from '../messages';

Expand Down Expand Up @@ -75,6 +75,64 @@ export class RulesActionSummaryViewer extends AbstractActionSummaryViewer {
this.display.displayLog(indent(getMessage(BundleName.ActionSummaryViewer, 'rules-action.rules-item', [ruleCountForEngine, engineName])));
}
}
}

export class RunActionSummaryViewer extends AbstractActionSummaryViewer {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code is almost identical to the code from RunSummaryViewer.ts, but it's different to accomplish the goals of the story.

public constructor(display: Display) {
super(display);
}

public view(results: RunResults, logFile: string, outfiles: string[]): void {
// Start with separator to cleanly break from anything that's already been logged.
this.displayLineSeparator();
this.displaySummaryHeader();
this.displayLineSeparator();

if (results.getViolationCount() === 0) {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'run-action.found-no-violations'));
} else {
this.displayResultsSummary(results);
}
this.displayLineSeparator();

if (outfiles.length > 0) {
this.displayOutfiles(outfiles);
this.displayLineSeparator();
}

this.displayLogFileInfo(logFile);
}

private displayResultsSummary(results: RunResults): void {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'run-action.violations-total', [results.getViolationCount(), this.countUniqueFiles(results.getViolations())]));
for (const sev of Object.values(SeverityLevel)) {
// Some of the keys will be numbers, since the enum is numerical. Skip those.
if (typeof sev !== 'string') {
continue;
}
const sevCount = results.getViolationCountOfSeverity(SeverityLevel[sev] as SeverityLevel);
if (sevCount > 0) {
this.display.displayLog(indent(getMessage(BundleName.ActionSummaryViewer, 'run-action.violations-item', [sevCount, sev])));
}
}
}

private countUniqueFiles(violations: Violation[]): number {
const fileSet: Set<string> = new Set();
violations.forEach(v => {
const primaryLocation = v.getCodeLocations()[v.getPrimaryLocationIndex()];
const file = primaryLocation.getFile();
if (file) {
fileSet.add(file);
}
});
return fileSet.size;
}

private displayOutfiles(outfiles: string[]): void {
this.display.displayLog(getMessage(BundleName.ActionSummaryViewer, 'run-action.outfiles-total'));
for (const outfile of outfiles) {
this.display.displayLog(indent(outfile));
}
}
}
33 changes: 12 additions & 21 deletions src/lib/viewers/ResultsViewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,14 @@ abstract class AbstractResultsDisplayer implements ResultsViewer {
if (results.getViolationCount() === 0) {
return;
} else {
this.displayLineSeparator();
this._view(results);
this.display.displayLog('\n');
this.displayLineSeparator();
}
}

protected countUniqueFiles(violations: Violation[]): number {
const fileSet: Set<string> = new Set();
violations.forEach(v => {
const primaryLocation = v.getCodeLocations()[v.getPrimaryLocationIndex()];
const file = primaryLocation.getFile();
if (file) {
fileSet.add(file);
}
});
return fileSet.size;
protected displayLineSeparator(): void {
this.display.displayLog('');
}

protected abstract _view(results: RunResults): void;
Expand All @@ -44,8 +37,6 @@ export class ResultsDetailDisplayer extends AbstractResultsDisplayer {
protected _view(results: RunResults): void {
const violations = sortViolations(results.getViolations());

this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.detail.found-results', [
violations.length, this.countUniqueFiles(violations)]));
this.displayDetails(violations);
}

Expand Down Expand Up @@ -147,8 +138,7 @@ export class ResultsTableDisplayer extends AbstractResultsDisplayer {
}
});

this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.table.found-results', [
violations.length, this.countUniqueFiles(violations), parentFolder]));
this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.table.results-relative-to', [parentFolder]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to show this if there are no results?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It doesn't show if there are no results. The method is only called when there are results to show.

this.display.displayTable(resultRows, TABLE_COLUMNS);
}
}
Expand All @@ -164,22 +154,22 @@ function sortViolations(violations: Violation[]): Violation[] {
// Next, compare file names.
const v1PrimaryLocation = v1.getCodeLocations()[v1.getPrimaryLocationIndex()];
const v2PrimaryLocation = v2.getCodeLocations()[v2.getPrimaryLocationIndex()];
const v1File = v1PrimaryLocation.getFile() || '';
const v2File = v2PrimaryLocation.getFile() || '';
const v1File = v1PrimaryLocation.getFile() || /* istanbul ignore next */ '';
const v2File = v2PrimaryLocation.getFile() || /* istanbul ignore next */ '';
if (v1File !== v2File) {
return v1File.localeCompare(v2File);
}

// Next, compare start lines.
const v1StartLine = v1PrimaryLocation.getStartLine() || 0;
const v2StartLine = v2PrimaryLocation.getStartLine() || 0;
const v1StartLine = v1PrimaryLocation.getStartLine() || /* istanbul ignore next */ 0;
const v2StartLine = v2PrimaryLocation.getStartLine() || /* istanbul ignore next */ 0;
if (v1StartLine !== v2StartLine) {
return v1StartLine - v2StartLine;
}

// Next, compare start columns.
const v1StartColumn = v1PrimaryLocation.getStartColumn() || 0;
const v2StartColumn = v2PrimaryLocation.getStartColumn() || 0;
const v1StartColumn = v1PrimaryLocation.getStartColumn() || /* istanbul ignore next */ 0;
const v2StartColumn = v2PrimaryLocation.getStartColumn() || /* istanbul ignore next */ 0;
return v1StartColumn - v2StartColumn;
});
}
Expand All @@ -196,6 +186,7 @@ function getPrimaryLocation(violation: Violation): CodeLocation {
export function findLongestCommonParentFolderOf(filePaths: string[]): string {
const roots: string[] = filePaths.map(filePath => path.parse(filePath).root);
const commonRoot: string = (new Set(roots)).size === 1 ? roots[0] : '';
// istanbul ignore next - Hard to test outside of Windows
if (!commonRoot) {
return '';
}
Expand Down
62 changes: 0 additions & 62 deletions src/lib/viewers/RunSummaryViewer.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

=== Summary

Found 0 violations.

Additional log information written to:
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

=== Summary

Found 1 violation(s) across 1 file(s):
1 Low severity violation(s) found.

Results written to:
{{PATH_TO_FILE1}}
{{PATH_TO_FILE2}}

Additional log information written to:
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

=== Summary

Found 2 violation(s) across 1 file(s):
1 High severity violation(s) found.
1 Low severity violation(s) found.

Additional log information written to:
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Found 4 violation(s) across 1 file(s):

=== 1. stub1RuleA
severity: 4 (Low)
engine: stubEngine1
Expand Down
Loading