Merge pull request #3040 from github/koesie10/duplicate-query-packs

Prevent duplicate query packs when creating a query

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

@@ -1,12 +1,14 @@
 import { mkdir, writeFile } from "fs-extra";
 import { dump } from "js-yaml";
-import { join } from "path";
+import { dirname, join } from "path";
 import { Uri } from "vscode";
 import { CodeQLCliServer } from "../codeql-cli/cli";
 import { QueryLanguage } from "../common/query-language";
+import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders";
+import { basename } from "../common/path";
 
 export class QlPackGenerator {
-  private readonly qlpackName: string;
+  private qlpackName: string | undefined;
   private readonly qlpackVersion: string;
   private readonly header: string;
   private readonly qlpackFileName: string;
@@ -16,8 +18,8 @@ export class QlPackGenerator {
     private readonly queryLanguage: QueryLanguage,
     private readonly cliServer: CodeQLCliServer,
     private readonly storagePath: string,
+    private readonly includeFolderNameInQlpackName: boolean = false,
   ) {
-    this.qlpackName = `getting-started/codeql-extra-queries-${this.queryLanguage}`;
     this.qlpackVersion = "1.0.0";
     this.header = "# This is an automatically generated file.\n\n";
 
@@ -26,6 +28,8 @@ export class QlPackGenerator {
   }
 
   public async generate() {
+    this.qlpackName = await this.determineQlpackName();
+
     // create QL pack folder and add to workspace
     await this.createWorkspaceFolder();
 
@@ -39,6 +43,37 @@ export class QlPackGenerator {
     await this.createCodeqlPackLockYaml();
   }
 
+  private async determineQlpackName(): Promise<string> {
+    let qlpackBaseName = `getting-started/codeql-extra-queries-${this.queryLanguage}`;
+    if (this.includeFolderNameInQlpackName) {
+      const folderBasename = basename(dirname(this.folderUri.fsPath));
+      if (
+        folderBasename.includes("codeql") ||
+        folderBasename.includes("queries")
+      ) {
+        // If the user has already included "codeql" or "queries" in the folder name, don't include it twice
+        qlpackBaseName = `getting-started/${folderBasename}-${this.queryLanguage}`;
+      } else {
+        qlpackBaseName = `getting-started/codeql-extra-queries-${folderBasename}-${this.queryLanguage}`;
+      }
+    }
+
+    const existingQlPacks = await this.cliServer.resolveQlpacks(
+      getOnDiskWorkspaceFolders(),
+    );
+    const existingQlPackNames = Object.keys(existingQlPacks);
+
+    let qlpackName = qlpackBaseName;
+    let i = 0;
+    while (existingQlPackNames.includes(qlpackName)) {
+      i++;
+
+      qlpackName = `${qlpackBaseName}-${i}`;
+    }
+
+    return qlpackName;
+  }
+
   private async createWorkspaceFolder() {
     await mkdir(this.folderUri.fsPath);
   }

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

@@ -34,7 +34,7 @@ import { showInformationMessageWithAction } from "../common/vscode/dialog";
 import { redactableError } from "../common/errors";
 import { App } from "../common/app";
 import { QueryTreeViewItem } from "../queries-panel/query-tree-view-item";
-import { containsPath } from "../common/files";
+import { containsPath, pathsEqual } from "../common/files";
 import { getQlPackPath } from "../common/ql";
 import { load } from "js-yaml";
 import { QlPackFile } from "../packaging/qlpack-file";
@@ -284,13 +284,6 @@ export class SkeletonQueryWizard {
   }
 
   private async createQlPack() {
-    if (this.qlPackStoragePath === undefined) {
-      throw new Error("Query pack storage path is undefined");
-    }
-    if (this.language === undefined) {
-      throw new Error("Language is undefined");
-    }
-
     this.progress({
       message: "Creating skeleton QL pack around query",
       step: 2,
@@ -298,11 +291,7 @@ export class SkeletonQueryWizard {
     });
 
     try {
-      const qlPackGenerator = new QlPackGenerator(
-        this.language,
-        this.cliServer,
-        this.qlPackStoragePath,
-      );
+      const qlPackGenerator = this.createQlPackGenerator();
 
       await qlPackGenerator.generate();
     } catch (e: unknown) {
@@ -313,13 +302,6 @@ export class SkeletonQueryWizard {
   }
 
   private async createExampleFile() {
-    if (this.qlPackStoragePath === undefined) {
-      throw new Error("Folder name is undefined");
-    }
-    if (this.language === undefined) {
-      throw new Error("Language is undefined");
-    }
-
     this.progress({
       message:
         "Skeleton query pack already exists. Creating additional query example file.",
@@ -328,11 +310,7 @@ export class SkeletonQueryWizard {
     });
 
     try {
-      const qlPackGenerator = new QlPackGenerator(
-        this.language,
-        this.cliServer,
-        this.qlPackStoragePath,
-      );
+      const qlPackGenerator = this.createQlPackGenerator();
 
       this.fileName = await this.determineNextFileName();
       await qlPackGenerator.createExampleQlFile(this.fileName);
@@ -475,6 +453,29 @@ export class SkeletonQueryWizard {
     return `[${this.fileName}](command:vscode.open?${queryString})`;
   }
 
+  private createQlPackGenerator() {
+    if (this.qlPackStoragePath === undefined) {
+      throw new Error("Query pack storage path is undefined");
+    }
+    if (this.language === undefined) {
+      throw new Error("Language is undefined");
+    }
+
+    const parentFolder = dirname(this.qlPackStoragePath);
+
+    // Only include the folder name in the qlpack name if the qlpack is not in the root of the workspace.
+    const includeFolderNameInQlpackName = !getOnDiskWorkspaceFolders().some(
+      (workspaceFolder) => pathsEqual(workspaceFolder, parentFolder),
+    );
+
+    return new QlPackGenerator(
+      this.language,
+      this.cliServer,
+      this.qlPackStoragePath,
+      includeFolderNameInQlpackName,
+    );
+  }
+
   public static async findDatabaseItemByNwo(
     language: string,
     databaseNwo: string,

extensions/ql-vscode/test/vscode-tests/minimal-workspace/qlpack-generator.test.ts

@@ -7,6 +7,9 @@ import { Uri, workspace } from "vscode";
 import { getErrorMessage } from "../../../src/common/helpers-pure";
 import * as tmp from "tmp";
 import { mockedObject } from "../utils/mocking.helpers";
+import { ensureDir, readFile } from "fs-extra";
+import { load } from "js-yaml";
+import { QlPackFile } from "../../../src/packaging/qlpack-file";
 
 describe("QlPackGenerator", () => {
   let packFolderPath: string;
@@ -14,7 +17,11 @@ describe("QlPackGenerator", () => {
   let exampleQlFilePath: string;
   let language: string;
   let generator: QlPackGenerator;
-  let packAddSpy: jest.Mock<any, []>;
+  let packAddSpy: jest.MockedFunction<typeof CodeQLCliServer.prototype.packAdd>;
+  let resolveQlpacksSpy: jest.MockedFunction<
+    typeof CodeQLCliServer.prototype.resolveQlpacks
+  >;
+  let mockCli: CodeQLCliServer;
   let dir: tmp.DirResult;
 
   beforeEach(async () => {
@@ -29,8 +36,10 @@ describe("QlPackGenerator", () => {
     exampleQlFilePath = join(packFolderPath, "example.ql");
 
     packAddSpy = jest.fn();
-    const mockCli = mockedObject<CodeQLCliServer>({
+    resolveQlpacksSpy = jest.fn().mockResolvedValue({});
+    mockCli = mockedObject<CodeQLCliServer>({
       packAdd: packAddSpy,
+      resolveQlpacks: resolveQlpacksSpy,
     });
 
     generator = new QlPackGenerator(
@@ -71,5 +80,137 @@ describe("QlPackGenerator", () => {
     expect(existsSync(exampleQlFilePath)).toBe(true);
 
     expect(packAddSpy).toHaveBeenCalledWith(packFolderPath, language);
+
+    const qlpack = load(
+      await readFile(qlPackYamlFilePath, "utf8"),
+    ) as QlPackFile;
+    expect(qlpack).toEqual(
+      expect.objectContaining({
+        name: "getting-started/codeql-extra-queries-ruby",
+      }),
+    );
+  });
+
+  describe("when a pack with the same name already exists", () => {
+    beforeEach(() => {
+      resolveQlpacksSpy.mockResolvedValue({
+        "getting-started/codeql-extra-queries-ruby": ["/path/to/pack"],
+      });
+    });
+
+    it("should change the name of the pack", async () => {
+      await generator.generate();
+
+      const qlpack = load(
+        await readFile(qlPackYamlFilePath, "utf8"),
+      ) as QlPackFile;
+      expect(qlpack).toEqual(
+        expect.objectContaining({
+          name: "getting-started/codeql-extra-queries-ruby-1",
+        }),
+      );
+    });
+  });
+
+  describe("when the folder name is included in the pack name", () => {
+    beforeEach(async () => {
+      const parentFolderPath = join(dir.name, "my-folder");
+
+      packFolderPath = Uri.file(
+        join(parentFolderPath, `test-ql-pack-${language}`),
+      ).fsPath;
+      await ensureDir(parentFolderPath);
+
+      qlPackYamlFilePath = join(packFolderPath, "codeql-pack.yml");
+      exampleQlFilePath = join(packFolderPath, "example.ql");
+
+      generator = new QlPackGenerator(
+        language as QueryLanguage,
+        mockCli,
+        packFolderPath,
+        true,
+      );
+    });
+
+    it("should set the name of the pack", async () => {
+      await generator.generate();
+
+      const qlpack = load(
+        await readFile(qlPackYamlFilePath, "utf8"),
+      ) as QlPackFile;
+      expect(qlpack).toEqual(
+        expect.objectContaining({
+          name: "getting-started/codeql-extra-queries-my-folder-ruby",
+        }),
+      );
+    });
+
+    describe("when the folder name includes codeql", () => {
+      beforeEach(async () => {
+        const parentFolderPath = join(dir.name, "my-codeql");
+
+        packFolderPath = Uri.file(
+          join(parentFolderPath, `test-ql-pack-${language}`),
+        ).fsPath;
+        await ensureDir(parentFolderPath);
+
+        qlPackYamlFilePath = join(packFolderPath, "codeql-pack.yml");
+        exampleQlFilePath = join(packFolderPath, "example.ql");
+
+        generator = new QlPackGenerator(
+          language as QueryLanguage,
+          mockCli,
+          packFolderPath,
+          true,
+        );
+      });
+
+      it("should set the name of the pack", async () => {
+        await generator.generate();
+
+        const qlpack = load(
+          await readFile(qlPackYamlFilePath, "utf8"),
+        ) as QlPackFile;
+        expect(qlpack).toEqual(
+          expect.objectContaining({
+            name: "getting-started/my-codeql-ruby",
+          }),
+        );
+      });
+    });
+
+    describe("when the folder name includes queries", () => {
+      beforeEach(async () => {
+        const parentFolderPath = join(dir.name, "my-queries");
+
+        packFolderPath = Uri.file(
+          join(parentFolderPath, `test-ql-pack-${language}`),
+        ).fsPath;
+        await ensureDir(parentFolderPath);
+
+        qlPackYamlFilePath = join(packFolderPath, "codeql-pack.yml");
+        exampleQlFilePath = join(packFolderPath, "example.ql");
+
+        generator = new QlPackGenerator(
+          language as QueryLanguage,
+          mockCli,
+          packFolderPath,
+          true,
+        );
+      });
+
+      it("should set the name of the pack", async () => {
+        await generator.generate();
+
+        const qlpack = load(
+          await readFile(qlPackYamlFilePath, "utf8"),
+        ) as QlPackFile;
+        expect(qlpack).toEqual(
+          expect.objectContaining({
+            name: "getting-started/my-queries-ruby",
+          }),
+        );
+      });
+    });
   });
 });