diff --git a/extensions/ql-vscode/src/compare/sarif-diff.ts b/extensions/ql-vscode/src/compare/sarif-diff.ts index 9f80895260b..8f8dafd3a9a 100644 --- a/extensions/ql-vscode/src/compare/sarif-diff.ts +++ b/extensions/ql-vscode/src/compare/sarif-diff.ts @@ -1,5 +1,40 @@ import type { Result } from "sarif"; +function toCanonicalResult(result: Result): Result { + const canonicalResult = { + ...result, + }; + + if (canonicalResult.locations) { + canonicalResult.locations = canonicalResult.locations.map((location) => { + if (location.physicalLocation?.artifactLocation?.index !== undefined) { + const canonicalLocation = { + ...location, + }; + + canonicalLocation.physicalLocation = { + ...canonicalLocation.physicalLocation, + }; + + canonicalLocation.physicalLocation.artifactLocation = { + ...canonicalLocation.physicalLocation.artifactLocation, + }; + + // The index is dependent on the result of the SARIF file and usually doesn't really tell + // us anything useful, so we remove it from the comparison. + delete canonicalLocation.physicalLocation.artifactLocation.index; + + return canonicalLocation; + } + + // Don't create a new object if we don't need to + return location; + }); + } + + return canonicalResult; +} + /** * Compare the alerts of two queries. Use deep equality to determine if * results have been added or removed across two invocations of a query. @@ -25,9 +60,12 @@ export function sarifDiff(fromResults: Result[], toResults: Result[]) { throw new Error("CodeQL Compare: Target query has no results."); } + const canonicalFromResults = fromResults.map(toCanonicalResult); + const canonicalToResults = toResults.map(toCanonicalResult); + const results = { - from: arrayDiff(fromResults, toResults), - to: arrayDiff(toResults, fromResults), + from: arrayDiff(canonicalFromResults, canonicalToResults), + to: arrayDiff(canonicalToResults, canonicalFromResults), }; if ( diff --git a/extensions/ql-vscode/test/unit-tests/compare/sarif-diff.test.ts b/extensions/ql-vscode/test/unit-tests/compare/sarif-diff.test.ts new file mode 100644 index 00000000000..ac6190dcfa1 --- /dev/null +++ b/extensions/ql-vscode/test/unit-tests/compare/sarif-diff.test.ts @@ -0,0 +1,183 @@ +import type { Result } from "sarif"; +import { sarifDiff } from "../../../src/compare/sarif-diff"; + +describe("sarifDiff", () => { + const result1: Result = { + message: { + text: "result1", + }, + }; + const result2: Result = { + message: { + text: "result2", + }, + }; + const result3: Result = { + message: { + text: "result3", + }, + }; + + it("throws an error when the source query has no results", () => { + expect(() => sarifDiff([], [result1, result2])).toThrow( + "CodeQL Compare: Source query has no results.", + ); + }); + + it("throws an error when the target query has no results", () => { + expect(() => sarifDiff([result1, result2], [])).toThrow( + "CodeQL Compare: Target query has no results.", + ); + }); + + it("throws an error when there is no overlap between the source and target queries", () => { + expect(() => sarifDiff([result1], [result2])).toThrow( + "CodeQL Compare: No overlap between the selected queries.", + ); + }); + + it("return an empty comparison when the results are the same", () => { + expect(sarifDiff([result1, result2], [result1, result2])).toEqual({ + from: [], + to: [], + }); + }); + + it("returns the added and removed results", () => { + expect(sarifDiff([result1, result3], [result1, result2])).toEqual({ + from: [result3], + to: [result2], + }); + }); + + it("does not use reference equality to compare results", () => { + const result = { + message: { + text: "result1", + }, + }; + expect(sarifDiff([result], [result1])).toEqual({ + from: [], + to: [], + }); + }); + + it("does not take into account the index of the artifact location", () => { + const result1: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file1", + uriBaseId: "%SRCROOT%", + index: 1, + }, + }, + }, + ], + }; + const result2: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file1", + uriBaseId: "%SRCROOT%", + index: 2, + }, + }, + }, + ], + }; + expect(sarifDiff([result1], [result2])).toEqual({ + from: [], + to: [], + }); + }); + + it("takes into account the other properties of the artifact location", () => { + const result1: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file1", + uriBaseId: "%SRCROOT%", + }, + }, + }, + ], + }; + const result2: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file2", + uriBaseId: "%SRCROOT%", + }, + }, + }, + ], + }; + expect(sarifDiff([result1], [result1, result2])).toEqual({ + from: [], + to: [result2], + }); + }); + + it("does not modify the input", () => { + const result1: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file1", + uriBaseId: "%SRCROOT%", + index: 1, + }, + }, + }, + ], + }; + const result1Copy = JSON.parse(JSON.stringify(result1)); + const result2: Result = { + message: { + text: "result1", + }, + locations: [ + { + physicalLocation: { + artifactLocation: { + uri: "file:///path/to/file1", + uriBaseId: "%SRCROOT%", + index: 2, + }, + }, + }, + ], + }; + const result2Copy = JSON.parse(JSON.stringify(result2)); + expect(sarifDiff([result1], [result2])).toEqual({ + from: [], + to: [], + }); + expect(result1).toEqual(result1Copy); + expect(result2).toEqual(result2Copy); + }); +});