FIX: @W-19184117@: Our JSON schema fields primaryLocationIndex, locat… (#319)

Author: stephen-carter-at-sf

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",

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)

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.`,
 

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()
     }
-}
+}

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[]
+    // An non-empty array of code locations associated with the violation
+    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

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 });
     }
-}
+}

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 {

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;

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 {

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": []