Skip to content

NEW @W-17159591@ Added version property to output #142

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.

Sign up for GitHub

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

Merged
jfeingold35 merged 1 commit into from
Dec 3, 2024
Merged

NEW @W-17159591@ Added version property to output #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion packages/code-analyzer-core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"main": "dist/index.js",
"types": "dist/index.d.ts",
"dependencies": {
"@salesforce/code-analyzer-engine-api": "0.14.0",
"@salesforce/code-analyzer-engine-api": "0.15.0-SNAPSHOT",
"@types/js-yaml": "^4.0.9",
"@types/node": "^20.0.0",
"@types/sarif": "^2.1.7",
Expand Down
4 changes: 2 additions & 2 deletions packages/code-analyzer-core/src/code-analyzer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,11 @@ export class CodeAnalyzer {
try {
apiEngineRunResults = await engine.runRules(rulesToRun, engineRunOptions);
} catch (error) {
return new UnexpectedErrorEngineRunResults(engineName, error as Error);
return new UnexpectedErrorEngineRunResults(engineName, await engine.getEngineVersion(), error as Error);
}

validateEngineRunResults(engineName, apiEngineRunResults, ruleSelection);
const engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, apiEngineRunResults, ruleSelection);
const engineRunResults: EngineRunResults = new EngineRunResultsImpl(engineName, await engine.getEngineVersion(), apiEngineRunResults, ruleSelection);

this.emitEvent<EngineRunProgressEvent>({
type: EventType.EngineRunProgressEvent, timestamp: this.clock.now(), engineName: engineName, percentComplete: 100
Expand Down
15 changes: 15 additions & 0 deletions packages/code-analyzer-core/src/output-formats/json-output-format.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@ export type JsonResultsOutput = {
sev4: number
sev5: number
}
versions: JsonVersionOutput
violations: JsonViolationOutput[]
}

export type JsonVersionOutput = {
[engine: string]: string
}

export type JsonViolationOutput = {
rule: string
engine: string
Expand Down Expand Up @@ -53,10 +58,20 @@ export function toJsonResultsOutput(results: RunResults, sanitizeFcn: (text: str
sev4: results.getViolationCountOfSeverity(SeverityLevel.Low),
sev5: results.getViolationCountOfSeverity(SeverityLevel.Info),
},
versions: toJsonVersionObject(results),
violations: toJsonViolationOutputArray(results.getViolations(), results.getRunDirectory(), sanitizeFcn)
};
}

function toJsonVersionObject(results: RunResults): JsonVersionOutput {
const versions: JsonVersionOutput = {};
const engineNames: string[] = results.getEngineNames();
for (const engineName of engineNames) {
versions[engineName] = results.getEngineRunResults(engineName).getEngineVersion();
}
return versions;
}

export function toJsonViolationOutputArray(violations: Violation[], runDir: string, sanitizeFcn: (text: string) => string): JsonViolationOutput[] {
return violations.map(v => toJsonViolationOutput(v, runDir, sanitizeFcn));
}
Expand Down
1 change: 1 addition & 0 deletions packages/code-analyzer-core/src/output-formats/sarif-output-format.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function toSarifRun(engineRunResults: EngineRunResults, runDir: string): sarif.R
tool: {
driver: {
name: engineRunResults.getEngineName(),
semanticVersion: engineRunResults.getEngineVersion(),
Copy link
Contributor Author

@jfeingold35 jfeingold35 Dec 3, 2024

Choose a reason for hiding this comment

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

SARIF supports both semanticVersion and version. semanticVersion should be used if the version is known to conform to Semantic Version 2 schema. Since all of the versions we're returning are pulled directly from the package.json, they therefore definitionally conform to the schema.
My assumption is that external plugin authors will also return something that resembles a SemVer, and thus this should be valid for them too. If that assumption is incorrect, I can just change to the version property trivially.

Copy link
Contributor

@stephen-carter-at-sf stephen-carter-at-sf Dec 3, 2024

Choose a reason for hiding this comment

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

Does the validator https://sarifweb.azurewebsites.net/Validation validate with semanticVersion supplied but not version?

Also yeah, not sure how things will work out given that we are not forcing SemVer in the engine api... so I wonder if we just use "version". Is "version" what we used with v4?

Alternatively, we could just see if it is of a specific format (X.Y.Z) and if so, then use semanticVersion and if not then just fall back to version.

informationUri: "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
rules: rules.map(toSarifReportingDescriptor),
}
Expand Down
6 changes: 6 additions & 0 deletions packages/code-analyzer-core/src/output-formats/xml-output-format.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export class XmlOutputFormatter implements OutputFormatter {
violationCountsNode.node('sev4').text(`${resultsOutput.violationCounts.sev4}`);
violationCountsNode.node('sev5').text(`${resultsOutput.violationCounts.sev5}`);

const versionsNode: xmlbuilder.XMLElement = resultsNode.node('versions');
const engineNames: string[] = results.getEngineNames();
for (const engineName of engineNames) {
versionsNode.node(engineName).text(results.getEngineRunResults(engineName).getEngineVersion());
Copy link
Contributor

@stephen-carter-at-sf stephen-carter-at-sf Dec 3, 2024

Choose a reason for hiding this comment

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

Hmmm... This would be the first time we would be generating dynamic tags. Thankfully tag names can contain hyphens. That is, we would have <versions><pmd>X.Y.Z</pmd><retire-js>X.Y.Z</retire-js></versions> for example.

This is fine but we have to keep in mind that if we ever publish an xsd associated with these dynamic tags, we'd have to use something like <xs:any minOccurs="0" maxOccurs="unbounded"/> when documenting the tags under the versions tag.

}

const violationsNode: xmlbuilder.XMLElement = resultsNode.node('violations');
for (const violationOutput of resultsOutput.violations) {
const violationNode: xmlbuilder.XMLElement = violationsNode.node('violation');
Expand Down
17 changes: 15 additions & 2 deletions packages/code-analyzer-core/src/results.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface Violation {

export interface EngineRunResults {
getEngineName(): string
getEngineVersion(): string
getViolationCount(): number
getViolationCountOfSeverity(severity: SeverityLevel): number
getViolations(): Violation[]
Expand Down Expand Up @@ -169,11 +170,13 @@ export class UnexpectedEngineErrorViolation implements Violation {

export class EngineRunResultsImpl implements EngineRunResults {
private readonly engineName: string;
private readonly engineVersion: string;
private readonly apiEngineRunResults: engApi.EngineRunResults;
private readonly ruleSelection: RuleSelection;

constructor(engineName: string, apiEngineRunResults: engApi.EngineRunResults, ruleSelection: RuleSelection) {
constructor(engineName: string, engineVersion: string, apiEngineRunResults: engApi.EngineRunResults, ruleSelection: RuleSelection) {
this.engineName = engineName;
this.engineVersion = engineVersion;
this.apiEngineRunResults = apiEngineRunResults;
this.ruleSelection = ruleSelection;
}
Expand All @@ -182,6 +185,10 @@ export class EngineRunResultsImpl implements EngineRunResults {
return this.engineName;
}

getEngineVersion(): string {
return this.engineVersion;
}

getViolationCount(): number {
return this.apiEngineRunResults.violations.length;
}
Expand All @@ -198,17 +205,23 @@ export class EngineRunResultsImpl implements EngineRunResults {

export class UnexpectedErrorEngineRunResults implements EngineRunResults {
private readonly engineName: string;
private readonly engineVersion: string;
private readonly violation: Violation;

constructor(engineName: string, error: Error) {
constructor(engineName: string, engineVersion: string, error: Error) {
this.engineName = engineName;
this.engineVersion = engineVersion;
this.violation = new UnexpectedEngineErrorViolation(engineName, error);
}

getEngineName(): string {
return this.engineName;
}

getEngineVersion(): string {
return this.engineVersion;
}

getViolationCount(): number {
return 1;
}
Expand Down
6 changes: 6 additions & 0 deletions packages/code-analyzer-core/test/run.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {

const stubEngine1Results = overallResults.getEngineRunResults('stubEngine1');
expect(stubEngine1Results.getEngineName()).toEqual('stubEngine1');
expect(stubEngine1Results.getEngineVersion()).toEqual('0.0.1');
expect(stubEngine1Results.getViolationCount()).toEqual(0);
for (const severityLevel of getAllSeverityLevels()) {
expect(stubEngine1Results.getViolationCountOfSeverity(severityLevel)).toEqual(0);
Expand All @@ -269,6 +270,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {

const stubEngine2Results = overallResults.getEngineRunResults('stubEngine2');
expect(stubEngine2Results.getEngineName()).toEqual('stubEngine2');
expect(stubEngine2Results.getEngineVersion()).toEqual('0.1.0');
expect(stubEngine2Results.getViolationCount()).toEqual(0);
for (const severityLevel of getAllSeverityLevels()) {
expect(stubEngine2Results.getViolationCountOfSeverity(severityLevel)).toEqual(0);
Expand All @@ -277,6 +279,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {

const stubEngine3Results = overallResults.getEngineRunResults('stubEngine3');
expect(stubEngine3Results.getEngineName()).toEqual('stubEngine3');
expect(stubEngine3Results.getEngineVersion()).toEqual('1.0.0');
expect(stubEngine3Results.getViolationCount()).toEqual(0);
for (const severityLevel of getAllSeverityLevels()) {
expect(stubEngine3Results.getViolationCountOfSeverity(severityLevel)).toEqual(0);
Expand All @@ -303,6 +306,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {

const engine1Results = overallResults.getEngineRunResults('stubEngine1');
expect(engine1Results.getEngineName()).toEqual('stubEngine1');
expect(engine1Results.getEngineVersion()).toEqual('0.0.1');
expect(engine1Results.getViolationCount()).toEqual(2);
expect(engine1Results.getViolationCountOfSeverity(SeverityLevel.Critical)).toEqual(0);
expect(engine1Results.getViolationCountOfSeverity(SeverityLevel.High)).toEqual(0);
Expand Down Expand Up @@ -332,6 +336,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {

const engine2Results = overallResults.getEngineRunResults('stubEngine2');
expect(engine2Results.getEngineName()).toEqual('stubEngine2');
expect(engine2Results.getEngineVersion()).toEqual('0.1.0');
expect(engine2Results.getViolationCount()).toEqual(1);
expect(engine2Results.getViolationCountOfSeverity(SeverityLevel.Critical)).toEqual(0);
expect(engine2Results.getViolationCountOfSeverity(SeverityLevel.High)).toEqual(1);
Expand Down Expand Up @@ -495,6 +500,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
const violations: Violation[] = overallResults.getViolations();
expect(violations).toHaveLength(1);
const engineRunResults: EngineRunResults = overallResults.getEngineRunResults('throwingEngine');
expect(engineRunResults.getEngineVersion()).toEqual('3.0.0');
expect(engineRunResults.getViolations()).toEqual(violations);
expect(violations[0].getRule()).toEqual(new UnexpectedEngineErrorRule('throwingEngine'));
expect(violations[0].getRule().getDescription()).toEqual(getMessage('UnexpectedEngineErrorRuleDescription', 'throwingEngine'));
Expand Down
24 changes: 24 additions & 0 deletions packages/code-analyzer-core/test/stubs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ export class StubEngine1 extends engApi.Engine {
return "stubEngine1";
}

getEngineVersion(): Promise<string> {
return Promise.resolve('0.0.1');
}

async describeRules(describeOptions: DescribeOptions): Promise<engApi.RuleDescription[]> {
this.describeRulesCallHistory.push({describeOptions});
this.emitDescribeRulesProgressEvent(20);
Expand Down Expand Up @@ -157,6 +161,10 @@ export class StubEngine2 extends engApi.Engine {
return "stubEngine2";
}

getEngineVersion(): Promise<string> {
return Promise.resolve('0.1.0');
}

async describeRules(describeOptions: DescribeOptions): Promise<engApi.RuleDescription[]> {
this.describeRulesCallHistory.push({describeOptions});
this.emitDescribeRulesProgressEvent(30);
Expand Down Expand Up @@ -211,6 +219,10 @@ export class StubEngine3 extends engApi.Engine {
return 'stubEngine3';
}

getEngineVersion(): Promise<string> {
return Promise.resolve('1.0.0');
}

async describeRules(describeOptions: DescribeOptions): Promise<engApi.RuleDescription[]> {
this.describeRulesCallHistory.push({describeOptions});
this.emitDescribeRulesProgressEvent(50);
Expand Down Expand Up @@ -376,6 +388,10 @@ class FutureEngine extends engApi.Engine {
return "future";
}

getEngineVersion(): Promise<string> {
return Promise.resolve('2.0.0');
}

async describeRules(_describeOptions: DescribeOptions): Promise<engApi.RuleDescription[]> {
return [];
}
Expand Down Expand Up @@ -483,6 +499,10 @@ class ThrowingEngine extends StubEngine1 {
return "throwingEngine";
}

getEngineVersion(): Promise<string> {
return Promise.resolve('3.0.0');
}

async runRules(_ruleNames: string[], _runOptions: engApi.RunOptions): Promise<engApi.EngineRunResults> {
throw new Error('SomeErrorMessageFromThrowingEngine');
}
Expand All @@ -509,6 +529,10 @@ class RepeatedRuleNameEngine extends engApi.Engine {
return 'repeatedRuleNameEngine';
}

getEngineVersion(): Promise<string> {
return Promise.resolve('4.0.0');
}

async describeRules(_describeOptions: DescribeOptions): Promise<RuleDescription[]> {
return [
{
Expand Down
5 changes: 5 additions & 0 deletions ...es/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@
"sev4": 1,
"sev5": 0
},
"versions": {
"stubEngine1": "0.0.1",
"stubEngine2": "0.1.0",
"stubEngine3": "1.0.0"
},
"violations": [
{
"rule": "stub1RuleA",
Expand Down
3 changes: 3 additions & 0 deletions ...s/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"tool": {
"driver": {
"name": "stubEngine1",
"semanticVersion": "0.0.1",
"informationUri": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
"rules": [
{
Expand Down Expand Up @@ -169,6 +170,7 @@
"tool": {
"driver": {
"name": "stubEngine2",
"semanticVersion": "0.1.0",
"informationUri": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
"rules": [
{
Expand Down Expand Up @@ -255,6 +257,7 @@
"tool": {
"driver": {
"name": "stubEngine3",
"semanticVersion": "1.0.0",
"informationUri": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
"rules": [
{
Expand Down
5 changes: 5 additions & 0 deletions ...ges/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
<sev4>1</sev4>
<sev5>0</sev5>
</violationCounts>
<versions>
<stubEngine1>0.0.1</stubEngine1>
<stubEngine2>0.1.0</stubEngine2>
<stubEngine3>1.0.0</stubEngine3>
</versions>
<violations>
<violation>
<rule>stub1RuleA</rule>
Expand Down
3 changes: 3 additions & 0 deletions ...yzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
"sev4": 0,
"sev5": 0
},
"versions": {
"throwingEngine": "3.0.0"
},
"violations": [
{
"rule": "UnexpectedEngineError",
Expand Down
1 change: 1 addition & 0 deletions ...zer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"tool": {
"driver": {
"name": "throwingEngine",
"semanticVersion": "3.0.0",
"informationUri": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/guide/version-5.html",
"rules": [
{
Expand Down
3 changes: 3 additions & 0 deletions ...lyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
<sev4>0</sev4>
<sev5>0</sev5>
</violationCounts>
<versions>
<throwingEngine>3.0.0</throwingEngine>
</versions>
<violations>
<violation>
<rule>UnexpectedEngineError</rule>
Expand Down
1 change: 1 addition & 0 deletions packages/code-analyzer-core/test/test-data/expectedOutputFiles/zeroViolations.goldfile.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
"sev4": 0,
"sev5": 0
},
"versions": {},
"violations": []
}
1 change: 1 addition & 0 deletions packages/code-analyzer-core/test/test-data/expectedOutputFiles/zeroViolations.goldfile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@
<sev4>0</sev4>
<sev5>0</sev5>
</violationCounts>
<versions></versions>
<violations></violations>
</results>
Loading