diff --git a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts index 97fa3c45aff..8b0c6bb3429 100644 --- a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts @@ -121,10 +121,12 @@ export class AnalysesResultsManager { throw new Error(`Could not download the analysis results for ${analysis.nwo}: ${e.message}`); } + const fileLinkPrefix = this.createFileLinkPrefix(analysis.nwo, analysis.databaseSha); + let newAnaysisResults: AnalysisResults; const fileExtension = path.extname(artifactPath); if (fileExtension === '.sarif') { - const queryResults = await this.readSarifResults(artifactPath); + const queryResults = await this.readSarifResults(artifactPath, fileLinkPrefix); newAnaysisResults = { ...analysisResults, interpretedResults: queryResults, @@ -152,11 +154,11 @@ export class AnalysesResultsManager { return await extractRawResults(this.cliServer, this.logger, filePath); } - private async readSarifResults(filePath: string): Promise { + private async readSarifResults(filePath: string, fileLinkPrefix: string): Promise { const sarifLog = await sarifParser(filePath); - const processedSarif = extractAnalysisAlerts(sarifLog); - if (processedSarif.errors) { + const processedSarif = extractAnalysisAlerts(sarifLog, fileLinkPrefix); + if (processedSarif.errors.length) { void this.logger.log(`Error processing SARIF file: ${os.EOL}${processedSarif.errors.join(os.EOL)}`); } @@ -166,4 +168,8 @@ export class AnalysesResultsManager { private isAnalysisInMemory(analysis: AnalysisSummary): boolean { return this.internalGetAnalysesResults(analysis.downloadLink.queryId).some(x => x.nwo === analysis.nwo); } + + private createFileLinkPrefix(nwo: string, sha: string): string { + return `https://github.com/${nwo}/blob/${sha}`; + } } diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts index 0ae886fd1ff..25de2f84f43 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts @@ -15,7 +15,8 @@ import { const defaultSeverity = 'Warning'; export function extractAnalysisAlerts( - sarifLog: sarif.Log + sarifLog: sarif.Log, + fileLinkPrefix: string ): { alerts: AnalysisAlert[], errors: string[] @@ -26,7 +27,7 @@ export function extractAnalysisAlerts( for (const run of sarifLog.runs ?? []) { for (const result of run.results ?? []) { try { - alerts.push(...extractResultAlerts(run, result)); + alerts.push(...extractResultAlerts(run, result, fileLinkPrefix)); } catch (e) { errors.push(`Error when processing SARIF result: ${e}`); continue; @@ -39,14 +40,15 @@ export function extractAnalysisAlerts( function extractResultAlerts( run: sarif.Run, - result: sarif.Result + result: sarif.Result, + fileLinkPrefix: string ): AnalysisAlert[] { const alerts: AnalysisAlert[] = []; - const message = getMessage(result); + const message = getMessage(result, fileLinkPrefix); const rule = tryGetRule(run, result); const severity = tryGetSeverity(run, result, rule) || defaultSeverity; - const codeFlows = getCodeFlows(result); + const codeFlows = getCodeFlows(result, fileLinkPrefix); const shortDescription = getShortDescription(rule, message!); for (const location of result.locations ?? []) { @@ -60,7 +62,10 @@ function extractResultAlerts( const analysisAlert: AnalysisAlert = { message, shortDescription, - filePath, + fileLink: { + fileLinkPrefix, + filePath, + }, severity, codeSnippet, highlightedRegion, @@ -174,7 +179,8 @@ function getHighlightedRegion(region: sarif.Region): HighlightedRegion { } function getCodeFlows( - result: sarif.Result + result: sarif.Result, + fileLinkPrefix: string ): CodeFlow[] { const codeFlows = []; @@ -192,7 +198,10 @@ function getCodeFlows( : undefined; threadFlows.push({ - filePath, + fileLink: { + fileLinkPrefix, + filePath, + }, codeSnippet, highlightedRegion } as ThreadFlow); @@ -206,7 +215,7 @@ function getCodeFlows( return codeFlows; } -function getMessage(result: sarif.Result): AnalysisMessage { +function getMessage(result: sarif.Result, fileLinkPrefix: string): AnalysisMessage { const tokens: AnalysisMessageToken[] = []; const messageText = result.message!.text!; @@ -221,7 +230,10 @@ function getMessage(result: sarif.Result): AnalysisMessage { t: 'location', text: messagePart.text, location: { - filePath: relatedLocation!.physicalLocation!.artifactLocation!.uri!, + fileLink: { + fileLinkPrefix: fileLinkPrefix, + filePath: relatedLocation!.physicalLocation!.artifactLocation!.uri!, + }, highlightedRegion: getHighlightedRegion(relatedLocation!.physicalLocation!.region!), } }); 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 52c5177896c..cdd622a0fc2 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -19,12 +19,17 @@ export interface AnalysisAlert { message: AnalysisMessage; shortDescription: string; severity: ResultSeverity; - filePath: string; + fileLink: FileLink; codeSnippet: CodeSnippet; highlightedRegion?: HighlightedRegion; codeFlows: CodeFlow[]; } +export interface FileLink { + fileLinkPrefix: string; + filePath: string; +} + export interface CodeSnippet { startLine: number; endLine: number; @@ -43,7 +48,7 @@ export interface CodeFlow { } export interface ThreadFlow { - filePath: string; + fileLink: FileLink; codeSnippet: CodeSnippet; highlightedRegion?: HighlightedRegion; message?: AnalysisMessage; @@ -66,7 +71,7 @@ export interface AnalysisMessageLocationToken { t: 'location'; text: string; location: { - filePath: string; + fileLink: FileLink; highlightedRegion?: HighlightedRegion; }; } diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx index 408511d1666..736fe49c350 100644 --- a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -7,7 +7,7 @@ const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { const showPathsLink = alert.codeFlows.length > 0; return { - const filePath = codeFlow.threadFlows[codeFlow.threadFlows.length - 1].filePath; + const filePath = codeFlow.threadFlows[codeFlow.threadFlows.length - 1].fileLink.filePath; return filePath.substring(filePath.lastIndexOf('/') + 1); }; diff --git a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx index 1e0db50a5ea..9505a1f0d9e 100644 --- a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import styled from 'styled-components'; -import { CodeSnippet, HighlightedRegion, AnalysisMessage, ResultSeverity } from '../shared/analysis-result'; +import { CodeSnippet, FileLink, HighlightedRegion, AnalysisMessage, ResultSeverity } from '../shared/analysis-result'; import { Box, Link } from '@primer/react'; import VerticalSpace from './VerticalSpace'; @@ -8,6 +8,14 @@ const borderColor = 'var(--vscode-editor-snippetFinalTabstopHighlightBorder)'; const warningColor = '#966C23'; const highlightColor = '#534425'; +const createFileLink = (fileLink: FileLink, startLine?: number, endLine?: number) => { + if (startLine && endLine) { + return `${fileLink.fileLinkPrefix}/${fileLink.filePath}#L${startLine}-L${endLine}`; + } else { + return `${fileLink.fileLinkPrefix}/${fileLink.filePath}`; + } +}; + const getSeverityColor = (severity: ResultSeverity) => { switch (severity) { case 'Recommendation': @@ -106,7 +114,11 @@ const Message = ({ case 'text': return {token.text}; case 'location': - return {token.text}; + return + {token.text} + ; default: return <>; } @@ -165,14 +177,14 @@ const CodeLine = ({ }; const FileCodeSnippet = ({ - filePath, + fileLink, codeSnippet, highlightedRegion, severity, message, messageChildren, }: { - filePath: string, + fileLink: FileLink, codeSnippet: CodeSnippet, highlightedRegion?: HighlightedRegion, severity?: ResultSeverity, @@ -183,11 +195,12 @@ const FileCodeSnippet = ({ const code = codeSnippet.text.split('\n'); const startingLine = codeSnippet.startLine; + const endingLine = codeSnippet.endLine; return ( - {filePath} + {fileLink.filePath} {code.map((line, index) => ( diff --git a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx index 9bf2c746bd5..116569bd932 100644 --- a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx @@ -369,7 +369,7 @@ export function RemoteQueries(): JSX.Element { return
Waiting for results to load.
; } - const showAnalysesResults = false; + const showAnalysesResults = true; try { return
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 208db88d124..9315f28ecd9 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts @@ -378,12 +378,13 @@ describe('SARIF processing', () => { }); describe('extractAnalysisAlerts', () => { + const fakefileLinkPrefix = 'https://example.com'; it('should not return any results if no runs found in the SARIF', () => { const sarif = { // Runs are missing here. } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(0); @@ -401,7 +402,7 @@ describe('SARIF processing', () => { ] } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(0); @@ -411,7 +412,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.message.text = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -422,7 +423,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -433,7 +434,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.region = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(1); @@ -443,7 +444,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.artifactLocation = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -532,7 +533,7 @@ describe('SARIF processing', () => { ] } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(0); expect(result.alerts.length).to.equal(3); @@ -562,7 +563,7 @@ describe('SARIF processing', () => { } ]; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(0); @@ -574,7 +575,10 @@ describe('SARIF processing', () => { expect(message.tokens[1].t).to.equal('location'); expect(message.tokens[1].text).to.equal('absolute path'); expect((message.tokens[1] as AnalysisMessageLocationToken).location).to.deep.equal({ - filePath: 'npm-packages/meteor-installer/config.js', + fileLink: { + fileLinkPrefix: fakefileLinkPrefix, + filePath: 'npm-packages/meteor-installer/config.js', + }, highlightedRegion: { startLine: 35, startColumn: 20,