Skip to content

Commit f52a512

Browse files
Merge pull request #2672 from github/robertbrignull/sarif-processing-no-location
Handle when an alert message contains links to files outside of the repository source
2 parents daf389a + 6a12dc2 commit f52a512

File tree

4 files changed

+87
-21
lines changed

4 files changed

+87
-21
lines changed

extensions/ql-vscode/src/common/sarif-utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as Sarif from "sarif";
22
import type { HighlightedRegion } from "../variant-analysis/shared/analysis-result";
33
import { ResolvableLocationValue } from "../common/bqrs-cli-types";
44

5-
interface SarifLink {
5+
export interface SarifLink {
66
dest: number;
77
text: string;
88
}

extensions/ql-vscode/src/variant-analysis/sarif-processing.ts

+41-16
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as sarif from "sarif";
22
import {
3+
SarifLink,
34
parseHighlightedLine,
45
parseSarifPlainTextMessage,
56
parseSarifRegion,
@@ -14,6 +15,7 @@ import {
1415
ThreadFlow,
1516
CodeSnippet,
1617
HighlightedRegion,
18+
AnalysisMessageLocationTokenLocation,
1719
} from "./shared/analysis-result";
1820

1921
// A line of more than 8k characters is probably generated.
@@ -303,24 +305,47 @@ function getMessage(
303305
if (typeof messagePart === "string") {
304306
tokens.push({ t: "text", text: messagePart });
305307
} else {
306-
const relatedLocation = result.relatedLocations!.find(
307-
(rl) => rl.id === messagePart.dest,
308-
);
309-
tokens.push({
310-
t: "location",
311-
text: messagePart.text,
312-
location: {
313-
fileLink: {
314-
fileLinkPrefix,
315-
filePath: relatedLocation!.physicalLocation!.artifactLocation!.uri!,
316-
},
317-
highlightedRegion: getHighlightedRegion(
318-
relatedLocation!.physicalLocation!.region!,
319-
),
320-
},
321-
});
308+
const location = getRelatedLocation(messagePart, result, fileLinkPrefix);
309+
if (location === undefined) {
310+
tokens.push({ t: "text", text: messagePart.text });
311+
} else {
312+
tokens.push({
313+
t: "location",
314+
text: messagePart.text,
315+
location,
316+
});
317+
}
322318
}
323319
}
324320

325321
return { tokens };
326322
}
323+
324+
function getRelatedLocation(
325+
messagePart: SarifLink,
326+
result: sarif.Result,
327+
fileLinkPrefix: string,
328+
): AnalysisMessageLocationTokenLocation | undefined {
329+
const relatedLocation = result.relatedLocations!.find(
330+
(rl) => rl.id === messagePart.dest,
331+
);
332+
if (
333+
relatedLocation === undefined ||
334+
relatedLocation.physicalLocation?.artifactLocation?.uri === undefined ||
335+
relatedLocation.physicalLocation?.artifactLocation?.uri?.startsWith(
336+
"file:",
337+
) ||
338+
relatedLocation.physicalLocation?.region === undefined
339+
) {
340+
return undefined;
341+
}
342+
return {
343+
fileLink: {
344+
fileLinkPrefix,
345+
filePath: relatedLocation.physicalLocation.artifactLocation.uri,
346+
},
347+
highlightedRegion: getHighlightedRegion(
348+
relatedLocation.physicalLocation.region,
349+
),
350+
};
351+
}

extensions/ql-vscode/src/variant-analysis/shared/analysis-result.ts

+6-4
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,12 @@ interface AnalysisMessageTextToken {
6363
export interface AnalysisMessageLocationToken {
6464
t: "location";
6565
text: string;
66-
location: {
67-
fileLink: FileLink;
68-
highlightedRegion?: HighlightedRegion;
69-
};
66+
location: AnalysisMessageLocationTokenLocation;
67+
}
68+
69+
export interface AnalysisMessageLocationTokenLocation {
70+
fileLink: FileLink;
71+
highlightedRegion?: HighlightedRegion;
7072
}
7173

7274
export type ResultSeverity = "Recommendation" | "Warning" | "Error";

extensions/ql-vscode/test/unit-tests/sarif-processing.test.ts

+39
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,45 @@ describe("SARIF processing", () => {
720720
expectNoParsingError(result);
721721
expect(actualCodeSnippet).not.toBeUndefined();
722722
});
723+
724+
it("should be able to handle when a location has no uri", () => {
725+
const sarif = buildValidSarifLog();
726+
sarif.runs![0].results![0].message.text = "message [String](1)";
727+
sarif.runs![0].results![0].relatedLocations = [
728+
{
729+
id: 1,
730+
physicalLocation: {
731+
artifactLocation: {
732+
uri: "file:/modules/java.base/java/lang/String.class",
733+
index: 1,
734+
},
735+
},
736+
message: {
737+
text: "String",
738+
},
739+
},
740+
];
741+
742+
const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix);
743+
744+
expect(result).toBeTruthy();
745+
expectNoParsingError(result);
746+
expect(result.alerts[0].codeSnippet).not.toBeUndefined();
747+
expect(result.alerts[0].message.tokens).toStrictEqual([
748+
{
749+
t: "text",
750+
text: "message ",
751+
},
752+
{
753+
t: "text",
754+
text: "String",
755+
},
756+
{
757+
t: "text",
758+
text: "",
759+
},
760+
]);
761+
});
723762
});
724763

725764
function expectResultParsingError(msg: string) {

0 commit comments

Comments
 (0)