Fix PR feedback

Author: Dave Bartolomeo

extensions/ql-vscode/src/codeql-cli/cli.ts

@@ -1420,16 +1420,28 @@ export class CodeQLCliServer implements Disposable {
     );
   }
 
+  /**
+   * Compile a CodeQL pack and bundle it into a single file.
+   *
+   * @param sourcePackDir The directory of the input CodeQL pack.
+   * @param workspaceFolders The workspace folders to search for additional packs.
+   * @param outputBundleFile The path to the output bundle file.
+   * @param outputPackDir The directory to contain the unbundled output pack.
+   * @param moreOptions Additional options to be passed to `codeql pack bundle`.
+   */
   async packBundle(
-    dir: string,
+    sourcePackDir: string,
     workspaceFolders: string[],
-    outputPath: string,
+    outputBundleFile: string,
+    outputPackDir: string,
     moreOptions: string[],
   ): Promise<void> {
     const args = [
       "-o",
-      outputPath,
-      dir,
+      outputBundleFile,
+      sourcePackDir,
+      "--pack-path",
+      outputPackDir,
       ...moreOptions,
       ...this.getAdditionalPacksArg(workspaceFolders),
     ];
@@ -1492,7 +1504,7 @@ export class CodeQLCliServer implements Disposable {
     return (await this.getVersionAndFeatures()).features;
   }
 
-  public async getVersionAndFeatures(): Promise<VersionAndFeatures> {
+  private async getVersionAndFeatures(): Promise<VersionAndFeatures> {
     if (!this._versionAndFeatures) {
       try {
         const newVersionAndFeatures = await this.refreshVersion();

extensions/ql-vscode/src/variant-analysis/run-remote-query.ts

@@ -79,11 +79,10 @@ async function generateQueryPack(
     ? await askForLanguage(cliServer) // open popup to ask for language if not already hardcoded
     : await findLanguage(cliServer, Uri.file(queryFile));
   if (!language) {
-    throw new UserCancellationException("Could not determine language.");
+    throw new UserCancellationException("Could not determine language");
   }
 
   let queryPackDir: string;
-  let precompilationOpts: string[];
   let needsInstall: boolean;
   if (mustSynthesizePack) {
     // This section applies whether or not the CLI supports MRVA pack creation directly.
@@ -136,8 +135,7 @@ async function generateQueryPack(
     await cliServer.clearCache();
   }
 
-  // Clear the CLI cache so that the most recent qlpack lock file is used.
-  await cliServer.clearCache();
+  let precompilationOpts: string[];
   if (cliSupportsMrvaPackCreate) {
     precompilationOpts = [
       "--mrva",
@@ -151,11 +149,11 @@ async function generateQueryPack(
     if (await cliServer.cliConstraints.usesGlobalCompilationCache()) {
       precompilationOpts = ["--qlx"];
     } else {
-      const ccache = join(originalPackRoot, ".cache");
+      const cache = join(originalPackRoot, ".cache");
       precompilationOpts = [
         "--qlx",
         "--no-default-compilation-cache",
-        `--compilation-cache=${ccache}`,
+        `--compilation-cache=${cache}`,
       ];
     }
 
@@ -168,11 +166,13 @@ async function generateQueryPack(
   void extLogger.log(
     `Compiling and bundling query pack from ${queryPackDir} to ${bundlePath}. (This may take a while.)`,
   );
-  await cliServer.packBundle(queryPackDir, workspaceFolders, bundlePath, [
-    "--pack-path",
+  await cliServer.packBundle(
+    queryPackDir,
+    workspaceFolders,
+    bundlePath,
     tmpDir.compiledPackDir,
-    ...precompilationOpts,
-  ]);
+    precompilationOpts,
+  );
   const base64Pack = (await readFile(bundlePath)).toString("base64");
   return {
     base64Pack,

extensions/ql-vscode/test/matchers/toExistInCodeQLPack.ts

@@ -0,0 +1,43 @@
+import { expect } from "@jest/globals";
+import type { ExpectationResult } from "expect";
+import type { QueryPackFS } from "../vscode-tests/utils/bundled-pack-helpers";
+import { EOL } from "os";
+
+/**
+ * Custom Jest matcher to check if a file exists in a query pack.
+ */
+function toExistInPack(
+  this: jest.MatcherContext,
+  actual: unknown,
+  packFS: QueryPackFS,
+): ExpectationResult {
+  if (typeof actual !== "string") {
+    throw new TypeError(
+      `Expected actual value to be a string. Found ${typeof actual}`,
+    );
+  }
+
+  const pass = packFS.fileExists(actual);
+  if (pass) {
+    return {
+      pass: true,
+      message: () => `expected ${actual} not to exist in pack`,
+    };
+  } else {
+    const files = packFS.allFiles();
+    const filesString = files.length > 0 ? files.join(EOL) : "<none>";
+    return {
+      pass: false,
+      message: () =>
+        `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`,
+    };
+  }
+}
+
+expect.extend({ toExistInPack });
+
+declare module "expect" {
+  interface Matchers<R> {
+    toExistInCodeQLPack(packFS: QueryPackFS): R;
+  }
+}

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

@@ -20,50 +20,11 @@ import { ExtensionApp } from "../../../../src/common/vscode/vscode-app";
 import { DbConfigStore } from "../../../../src/databases/config/db-config-store";
 import { mockedQuickPickItem } from "../../utils/mocking.helpers";
 import { QueryLanguage } from "../../../../src/common/query-language";
-import type { QueryPackFS } from "../../utils/bundled-pack-helpers";
 import { readBundledPack } from "../../utils/bundled-pack-helpers";
 import { load } from "js-yaml";
 import type { ExtensionPackMetadata } from "../../../../src/model-editor/extension-pack-metadata";
 import type { QlPackLockFile } from "../../../../src/packaging/qlpack-lock-file";
 import { expect } from "@jest/globals";
-import type { ExpectationResult } from "expect";
-
-/**
- * Custom Jest matcher to check if a file exists in a query pack.
- */
-function toExistInPack(
-  this: jest.MatcherContext,
-  actual: unknown,
-  packFS: QueryPackFS,
-): ExpectationResult {
-  if (typeof actual !== "string") {
-    throw new TypeError("Expected actual value to be a string.");
-  }
-
-  const pass = packFS.fileExists(actual);
-  const files = packFS.allFiles();
-  const filesString = files.length > 0 ? files.join("\n") : "<none>";
-  if (pass) {
-    return {
-      pass: true,
-      message: () => `expected ${actual} not to exist in pack`,
-    };
-  } else {
-    return {
-      pass: false,
-      message: () =>
-        `expected ${actual} to exist in pack.\nThe following files were found in the pack:\n${filesString}`,
-    };
-  }
-}
-
-expect.extend({ toExistInPack });
-
-declare module "expect" {
-  interface Matchers<R> {
-    toExistInPack(packFS: QueryPackFS): R;
-  }
-}
 
 describe("Variant Analysis Manager", () => {
   let cli: CodeQLCliServer;
@@ -371,14 +332,14 @@ describe("Variant Analysis Manager", () => {
 
       const packFS = await readBundledPack(request.query.pack);
       filesThatExist.forEach((file) => {
-        expect(file).toExistInPack(packFS);
+        expect(file).toExistInCodeQLPack(packFS);
       });
 
       qlxFilesThatExist.forEach((file) => {
-        expect(file).toExistInPack(packFS);
+        expect(file).toExistInCodeQLPack(packFS);
       });
       filesThatDoNotExist.forEach((file) => {
-        expect(file).not.toExistInPack(packFS);
+        expect(file).not.toExistInCodeQLPack(packFS);
       });
 
       expect(

extensions/ql-vscode/test/vscode-tests/cli.ts

@@ -1,3 +1,4 @@
+import { dirname } from "path";
 import { workspace } from "vscode";
 
 /**
@@ -6,10 +7,10 @@ import { workspace } from "vscode";
  */
 function hasCodeQL() {
   const folders = workspace.workspaceFolders;
-  return !!folders?.some(
-    (folder) =>
-      folder.uri.path.endsWith("/codeql") || folder.uri.path.endsWith("/ql"),
-  );
+  return !!folders?.some((folder) => {
+    const name = dirname(folder.uri.fsPath);
+    return name === "codeql" || name === "ql";
+  });
 }
 
 // describeWithCodeQL will be equal to describe if the CodeQL libraries are