Skip to content

Commit

Permalink
CHANGE @W-17053004@ Polished run command output
Browse files Browse the repository at this point in the history
  • Loading branch information
jfeingold35 committed Nov 8, 2024
1 parent 865619b commit 6f2a293
Show file tree
Hide file tree
Showing 25 changed files with 330 additions and 297 deletions.
16 changes: 16 additions & 0 deletions messages/action-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
5 changes: 3 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 @@ -60,6 +60,7 @@ export default class RunCommand extends SfCommand<void> implements Displayable {
description: getMessage(BundleName.RunCommand, 'flags.view.description'),
char: 'v',
default: View.TABLE,
exclusive: ['output-file'],
options: Object.values(View)
}),
'output-file': Flags.string({
Expand Down Expand Up @@ -108,7 +109,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
12 changes: 7 additions & 5 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 @@ -78,9 +78,11 @@ export class RunAction {
// or file handlers that must be gracefully ended.
this.dependencies.progressListeners.forEach(listener => listener.stopListening());
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']);
const resultsWritten: boolean = this.dependencies.writer.write(results);
if (!resultsWritten) {
this.dependencies.resultsViewer.view(results);
}
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 {
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));
}
}
}
20 changes: 5 additions & 15 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 @@ -123,8 +114,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]));
this.display.displayTable(resultRows, TABLE_COLUMNS);
}
}
Expand Down
62 changes: 0 additions & 62 deletions src/lib/viewers/RunSummaryViewer.ts

This file was deleted.

13 changes: 9 additions & 4 deletions src/lib/writers/ResultsWriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {OutputFormat, RunResults} from '@salesforce/code-analyzer-core';
import {BundleName, getMessage} from '../messages';

export interface ResultsWriter {
write(results: RunResults): void;
write(results: RunResults): boolean;
}

export class CompositeResultsWriter implements ResultsWriter {
Expand All @@ -14,8 +14,12 @@ export class CompositeResultsWriter implements ResultsWriter {
this.writers = writers;
}

public write(results: RunResults): void {
this.writers.forEach(w => w.write(results));
public write(results: RunResults): boolean {
let written: boolean = false;
for (const writer of this.writers) {
written = writer.write(results) || /* istanbul ignore next */ written;
}
return written;
}

public static fromFiles(files: string[]): CompositeResultsWriter {
Expand Down Expand Up @@ -49,7 +53,8 @@ export class ResultsFileWriter implements ResultsWriter {
}
}

public write(results: RunResults): void {
public write(results: RunResults): boolean {
fs.writeFileSync(this.file, results.toFormattedOutput(this.format));
return true;
}
}
10 changes: 10 additions & 0 deletions test/commands/code-analyzer/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,5 +334,15 @@ describe('`code-analyzer run` tests', () => {
expect(receivedActionDependencies.resultsViewer.constructor.name).toEqual('ResultsDetailDisplayer');
});
});

describe('Flag interactions', () => {
it('--output-file and --view are mutually exclusive', async () => {
const outfileInputValue = './somefile.json';
const viewInputValue = 'detail';
const executionPromise = RunCommand.run(['--output-file', outfileInputValue, '--view', viewInputValue]);
await expect(executionPromise).rejects.toThrow('--output-file=./somefile.json cannot also be provided when using --view');
expect(executeSpy).not.toHaveBeenCalled();
});
});
});

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

=== Summary

Found 0 violations.

Additional log information written to:
Loading

0 comments on commit 6f2a293

Please sign in to comment.