From eeaff9b7da398d9509a8fbabede9b10a22a9eaa2 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Fri, 1 Aug 2025 11:59:15 -0400 Subject: [PATCH 1/2] FIX: @W-19184117@: Our JSON schema fields primaryLocationIndex, locations, resources should not be marked as optional --- packages/code-analyzer-core/package.json | 2 +- .../src/output-formats/results/json-run-results-format.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/code-analyzer-core/package.json b/packages/code-analyzer-core/package.json index b580d82e..1c89c888 100644 --- a/packages/code-analyzer-core/package.json +++ b/packages/code-analyzer-core/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-core", "description": "Core Package for the Salesforce Code Analyzer", - "version": "0.31.0", + "version": "0.32.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", diff --git a/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts b/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts index 726b4c35..ec9023d7 100644 --- a/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts +++ b/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts @@ -54,16 +54,16 @@ export type JsonViolationOutput = { tags: string[] // The index of the primary code location within the code locations array - primaryLocationIndex?: number + primaryLocationIndex: number // An array of code locations associated with the violation - locations?: JsonCodeLocationOutput[] + locations: JsonCodeLocationOutput[] // The violation message message: string // An array of urls for resources associated with the violation - resources?: string[] + resources: string[] } export type JsonCodeLocationOutput = { // The path, relative to runDir, of the file associated with the violation From f515a2158dd809c88f9d0109b9f302e0f2fc403e Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Fri, 1 Aug 2025 12:56:54 -0400 Subject: [PATCH 2/2] Enforce engines must return violations with at least one code location --- .../code-analyzer-core/src/code-analyzer.ts | 5 +- packages/code-analyzer-core/src/messages.ts | 6 ++ .../results/csv-run-results-format.ts | 3 +- .../results/json-run-results-format.ts | 2 +- .../results/xml-run-results-format.ts | 59 +++++++++---------- packages/code-analyzer-core/src/results.ts | 5 +- .../test/code-analyzer.test.ts | 16 +++++ ...expectedEngineErrorViolation.goldfile.html | 2 +- ...expectedEngineErrorViolation.goldfile.json | 4 +- ...nexpectedEngineErrorViolation.goldfile.xml | 4 +- 10 files changed, 66 insertions(+), 40 deletions(-) diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index a1f98ae5..6e435704 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -635,8 +635,8 @@ function validateTargetLivesWithinWorkspace(target: string, workspaceFilesAndFol function validateEngineRunResults(engineName: string, apiEngineRunResults: engApi.EngineRunResults, ruleSelection: RuleSelection): void { for (const violation of apiEngineRunResults.violations) { validateViolationRuleName(violation, engineName, ruleSelection); - validateViolationPrimaryLocationIndex(violation, engineName); validateViolationCodeLocations(violation, engineName); + validateViolationPrimaryLocationIndex(violation, engineName); } } @@ -656,6 +656,9 @@ function validateViolationPrimaryLocationIndex(violation: engApi.Violation, engi } function validateViolationCodeLocations(violation: engApi.Violation, engineName: string): void { + if (violation.codeLocations.length === 0) { + throw new Error(getMessage('EngineReturnedViolationWithEmptyCodeLocationArray', engineName, violation.ruleName)); + } for (const codeLocation of violation.codeLocations) { const absFile: string = toAbsolutePath(codeLocation.file); fs.existsSync(absFile) diff --git a/packages/code-analyzer-core/src/messages.ts b/packages/code-analyzer-core/src/messages.ts index 75e566db..097687a8 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -136,6 +136,9 @@ const MESSAGE_CATALOG : MessageCatalog = { FileOrFolderDoesNotExist: `The file or folder '%s' does not exist.`, + UndefinedCodeLocationComment: + `Undefined Code Location`, + AtLeastOneFileOrFolderMustBeIncludedInWorkspace: `At least one file or folder must be included in the workspace.`, @@ -169,6 +172,9 @@ const MESSAGE_CATALOG : MessageCatalog = { EngineReturnedViolationForUnselectedRule: `Engine failure. The engine '%s' returned a violation for rule '%s' which was not selected.`, + EngineReturnedViolationWithEmptyCodeLocationArray: + `Engine failure. The engine '%s' returned a violation for rule '%s' that contains an an empty code location array. Rule violations must have at least one code location object.`, + EngineReturnedViolationWithInvalidPrimaryLocationIndex: `Engine failure. The engine '%s' returned a violation for rule '%s' that contains an out of bounds primary location index value of %d. Expected a non-negative integer that is less than %d.`, diff --git a/packages/code-analyzer-core/src/output-formats/results/csv-run-results-format.ts b/packages/code-analyzer-core/src/output-formats/results/csv-run-results-format.ts index 335cb275..476664d6 100644 --- a/packages/code-analyzer-core/src/output-formats/results/csv-run-results-format.ts +++ b/packages/code-analyzer-core/src/output-formats/results/csv-run-results-format.ts @@ -21,6 +21,7 @@ export class CsvRunResultsFormatter implements RunResultsFormatter { 'endLine', 'endColumn', 'message', 'resources'], cast: { object: value => { + /* istanbul ignore else */ if (Array.isArray(value)) { return { value: value.join(','), quoted: true }; } @@ -64,4 +65,4 @@ function toCsvRow(violation: Violation, runDir: string): CsvRow { message: violation.getMessage(), resources: violation.getResourceUrls() } -} \ No newline at end of file +} diff --git a/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts b/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts index ec9023d7..f0193656 100644 --- a/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts +++ b/packages/code-analyzer-core/src/output-formats/results/json-run-results-format.ts @@ -56,7 +56,7 @@ export type JsonViolationOutput = { // The index of the primary code location within the code locations array primaryLocationIndex: number - // An array of code locations associated with the violation + // An non-empty array of code locations associated with the violation locations: JsonCodeLocationOutput[] // The violation message diff --git a/packages/code-analyzer-core/src/output-formats/results/xml-run-results-format.ts b/packages/code-analyzer-core/src/output-formats/results/xml-run-results-format.ts index 70a40a7c..82a46382 100644 --- a/packages/code-analyzer-core/src/output-formats/results/xml-run-results-format.ts +++ b/packages/code-analyzer-core/src/output-formats/results/xml-run-results-format.ts @@ -38,42 +38,39 @@ export class XmlRunResultsFormatter implements RunResultsFormatter { for (const tag of violationOutput.tags) { tagsNode.node('tag').text(tag); } - if (violationOutput.primaryLocationIndex != null) { - violationNode.node('primaryLocationIndex').text(`${violationOutput.primaryLocationIndex}`); - } - if (violationOutput.locations) { - const pathLocationsNode: xmlbuilder.XMLElement = violationNode.node('locations'); - for (const location of violationOutput.locations) { - const locationNode: xmlbuilder.XMLElement = pathLocationsNode.node('location'); - if (location.file !== undefined) { - locationNode.node('file').text(location.file); - } - if (location.startLine !== undefined) { - locationNode.node('startLine').text(`${location.startLine}`); - } - if (location.startColumn !== undefined) { - locationNode.node('startColumn').text(`${location.startColumn}`); - } - if (location.endLine !== undefined) { - locationNode.node('endLine').text(`${location.endLine}`); - } - if (location.endColumn !== undefined) { - locationNode.node('endColumn').text(`${location.endColumn}`); - } - if (location.comment !== undefined) { - locationNode.node('comment').text(location.comment); - } + violationNode.node('primaryLocationIndex').text(`${violationOutput.primaryLocationIndex}`); + + const pathLocationsNode: xmlbuilder.XMLElement = violationNode.node('locations'); + for (const location of violationOutput.locations) { + const locationNode: xmlbuilder.XMLElement = pathLocationsNode.node('location'); + if (location.file !== undefined) { + locationNode.node('file').text(location.file); + } + if (location.startLine !== undefined) { + locationNode.node('startLine').text(`${location.startLine}`); + } + if (location.startColumn !== undefined) { + locationNode.node('startColumn').text(`${location.startColumn}`); + } + if (location.endLine !== undefined) { + locationNode.node('endLine').text(`${location.endLine}`); + } + if (location.endColumn !== undefined) { + locationNode.node('endColumn').text(`${location.endColumn}`); + } + if (location.comment !== undefined) { + locationNode.node('comment').text(location.comment); } } + violationNode.node('message').text(violationOutput.message); - if (violationOutput.resources) { - const resourcesNode: xmlbuilder.XMLElement = violationNode.node('resources'); - for (const resource of violationOutput.resources) { - resourcesNode.node('resource').text(resource); - } + + const resourcesNode: xmlbuilder.XMLElement = violationNode.node('resources'); + for (const resource of violationOutput.resources) { + resourcesNode.node('resource').text(resource); } } return violationsNode.end({ pretty: true, allowEmpty: true }); } -} \ No newline at end of file +} diff --git a/packages/code-analyzer-core/src/results.ts b/packages/code-analyzer-core/src/results.ts index 31630635..111aa2b3 100644 --- a/packages/code-analyzer-core/src/results.ts +++ b/packages/code-analyzer-core/src/results.ts @@ -163,9 +163,8 @@ export class UndefinedCodeLocation implements CodeLocation { return undefined; } - // istanbul ignore next - Unused method, required for interface - getComment(): undefined { - return undefined; + getComment(): string { + return getMessage('UndefinedCodeLocationComment'); } getEndLine(): undefined { diff --git a/packages/code-analyzer-core/test/code-analyzer.test.ts b/packages/code-analyzer-core/test/code-analyzer.test.ts index 10ca2a76..e22df4e3 100644 --- a/packages/code-analyzer-core/test/code-analyzer.test.ts +++ b/packages/code-analyzer-core/test/code-analyzer.test.ts @@ -416,6 +416,22 @@ describe("Tests for the run method of CodeAnalyzer", () => { getMessage('EngineReturnedViolationWithInvalidPrimaryLocationIndex', 'stubEngine2', 'stub2RuleC', -2, 3)); }); + it("When an engine returns a violatoin that has zero code locations, then an error is thrown", async() => { + const badViolation: engApi.Violation = { + ruleName: 'stub1RuleC', + message: 'SomeViolationMessage2', + codeLocations: [], + primaryLocationIndex: 0, + resourceUrls: ["https://example.com/aViolationSpecificUrl1",] + }; + badViolation.primaryLocationIndex = 0; + stubEngine1.resultsToReturn = { + violations: [badViolation] + }; + await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow( + getMessage('EngineReturnedViolationWithEmptyCodeLocationArray', 'stubEngine1', 'stub1RuleC')); + }); + it("When an engine returns a violation that has a primary location index that is not an integer, then an error is thrown", async () => { const badViolation: engApi.Violation = stubs.getSampleViolationForStub1RuleC(); badViolation.primaryLocationIndex = 0.5; diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.html b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.html index 78a3e204..0183db7c 100644 --- a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.html +++ b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.html @@ -49,7 +49,7 @@ })(); // ==== START OF VIOLATIONS ==== - const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":1,"sev1":1,"sev2":0,"sev3":0,"sev4":0,"sev5":0},"versions":{"code-analyzer":"{{CORE_VERSION}}","throwingEngine":"3.0.0"},"violations":[{"rule":"UnexpectedEngineError","engine":"throwingEngine","severity":1,"tags":[],"primaryLocationIndex":0,"locations":[{}],"message":"The engine with name 'throwingEngine' threw an unexpected error: SomeErrorMessageFromThrowingEngine","resources":[]}]}; + const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":1,"sev1":1,"sev2":0,"sev3":0,"sev4":0,"sev5":0},"versions":{"code-analyzer":"{{CORE_VERSION}}","throwingEngine":"3.0.0"},"violations":[{"rule":"UnexpectedEngineError","engine":"throwingEngine","severity":1,"tags":[],"primaryLocationIndex":0,"locations":[{"comment":"Undefined Code Location"}],"message":"The engine with name 'throwingEngine' threw an unexpected error: SomeErrorMessageFromThrowingEngine","resources":[]}]}; // ==== END OF VIOLATIONS ==== class Model { diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.json b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.json index 9b46e90c..1a7b6eca 100644 --- a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.json +++ b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.json @@ -20,7 +20,9 @@ "tags": [], "primaryLocationIndex": 0, "locations": [ - {} + { + "comment": "Undefined Code Location" + } ], "message": "The engine with name 'throwingEngine' threw an unexpected error: SomeErrorMessageFromThrowingEngine", "resources": [] diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.xml b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.xml index 977bd0a6..eff3e74b 100644 --- a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.xml +++ b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/unexpectedEngineErrorViolation.goldfile.xml @@ -21,7 +21,9 @@ 0 - + + Undefined Code Location + The engine with name 'throwingEngine' threw an unexpected error: SomeErrorMessageFromThrowingEngine