Merge branch 'main' into koesie10/access-paths-query-bqrs

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),
+    from: arrayDiff(canonicalFromResults, canonicalToResults),
-    to: arrayDiff(toResults, fromResults),
+    to: arrayDiff(canonicalToResults, canonicalFromResults),
   };
 
   if (

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,

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) => {

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,

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<AccessPathSuggestionRow[]>;
+  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<string | undefined> {
@@ -156,14 +161,7 @@ async function resolveSuggestionsQuery(
   const queries = await resolveQueriesFromPacks(
     cliServer,
     packsToSearch,
-    {
+    queryConstraints,
-      kind: "table",
-      "tags contain all": [
-        "modeleditor",
-        "access-path-suggestions",
-        modeTag(mode),
-      ],
-    },
     additionalPackPaths,
   );
   if (queries.length > 1) {

extensions/ql-vscode/src/view/model-editor/MethodRow.tsx

@@ -278,6 +278,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
                     <ModelInputSuggestBox
                       modeledMethod={modeledMethod}
                       suggestions={inputAccessPathSuggestions}
+                      typePathSuggestions={outputAccessPathSuggestions ?? []}
                       onChange={modeledMethodChangedHandlers[index]}
                     />
                   )}

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<string | undefined>(
@@ -75,7 +77,13 @@ export const ModelInputSuggestBox = ({
   );
 
   if (modeledMethod?.type === "type") {
-    return <ReadonlyDropdown value={modeledMethod.path} aria-label="Path" />;
+    return (
+      <ModelTypePathSuggestBox
+        modeledMethod={modeledMethod}
+        suggestions={typePathSuggestions}
+        onChange={onChange}
+      />
+    );
   }
 
   return (

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 (
-      <ReadonlyDropdown
+      <ModelTypeTextbox
-        value={modeledMethod.relatedTypeName}
+        modeledMethod={modeledMethod}
+        typeInfo="relatedTypeName"
+        onChange={onChange}
         aria-label="Related type name"
       />
     );

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) => (
+  <ModelSuggestionIcon name={option.icon} />
+);
+
+const getDetails = (option: AccessPathOption) => option.details;
+
+export const ModelTypePathSuggestBox = ({
+  modeledMethod,
+  suggestions,
+  onChange,
+}: Props) => {
+  const [value, setValue] = useState<string | undefined>(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 (
+    <SuggestBox<AccessPathOption, AccessPathDiagnostic>
+      value={value}
+      options={suggestions}
+      onChange={setValue}
+      parseValueToTokens={parseValueToTokens}
+      validateValue={validateAccessPath}
+      getIcon={getIcon}
+      getDetails={getDetails}
+      aria-label="Path"
+    />
+  );
+};

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);
+  });
+});

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<CodeQLCliServer>({
         resolveQlpacks: jest.fn().mockResolvedValue({
           "my/extensions": "/a/b/c/",
         }),
-        resolveQueriesInSuite: jest
+        resolveQueriesInSuite,
-          .fn()
-          .mockResolvedValue(["/a/b/c/FrameworkModeAccessPathSuggestions.ql"]),
         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);