Merge pull request #2185 from github/robertbrignull/remove_internal_commands

Remove two more internal-only commands

extensions/ql-vscode/src/extension.ts

@@ -1146,15 +1146,6 @@ async function activateWithInstalledDistribution(
     ),
   );
 
-  ctx.subscriptions.push(
-    commandRunner(
-      "codeQL.cancelVariantAnalysis",
-      async (variantAnalysisId: number) => {
-        await variantAnalysisManager.cancelVariantAnalysis(variantAnalysisId);
-      },
-    ),
-  );
-
   ctx.subscriptions.push(
     commandRunner("codeQL.exportSelectedVariantAnalysisResults", async () => {
       await exportSelectedVariantAnalysisResults(qhm);
@@ -1208,15 +1199,6 @@ async function activateWithInstalledDistribution(
     ),
   );
 
-  ctx.subscriptions.push(
-    commandRunner(
-      "codeQL.openVariantAnalysisQueryText",
-      async (variantAnalysisId: number) => {
-        await variantAnalysisManager.openQueryText(variantAnalysisId);
-      },
-    ),
-  );
-
   ctx.subscriptions.push(
     commandRunner("codeQL.openReferencedFile", async (selectedQuery: Uri) => {
       await openReferencedFile(qs, cliServer, selectedQuery);

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

@@ -894,8 +894,7 @@ export class QueryHistoryManager extends DisposableObject {
         if (item.t === "local") {
           item.cancel();
         } else if (item.t === "variant-analysis") {
-          await commands.executeCommand(
+          await this.variantAnalysisManager.cancelVariantAnalysis(
-            "codeQL.cancelVariantAnalysis",
             item.variantAnalysis.id,
           );
         } else {
@@ -921,8 +920,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.t === "variant-analysis") {
-      await commands.executeCommand(
+      await this.variantAnalysisManager.openQueryText(
-        "codeQL.openVariantAnalysisQueryText",
         finalSingleItem.variantAnalysis.id,
       );
       return;

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

@@ -25,4 +25,6 @@ export interface VariantAnalysisViewManager<
     variantAnalysisId: number,
   ): Promise<VariantAnalysisScannedRepositoryState[]>;
   openQueryFile(variantAnalysisId: number): Promise<void>;
+  openQueryText(variantAnalysisId: number): Promise<void>;
+  cancelVariantAnalysis(variantAnalysisId: number): Promise<void>;
 }

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

@@ -115,10 +115,7 @@ export class VariantAnalysisView
 
         break;
       case "cancelVariantAnalysis":
-        void commands.executeCommand(
+        await this.manager.cancelVariantAnalysis(this.variantAnalysisId);
-          "codeQL.cancelVariantAnalysis",
-          this.variantAnalysisId,
-        );
         break;
       case "requestRepositoryResults":
         void commands.executeCommand(
@@ -131,10 +128,7 @@ export class VariantAnalysisView
         await this.manager.openQueryFile(this.variantAnalysisId);
         break;
       case "openQueryText":
-        void commands.executeCommand(
+        await this.manager.openQueryText(this.variantAnalysisId);
-          "codeQL.openVariantAnalysisQueryText",
-          this.variantAnalysisId,
-        );
         break;
       case "copyRepositoryList":
         void commands.executeCommand(

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx

@@ -32,12 +32,14 @@ const openQueryText = () => {
   vscode.postMessage({
     t: "openQueryText",
   });
+  sendTelemetry("variant-analysis-open-query-text");
 };
 
 const stopQuery = () => {
   vscode.postMessage({
     t: "cancelVariantAnalysis",
   });
+  sendTelemetry("variant-analysis-cancel");
 };
 
 const openLogs = () => {

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts

@@ -41,6 +41,9 @@ describe("QueryHistoryManager", () => {
   let executeCommandSpy: jest.SpiedFunction<
     typeof vscode.commands.executeCommand
   >;
+  let cancelVariantAnalysisSpy: jest.SpiedFunction<
+    typeof variantAnalysisManagerStub.cancelVariantAnalysis
+  >;
   const doCompareCallback = jest.fn();
 
   let queryHistoryManager: QueryHistoryManager;
@@ -82,9 +85,14 @@ describe("QueryHistoryManager", () => {
       onVariantAnalysisStatusUpdated: jest.fn(),
       onVariantAnalysisRemoved: jest.fn(),
       removeVariantAnalysis: jest.fn(),
+      cancelVariantAnalysis: jest.fn(),
       showView: jest.fn(),
     } as any as VariantAnalysisManager;
 
+    cancelVariantAnalysisSpy = jest
+      .spyOn(variantAnalysisManagerStub, "cancelVariantAnalysis")
+      .mockResolvedValue(undefined);
+
     localQueryHistory = [
       // completed
       createMockLocalQueryInfo({
@@ -729,8 +737,7 @@ describe("QueryHistoryManager", () => {
         const inProgress1 = variantAnalysisHistory[1];
 
         await queryHistoryManager.handleCancel(inProgress1, [inProgress1]);
-        expect(executeCommandSpy).toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           inProgress1.variantAnalysis.id,
         );
       });
@@ -746,12 +753,10 @@ describe("QueryHistoryManager", () => {
           inProgress1,
           inProgress2,
         ]);
-        expect(executeCommandSpy).toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           inProgress1.variantAnalysis.id,
         );
-        expect(executeCommandSpy).toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           inProgress2.variantAnalysis.id,
         );
       });
@@ -793,8 +798,7 @@ describe("QueryHistoryManager", () => {
         await queryHistoryManager.handleCancel(completedVariantAnalysis, [
           completedVariantAnalysis,
         ]);
-        expect(executeCommandSpy).not.toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).not.toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           completedVariantAnalysis.variantAnalysis,
         );
       });
@@ -810,12 +814,10 @@ describe("QueryHistoryManager", () => {
           completedVariantAnalysis,
           failedVariantAnalysis,
         ]);
-        expect(executeCommandSpy).not.toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).not.toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           completedVariantAnalysis.variantAnalysis.id,
         );
-        expect(executeCommandSpy).not.toBeCalledWith(
+        expect(cancelVariantAnalysisSpy).not.toBeCalledWith(
-          "codeQL.cancelVariantAnalysis",
           failedVariantAnalysis.variantAnalysis.id,
         );
       });

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/variant-analysis-history.test.ts

@@ -8,7 +8,7 @@ import {
 } from "fs-extra";
 import { join } from "path";
 
-import { commands, ExtensionContext, Uri } from "vscode";
+import { ExtensionContext, Uri } from "vscode";
 import { DatabaseManager } from "../../../../src/local-databases";
 import { tmpDir, walkDirectory } from "../../../../src/helpers";
 import { DisposableBucket } from "../../disposable-bucket";
@@ -54,9 +54,12 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     rehydrateVariantAnalysis: rehydrateVariantAnalysisStub,
     onVariantAnalysisStatusUpdated: jest.fn(),
     showView: showViewStub,
+    openQueryText: jest.fn(),
   } as any as VariantAnalysisManager;
 
-  let executeCommandSpy: jest.SpiedFunction<typeof commands.executeCommand>;
+  let openQueryTextSpy: jest.SpiedFunction<
+    typeof variantAnalysisManagerStub.openQueryText
+  >;
 
   beforeEach(async () => {
     // Since these tests change the state of the query history manager, we need to copy the original
@@ -95,8 +98,8 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     );
     disposables.push(qhm);
 
-    executeCommandSpy = jest
+    openQueryTextSpy = jest
-      .spyOn(commands, "executeCommand")
+      .spyOn(variantAnalysisManagerStub, "openQueryText")
       .mockResolvedValue(undefined);
   });
 
@@ -180,8 +183,7 @@ describe("Variant Analyses and QueryHistoryManager", () => {
     await qhm.readQueryHistory();
     await qhm.handleShowQueryText(qhm.treeDataProvider.allHistory[0], []);
 
-    expect(executeCommandSpy).toHaveBeenCalledWith(
+    expect(openQueryTextSpy).toHaveBeenCalledWith(
-      "codeQL.openVariantAnalysisQueryText",
       rawQueryHistory[0].variantAnalysis.id,
     );
   });