NEW: @W-15951478@: Allow resourceUrls to be provided by engine from t…

…he violation and the rule (#20)
forcedotcom · Jun 6, 2024 · db2d29e · db2d29e
1 parent f728b5d
commit db2d29e
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 5 deletions.
diff --git a/packages/code-analyzer-core/src/output-format.ts b/packages/code-analyzer-core/src/output-format.ts
@@ -191,7 +191,7 @@ function createViolationOutput(id: number, violation: Violation, runDir: string)
         endColumn: primaryLocation.getEndColumn(),
         pathLocations: [RuleType.DataFlow, RuleType.Flow].includes(rule.getType()) ? createPathLocations(codeLocations, runDir) : undefined,
         message: violation.getMessage(),
-        resources: violation.getRule().getResourceUrls()
+        resources: violation.getResourceUrls()
     };
 }
 

diff --git a/packages/code-analyzer-core/src/results.ts b/packages/code-analyzer-core/src/results.ts
@@ -18,6 +18,7 @@ export interface Violation {
     getMessage(): string
     getCodeLocations(): CodeLocation[]
     getPrimaryLocationIndex(): number
+    getResourceUrls(): string[]
 }
 
 export interface EngineRunResults {
@@ -115,6 +116,13 @@ export class ViolationImpl implements Violation {
     getPrimaryLocationIndex(): number {
         return this.apiViolation.primaryLocationIndex;
     }
+
+    getResourceUrls(): string[] {
+        // Returns the urls from the rule and then appends any urls from the violation that are not already from the rule.
+        const urls: string[] = this.rule.getResourceUrls();
+        return !this.apiViolation.resourceUrls ? urls :
+            [...urls, ...this.apiViolation.resourceUrls.filter(url => !urls.includes(url))];
+    }
 }
 
 export class UnexpectedEngineErrorViolation implements Violation {
@@ -143,6 +151,10 @@ export class UnexpectedEngineErrorViolation implements Violation {
     getPrimaryLocationIndex(): number {
         return 0;
     }
+
+    getResourceUrls(): string[] {
+        return [];
+    }
 }
 
 export class EngineRunResultsImpl implements EngineRunResults {

diff --git a/packages/code-analyzer-core/test/run.test.ts b/packages/code-analyzer-core/test/run.test.ts
@@ -328,12 +328,18 @@ describe("Tests for the run method of CodeAnalyzer", () => {
         expect(engine1Violation1CodeLocations).toHaveLength(1);
         assertCodeLocation(engine1Violation1CodeLocations[0], path.resolve('test', 'config.test.ts'), 3, 6, 11, 8);
         expect(engine1Violations[0].getPrimaryLocationIndex()).toEqual(0);
+        expect(engine1Violations[0].getResourceUrls()).toEqual(["https://example.com/stub1RuleA"]);
         expect(engine1Violations[1].getRule()).toEqual(selection.getRule('stubEngine1', 'stub1RuleC'));
         expect(engine1Violations[1].getMessage()).toEqual('SomeViolationMessage2');
         const engine1Violation2CodeLocations: CodeLocation[] = engine1Violations[1].getCodeLocations();
         expect(engine1Violation2CodeLocations).toHaveLength(1);
         assertCodeLocation(engine1Violation2CodeLocations[0], path.resolve('test', 'run.test.ts'), 21, 7, 25, 4);
         expect(engine1Violations[1].getPrimaryLocationIndex()).toEqual(0);
+        expect(engine1Violations[1].getResourceUrls()).toEqual([
+            "https://example.com/stub1RuleC",
+            "https://example.com/aViolationSpecificUrl1",
+            "https://example.com/violationSpecificUrl2"
+        ]);
 
         const engine2Results = overallResults.getEngineRunResults('stubEngine2');
         expect(engine2Results.getEngineName()).toEqual('stubEngine2');
@@ -353,6 +359,7 @@ describe("Tests for the run method of CodeAnalyzer", () => {
         assertCodeLocation(engine2Violation1CodeLocations[1], path.resolve('test', 'test-helpers.ts'), 9, 1);
         assertCodeLocation(engine2Violation1CodeLocations[2], path.resolve('test', 'stubs.ts'), 76, 8);
         expect(engine2Violations[0].getPrimaryLocationIndex()).toEqual(2);
+        expect(engine2Violations[0].getResourceUrls()).toEqual([]);
 
         expect(overallResults.getViolations()).toEqual([...engine1Violations,...engine2Violations]);
     });

diff --git a/packages/code-analyzer-core/test/stubs.ts b/packages/code-analyzer-core/test/stubs.ts
@@ -206,7 +206,10 @@ export function getSampleViolationForStub1RuleA(): engApi.Violation {
                 endColumn: 8
             }
         ],
-        primaryLocationIndex: 0
+        primaryLocationIndex: 0,
+        resourceUrls: [
+            "https://example.com/stub1RuleA" // Same url as rule's url... to test that we don't duplicate it
+        ]
     };
 }
 
@@ -223,7 +226,11 @@ export function getSampleViolationForStub1RuleC(): engApi.Violation {
                 endColumn: 4
             }
         ],
-        primaryLocationIndex: 0
+        primaryLocationIndex: 0,
+        resourceUrls: [
+            "https://example.com/aViolationSpecificUrl1", // starting with "aViolation" so that we can test that this url comes after the rule url even though alphabetically it comes first
+            "https://example.com/violationSpecificUrl2",
+        ]
     };
 }
 

diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.csv b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.csv
@@ -1,6 +1,6 @@
 "id","rule","engine","severity","type","tags","file","line","column","endLine","endColumn","pathLocations","message","resources"
 1,"stub1RuleA","stubEngine1",4,"Standard","Recommended,CodeStyle","test{{PATHSEP}}config.test.ts",3,6,11,8,,"SomeViolationMessage1","https://example.com/stub1RuleA"
-2,"stub1RuleC","stubEngine1",3,"Standard","Recommended,Performance,Custom","test{{PATHSEP}}run.test.ts",21,7,25,4,,"SomeViolationMessage2","https://example.com/stub1RuleC"
+2,"stub1RuleC","stubEngine1",3,"Standard","Recommended,Performance,Custom","test{{PATHSEP}}run.test.ts",21,7,25,4,,"SomeViolationMessage2","https://example.com/stub1RuleC,https://example.com/aViolationSpecificUrl1,https://example.com/violationSpecificUrl2"
 3,"stub1RuleE","stubEngine1",3,"Standard","Performance","test{{PATHSEP}}run.test.ts",56,4,,,,"Some Violation that contains
 a new line in `it` and ""various"" 'quotes'. Also it has <brackets> that may need to be {escaped}.","https://example.com/stub1RuleE,https://example.com/stub1RuleE_2"
 4,"stub2RuleC","stubEngine2",2,"DataFlow","Recommended,BestPractice","test{{PATHSEP}}stubs.ts",76,8,,,"test{{PATHSEP}}stubs.ts:4:13,test{{PATHSEP}}test-helpers.ts:9:1,test{{PATHSEP}}stubs.ts:76:8","SomeViolationMessage3",
diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.json b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.json
@@ -47,7 +47,9 @@
       "endColumn": 4,
       "message": "SomeViolationMessage2",
       "resources": [
-        "https://example.com/stub1RuleC"
+        "https://example.com/stub1RuleC",
+        "https://example.com/aViolationSpecificUrl1",
+        "https://example.com/violationSpecificUrl2"
       ]
     },
     {

diff --git a/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.xml b/packages/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.xml
@@ -47,6 +47,8 @@
       <message>SomeViolationMessage2</message>
       <resources>
         <resource>https://example.com/stub1RuleC</resource>
+        <resource>https://example.com/aViolationSpecificUrl1</resource>
+        <resource>https://example.com/violationSpecificUrl2</resource>
       </resources>
     </violation>
     <violation id="3">

diff --git a/packages/code-analyzer-engine-api/src/results.ts b/packages/code-analyzer-engine-api/src/results.ts
@@ -11,6 +11,7 @@ export type Violation = {
     message: string
     codeLocations: CodeLocation[]
     primaryLocationIndex: number
+    resourceUrls?: string[]
 }
 
 export type EngineRunResults = {