Merge branch 'main' into robertbrignull/move_language_pack

Author: robertbrignull

extensions/ql-vscode/CHANGELOG.md

@@ -5,6 +5,7 @@
 - Enable collection of telemetry for the `codeQL.addingDatabases.addDatabaseSourceToWorkspace` setting. [#3238](https://github.com/github/vscode-codeql/pull/3238)
 - In the CodeQL model editor, you can now select individual method rows and save changes to only the selected rows, instead of having to save the entire library model. [#3156](https://github.com/github/vscode-codeql/pull/3156)
 - If you run a query without having selected a database, we show a more intuitive prompt to help you select a database. [#3214](https://github.com/github/vscode-codeql/pull/3214)
+- Error messages returned from the CodeQL CLI are now less verbose and more user-friendly. [#3259](https://github.com/github/vscode-codeql/pull/3259)
 - The UI for browsing and running CodeQL tests has moved to use VS Code's built-in test UI. This makes the CodeQL test UI more consistent with the test UIs for other languages.
   This change means that this extension no longer depends on the "Test Explorer UI" and "Test Adapter Converter" extensions. You can uninstall those two extensions if they are
   not being used by any other extensions you may have installed.

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

@@ -0,0 +1,102 @@
+import { asError, getErrorMessage } from "../common/helpers-pure";
+
+// https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/exit-codes
+const EXIT_CODE_USER_ERROR = 2;
+const EXIT_CODE_CANCELLED = 98;
+
+export class ExitCodeError extends Error {
+  constructor(public readonly exitCode: number | null) {
+    super(`Process exited with code ${exitCode}`);
+  }
+}
+
+export class CliError extends Error {
+  constructor(
+    message: string,
+    public readonly stderr: string | undefined,
+    public readonly cause: Error,
+    public readonly commandDescription: string,
+    public readonly commandArgs: string[],
+  ) {
+    super(message);
+  }
+}
+
+export function getCliError(
+  e: unknown,
+  stderr: string | undefined,
+  commandDescription: string,
+  commandArgs: string[],
+): CliError {
+  const error = asError(e);
+
+  if (!(error instanceof ExitCodeError) || !stderr) {
+    return formatCliErrorFallback(
+      error,
+      stderr,
+      commandDescription,
+      commandArgs,
+    );
+  }
+
+  switch (error.exitCode) {
+    case EXIT_CODE_USER_ERROR: {
+      // This is an error that we should try to format nicely
+      const fatalErrorIndex = stderr.lastIndexOf("A fatal error occurred: ");
+      if (fatalErrorIndex !== -1) {
+        return new CliError(
+          stderr.slice(fatalErrorIndex),
+          stderr,
+          error,
+          commandDescription,
+          commandArgs,
+        );
+      }
+
+      break;
+    }
+    case EXIT_CODE_CANCELLED: {
+      const cancellationIndex = stderr.lastIndexOf(
+        "Computation was cancelled: ",
+      );
+      if (cancellationIndex !== -1) {
+        return new CliError(
+          stderr.slice(cancellationIndex),
+          stderr,
+          error,
+          commandDescription,
+          commandArgs,
+        );
+      }
+
+      break;
+    }
+  }
+
+  return formatCliErrorFallback(error, stderr, commandDescription, commandArgs);
+}
+
+function formatCliErrorFallback(
+  error: Error,
+  stderr: string | undefined,
+  commandDescription: string,
+  commandArgs: string[],
+): CliError {
+  if (stderr) {
+    return new CliError(
+      stderr,
+      undefined,
+      error,
+      commandDescription,
+      commandArgs,
+    );
+  }
+
+  return new CliError(
+    getErrorMessage(error),
+    undefined,
+    error,
+    commandDescription,
+    commandArgs,
+  );
+}

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

@@ -35,6 +35,7 @@ import { LINE_ENDINGS, splitStreamAtSeparators } from "../common/split-stream";
 import type { Position } from "../query-server/messages";
 import { LOGGING_FLAGS } from "./cli-command";
 import type { CliFeatures, VersionAndFeatures } from "./cli-version";
+import { ExitCodeError, getCliError } from "./cli-errors";
 
 /**
  * The version of the SARIF format that we are using.
@@ -420,7 +421,9 @@ export class CodeQLCliServer implements Disposable {
             stderrBuffers.push(newData);
           });
           // Listen for process exit.
-          process.addListener("close", (code) => reject(code));
+          process.addListener("close", (code) =>
+            reject(new ExitCodeError(code)),
+          );
           // Write the command followed by a null terminator.
           process.stdin.write(JSON.stringify(args), "utf8");
           process.stdin.write(this.nullBuffer);
@@ -436,19 +439,18 @@ export class CodeQLCliServer implements Disposable {
       } catch (err) {
         // Kill the process if it isn't already dead.
         this.killProcessIfRunning();
+
         // Report the error (if there is a stderr then use that otherwise just report the error code or nodejs error)
-        const newError =
-          stderrBuffers.length === 0
-            ? new Error(
-                `${description} failed with args:${EOL}    ${argsString}${EOL}${err}`,
-              )
-            : new Error(
-                `${description} failed with args:${EOL}    ${argsString}${EOL}${Buffer.concat(
-                  stderrBuffers,
-                ).toString("utf8")}`,
-              );
-        newError.stack += getErrorStack(err);
-        throw newError;
+        const cliError = getCliError(
+          err,
+          stderrBuffers.length > 0
+            ? Buffer.concat(stderrBuffers).toString("utf8")
+            : undefined,
+          description,
+          args,
+        );
+        cliError.stack += getErrorStack(err);
+        throw cliError;
       } finally {
         if (!silent) {
           void this.logger.log(Buffer.concat(stderrBuffers).toString("utf8"));

extensions/ql-vscode/src/common/vscode/commands.ts

@@ -13,6 +13,8 @@ import { redactableError } from "../../common/errors";
 import { UserCancellationException } from "./progress";
 import { telemetryListener } from "./telemetry";
 import type { AppTelemetry } from "../telemetry";
+import { CliError } from "../../codeql-cli/cli-errors";
+import { EOL } from "os";
 
 /**
  * Create a command manager for VSCode, wrapping registerCommandWithErrorHandling
@@ -62,6 +64,16 @@ export function registerCommandWithErrorHandling(
         } else {
           void showAndLogWarningMessage(logger, errorMessage.fullMessage);
         }
+      } else if (e instanceof CliError) {
+        const fullMessage = `${e.commandDescription} failed with args:${EOL}    ${e.commandArgs.join(" ")}${EOL}${
+          e.stderr ?? e.cause
+        }`;
+        void showAndLogExceptionWithTelemetry(logger, telemetry, errorMessage, {
+          fullMessage,
+          extraTelemetryProperties: {
+            command: commandId,
+          },
+        });
       } else {
         // Include the full stack in the error log only.
         const fullMessage = errorMessage.fullMessageWithStack;

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

@@ -158,6 +158,7 @@ export interface VariantAnalysisSubmission {
 // unclear what it will look like in the future.
 export interface VariantAnalysisQueries {
   language: QueryLanguage;
+  count: number;
 }
 
 export async function isVariantAnalysisComplete(

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

@@ -405,6 +405,7 @@ export class VariantAnalysisManager
         ? undefined
         : {
             language: qlPackDetails.language,
+            count: qlPackDetails.queryFiles.length,
           };
 
     const variantAnalysisSubmission: VariantAnalysisSubmission = {

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

@@ -218,9 +218,15 @@ export class VariantAnalysisView
   }
 
   private getTitle(variantAnalysis: VariantAnalysis | undefined): string {
-    return variantAnalysis
-      ? `${variantAnalysis.query.name} - Variant Analysis Results`
-      : `Variant Analysis ${this.variantAnalysisId} - Results`;
+    if (!variantAnalysis) {
+      return `Variant Analysis ${this.variantAnalysisId} - Results`;
+    }
+
+    if (variantAnalysis.queries) {
+      return `Variant Analysis using multiple queries - Results`;
+    } else {
+      return `${variantAnalysis.query.name} - Variant Analysis Results`;
+    }
   }
 
   private async showDataFlows(dataFlows: DataFlowPaths): Promise<void> {

extensions/ql-vscode/src/view/common/SuggestBox/SuggestBox.tsx

@@ -20,6 +20,7 @@ import { findMatchingOptions } from "./options";
 import { SuggestBoxItem } from "./SuggestBoxItem";
 import { LabelText } from "./LabelText";
 import type { Diagnostic } from "./diagnostics";
+import { useOpenKey } from "./useOpenKey";
 
 const Input = styled(VSCodeTextField)<{ $error: boolean }>`
   width: 430px;
@@ -152,6 +153,7 @@ export const SuggestBox = <
   const focus = useFocus(context);
   const role = useRole(context, { role: "listbox" });
   const dismiss = useDismiss(context);
+  const openKey = useOpenKey(context);
   const listNav = useListNavigation(context, {
     listRef,
     activeIndex,
@@ -161,7 +163,7 @@ export const SuggestBox = <
   });
 
   const { getReferenceProps, getFloatingProps, getItemProps } = useInteractions(
-    [focus, role, dismiss, listNav],
+    [focus, role, dismiss, openKey, listNav],
   );
 
   const handleInput = useCallback(

extensions/ql-vscode/src/view/common/SuggestBox/__tests__/useEffectEvent.test.ts

@@ -0,0 +1,26 @@
+import { renderHook } from "@testing-library/react";
+import { useEffectEvent } from "../useEffectEvent";
+
+describe("useEffectEvent", () => {
+  it("does not change reference when changing the callback function", () => {
+    const callback1 = jest.fn();
+    const callback2 = jest.fn();
+
+    const { result, rerender } = renderHook(
+      (callback) => useEffectEvent(callback),
+      {
+        initialProps: callback1,
+      },
+    );
+
+    const callbackResult = result.current;
+
+    rerender();
+
+    expect(result.current).toBe(callbackResult);
+
+    rerender(callback2);
+
+    expect(result.current).toBe(callbackResult);
+  });
+});