Merge branch 'main' into elena/yer-a-windows-query

extensions/ql-vscode/CHANGELOG.md

@@ -2,8 +2,12 @@
 
 ## [UNRELEASED]
 
+## 1.8.2 - 12 April 2023
+
+- Fix bug where users could end up with the managed CodeQL CLI getting uninstalled during upgrades and not reinstalled. [#2294](https://github.com/github/vscode-codeql/pull/2294)
 - Fix bug that was causing code flows to not get updated when switching between results. [#2288](https://github.com/github/vscode-codeql/pull/2288)
 - Restart the CodeQL language server whenever the _CodeQL: Restart Query Server_ command is invoked. This avoids bugs where the CLI version changes to support new language features, but the language server is not updated. [#2238](https://github.com/github/vscode-codeql/pull/2238)
+- Avoid requiring a manual restart of the query server when the [external CLI config file](https://docs.github.com/en/code-security/codeql-cli/using-the-codeql-cli/specifying-command-options-in-a-codeql-configuration-file#using-a-codeql-configuration-file) changes. [#2289](https://github.com/github/vscode-codeql/pull/2289)
 
 ## 1.8.1 - 23 March 2023
 

extensions/ql-vscode/package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "vscode-codeql",
-  "version": "1.8.2",
+  "version": "1.8.3",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "vscode-codeql",
-      "version": "1.8.2",
+      "version": "1.8.3",
       "hasInstallScript": true,
       "license": "MIT",
       "dependencies": {

extensions/ql-vscode/package.json

@@ -4,7 +4,7 @@
   "description": "CodeQL for Visual Studio Code",
   "author": "GitHub",
   "private": true,
-  "version": "1.8.2",
+  "version": "1.8.3",
   "publisher": "GitHub",
   "license": "MIT",
   "icon": "media/VS-marketplace-CodeQL-icon.png",

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

@@ -71,6 +71,7 @@ export type BaseCommands = {
   "codeQL.restartQueryServer": () => Promise<void>;
   "codeQL.restartQueryServerOnConfigChange": () => Promise<void>;
   "codeQL.restartLegacyQueryServerOnConfigChange": () => Promise<void>;
+  "codeQL.restartQueryServerOnExternalConfigChange": () => Promise<void>;
 };
 
 // Commands used when working with queries in the editor

extensions/ql-vscode/src/databaseFetcher.ts

@@ -122,12 +122,11 @@ export async function askForGitHubRepo(
   progress?: ProgressCallback,
   suggestedValue?: string,
 ): Promise<string | undefined> {
-  progress &&
-    progress({
-      message: "Choose repository",
-      step: 1,
-      maxStep: 2,
-    });
+  progress?.({
+    message: "Choose repository",
+    step: 1,
+    maxStep: 2,
+  });
 
   const options: InputBoxOptions = {
     title:

extensions/ql-vscode/src/distribution.ts

@@ -215,6 +215,9 @@ export class DistributionManager implements DistributionProvider {
     minSecondsSinceLastUpdateCheck: number,
   ): Promise<DistributionUpdateCheckResult> {
     const distribution = await this.getDistributionWithoutVersionCheck();
+    if (distribution === undefined) {
+      minSecondsSinceLastUpdateCheck = 0;
+    }
     const extensionManagedCodeQlPath =
       await this.extensionSpecificDistributionManager.getCodeQlPathWithoutVersionCheck();
     if (distribution?.codeQlPath !== extensionManagedCodeQlPath) {

extensions/ql-vscode/src/extension.ts

@@ -13,12 +13,13 @@ import {
   workspace,
 } from "vscode";
 import { LanguageClient } from "vscode-languageclient/node";
-import { arch, platform } from "os";
+import { arch, platform, homedir } from "os";
 import { ensureDir } from "fs-extra";
 import { join } from "path";
 import { dirSync } from "tmp-promise";
 import { testExplorerExtensionId, TestHub } from "vscode-test-adapter-api";
 import { lt, parse } from "semver";
+import { watch } from "chokidar";
 
 import { AstViewer } from "./astViewer";
 import {
@@ -194,6 +195,7 @@ function getCommands(
     "codeQL.restartQueryServer": restartQueryServer,
     "codeQL.restartQueryServerOnConfigChange": restartQueryServer,
     "codeQL.restartLegacyQueryServerOnConfigChange": restartQueryServer,
+    "codeQL.restartQueryServerOnExternalConfigChange": restartQueryServer,
     "codeQL.copyVersion": async () => {
       const text = `CodeQL extension version: ${
         extension?.packageJSON.version
@@ -672,6 +674,7 @@ async function activateWithInstalledDistribution(
     extLogger,
   );
   ctx.subscriptions.push(cliServer);
+  watchExternalConfigFile(app, ctx);
 
   const statusBar = new CodeQlStatusBarHandler(
     cliServer,
@@ -1011,6 +1014,34 @@ async function activateWithInstalledDistribution(
   };
 }
 
+/**
+ * Handle changes to the external config file. This is used to restart the query server
+ * when the user changes options.
+ * See https://docs.github.com/en/code-security/codeql-cli/using-the-codeql-cli/specifying-command-options-in-a-codeql-configuration-file#using-a-codeql-configuration-file
+ */
+function watchExternalConfigFile(app: ExtensionApp, ctx: ExtensionContext) {
+  const home = homedir();
+  if (home) {
+    const configPath = join(home, ".config", "codeql", "config");
+    const configWatcher = watch(configPath, {
+      // These options avoid firing the event twice.
+      persistent: true,
+      ignoreInitial: true,
+      awaitWriteFinish: true,
+    });
+    configWatcher.on("all", async () => {
+      await app.commands.execute(
+        "codeQL.restartQueryServerOnExternalConfigChange",
+      );
+    });
+    ctx.subscriptions.push({
+      dispose: () => {
+        void configWatcher.close();
+      },
+    });
+  }
+}
+
 async function showResultsForComparison(
   compareView: CompareView,
   from: CompletedLocalQueryInfo,

extensions/ql-vscode/src/view/results/__tests__/results.spec.tsx

@@ -1,5 +1,5 @@
 import * as React from "react";
-import { render as reactRender, screen } from "@testing-library/react";
+import { act, render as reactRender, screen } from "@testing-library/react";
 import { ResultsApp } from "../results";
 import {
   Interpretation,
@@ -20,18 +20,20 @@ const exampleSarif = fs.readJSONSync(
 describe(ResultsApp.name, () => {
   const render = () => reactRender(<ResultsApp />);
   const postMessage = async (msg: IntoResultsViewMsg) => {
-    // window.postMessage doesn't set the origin correctly, see
-    // https://github.com/jsdom/jsdom/issues/2745
-    window.dispatchEvent(
-      new MessageEvent("message", {
-        source: window,
-        origin: window.location.origin,
-        data: msg,
-      }),
-    );
+    await act(async () => {
+      // window.postMessage doesn't set the origin correctly, see
+      // https://github.com/jsdom/jsdom/issues/2745
+      window.dispatchEvent(
+        new MessageEvent("message", {
+          source: window,
+          origin: window.location.origin,
+          data: msg,
+        }),
+      );
 
-    // The event is dispatched asynchronously, so we need to wait for it to be handled.
-    await new Promise((resolve) => setTimeout(resolve, 0));
+      // The event is dispatched asynchronously, so we need to wait for it to be handled.
+      await new Promise((resolve) => setTimeout(resolve, 0));
+    });
   };
 
   it("renders results", async () => {
@@ -95,6 +97,7 @@ describe(ResultsApp.name, () => {
         },
       },
     };
+
     await postMessage(message);
 
     expect(
@@ -117,4 +120,84 @@ describe(ResultsApp.name, () => {
       screen.getByText("'x' is assigned a value but never used."),
     ).toBeInTheDocument();
   });
+
+  it("renders results when switching between queries with different result set names", async () => {
+    render();
+
+    await postMessage({
+      t: "setState",
+      interpretation: undefined,
+      origResultsPaths: {
+        resultsPath: "/a/b/c/results.bqrs",
+        interpretedResultsPath: "/a/b/c/interpretedResults.sarif",
+      },
+      resultsPath: "/a/b/c/results.bqrs",
+      parsedResultSets: {
+        pageNumber: 0,
+        pageSize: 200,
+        numPages: 1,
+        numInterpretedPages: 0,
+        resultSet: {
+          schema: {
+            name: "#select",
+            rows: 1,
+            columns: [{ kind: "s" }],
+            pagination: { "step-size": 200, offsets: [13] },
+          },
+          rows: [["foobar1"]],
+          t: "RawResultSet",
+        },
+        resultSetNames: ["#select"],
+      },
+      sortedResultsMap: {},
+      database: {
+        name: "test-db",
+        databaseUri: "test-db-uri",
+      },
+      shouldKeepOldResultsWhileRendering: false,
+      metadata: {},
+      queryName: "empty.ql",
+      queryPath: "/a/b/c/empty.ql",
+    });
+
+    expect(screen.getByText("foobar1")).toBeInTheDocument();
+
+    await postMessage({
+      t: "setState",
+      interpretation: undefined,
+      origResultsPaths: {
+        resultsPath: "/a/b/c/results.bqrs",
+        interpretedResultsPath: "/a/b/c/interpretedResults.sarif",
+      },
+      resultsPath: "/a/b/c/results.bqrs",
+      parsedResultSets: {
+        pageNumber: 0,
+        pageSize: 200,
+        numPages: 1,
+        numInterpretedPages: 0,
+        resultSet: {
+          schema: {
+            name: "#Quick_evaluation_of_expression",
+            rows: 1,
+            columns: [{ name: "#expr_result", kind: "s" }],
+            pagination: { "step-size": 200, offsets: [49] },
+          },
+          rows: [["foobar2"]],
+          t: "RawResultSet",
+        },
+        resultSetNames: ["#Quick_evaluation_of_expression"],
+      },
+      sortedResultsMap: {},
+      database: {
+        name: "test-db",
+        databaseUri: "test-db-uri",
+      },
+      shouldKeepOldResultsWhileRendering: false,
+      metadata: {},
+      queryName: "Quick evaluation of empty.ql:1",
+      queryPath: "/a/b/c/empty.ql",
+    });
+
+    expect(screen.getByText("foobar2")).toBeInTheDocument();
+  });
 });

extensions/ql-vscode/src/view/results/result-tables.tsx

@@ -78,6 +78,52 @@ function renderResultCountString(resultSet: ResultSet): JSX.Element {
   );
 }
 
+function getInterpretedTableName(interpretation: Interpretation): string {
+  return interpretation.data.t === "GraphInterpretationData"
+    ? GRAPH_TABLE_NAME
+    : ALERTS_TABLE_NAME;
+}
+
+function getResultSetNames(
+  interpretation: Interpretation | undefined,
+  parsedResultSets: ParsedResultSets,
+): string[] {
+  return interpretation
+    ? parsedResultSets.resultSetNames.concat([
+        getInterpretedTableName(interpretation),
+      ])
+    : parsedResultSets.resultSetNames;
+}
+
+function getResultSets(
+  rawResultSets: readonly ResultSet[],
+  interpretation: Interpretation | undefined,
+): ResultSet[] {
+  const resultSets: ResultSet[] =
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore 2783 Avoid compilation error for overwriting the t property
+    rawResultSets.map((rs) => ({ t: "RawResultSet", ...rs }));
+
+  if (interpretation !== undefined) {
+    const tableName = getInterpretedTableName(interpretation);
+    resultSets.push({
+      t: "InterpretedResultSet",
+      // FIXME: The values of version, columns, tupleCount are
+      // unused stubs because a InterpretedResultSet schema isn't used the
+      // same way as a RawResultSet. Probably should pull `name` field
+      // out.
+      schema: {
+        name: tableName,
+        rows: 1,
+        columns: [],
+      },
+      name: tableName,
+      interpretation,
+    });
+  }
+  return resultSets;
+}
+
 /**
  * Displays multiple `ResultTable` tables, where the table to be displayed is selected by a
  * dropdown.
@@ -86,51 +132,13 @@ export class ResultTables extends React.Component<
   ResultTablesProps,
   ResultTablesState
 > {
-  private getResultSets(): ResultSet[] {
-    const resultSets: ResultSet[] =
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore 2783
-      this.props.rawResultSets.map((rs) => ({ t: "RawResultSet", ...rs }));
-
-    if (this.props.interpretation !== undefined) {
-      const tableName = this.getInterpretedTableName();
-      resultSets.push({
-        t: "InterpretedResultSet",
-        // FIXME: The values of version, columns, tupleCount are
-        // unused stubs because a InterpretedResultSet schema isn't used the
-        // same way as a RawResultSet. Probably should pull `name` field
-        // out.
-        schema: {
-          name: tableName,
-          rows: 1,
-          columns: [],
-        },
-        name: tableName,
-        interpretation: this.props.interpretation,
-      });
-    }
-    return resultSets;
-  }
-
-  private getInterpretedTableName(): string {
-    return this.props.interpretation?.data.t === "GraphInterpretationData"
-      ? GRAPH_TABLE_NAME
-      : ALERTS_TABLE_NAME;
-  }
-
-  private getResultSetNames(): string[] {
-    return this.props.interpretation
-      ? this.props.parsedResultSets.resultSetNames.concat([
-          this.getInterpretedTableName(),
-        ])
-      : this.props.parsedResultSets.resultSetNames;
-  }
-
   constructor(props: ResultTablesProps) {
     super(props);
     const selectedTable =
       props.parsedResultSets.selectedTable ||
-      getDefaultResultSet(this.getResultSets());
+      getDefaultResultSet(
+        getResultSets(props.rawResultSets, props.interpretation),
+      );
     const selectedPage = `${props.parsedResultSets.pageNumber + 1}`;
     this.state = {
       selectedTable,
@@ -139,6 +147,36 @@ export class ResultTables extends React.Component<
     };
   }
 
+  componentDidUpdate(
+    prevProps: Readonly<ResultTablesProps>,
+    prevState: Readonly<ResultTablesState>,
+    snapshot?: any,
+  ) {
+    const resultSetExists =
+      this.props.parsedResultSets.resultSetNames.some(
+        (v) => this.state.selectedTable === v,
+      ) ||
+      getResultSets(this.props.rawResultSets, this.props.interpretation).some(
+        (v) => this.state.selectedTable === v.schema.name,
+      );
+
+    // If the selected result set does not exist, select the default result set.
+    if (!resultSetExists) {
+      this.setState((state, props) => {
+        const selectedTable =
+          props.parsedResultSets.selectedTable ||
+          getDefaultResultSet(
+            getResultSets(props.rawResultSets, props.interpretation),
+          );
+
+        return {
+          selectedTable,
+          selectedPage: `${props.parsedResultSets.pageNumber + 1}`,
+        };
+      });
+    }
+  }
+
   untoggleProblemsView() {
     this.setState({
       problemsViewSelected: false,
@@ -303,8 +341,14 @@ export class ResultTables extends React.Component<
 
   render(): React.ReactNode {
     const { selectedTable } = this.state;
-    const resultSets = this.getResultSets();
-    const resultSetNames = this.getResultSetNames();
+    const resultSets = getResultSets(
+      this.props.rawResultSets,
+      this.props.interpretation,
+    );
+    const resultSetNames = getResultSetNames(
+      this.props.interpretation,
+      this.props.parsedResultSets,
+    );
 
     const resultSet = resultSets.find(
       (resultSet) => resultSet.schema.name === selectedTable,