Skip to content

Commit

Permalink
FIX @W-17380965@ SARIF format handles spaces in file paths (#157)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jfeingold35
jfeingold35 authored Dec 10, 2024
1 parent 54f6986 commit 2d686e1
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 37 deletions.
2 changes: 1 addition & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/code-analyzer-core/src/output-formats/sarif-output-format.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ function toSarifLocation(codeLocation: CodeLocation, runDir: string): sarif.Loca
return {
physicalLocation: {
artifactLocation: {
uri: path.relative(runDir, codeLocation.getFile()!),
uriBaseId: runDir
uri: encodeURI(path.relative(runDir, codeLocation.getFile()!)),
uriBaseId: encodeURI(runDir)
},
region: {
startLine: codeLocation.getStartLine(),
Expand Down
10 changes: 9 additions & 1 deletion packages/code-analyzer-core/test/output-format.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ beforeAll(async () => {
const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin();
await codeAnalyzer.addEnginePlugin(stubPlugin);
(stubPlugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1).resultsToReturn = {
violations: [stubs.getSampleViolationForStub1RuleA(), stubs.getSampleViolationForStub1RuleC(), stubs.getSampleViolationForStub1RuleE()]
violations: [
stubs.getSampleViolationForStub1RuleA(), stubs.getSampleViolationForStub1RuleAFromDirectoryWithSpaces(),
stubs.getSampleViolationForStub1RuleC(), stubs.getSampleViolationForStub1RuleE()
]
};
(stubPlugin.getCreatedEngine('stubEngine2') as stubs.StubEngine2).resultsToReturn = {
violations: [stubs.getSampleViolationForStub2RuleC()]
Expand Down Expand Up @@ -161,15 +164,20 @@ function getContentsOfExpectedOutputFile(expectedOutputFileName: string, escapeB
const contents: string = fs.readFileSync(path.resolve('test','test-data','expectedOutputFiles',expectedOutputFileName), 'utf-8');
let pathSepVar: string = path.sep;
let runDirVar: string = process.cwd() + path.sep;
const encodedRunDir: string = encodeURI(runDirVar);
const escapedRunDir: string = runDirVar.replaceAll('\\', '\\\\');
if (escapeBackslashesOnPaths) {
pathSepVar = pathSepVar.replaceAll('\\','\\\\');
}
if (escapeBackslashesOnRunDir) {
runDirVar = runDirVar.replaceAll('\\','\\\\');
}
const encodedPathSepVar: string = encodeURI(path.sep);

return contents.replaceAll('{{PATHSEP}}', pathSepVar)
.replaceAll(`{{ENCODEDPATHSEP}}`, encodedPathSepVar)
.replaceAll('{{RUNDIR}}', runDirVar)
.replaceAll('{{ENCODEDRUNDIR}}', encodedRunDir)
.replaceAll('{{ESCAPEDRUNDIR}}', escapedRunDir)
.replaceAll('{{###RUNDIR###}}', runDirVar)
.replaceAll('{{###TIMESTAMP###}}', fixedTime.toLocaleString('en-us', {year: "numeric", month: "short",
Expand Down
20 changes: 20 additions & 0 deletions packages/code-analyzer-core/test/stubs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,26 @@ export function getSampleViolationForStub1RuleA(): engApi.Violation {
};
}

export function getSampleViolationForStub1RuleAFromDirectoryWithSpaces(): engApi.Violation {
return {
ruleName: 'stub1RuleA',
message: 'SomeViolationMessage1',
codeLocations: [
{
file: 'test/test-data/sample-input-files/subfolder with spaces/some-target-file.ts',
startLine: 10,
startColumn: 4,
endLine: 11,
endColumn: 2
}
],
primaryLocationIndex: 0,
resourceUrls: [
"https://example.com/stub1RuleA" // Same url as rule's url... to test that we don't duplicate it
]
}
}

export function getSampleViolationForStub1RuleC(): engApi.Violation {
return {
ruleName: 'stub1RuleC',
Expand Down
1 change: 1 addition & 0 deletions ...ges/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.csv
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"rule","engine","severity","tags","file","startLine","startColumn","endLine","endColumn","message","resources"
"stub1RuleA","stubEngine1",4,"Recommended,CodeStyle","test{{PATHSEP}}config.test.ts",3,6,11,8,"SomeViolationMessage1","https://example.com/stub1RuleA"
"stub1RuleA","stubEngine1",4,"Recommended,CodeStyle","test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts",10,4,11,2,"SomeViolationMessage1","https://example.com/stub1RuleA"
"stub1RuleC","stubEngine1",3,"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"
"stub1RuleE","stubEngine1",3,"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"
Expand Down
2 changes: 1 addition & 1 deletion ...es/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
})();

// ==== START OF VIOLATIONS ====
const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":5,"sev1":0,"sev2":1,"sev3":3,"sev4":1,"sev5":0},"versions":{"stubEngine1":"0.0.1","stubEngine2":"0.1.0","stubEngine3":"1.0.0"},"violations":[{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}config.test.ts","startLine":3,"startColumn":6,"endLine":11,"endColumn":8}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleC","engine":"stubEngine1","severity":3,"tags":["Recommended","Performance","Custom"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":21,"startColumn":7,"endLine":25,"endColumn":4}],"message":"SomeViolationMessage2","resources":["https://example.com/stub1RuleC","https://example.com/aViolationSpecificUrl1","https://example.com/violationSpecificUrl2"]},{"rule":"stub1RuleE","engine":"stubEngine1","severity":3,"tags":["Performance"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":56,"startColumn":4}],"message":"Some Violation that contains\na new line in `it` and &quot;various&quot; &#39;quotes&#39;. Also it has &lt;brackets&gt; that may need to be {escaped}.","resources":["https://example.com/stub1RuleE","https://example.com/stub1RuleE_2"]},{"rule":"stub2RuleC","engine":"stubEngine2","severity":2,"tags":["Recommended","BestPractice"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":4,"startColumn":13},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":9,"startColumn":1},{"file":"test{{PATHSEP}}stubs.ts","startLine":76,"startColumn":8}],"message":"SomeViolationMessage3","resources":[]},{"rule":"stub3RuleA","engine":"stubEngine3","severity":3,"tags":["Recommended","ErrorProne"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":20,"startColumn":10,"endLine":22,"endColumn":25,"comment":"Comment at location 1"},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":5,"startColumn":10,"comment":"Comment at location 2"},{"file":"test{{PATHSEP}}stubs.ts","startLine":90,"startColumn":1,"endLine":95,"endColumn":10}],"message":"SomeViolationMessage4","resources":[]}]};
const data = {"runDir":"{{ESCAPEDRUNDIR}}","violationCounts":{"total":6,"sev1":0,"sev2":1,"sev3":3,"sev4":2,"sev5":0},"versions":{"stubEngine1":"0.0.1","stubEngine2":"0.1.0","stubEngine3":"1.0.0"},"violations":[{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}config.test.ts","startLine":3,"startColumn":6,"endLine":11,"endColumn":8}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleA","engine":"stubEngine1","severity":4,"tags":["Recommended","CodeStyle"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts","startLine":10,"startColumn":4,"endLine":11,"endColumn":2}],"message":"SomeViolationMessage1","resources":["https://example.com/stub1RuleA"]},{"rule":"stub1RuleC","engine":"stubEngine1","severity":3,"tags":["Recommended","Performance","Custom"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":21,"startColumn":7,"endLine":25,"endColumn":4}],"message":"SomeViolationMessage2","resources":["https://example.com/stub1RuleC","https://example.com/aViolationSpecificUrl1","https://example.com/violationSpecificUrl2"]},{"rule":"stub1RuleE","engine":"stubEngine1","severity":3,"tags":["Performance"],"primaryLocationIndex":0,"locations":[{"file":"test{{PATHSEP}}run.test.ts","startLine":56,"startColumn":4}],"message":"Some Violation that contains\na new line in `it` and &quot;various&quot; &#39;quotes&#39;. Also it has &lt;brackets&gt; that may need to be {escaped}.","resources":["https://example.com/stub1RuleE","https://example.com/stub1RuleE_2"]},{"rule":"stub2RuleC","engine":"stubEngine2","severity":2,"tags":["Recommended","BestPractice"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":4,"startColumn":13},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":9,"startColumn":1},{"file":"test{{PATHSEP}}stubs.ts","startLine":76,"startColumn":8}],"message":"SomeViolationMessage3","resources":[]},{"rule":"stub3RuleA","engine":"stubEngine3","severity":3,"tags":["Recommended","ErrorProne"],"primaryLocationIndex":2,"locations":[{"file":"test{{PATHSEP}}stubs.ts","startLine":20,"startColumn":10,"endLine":22,"endColumn":25,"comment":"Comment at location 1"},{"file":"test{{PATHSEP}}test-helpers.ts","startLine":5,"startColumn":10,"comment":"Comment at location 2"},{"file":"test{{PATHSEP}}stubs.ts","startLine":90,"startColumn":1,"endLine":95,"endColumn":10}],"message":"SomeViolationMessage4","resources":[]}]};
// ==== END OF VIOLATIONS ====

class Model {
Expand Down
27 changes: 25 additions & 2 deletions ...es/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.json
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"runDir": "{{RUNDIR}}",
"violationCounts": {
"total": 5,
"total": 6,
"sev1": 0,
"sev2": 1,
"sev3": 3,
"sev4": 1,
"sev4": 2,
"sev5": 0
},
"versions": {
Expand Down Expand Up @@ -37,6 +37,29 @@
"https://example.com/stub1RuleA"
]
},
{
"rule": "stub1RuleA",
"engine": "stubEngine1",
"severity": 4,
"tags": [
"Recommended",
"CodeStyle"
],
"primaryLocationIndex": 0,
"locations": [
{
"file": "test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts",
"startLine": 10,
"startColumn": 4,
"endLine": 11,
"endColumn": 2
}
],
"message": "SomeViolationMessage1",
"resources": [
"https://example.com/stub1RuleA"
]
},
{
"rule": "stub1RuleC",
"engine": "stubEngine1",
Expand Down
96 changes: 68 additions & 28 deletions ...s/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.sarif
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}config.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}config.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 3,
Expand All @@ -73,8 +73,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}config.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}config.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 3,
Expand All @@ -86,6 +86,46 @@
}
]
},
{
"ruleId": "stub1RuleA",
"ruleIndex": 0,
"level": "warning",
"message": {
"text": "SomeViolationMessage1"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}test-data{{ENCODEDPATHSEP}}sample-input-files{{ENCODEDPATHSEP}}subfolder%20with%20spaces{{ENCODEDPATHSEP}}some-target-file.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 10,
"startColumn": 4,
"endLine": 11,
"endColumn": 2
}
}
}
],
"relatedLocations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{ENCODEDPATHSEP}}test-data{{ENCODEDPATHSEP}}sample-input-files{{ENCODEDPATHSEP}}subfolder%20with%20spaces{{ENCODEDPATHSEP}}some-target-file.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 10,
"startColumn": 4,
"endLine": 11,
"endColumn": 2
}
}
}
]
},
{
"ruleId": "stub1RuleC",
"ruleIndex": 1,
Expand All @@ -97,8 +137,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}run.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 21,
Expand All @@ -113,8 +153,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}run.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 21,
Expand All @@ -137,8 +177,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}run.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 56,
Expand All @@ -151,8 +191,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}run.test.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}run.test.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 56,
Expand Down Expand Up @@ -204,8 +244,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 76,
Expand All @@ -218,8 +258,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 4,
Expand All @@ -230,8 +270,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}test-helpers.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}test-helpers.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 9,
Expand All @@ -242,8 +282,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 76,
Expand Down Expand Up @@ -295,8 +335,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 90,
Expand All @@ -311,8 +351,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 20,
Expand All @@ -325,8 +365,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}test-helpers.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}test-helpers.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 5,
Expand All @@ -337,8 +377,8 @@
{
"physicalLocation": {
"artifactLocation": {
"uri": "test{{PATHSEP}}stubs.ts",
"uriBaseId": "{{RUNDIR}}"
"uri": "test{{ENCODEDPATHSEP}}stubs.ts",
"uriBaseId": "{{ENCODEDRUNDIR}}"
},
"region": {
"startLine": 90,
Expand Down
27 changes: 25 additions & 2 deletions ...ges/code-analyzer-core/test/test-data/expectedOutputFiles/multipleViolations.goldfile.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
<results>
<runDir>{{RUNDIR}}</runDir>
<violationCounts>
<total>5</total>
<total>6</total>
<sev1>0</sev1>
<sev2>1</sev2>
<sev3>3</sev3>
<sev4>1</sev4>
<sev4>2</sev4>
<sev5>0</sev5>
</violationCounts>
<versions>
Expand Down Expand Up @@ -38,6 +38,29 @@
<resource>https://example.com/stub1RuleA</resource>
</resources>
</violation>
<violation>
<rule>stub1RuleA</rule>
<engine>stubEngine1</engine>
<severity>4</severity>
<tags>
<tag>Recommended</tag>
<tag>CodeStyle</tag>
</tags>
<primaryLocationIndex>0</primaryLocationIndex>
<locations>
<location>
<file>test{{PATHSEP}}test-data{{PATHSEP}}sample-input-files{{PATHSEP}}subfolder with spaces{{PATHSEP}}some-target-file.ts</file>
<startLine>10</startLine>
<startColumn>4</startColumn>
<endLine>11</endLine>
<endColumn>2</endColumn>
</location>
</locations>
<message>SomeViolationMessage1</message>
<resources>
<resource>https://example.com/stub1RuleA</resource>
</resources>
</violation>
<violation>
<rule>stub1RuleC</rule>
<engine>stubEngine1</engine>
Expand Down
Loading

0 comments on commit 2d686e1

Please sign in to comment.