Merge pull request #2310 from github/yer-a-workspace-query

Stop pushing QL pack as top level folder to avoid confusing the user

extensions/ql-vscode/src/helpers.ts

@@ -8,7 +8,7 @@ import {
 } from "fs-extra";
 import { glob } from "glob";
 import { load } from "js-yaml";
-import { join, basename } from "path";
+import { join, basename, dirname } from "path";
 import { dirSync } from "tmp-promise";
 import {
   ExtensionContext,
@@ -791,3 +791,39 @@ export async function* walkDirectory(
     }
   }
 }
+
+/**
+ * Returns the path of the first folder in the workspace.
+ * This is used to decide where to create skeleton QL packs.
+ *
+ * If the first folder is a QL pack, then the parent folder is returned.
+ * This is because the vscode-codeql-starter repo contains a ql pack in
+ * the first folder.
+ *
+ * This is a temporary workaround until we can retire the
+ * vscode-codeql-starter repo.
+ */
+
+export function getFirstWorkspaceFolder() {
+  const workspaceFolders = getOnDiskWorkspaceFolders();
+
+  if (!workspaceFolders || workspaceFolders.length === 0) {
+    throw new Error("No workspace folders found");
+  }
+
+  const firstFolderFsPath = workspaceFolders[0];
+
+  // For the vscode-codeql-starter repo, the first folder will be a ql pack
+  // so we need to get the parent folder
+  if (
+    firstFolderFsPath.includes(
+      join("vscode-codeql-starter", "codeql-custom-queries"),
+    )
+  ) {
+    // return the parent folder
+    return dirname(firstFolderFsPath);
+  } else {
+    // if the first folder is not a ql pack, then we are in a normal workspace
+    return firstFolderFsPath;
+  }
+}

extensions/ql-vscode/src/local-databases.ts

@@ -11,6 +11,7 @@ import {
   showAndLogExceptionWithTelemetry,
   isFolderAlreadyInWorkspace,
   showBinaryChoiceDialog,
+  getFirstWorkspaceFolder,
 } from "./helpers";
 import { ProgressCallback, withProgress } from "./progress";
 import {
@@ -29,6 +30,7 @@ import { isCodespacesTemplate } from "./config";
 import { QlPackGenerator } from "./qlpack-generator";
 import { QueryLanguage } from "./common/query-language";
 import { App } from "./common/app";
+import { existsSync } from "fs";
 
 /**
  * databases.ts
@@ -662,8 +664,13 @@ export class DatabaseManager extends DisposableObject {
       return;
     }
 
+    const firstWorkspaceFolder = getFirstWorkspaceFolder();
     const folderName = `codeql-custom-queries-${databaseItem.language}`;
-    if (isFolderAlreadyInWorkspace(folderName)) {
+
+    if (
+      existsSync(join(firstWorkspaceFolder, folderName)) ||
+      isFolderAlreadyInWorkspace(folderName)
+    ) {
       return;
     }
 
@@ -680,7 +687,7 @@ export class DatabaseManager extends DisposableObject {
         folderName,
         databaseItem.language as QueryLanguage,
         this.cli,
-        this.ctx.storageUri?.fsPath,
+        firstWorkspaceFolder,
       );
       await qlPackGenerator.generate();
     } catch (e: unknown) {

extensions/ql-vscode/src/qlpack-generator.ts

@@ -1,7 +1,7 @@
-import { writeFile } from "fs-extra";
+import { mkdir, writeFile } from "fs-extra";
 import { dump } from "js-yaml";
 import { join } from "path";
-import { Uri, workspace } from "vscode";
+import { Uri } from "vscode";
 import { CodeQLCliServer } from "./cli";
 import { QueryLanguage } from "./common/query-language";
 
@@ -44,14 +44,7 @@ export class QlPackGenerator {
   }
 
   private async createWorkspaceFolder() {
-    await workspace.fs.createDirectory(this.folderUri);
-
-    const end = (workspace.workspaceFolders || []).length;
-
-    workspace.updateWorkspaceFolders(end, 0, {
-      name: this.folderName,
-      uri: this.folderUri,
-    });
+    await mkdir(this.folderUri.fsPath);
   }
 
   private async createQlPackYaml() {

extensions/ql-vscode/src/skeleton-query-wizard.ts

@@ -1,15 +1,20 @@
-import { join, dirname } from "path";
+import { join } from "path";
 import { CancellationToken, Uri, workspace, window as Window } from "vscode";
 import { CodeQLCliServer } from "./cli";
 import { OutputChannelLogger } from "./common";
 import { Credentials } from "./common/authentication";
 import { QueryLanguage } from "./common/query-language";
-import { askForLanguage, isFolderAlreadyInWorkspace } from "./helpers";
+import {
+  askForLanguage,
+  getFirstWorkspaceFolder,
+  isFolderAlreadyInWorkspace,
+} from "./helpers";
 import { getErrorMessage } from "./pure/helpers-pure";
 import { QlPackGenerator } from "./qlpack-generator";
 import { DatabaseItem, DatabaseManager } from "./local-databases";
 import { ProgressCallback, UserCancellationException } from "./progress";
 import { askForGitHubRepo, downloadGitHubDatabase } from "./databaseFetcher";
+import { existsSync } from "fs";
 
 type QueryLanguagesToDatabaseMap = Record<string, string>;
 
@@ -50,11 +55,11 @@ export class SkeletonQueryWizard {
       return;
     }
 
-    this.qlPackStoragePath = this.getFirstStoragePath();
+    this.qlPackStoragePath = getFirstWorkspaceFolder();
 
-    const skeletonPackAlreadyExists = isFolderAlreadyInWorkspace(
-      this.folderName,
-    );
+    const skeletonPackAlreadyExists =
+      existsSync(join(this.qlPackStoragePath, this.folderName)) ||
+      isFolderAlreadyInWorkspace(this.folderName);
 
     if (skeletonPackAlreadyExists) {
       // just create a new example query file in skeleton QL pack
@@ -93,27 +98,6 @@ export class SkeletonQueryWizard {
     });
   }
 
-  public getFirstStoragePath() {
-    const workspaceFolders = workspace.workspaceFolders;
-
-    if (!workspaceFolders || workspaceFolders.length === 0) {
-      throw new Error("No workspace folders found");
-    }
-
-    const firstFolder = workspaceFolders[0];
-    const firstFolderFsPath = firstFolder.uri.fsPath;
-
-    // For the vscode-codeql-starter repo, the first folder will be a ql pack
-    // so we need to get the parent folder
-    if (firstFolderFsPath.includes("codeql-custom-queries")) {
-      // return the parent folder
-      return dirname(firstFolderFsPath);
-    } else {
-      // if the first folder is not a ql pack, then we are in a normal workspace
-      return firstFolderFsPath;
-    }
-  }
-
   private async chooseLanguage() {
     this.progress({
       message: "Choose language",

extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts

@@ -82,11 +82,11 @@ describe("SkeletonQueryWizard", () => {
     jest.spyOn(workspace, "workspaceFolders", "get").mockReturnValue([
       {
         name: `codespaces-codeql`,
-        uri: { fsPath: storagePath },
+        uri: { fsPath: storagePath, scheme: "file" },
       },
       {
         name: "/second/folder/path",
-        uri: { fsPath: storagePath },
+        uri: { fsPath: storagePath, scheme: "file" },
       },
     ] as WorkspaceFolder[]);
 
@@ -302,66 +302,6 @@ describe("SkeletonQueryWizard", () => {
     });
   });
 
-  describe("getFirstStoragePath", () => {
-    it("should return the first workspace folder", async () => {
-      jest.spyOn(workspace, "workspaceFolders", "get").mockReturnValue([
-        {
-          name: "codespaces-codeql",
-          uri: { fsPath: "codespaces-codeql" },
-        },
-      ] as WorkspaceFolder[]);
-
-      wizard = new SkeletonQueryWizard(
-        mockCli,
-        jest.fn(),
-        credentials,
-        extLogger,
-        mockDatabaseManager,
-        token,
-        storagePath,
-      );
-
-      expect(wizard.getFirstStoragePath()).toEqual("codespaces-codeql");
-    });
-
-    describe("if user is in vscode-codeql-starter workspace", () => {
-      it("should set storage path to parent folder", async () => {
-        jest.spyOn(workspace, "workspaceFolders", "get").mockReturnValue([
-          {
-            name: "codeql-custom-queries-cpp",
-            uri: {
-              fsPath: join(
-                "vscode-codeql-starter",
-                "codeql-custom-queries-cpp",
-              ),
-            },
-          },
-          {
-            name: "codeql-custom-queries-csharp",
-            uri: {
-              fsPath: join(
-                "vscode-codeql-starter",
-                "codeql-custom-queries-csharp",
-              ),
-            },
-          },
-        ] as WorkspaceFolder[]);
-
-        wizard = new SkeletonQueryWizard(
-          mockCli,
-          jest.fn(),
-          credentials,
-          extLogger,
-          mockDatabaseManager,
-          token,
-          storagePath,
-        );
-
-        expect(wizard.getFirstStoragePath()).toEqual("vscode-codeql-starter");
-      });
-    });
-  });
-
   describe("findDatabaseItemByNwo", () => {
     describe("when the item exists", () => {
       it("should return the database item", async () => {

extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts

@@ -687,6 +687,22 @@ describe("local databases", () => {
         );
       });
     });
+
+    describe("when the QL pack already exists", () => {
+      beforeEach(() => {
+        fs.mkdirSync(join(dir.name, `codeql-custom-queries-${language}`));
+      });
+
+      it("should exit early", async () => {
+        showBinaryChoiceDialogSpy = jest
+          .spyOn(helpers, "showBinaryChoiceDialog")
+          .mockResolvedValue(false);
+
+        await (databaseManager as any).createSkeletonPacks(mockDbItem);
+
+        expect(generateSpy).not.toBeCalled();
+      });
+    });
   });
 
   describe("openDatabase", () => {

extensions/ql-vscode/test/vscode-tests/no-workspace/helpers.test.ts

@@ -26,6 +26,7 @@ import {
 import { DirResult } from "tmp";
 
 import {
+  getFirstWorkspaceFolder,
   getInitialQueryContents,
   InvocationRateLimiter,
   isFolderAlreadyInWorkspace,
@@ -678,3 +679,42 @@ describe("prepareCodeTour", () => {
     });
   });
 });
+
+describe("getFirstWorkspaceFolder", () => {
+  it("should return the first workspace folder", async () => {
+    jest.spyOn(workspace, "workspaceFolders", "get").mockReturnValue([
+      {
+        name: "codespaces-codeql",
+        uri: { fsPath: "codespaces-codeql", scheme: "file" },
+      },
+    ] as WorkspaceFolder[]);
+
+    expect(getFirstWorkspaceFolder()).toEqual("codespaces-codeql");
+  });
+
+  describe("if user is in vscode-codeql-starter workspace", () => {
+    it("should set storage path to parent folder", async () => {
+      jest.spyOn(workspace, "workspaceFolders", "get").mockReturnValue([
+        {
+          name: "codeql-custom-queries-cpp",
+          uri: {
+            fsPath: join("vscode-codeql-starter", "codeql-custom-queries-cpp"),
+            scheme: "file",
+          },
+        },
+        {
+          name: "codeql-custom-queries-csharp",
+          uri: {
+            fsPath: join(
+              "vscode-codeql-starter",
+              "codeql-custom-queries-csharp",
+            ),
+            scheme: "file",
+          },
+        },
+      ] as WorkspaceFolder[]);
+
+      expect(getFirstWorkspaceFolder()).toEqual("vscode-codeql-starter");
+    });
+  });
+});