Merge branch 'main' into aeisenberg/truncated-log-msg

extensions/ql-vscode/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Add settings `codeQL.variantAnalysis.defaultResultsFilter` and `codeQL.variantAnalysis.defaultResultsSort` for configuring how variant analysis results are filtered and sorted in the results view. The default is to show all repositories, and to sort by the number of results. [#2392](https://github.com/github/vscode-codeql/pull/2392)
 - Fix bug to ensure error messages have complete stack trace in message logs. [#2425](https://github.com/github/vscode-codeql/pull/2425)
+- Fix bug where the `CodeQL: Compare Query` command did not work for comparing quick-eval queries. [#2422](https://github.com/github/vscode-codeql/pull/2422)
 
 ## 1.8.4 - 3 May 2023
 

extensions/ql-vscode/src/common/commands.ts

@@ -251,6 +251,9 @@ export type VariantAnalysisCommands = {
   "codeQL.monitorRehydratedVariantAnalysis": (
     variantAnalysis: VariantAnalysis,
   ) => Promise<void>;
+  "codeQL.monitorReauthenticatedVariantAnalysis": (
+    variantAnalysis: VariantAnalysis,
+  ) => Promise<void>;
   "codeQL.openVariantAnalysisLogs": (
     variantAnalysisId: number,
   ) => Promise<void>;

extensions/ql-vscode/src/common/vscode/authentication.ts

@@ -3,7 +3,7 @@ import * as Octokit from "@octokit/rest";
 import { retry } from "@octokit/plugin-retry";
 import { Credentials } from "../authentication";
 
-const GITHUB_AUTH_PROVIDER_ID = "github";
+export const GITHUB_AUTH_PROVIDER_ID = "github";
 
 // We need 'repo' scope for triggering workflows, 'gist' scope for exporting results to Gist,
 // and 'read:packages' for reading private CodeQL packages.

extensions/ql-vscode/src/compare/compare-view.ts

@@ -175,21 +175,40 @@ export class CompareView extends AbstractWebview<
     const commonResultSetNames = fromSchemaNames.filter((name) =>
       toSchemaNames.includes(name),
     );
+
+    // Fall back on the default result set names if there are no common ones.
+    const defaultFromResultSetName = fromSchemaNames.find((name) =>
+      name.startsWith("#"),
+    );
+    const defaultToResultSetName = toSchemaNames.find((name) =>
+      name.startsWith("#"),
+    );
+
+    if (
+      commonResultSetNames.length === 0 &&
+      !(defaultFromResultSetName || defaultToResultSetName)
+    ) {
+      throw new Error(
+        "No common result sets found between the two queries. Please check that the queries are compatible.",
+      );
+    }
+
     const currentResultSetName =
       selectedResultSetName || commonResultSetNames[0];
     const fromResultSet = await this.getResultSet(
       fromSchemas,
-      currentResultSetName,
+      currentResultSetName || defaultFromResultSetName!,
       from.completedQuery.query.resultsPaths.resultsPath,
     );
     const toResultSet = await this.getResultSet(
       toSchemas,
-      currentResultSetName,
+      currentResultSetName || defaultToResultSetName!,
       to.completedQuery.query.resultsPaths.resultsPath,
     );
     return [
       commonResultSetNames,
-      currentResultSetName,
+      currentResultSetName ||
+        `${defaultFromResultSetName} <-> ${defaultToResultSetName}`,
       fromResultSet,
       toResultSet,
     ];

extensions/ql-vscode/src/queries-panel/query-tree-data-provider.ts

@@ -17,11 +17,20 @@ export class QueryTreeDataProvider
   private createTree(): QueryTreeViewItem[] {
     // Temporary mock data, just to populate the tree view.
     return [
-      {
+      new QueryTreeViewItem("custom-pack", [
-        label: "name1",
+        new QueryTreeViewItem("custom-pack/example.ql", []),
-        tooltip: "path1",
+      ]),
-        children: [],
+      new QueryTreeViewItem("ql", [
-      },
+        new QueryTreeViewItem("ql/javascript", [
+          new QueryTreeViewItem("ql/javascript/example.ql", []),
+        ]),
+        new QueryTreeViewItem("ql/go", [
+          new QueryTreeViewItem("ql/go/security", [
+            new QueryTreeViewItem("ql/go/security/query1.ql", []),
+            new QueryTreeViewItem("ql/go/security/query2.ql", []),
+          ]),
+        ]),
+      ]),
     ];
   }
 
@@ -30,9 +39,7 @@ export class QueryTreeDataProvider
    * @param item The item to represent.
    * @returns The UI presentation of the item.
    */
-  public getTreeItem(
+  public getTreeItem(item: QueryTreeViewItem): vscode.TreeItem {
-    item: QueryTreeViewItem,
-  ): vscode.TreeItem | Thenable<vscode.TreeItem> {
     return item;
   }
 
@@ -41,14 +48,12 @@ export class QueryTreeDataProvider
    * @param item The item to expand.
    * @returns The children of the item.
    */
-  public getChildren(
+  public getChildren(item?: QueryTreeViewItem): QueryTreeViewItem[] {
-    item?: QueryTreeViewItem,
-  ): vscode.ProviderResult<QueryTreeViewItem[]> {
     if (!item) {
       // We're at the root.
-      return Promise.resolve(this.queryTreeItems);
+      return this.queryTreeItems;
     } else {
-      return Promise.resolve(item.children);
+      return item.children;
     }
   }
 }

extensions/ql-vscode/src/queries-panel/query-tree-view-item.ts

@@ -1,11 +1,19 @@
 import * as vscode from "vscode";
+import { basename } from "path";
 
 export class QueryTreeViewItem extends vscode.TreeItem {
-  constructor(
+  constructor(path: string, public readonly children: QueryTreeViewItem[]) {
-    public readonly label: string,
+    super(basename(path));
-    public readonly tooltip: string | undefined,
+    this.tooltip = path;
-    public readonly children: QueryTreeViewItem[],
+    this.collapsibleState = this.children.length
-  ) {
+      ? vscode.TreeItemCollapsibleState.Collapsed
-    super(label);
+      : vscode.TreeItemCollapsibleState.None;
+    if (this.children.length === 0) {
+      this.command = {
+        title: "Open",
+        command: "vscode.open",
+        arguments: [vscode.Uri.file(path)],
+      };
+    }
   }
 }

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

@@ -5,6 +5,8 @@ import {
   getVariantAnalysisRepo,
 } from "./gh-api/gh-api-client";
 import {
+  authentication,
+  AuthenticationSessionsChangeEvent,
   CancellationToken,
   env,
   EventEmitter,
@@ -72,6 +74,7 @@ import {
   REPO_STATES_FILENAME,
   writeRepoStates,
 } from "./repo-states-store";
+import { GITHUB_AUTH_PROVIDER_ID } from "../common/vscode/authentication";
 
 export class VariantAnalysisManager
   extends DisposableObject
@@ -131,6 +134,10 @@ export class VariantAnalysisManager
     this.variantAnalysisResultsManager.onResultLoaded(
       this.onRepoResultLoaded.bind(this),
     );
+
+    this.push(
+      authentication.onDidChangeSessions(this.onDidChangeSessions.bind(this)),
+    );
   }
 
   getCommands(): VariantAnalysisCommands {
@@ -144,6 +151,8 @@ export class VariantAnalysisManager
         this.monitorVariantAnalysis.bind(this),
       "codeQL.monitorRehydratedVariantAnalysis":
         this.monitorVariantAnalysis.bind(this),
+      "codeQL.monitorReauthenticatedVariantAnalysis":
+        this.monitorVariantAnalysis.bind(this),
       "codeQL.openVariantAnalysisLogs": this.openVariantAnalysisLogs.bind(this),
       "codeQL.openVariantAnalysisView": this.showView.bind(this),
       "codeQL.runVariantAnalysis":
@@ -504,6 +513,38 @@ export class VariantAnalysisManager
     repoStates[repoState.repositoryId] = repoState;
   }
 
+  private async onDidChangeSessions(
+    event: AuthenticationSessionsChangeEvent,
+  ): Promise<void> {
+    if (event.provider.id !== GITHUB_AUTH_PROVIDER_ID) {
+      return;
+    }
+
+    for (const variantAnalysis of this.variantAnalyses.values()) {
+      if (
+        this.variantAnalysisMonitor.isMonitoringVariantAnalysis(
+          variantAnalysis.id,
+        )
+      ) {
+        continue;
+      }
+
+      if (
+        await isVariantAnalysisComplete(
+          variantAnalysis,
+          this.makeResultDownloadChecker(variantAnalysis),
+        )
+      ) {
+        continue;
+      }
+
+      void this.app.commands.execute(
+        "codeQL.monitorReauthenticatedVariantAnalysis",
+        variantAnalysis,
+      );
+    }
+  }
+
   public async monitorVariantAnalysis(
     variantAnalysis: VariantAnalysis,
   ): Promise<void> {

extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts

@@ -1,5 +1,6 @@
 import { env, EventEmitter } from "vscode";
 import { getVariantAnalysis } from "./gh-api/gh-api-client";
+import { RequestError } from "@octokit/request-error";
 
 import {
   isFinalVariantAnalysisStatus,
@@ -27,6 +28,8 @@ export class VariantAnalysisMonitor extends DisposableObject {
   );
   readonly onVariantAnalysisChange = this._onVariantAnalysisChange.event;
 
+  private readonly monitoringVariantAnalyses = new Set<number>();
+
   constructor(
     private readonly app: App,
     private readonly shouldCancelMonitor: (
@@ -36,9 +39,37 @@ export class VariantAnalysisMonitor extends DisposableObject {
     super();
   }
 
+  public isMonitoringVariantAnalysis(variantAnalysisId: number): boolean {
+    return this.monitoringVariantAnalyses.has(variantAnalysisId);
+  }
+
   public async monitorVariantAnalysis(
     variantAnalysis: VariantAnalysis,
   ): Promise<void> {
+    if (this.monitoringVariantAnalyses.has(variantAnalysis.id)) {
+      void extLogger.log(
+        `Already monitoring variant analysis ${variantAnalysis.id}`,
+      );
+      return;
+    }
+
+    this.monitoringVariantAnalyses.add(variantAnalysis.id);
+    try {
+      await this._monitorVariantAnalysis(variantAnalysis);
+    } finally {
+      this.monitoringVariantAnalyses.delete(variantAnalysis.id);
+    }
+  }
+
+  private async _monitorVariantAnalysis(
+    variantAnalysis: VariantAnalysis,
+  ): Promise<void> {
+    const variantAnalysisLabel = `${variantAnalysis.query.name} (${
+      variantAnalysis.query.language
+    }) [${new Date(variantAnalysis.executionStartTime).toLocaleString(
+      env.language,
+    )}]`;
+
     let attemptCount = 0;
     const scannedReposDownloaded: number[] = [];
 
@@ -61,11 +92,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
       } catch (e) {
         const errorMessage = getErrorMessage(e);
 
-        const message = `Error while monitoring variant analysis ${
+        const message = `Error while monitoring variant analysis ${variantAnalysisLabel}: ${errorMessage}`;
-          variantAnalysis.query.name
-        } (${variantAnalysis.query.language}) [${new Date(
-          variantAnalysis.executionStartTime,
-        ).toLocaleString(env.language)}]: ${errorMessage}`;
 
         // If we have already shown this error to the user, don't show it again.
         if (lastErrorShown === errorMessage) {
@@ -75,6 +102,19 @@ export class VariantAnalysisMonitor extends DisposableObject {
           lastErrorShown = errorMessage;
         }
 
+        if (e instanceof RequestError && e.status === 404) {
+          // We want to show the error message to the user, but we don't want to
+          // keep polling for the variant analysis if it no longer exists.
+          // Therefore, this block is down here rather than at the top of the
+          // catch block.
+          void extLogger.log(
+            `Variant analysis ${variantAnalysisLabel} no longer exists or is no longer accessible, stopping monitoring.`,
+          );
+          // Cancel monitoring on 404, as this probably means the user does not have access to it anymore
+          // e.g. lost access to repo, or repo was deleted
+          return;
+        }
+
         continue;
       }
 

extensions/ql-vscode/src/view/compare/Compare.tsx

@@ -59,9 +59,7 @@ export function Compare(_: Record<string, never>): JSX.Element {
     return (
       <>
         <div className="vscode-codeql__compare-header">
-          <div className="vscode-codeql__compare-header-item">
+          <div className="vscode-codeql__compare-header-item">Comparing:</div>
-            Table to compare:
-          </div>
           <CompareSelector
             availableResultSets={comparison.commonResultSetNames}
             currentResultSetName={comparison.currentResultSetName}

extensions/ql-vscode/src/view/compare/CompareSelector.tsx

@@ -7,7 +7,8 @@ interface Props {
 }
 
 export default function CompareSelector(props: Props) {
-  return (
+  return props.availableResultSets.length ? (
+    // Handle case where there are shared result sets
     <select
       value={props.currentResultSetName}
       onChange={(e) => props.updateResultSet(e.target.value)}
@@ -18,5 +19,8 @@ export default function CompareSelector(props: Props) {
         </option>
       ))}
     </select>
+  ) : (
+    // Handle case where there are no shared result sets
+    <div>{props.currentResultSetName}</div>
   );
 }

extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts

@@ -1,4 +1,5 @@
 import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client";
+import { RequestError } from "@octokit/request-error";
 import { VariantAnalysisMonitor } from "../../../../src/variant-analysis/variant-analysis-monitor";
 import {
   VariantAnalysis as VariantAnalysisApiResponse,
@@ -297,6 +298,55 @@ describe("Variant Analysis Monitor", () => {
         expect(mockEecuteCommand).not.toBeCalled();
       });
     });
+
+    describe("when a 404 is returned", () => {
+      let showAndLogWarningMessageSpy: jest.SpiedFunction<
+        typeof helpers.showAndLogWarningMessage
+      >;
+
+      beforeEach(async () => {
+        showAndLogWarningMessageSpy = jest
+          .spyOn(helpers, "showAndLogWarningMessage")
+          .mockResolvedValue(undefined);
+
+        const scannedRepos = createMockScannedRepos([
+          "pending",
+          "in_progress",
+          "in_progress",
+          "in_progress",
+          "pending",
+          "pending",
+        ]);
+        mockApiResponse = createMockApiResponse("in_progress", scannedRepos);
+        mockGetVariantAnalysis.mockResolvedValueOnce(mockApiResponse);
+
+        mockGetVariantAnalysis.mockRejectedValueOnce(
+          new RequestError("Not Found", 404, {
+            request: {
+              method: "GET",
+              url: "",
+              headers: {},
+            },
+            response: {
+              status: 404,
+              headers: {},
+              url: "",
+              data: {},
+            },
+          }),
+        );
+      });
+
+      it("should stop requesting the variant analysis", async () => {
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
+
+        expect(mockGetVariantAnalysis).toHaveBeenCalledTimes(2);
+        expect(showAndLogWarningMessageSpy).toHaveBeenCalledTimes(1);
+        expect(showAndLogWarningMessageSpy).toHaveBeenCalledWith(
+          expect.stringMatching(/not found/i),
+        );
+      });
+    });
   });
 
   function limitNumberOfAttemptsToMonitor() {