diff --git a/extensions/ql-vscode/src/compare/sarif-diff.ts b/extensions/ql-vscode/src/compare/sarif-diff.ts index 9f8089526..8f8dafd3a 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/src/model-editor/languages/models-as-data.ts b/extensions/ql-vscode/src/model-editor/languages/models-as-data.ts index 25ebd21d2..4fad24268 100644 --- a/extensions/ql-vscode/src/model-editor/languages/models-as-data.ts +++ b/extensions/ql-vscode/src/model-editor/languages/models-as-data.ts @@ -44,6 +44,7 @@ type ModelsAsDataLanguageModelGeneration = { }; type ModelsAsDataLanguageAccessPathSuggestions = { + queryConstraints: (mode: Mode) => QueryConstraints; parseResults: ( // The results of a single predicate of the query. bqrs: DecodedBqrsChunk, diff --git a/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts b/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts index e1b9d9a5b..faa1c6fc7 100644 --- a/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts +++ b/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts @@ -12,6 +12,7 @@ import { rubyPath, } from "./access-paths"; import { parseAccessPathSuggestionsResults } from "./suggestions"; +import { modeTag } from "../../mode-tag"; export const ruby: ModelsAsDataLanguage = { availableModes: [Mode.Framework], @@ -170,6 +171,10 @@ export const ruby: ModelsAsDataLanguage = { parseResults: parseGenerateModelResults, }, accessPathSuggestions: { + queryConstraints: (mode) => ({ + kind: "table", + "tags contain all": ["modeleditor", "access-paths", modeTag(mode)], + }), parseResults: parseAccessPathSuggestionsResults, }, getArgumentOptions: (method) => { diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index 0680f8f09..7bb5114d7 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -529,6 +529,7 @@ export class ModelEditorView extends AbstractWebview< modelsAsDataLanguage, this.app.logger, ), + queryConstraints: accessPathSuggestions.queryConstraints(mode), cliServer: this.cliServer, queryRunner: this.queryRunner, queryStorageDir: this.queryStorageDir, diff --git a/extensions/ql-vscode/src/model-editor/suggestion-queries.ts b/extensions/ql-vscode/src/model-editor/suggestion-queries.ts index ef96da2d3..37ce78fd3 100644 --- a/extensions/ql-vscode/src/model-editor/suggestion-queries.ts +++ b/extensions/ql-vscode/src/model-editor/suggestion-queries.ts @@ -1,7 +1,7 @@ import type { CodeQLCliServer } from "../codeql-cli/cli"; import type { Mode } from "./shared/mode"; +import type { QueryConstraints } from "../local-queries"; import { resolveQueriesFromPacks } from "../local-queries"; -import { modeTag } from "./mode-tag"; import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders"; import type { NotificationLogger } from "../common/logging"; import { showAndLogExceptionWithTelemetry } from "../common/logging"; @@ -22,6 +22,7 @@ type RunQueryOptions = { parseResults: ( results: DecodedBqrsChunk, ) => AccessPathSuggestionRow[] | Promise; + queryConstraints: QueryConstraints; cliServer: CodeQLCliServer; queryRunner: QueryRunner; @@ -39,6 +40,7 @@ export async function runSuggestionsQuery( mode: Mode, { parseResults, + queryConstraints, cliServer, queryRunner, logger, @@ -68,6 +70,7 @@ export async function runSuggestionsQuery( cliServer, databaseItem.language, mode, + queryConstraints, ); if (!queryPath) { void showAndLogExceptionWithTelemetry( @@ -141,6 +144,7 @@ export async function runSuggestionsQuery( * @param cliServer The CodeQL CLI server to use. * @param language The language of the query pack to use. * @param mode The mode to resolve the query for. + * @param queryConstraints Constraints to apply to the query. * @param additionalPackNames Additional pack names to search. * @param additionalPackPaths Additional pack paths to search. */ @@ -148,6 +152,7 @@ async function resolveSuggestionsQuery( cliServer: CodeQLCliServer, language: string, mode: Mode, + queryConstraints: QueryConstraints, additionalPackNames: string[] = [], additionalPackPaths: string[] = [], ): Promise { @@ -156,14 +161,7 @@ async function resolveSuggestionsQuery( const queries = await resolveQueriesFromPacks( cliServer, packsToSearch, - { - kind: "table", - "tags contain all": [ - "modeleditor", - "access-path-suggestions", - modeTag(mode), - ], - }, + queryConstraints, additionalPackPaths, ); if (queries.length > 1) { diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index e54e52d48..4b3b9e0df 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -278,6 +278,7 @@ const ModelableMethodRow = forwardRef( )} diff --git a/extensions/ql-vscode/src/view/model-editor/ModelInputSuggestBox.tsx b/extensions/ql-vscode/src/view/model-editor/ModelInputSuggestBox.tsx index 16ef1a330..b290dff43 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModelInputSuggestBox.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModelInputSuggestBox.tsx @@ -4,7 +4,6 @@ import { calculateNewProvenance, modeledMethodSupportsInput, } from "../../model-editor/modeled-method"; -import { ReadonlyDropdown } from "../common/ReadonlyDropdown"; import type { AccessPathOption } from "../../model-editor/suggestions"; import { SuggestBox } from "../common/SuggestBox"; import { useDebounceCallback } from "../common/useDebounceCallback"; @@ -14,10 +13,12 @@ import { validateAccessPath, } from "../../model-editor/shared/access-paths"; import { ModelSuggestionIcon } from "./ModelSuggestionIcon"; +import { ModelTypePathSuggestBox } from "./ModelTypePathSuggestBox"; type Props = { modeledMethod: ModeledMethod | undefined; suggestions: AccessPathOption[]; + typePathSuggestions: AccessPathOption[]; onChange: (modeledMethod: ModeledMethod) => void; }; @@ -33,6 +34,7 @@ const getDetails = (option: AccessPathOption) => option.details; export const ModelInputSuggestBox = ({ modeledMethod, suggestions, + typePathSuggestions, onChange, }: Props) => { const [value, setValue] = useState( @@ -75,7 +77,13 @@ export const ModelInputSuggestBox = ({ ); if (modeledMethod?.type === "type") { - return ; + return ( + + ); } return ( diff --git a/extensions/ql-vscode/src/view/model-editor/ModelOutputSuggestBox.tsx b/extensions/ql-vscode/src/view/model-editor/ModelOutputSuggestBox.tsx index d7ebfc4dd..0aeb3d6de 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModelOutputSuggestBox.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModelOutputSuggestBox.tsx @@ -4,7 +4,6 @@ import { calculateNewProvenance, modeledMethodSupportsOutput, } from "../../model-editor/modeled-method"; -import { ReadonlyDropdown } from "../common/ReadonlyDropdown"; import type { AccessPathOption } from "../../model-editor/suggestions"; import { SuggestBox } from "../common/SuggestBox"; import { useDebounceCallback } from "../common/useDebounceCallback"; @@ -14,6 +13,7 @@ import { validateAccessPath, } from "../../model-editor/shared/access-paths"; import { ModelSuggestionIcon } from "./ModelSuggestionIcon"; +import { ModelTypeTextbox } from "./ModelTypeTextbox"; type Props = { modeledMethod: ModeledMethod | undefined; @@ -76,8 +76,10 @@ export const ModelOutputSuggestBox = ({ if (modeledMethod?.type === "type") { return ( - ); diff --git a/extensions/ql-vscode/src/view/model-editor/ModelTypePathSuggestBox.tsx b/extensions/ql-vscode/src/view/model-editor/ModelTypePathSuggestBox.tsx new file mode 100644 index 000000000..044c65766 --- /dev/null +++ b/extensions/ql-vscode/src/view/model-editor/ModelTypePathSuggestBox.tsx @@ -0,0 +1,68 @@ +import { useEffect, useState } from "react"; +import type { TypeModeledMethod } from "../../model-editor/modeled-method"; +import type { AccessPathOption } from "../../model-editor/suggestions"; +import { SuggestBox } from "../common/SuggestBox"; +import { useDebounceCallback } from "../common/useDebounceCallback"; +import type { AccessPathDiagnostic } from "../../model-editor/shared/access-paths"; +import { + parseAccessPathTokens, + validateAccessPath, +} from "../../model-editor/shared/access-paths"; +import { ModelSuggestionIcon } from "./ModelSuggestionIcon"; + +type Props = { + modeledMethod: TypeModeledMethod; + suggestions: AccessPathOption[]; + onChange: (modeledMethod: TypeModeledMethod) => void; +}; + +const parseValueToTokens = (value: string) => + parseAccessPathTokens(value).map((t) => t.text); + +const getIcon = (option: AccessPathOption) => ( + +); + +const getDetails = (option: AccessPathOption) => option.details; + +export const ModelTypePathSuggestBox = ({ + modeledMethod, + suggestions, + onChange, +}: Props) => { + const [value, setValue] = useState(modeledMethod.path); + + useEffect(() => { + setValue(modeledMethod.path); + }, [modeledMethod]); + + // Debounce the callback to avoid updating the model too often. + // Not doing this results in a lot of lag when typing. + useDebounceCallback( + value, + (path: string | undefined) => { + if (path === undefined) { + return; + } + + onChange({ + ...modeledMethod, + path, + }); + }, + 500, + ); + + return ( + + value={value} + options={suggestions} + onChange={setValue} + parseValueToTokens={parseValueToTokens} + validateValue={validateAccessPath} + getIcon={getIcon} + getDetails={getDetails} + aria-label="Path" + /> + ); +}; 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 000000000..ac6190dcf --- /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); + }); +}); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/suggestion-queries.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/suggestion-queries.test.ts index 4450cd10b..f3ebc013b 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/suggestion-queries.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/suggestion-queries.test.ts @@ -1,3 +1,5 @@ +import { load } from "js-yaml"; +import { readFile } from "fs-extra"; import { createMockLogger } from "../../../__mocks__/loggerMock"; import type { DatabaseItem } from "../../../../src/databases/local-databases"; import { DatabaseKind } from "../../../../src/databases/local-databases"; @@ -144,15 +146,21 @@ describe("runSuggestionsQuery", () => { .mockResolvedValueOnce(mockInputSuggestions) .mockResolvedValueOnce(mockOutputSuggestions); + const resolveQueriesInSuite = jest + .fn() + .mockResolvedValue(["/a/b/c/FrameworkModeAccessPathSuggestions.ql"]); + const options = { parseResults, + queryConstraints: { + kind: "table", + "tags all": ["modeleditor", "access-paths", "ruby", "foo"], + }, cliServer: mockedObject({ resolveQlpacks: jest.fn().mockResolvedValue({ "my/extensions": "/a/b/c/", }), - resolveQueriesInSuite: jest - .fn() - .mockResolvedValue(["/a/b/c/FrameworkModeAccessPathSuggestions.ql"]), + resolveQueriesInSuite, packPacklist: jest .fn() .mockResolvedValue([ @@ -211,6 +219,20 @@ describe("runSuggestionsQuery", () => { undefined, undefined, ); + expect(options.cliServer.resolveQueriesInSuite).toHaveBeenCalledTimes(1); + + expect( + load(await readFile(resolveQueriesInSuite.mock.calls[0][0], "utf-8")), + ).toEqual([ + { + from: "codeql/ruby-queries", + include: { + kind: "table", + "tags all": ["modeleditor", "access-paths", "ruby", "foo"], + }, + queries: ".", + }, + ]); expect(options.parseResults).toHaveBeenCalledTimes(2);