Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -152,11 +154,11 @@ export class AnalysesResultsManager {
return await extractRawResults(this.cliServer, this.logger, filePath);
}

private async readSarifResults(filePath: string): Promise<AnalysisAlert[]> {
private async readSarifResults(filePath: string, fileLinkPrefix: string): Promise<AnalysisAlert[]> {
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)}`);
}

Expand All @@ -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}`;
}
}
32 changes: 22 additions & 10 deletions extensions/ql-vscode/src/remote-queries/sarif-processing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
const defaultSeverity = 'Warning';

export function extractAnalysisAlerts(
sarifLog: sarif.Log
sarifLog: sarif.Log,
fileLinkPrefix: string
): {
alerts: AnalysisAlert[],
errors: string[]
Expand All @@ -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;
Expand All @@ -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 ?? []) {
Expand All @@ -60,7 +62,10 @@ function extractResultAlerts(
const analysisAlert: AnalysisAlert = {
message,
shortDescription,
filePath,
fileLink: {
fileLinkPrefix,
filePath,
},
severity,
codeSnippet,
highlightedRegion,
Expand Down Expand Up @@ -174,7 +179,8 @@ function getHighlightedRegion(region: sarif.Region): HighlightedRegion {
}

function getCodeFlows(
result: sarif.Result
result: sarif.Result,
fileLinkPrefix: string
): CodeFlow[] {
const codeFlows = [];

Expand All @@ -192,7 +198,10 @@ function getCodeFlows(
: undefined;

threadFlows.push({
filePath,
fileLink: {
fileLinkPrefix,
filePath,
},
codeSnippet,
highlightedRegion
} as ThreadFlow);
Expand All @@ -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!;
Expand All @@ -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!),
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,7 +48,7 @@ export interface CodeFlow {
}

export interface ThreadFlow {
filePath: string;
fileLink: FileLink;
codeSnippet: CodeSnippet;
highlightedRegion?: HighlightedRegion;
message?: AnalysisMessage;
Expand All @@ -66,7 +71,7 @@ export interface AnalysisMessageLocationToken {
t: 'location';
text: string;
location: {
filePath: string;
fileLink: FileLink;
highlightedRegion?: HighlightedRegion;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => {
const showPathsLink = alert.codeFlows.length > 0;

return <FileCodeSnippet
filePath={alert.filePath}
fileLink={alert.fileLink}
codeSnippet={alert.codeSnippet}
highlightedRegion={alert.highlightedRegion}
severity={alert.severity}
Expand Down
4 changes: 2 additions & 2 deletions extensions/ql-vscode/src/remote-queries/view/CodePaths.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const CodePath = ({

<VerticalSpace size={2} />
<FileCodeSnippet
filePath={threadFlow.filePath}
fileLink={threadFlow.fileLink}
codeSnippet={threadFlow.codeSnippet}
highlightedRegion={threadFlow.highlightedRegion}
severity={severity}
Expand All @@ -82,7 +82,7 @@ const CodePath = ({
};

const getCodeFlowName = (codeFlow: CodeFlow) => {
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);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
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';

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':
Expand Down Expand Up @@ -106,7 +114,11 @@ const Message = ({
case 'text':
return <span key={`token-${index}`}>{token.text}</span>;
case 'location':
return <Link key={`token-${index}`} href='TODO'>{token.text}</Link>;
return <Link
key={`token-${index}`}
href={createFileLink(token.location.fileLink, token.location.highlightedRegion?.startLine, token.location.highlightedRegion?.endLine)}>
{token.text}
</Link>;
default:
return <></>;
}
Expand Down Expand Up @@ -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,
Expand All @@ -183,11 +195,12 @@ const FileCodeSnippet = ({
const code = codeSnippet.text.split('\n');

const startingLine = codeSnippet.startLine;
const endingLine = codeSnippet.endLine;

return (
<Container>
<TitleContainer>
<Link>{filePath}</Link>
<Link href={createFileLink(fileLink, startingLine, endingLine)}>{fileLink.filePath}</Link>
</TitleContainer>
<CodeContainer>
{code.map((line, index) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ export function RemoteQueries(): JSX.Element {
return <div>Waiting for results to load.</div>;
}

const showAnalysesResults = false;
const showAnalysesResults = true;

try {
return <div>
Expand Down
22 changes: 13 additions & 9 deletions extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down