Merge pull request #1822 from github/koesie10/consistent-query-text-file-commands

Make query text and file commands consistent

extensions/ql-vscode/src/extension.ts

@@ -1281,6 +1281,24 @@ async function activateWithInstalledDistribution(
     ),
   );
 
+  ctx.subscriptions.push(
+    commandRunner(
+      "codeQL.openVariantAnalysisQueryText",
+      async (variantAnalysisId: number) => {
+        await variantAnalysisManager.openQueryText(variantAnalysisId);
+      },
+    ),
+  );
+
+  ctx.subscriptions.push(
+    commandRunner(
+      "codeQL.openVariantAnalysisQueryFile",
+      async (variantAnalysisId: number) => {
+        await variantAnalysisManager.openQueryFile(variantAnalysisId);
+      },
+    ),
+  );
+
   ctx.subscriptions.push(
     commandRunner("codeQL.openReferencedFile", openReferencedFile),
   );

extensions/ql-vscode/src/query-history.ts

@@ -826,6 +826,14 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
+    if (finalSingleItem.t === "variant-analysis") {
+      await commands.executeCommand(
+        "codeQL.openVariantAnalysisQueryFile",
+        finalSingleItem.variantAnalysis.id,
+      );
+      return;
+    }
+
     let queryPath: string;
     switch (finalSingleItem.t) {
       case "local":
@@ -834,9 +842,6 @@ export class QueryHistoryManager extends DisposableObject {
       case "remote":
         queryPath = finalSingleItem.remoteQuery.queryFilePath;
         break;
-      case "variant-analysis":
-        queryPath = finalSingleItem.variantAnalysis.query.filePath;
-        break;
       default:
         assertNever(finalSingleItem);
     }
@@ -1340,6 +1345,14 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
+    if (finalSingleItem.t === "variant-analysis") {
+      await commands.executeCommand(
+        "codeQL.openVariantAnalysisQueryText",
+        finalSingleItem.variantAnalysis.id,
+      );
+      return;
+    }
+
     const params = new URLSearchParams({
       isQuickEval: String(
         !!(

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

@@ -11,6 +11,9 @@ import {
   EventEmitter,
   ExtensionContext,
   Uri,
+  ViewColumn,
+  window as Window,
+  workspace,
 } from "vscode";
 import { DisposableObject } from "../pure/disposable-object";
 import { Credentials } from "../authentication";
@@ -43,6 +46,7 @@ import {
   createTimestampFile,
   showAndLogErrorMessage,
   showAndLogInformationMessage,
+  showAndLogWarningMessage,
 } from "../helpers";
 import { readFile, readJson, remove, pathExists, outputJson } from "fs-extra";
 import { EOL } from "os";
@@ -54,6 +58,7 @@ import {
   filterAndSortRepositoriesWithResults,
   RepositoriesFilterSortStateWithIds,
 } from "../pure/variant-analysis-filter-sort";
+import { URLSearchParams } from "url";
 
 export class VariantAnalysisManager
   extends DisposableObject
@@ -267,6 +272,57 @@ export class VariantAnalysisManager
     return;
   }
 
+  public async openQueryText(variantAnalysisId: number): Promise<void> {
+    const variantAnalysis = await this.getVariantAnalysis(variantAnalysisId);
+    if (!variantAnalysis) {
+      void showAndLogWarningMessage(
+        "Could not open variant analysis query text. Variant analysis not found.",
+      );
+      return;
+    }
+
+    const filename = variantAnalysis.query.filePath;
+
+    try {
+      const params = new URLSearchParams({
+        variantAnalysisId: variantAnalysis.id.toString(),
+      });
+      const uri = Uri.from({
+        scheme: "codeql-variant-analysis",
+        path: filename,
+        query: params.toString(),
+      });
+      const doc = await workspace.openTextDocument(uri);
+      await Window.showTextDocument(doc, { preview: false });
+    } catch (error) {
+      void showAndLogWarningMessage(
+        "Could not open variant analysis query text. Failed to open text document.",
+      );
+    }
+  }
+
+  public async openQueryFile(variantAnalysisId: number): Promise<void> {
+    const variantAnalysis = await this.getVariantAnalysis(variantAnalysisId);
+
+    if (!variantAnalysis) {
+      void showAndLogWarningMessage(
+        "Could not open variant analysis query file",
+      );
+      return;
+    }
+
+    try {
+      const textDocument = await workspace.openTextDocument(
+        variantAnalysis.query.filePath,
+      );
+      await Window.showTextDocument(textDocument, ViewColumn.One);
+    } catch (error) {
+      void showAndLogWarningMessage(
+        `Could not open file: ${variantAnalysis.query.filePath}`,
+      );
+    }
+  }
+
   public registerView(view: VariantAnalysisView): void {
     if (this.views.has(view.variantAnalysisId)) {
       throw new Error(

extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts

@@ -1,12 +1,4 @@
-import {
-  commands,
-  ExtensionContext,
-  Uri,
-  ViewColumn,
-  window as Window,
-  workspace,
-} from "vscode";
-import { URLSearchParams } from "url";
+import { commands, ExtensionContext, Uri, ViewColumn } from "vscode";
 import { AbstractWebview, WebviewPanelConfig } from "../abstract-webview";
 import { extLogger } from "../common";
 import {
@@ -129,10 +121,16 @@ export class VariantAnalysisView
         );
         break;
       case "openQueryFile":
-        await this.openQueryFile();
+        void commands.executeCommand(
+          "codeQL.openVariantAnalysisQueryFile",
+          this.variantAnalysisId,
+        );
         break;
       case "openQueryText":
-        await this.openQueryText();
+        void commands.executeCommand(
+          "codeQL.openVariantAnalysisQueryText",
+          this.variantAnalysisId,
+        );
         break;
       case "copyRepositoryList":
         void commands.executeCommand(
@@ -186,61 +184,6 @@ export class VariantAnalysisView
     });
   }
 
-  private async openQueryFile(): Promise<void> {
-    const variantAnalysis = await this.manager.getVariantAnalysis(
-      this.variantAnalysisId,
-    );
-
-    if (!variantAnalysis) {
-      void showAndLogWarningMessage(
-        "Could not open variant analysis query file",
-      );
-      return;
-    }
-
-    try {
-      const textDocument = await workspace.openTextDocument(
-        variantAnalysis.query.filePath,
-      );
-      await Window.showTextDocument(textDocument, ViewColumn.One);
-    } catch (error) {
-      void showAndLogWarningMessage(
-        `Could not open file: ${variantAnalysis.query.filePath}`,
-      );
-    }
-  }
-
-  private async openQueryText(): Promise<void> {
-    const variantAnalysis = await this.manager.getVariantAnalysis(
-      this.variantAnalysisId,
-    );
-    if (!variantAnalysis) {
-      void showAndLogWarningMessage(
-        "Could not open variant analysis query text. Variant analysis not found.",
-      );
-      return;
-    }
-
-    const filename = variantAnalysis.query.filePath;
-
-    try {
-      const params = new URLSearchParams({
-        variantAnalysisId: variantAnalysis.id.toString(),
-      });
-      const uri = Uri.from({
-        scheme: "codeql-variant-analysis",
-        path: filename,
-        query: params.toString(),
-      });
-      const doc = await workspace.openTextDocument(uri);
-      await Window.showTextDocument(doc, { preview: false });
-    } catch (error) {
-      void showAndLogWarningMessage(
-        "Could not open variant analysis query text. Failed to open text document.",
-      );
-    }
-  }
-
   private async openLogs(): Promise<void> {
     const variantAnalysis = await this.manager.getVariantAnalysis(
       this.variantAnalysisId,

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts

@@ -4,8 +4,11 @@ import {
   env,
   extensions,
   QuickPickItem,
+  TextDocument,
+  TextEditor,
   Uri,
   window,
+  workspace,
 } from "vscode";
 import { CodeQLExtensionInterface } from "../../../extension";
 import { extLogger } from "../../../common";
@@ -1121,4 +1124,94 @@ describe("Variant Analysis Manager", () => {
       });
     });
   });
+
+  describe("openQueryText", () => {
+    let variantAnalysis: VariantAnalysis;
+    let variantAnalysisStorageLocation: string;
+
+    let showTextDocumentSpy: jest.SpiedFunction<typeof window.showTextDocument>;
+    let openTextDocumentSpy: jest.SpiedFunction<
+      typeof workspace.openTextDocument
+    >;
+
+    beforeEach(async () => {
+      variantAnalysis = createMockVariantAnalysis({});
+
+      variantAnalysisStorageLocation =
+        variantAnalysisManager.getVariantAnalysisStorageLocation(
+          variantAnalysis.id,
+        );
+      await createTimestampFile(variantAnalysisStorageLocation);
+      await variantAnalysisManager.rehydrateVariantAnalysis(variantAnalysis);
+
+      showTextDocumentSpy = jest
+        .spyOn(window, "showTextDocument")
+        .mockResolvedValue(undefined as unknown as TextEditor);
+      openTextDocumentSpy = jest
+        .spyOn(workspace, "openTextDocument")
+        .mockResolvedValue(undefined as unknown as TextDocument);
+    });
+
+    afterEach(() => {
+      fs.rmSync(variantAnalysisStorageLocation, { recursive: true });
+    });
+
+    it("opens the query text", async () => {
+      await variantAnalysisManager.openQueryText(variantAnalysis.id);
+
+      expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
+      expect(openTextDocumentSpy).toHaveBeenCalledTimes(1);
+
+      const uri: Uri = openTextDocumentSpy.mock.calls[0][0] as Uri;
+      expect(uri.scheme).toEqual("codeql-variant-analysis");
+      expect(uri.path).toEqual(variantAnalysis.query.filePath);
+      const params = new URLSearchParams(uri.query);
+      expect(Array.from(params.keys())).toEqual(["variantAnalysisId"]);
+      expect(params.get("variantAnalysisId")).toEqual(
+        variantAnalysis.id.toString(),
+      );
+    });
+  });
+
+  describe("openQueryFile", () => {
+    let variantAnalysis: VariantAnalysis;
+    let variantAnalysisStorageLocation: string;
+
+    let showTextDocumentSpy: jest.SpiedFunction<typeof window.showTextDocument>;
+    let openTextDocumentSpy: jest.SpiedFunction<
+      typeof workspace.openTextDocument
+    >;
+
+    beforeEach(async () => {
+      variantAnalysis = createMockVariantAnalysis({});
+
+      variantAnalysisStorageLocation =
+        variantAnalysisManager.getVariantAnalysisStorageLocation(
+          variantAnalysis.id,
+        );
+      await createTimestampFile(variantAnalysisStorageLocation);
+      await variantAnalysisManager.rehydrateVariantAnalysis(variantAnalysis);
+
+      showTextDocumentSpy = jest
+        .spyOn(window, "showTextDocument")
+        .mockResolvedValue(undefined as unknown as TextEditor);
+      openTextDocumentSpy = jest
+        .spyOn(workspace, "openTextDocument")
+        .mockResolvedValue(undefined as unknown as TextDocument);
+    });
+
+    afterEach(() => {
+      fs.rmSync(variantAnalysisStorageLocation, { recursive: true });
+    });
+
+    it("opens the query file", async () => {
+      await variantAnalysisManager.openQueryFile(variantAnalysis.id);
+
+      expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
+      expect(openTextDocumentSpy).toHaveBeenCalledTimes(1);
+
+      const filename: string = openTextDocumentSpy.mock.calls[0][0] as string;
+      expect(filename).toEqual(variantAnalysis.query.filePath);
+    });
+  });
 });

extensions/ql-vscode/src/vscode-tests/no-workspace/remote-queries/variant-analysis-history.test.ts

@@ -8,14 +8,7 @@ import {
 } from "fs-extra";
 import { join } from "path";
 
-import {
-  ExtensionContext,
-  TextDocument,
-  TextEditor,
-  Uri,
-  window,
-  workspace,
-} from "vscode";
+import { commands, ExtensionContext, Uri } from "vscode";
 import { QueryHistoryConfig } from "../../../config";
 import { DatabaseManager } from "../../../databases";
 import { tmpDir, walkDirectory } from "../../../helpers";
@@ -71,10 +64,7 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     showView: showViewStub,
   } as any as VariantAnalysisManager;
 
-  let showTextDocumentSpy: jest.SpiedFunction<typeof window.showTextDocument>;
-  let openTextDocumentSpy: jest.SpiedFunction<
-    typeof workspace.openTextDocument
-  >;
+  let executeCommandSpy: jest.SpiedFunction<typeof commands.executeCommand>;
 
   beforeEach(async () => {
     // Since these tests change the state of the query history manager, we need to copy the original
@@ -107,12 +97,9 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     );
     disposables.push(qhm);
 
-    showTextDocumentSpy = jest
-      .spyOn(window, "showTextDocument")
-      .mockResolvedValue(undefined as unknown as TextEditor);
-    openTextDocumentSpy = jest
-      .spyOn(workspace, "openTextDocument")
-      .mockResolvedValue(undefined as unknown as TextDocument);
+    executeCommandSpy = jest
+      .spyOn(commands, "executeCommand")
+      .mockResolvedValue(undefined);
   });
 
   afterEach(() => {
@@ -195,15 +182,9 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     await qhm.readQueryHistory();
     await qhm.handleShowQueryText(qhm.treeDataProvider.allHistory[0], []);
 
-    expect(showTextDocumentSpy).toBeCalledTimes(1);
-    expect(openTextDocumentSpy).toBeCalledTimes(1);
-
-    const uri: Uri = openTextDocumentSpy.mock.calls[0][0] as Uri;
-    expect(uri.scheme).toBe("codeql");
-    const params = new URLSearchParams(uri.query);
-    expect(params.get("isQuickEval")).toBe("false");
-    expect(params.get("queryText")).toBe(
-      rawQueryHistory[0].variantAnalysis.query.text,
+    expect(executeCommandSpy).toHaveBeenCalledWith(
+      "codeQL.openVariantAnalysisQueryText",
+      rawQueryHistory[0].variantAnalysis.id,
     );
   });