From 006cc8c52a9ef4ed47e29b44c14468a1ddf5ca78 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Tue, 29 Mar 2022 13:06:44 -0700 Subject: [PATCH] Undo sarif-processing change Will move to a different PR. --- .../src/remote-queries/sarif-processing.ts | 20 +++----- .../remote-queries/shared/analysis-result.ts | 2 +- .../remote-queries/view/FileCodeSnippet.tsx | 8 ++-- .../test/pure-tests/sarif-processing.test.ts | 47 +++---------------- 4 files changed, 19 insertions(+), 58 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts index bb4ef6d5a..d4ca0daea 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts @@ -54,9 +54,9 @@ function extractResultAlerts( for (const location of result.locations ?? []) { const physicalLocation = location.physicalLocation!; const filePath = physicalLocation.artifactLocation!.uri!; - const codeSnippet = getCodeSnippet(physicalLocation.contextRegion, physicalLocation.region); + const codeSnippet = getCodeSnippet(physicalLocation.contextRegion!); const highlightedRegion = physicalLocation.region - ? getHighlightedRegion(physicalLocation.region) + ? getHighlightedRegion(physicalLocation.region!) : undefined; const analysisAlert: AnalysisAlert = { @@ -156,21 +156,15 @@ export function tryGetRule( return undefined; } -function getCodeSnippet(region?: sarif.Region, alternateRegion?: sarif.Region): CodeSnippet | undefined { - region = region ?? alternateRegion; - - if (!region) { - return undefined; - } - - const text = region.snippet?.text || ''; +function getCodeSnippet(region: sarif.Region): CodeSnippet { + const text = region.snippet!.text!; const { startLine, endLine } = parseSarifRegion(region); return { startLine, endLine, text - }; + } as CodeSnippet; } function getHighlightedRegion(region: sarif.Region): HighlightedRegion { @@ -181,7 +175,7 @@ function getHighlightedRegion(region: sarif.Region): HighlightedRegion { startColumn, endLine, - // parseSarifRegion currently shifts the end column by 1 to account + // parseSarifRegion currently shifts the end column by 1 to account // for the way vscode counts columns so we need to shift it back. endColumn: endColumn + 1 }; @@ -201,7 +195,7 @@ function getCodeFlows( for (const threadFlowLocation of threadFlow.locations) { const physicalLocation = threadFlowLocation!.location!.physicalLocation!; const filePath = physicalLocation!.artifactLocation!.uri!; - const codeSnippet = getCodeSnippet(physicalLocation.contextRegion, physicalLocation.region); + const codeSnippet = getCodeSnippet(physicalLocation.contextRegion!); const highlightedRegion = physicalLocation.region ? getHighlightedRegion(physicalLocation.region) : undefined; diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index c40088391..f8d7613ad 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -21,7 +21,7 @@ export interface AnalysisAlert { shortDescription: string; severity: ResultSeverity; fileLink: FileLink; - codeSnippet?: CodeSnippet; + codeSnippet: CodeSnippet; highlightedRegion?: HighlightedRegion; codeFlows: CodeFlow[]; } diff --git a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx index 05a32ab75..d68de0721 100644 --- a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx @@ -181,17 +181,17 @@ const FileCodeSnippet = ({ messageChildren, }: { fileLink: FileLink, - codeSnippet?: CodeSnippet, + codeSnippet: CodeSnippet, highlightedRegion?: HighlightedRegion, severity?: ResultSeverity, message?: AnalysisMessage, messageChildren?: React.ReactNode, }) => { - const code = codeSnippet?.text.split('\n') || []; + const code = codeSnippet.text.split('\n'); - const startingLine = codeSnippet?.startLine || 0; - const endingLine = codeSnippet?.endLine || 0; + const startingLine = codeSnippet.startLine; + const endingLine = codeSnippet.endLine; const titleFileUri = createRemoteFileRef( fileLink, diff --git a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts index 3a9fb5138..16eb6dace 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts @@ -419,42 +419,15 @@ describe('SARIF processing', () => { expectResultParsingError(result.errors[0]); }); - it('should not return errors for result locations with no snippet', () => { - const sarif = buildValidSarifLog(); - sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion!.snippet = undefined; - - const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); - - const expectedCodeSnippet = { - startLine: result.alerts[0].codeSnippet!.startLine, - endLine: result.alerts[0].codeSnippet!.endLine, - text: '' - }; - - const actualCodeSnippet = result.alerts[0].codeSnippet; - - expect(result).to.be.ok; - expectNoParsingError(result); - expect(actualCodeSnippet).to.deep.equal(expectedCodeSnippet); - }); - - it('should not return errors for result locations with no contextRegion', () => { + it('should return errors for result locations with no context region', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion = undefined; const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); - const expectedCodeSnippet = { - startLine: result.alerts[0].highlightedRegion!.startLine, - endLine: result.alerts[0].highlightedRegion!.endLine, - text: '' - }; - - const actualCodeSnippet = result.alerts[0].codeSnippet; - expect(result).to.be.ok; - expectNoParsingError(result); - expect(actualCodeSnippet).to.deep.equal(expectedCodeSnippet); + expect(result.errors.length).to.equal(1); + expectResultParsingError(result.errors[0]); }); it('should not return errors for result locations with no region', () => { @@ -465,7 +438,6 @@ describe('SARIF processing', () => { expect(result).to.be.ok; expect(result.alerts.length).to.equal(1); - expectNoParsingError(result); }); it('should return errors for result locations with no physical location', () => { @@ -565,9 +537,9 @@ describe('SARIF processing', () => { expect(result).to.be.ok; expect(result.errors.length).to.equal(0); expect(result.alerts.length).to.equal(3); - expect(result.alerts.find(a => getMessageText(a.message) === 'msg1' && a.codeSnippet!.text === 'foo')).to.be.ok; - expect(result.alerts.find(a => getMessageText(a.message) === 'msg1' && a.codeSnippet!.text === 'bar')).to.be.ok; - expect(result.alerts.find(a => getMessageText(a.message) === 'msg2' && a.codeSnippet!.text === 'baz')).to.be.ok; + expect(result.alerts.find(a => getMessageText(a.message) === 'msg1' && a.codeSnippet.text === 'foo')).to.be.ok; + expect(result.alerts.find(a => getMessageText(a.message) === 'msg1' && a.codeSnippet.text === 'bar')).to.be.ok; + expect(result.alerts.find(a => getMessageText(a.message) === 'msg2' && a.codeSnippet.text === 'baz')).to.be.ok; expect(result.alerts.every(a => a.severity === 'Warning')).to.be.true; }); @@ -623,14 +595,9 @@ describe('SARIF processing', () => { expect(msg.startsWith('Error when processing SARIF result')).to.be.true; } - function expectNoParsingError(result: { errors: string[] | undefined }) { - const array = result.errors || []; - expect(array.length, array.join()).to.equal(0); - } - function buildValidSarifLog(): sarif.Log { return { - version: '2.1.0', + version: '0.0.1' as sarif.Log.version, runs: [ { results: [