Merge pull request #4194 from paldepind/use-bqrs-diff

Use `bqrs diff` when comparing query results

Author: paldepind

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

@@ -9,6 +9,7 @@ import { SemVer } from "semver";
 import type { Readable } from "stream";
 import tk from "tree-kill";
 import type { CancellationToken, Disposable, Uri } from "vscode";
+import { dir } from "tmp-promise";
 
 import type {
   BqrsInfo,
@@ -202,9 +203,11 @@ interface BqrsDecodeOptions {
   entities?: string[];
 }
 
-type OnLineCallback = (
-  line: string,
-) => Promise<string | undefined> | string | undefined;
+interface BqrsDiffOptions {
+  retainResultSets?: string[];
+}
+
+type OnLineCallback = (line: string) => Promise<string | undefined>;
 
 type VersionChangedListener = (
   newVersionAndFeatures: VersionAndFeatures | undefined,
@@ -368,12 +371,11 @@ export class CodeQLCliServer implements Disposable {
    */
   private async launchProcess(): Promise<ChildProcessWithoutNullStreams> {
     const codeQlPath = await this.getCodeQlPath();
-    const args = [];
-    if (shouldDebugCliServer()) {
-      args.push(
-        "-J=-agentlib:jdwp=transport=dt_socket,address=localhost:9012,server=n,suspend=y,quiet=y",
-      );
-    }
+    const args = shouldDebugCliServer()
+      ? [
+          "-J=-agentlib:jdwp=transport=dt_socket,address=localhost:9012,server=n,suspend=y,quiet=y",
+        ]
+      : [];
 
     return spawnServer(
       codeQlPath,
@@ -399,15 +401,11 @@ export class CodeQLCliServer implements Disposable {
     }
     this.commandInProcess = true;
     try {
-      //Launch the process if it doesn't exist
-      if (!this.process) {
-        this.process = await this.launchProcess();
-      }
-      // Grab the process so that typescript know that it is always defined.
-      const process = this.process;
+      // Launch the process if it doesn't exist
+      this.process ??= await this.launchProcess();
 
       // Compute the full args array
-      const args = command.concat(LOGGING_FLAGS).concat(commandArgs);
+      const args = command.concat(LOGGING_FLAGS, commandArgs);
       const argsString = args.join(" ");
       // If we are running silently, we don't want to print anything to the console.
       if (!silent) {
@@ -416,7 +414,7 @@ export class CodeQLCliServer implements Disposable {
         );
       }
       try {
-        return await this.handleProcessOutput(process, {
+        return await this.handleProcessOutput(this.process, {
           handleNullTerminator: true,
           onListenStart: (process) => {
             // Write the command followed by a null terminator.
@@ -451,7 +449,7 @@ export class CodeQLCliServer implements Disposable {
   ): Promise<string> {
     const codeqlPath = await this.getCodeQlPath();
 
-    const args = command.concat(LOGGING_FLAGS).concat(commandArgs);
+    const args = command.concat(LOGGING_FLAGS, commandArgs);
     const argsString = args.join(" ");
 
     // If we are running silently, we don't want to print anything to the console.
@@ -569,16 +567,15 @@ export class CodeQLCliServer implements Disposable {
 
           stdoutBuffers.push(newData);
 
-          if (handleNullTerminator) {
+          if (
+            handleNullTerminator &&
             // If the buffer ends in '0' then exit.
             // We don't have to check the middle as no output will be written after the null until
             // the next command starts
-            if (
-              newData.length > 0 &&
-              newData.readUInt8(newData.length - 1) === 0
-            ) {
-              resolve();
-            }
+            newData.length > 0 &&
+            newData.readUInt8(newData.length - 1) === 0
+          ) {
+            resolve();
           }
         };
         stderrListener = (newData: Buffer) => {
@@ -693,9 +690,7 @@ export class CodeQLCliServer implements Disposable {
    */
   private runNext(): void {
     const callback = this.commandQueue.shift();
-    if (callback) {
-      callback();
-    }
+    callback?.();
   }
 
   /**
@@ -813,7 +808,7 @@ export class CodeQLCliServer implements Disposable {
    *              is false or not specified, this option is ignored.
    * @returns The contents of the command's stdout, if the command succeeded.
    */
-  runCodeQlCliCommand(
+  private runCodeQlCliCommand(
     command: string[],
     commandArgs: string[],
     description: string,
@@ -825,9 +820,7 @@ export class CodeQLCliServer implements Disposable {
       token,
     }: RunOptions = {},
   ): Promise<string> {
-    if (progressReporter) {
-      progressReporter.report({ message: description });
-    }
+    progressReporter?.report({ message: description });
 
     if (runInNewProcess) {
       return this.runCodeQlCliInNewProcess(
@@ -874,18 +867,17 @@ export class CodeQLCliServer implements Disposable {
    * @param progressReporter Used to output progress messages, e.g. to the status bar.
    * @returns The contents of the command's stdout, if the command succeeded.
    */
-  async runJsonCodeQlCliCommand<OutputType>(
+  private async runJsonCodeQlCliCommand<OutputType>(
     command: string[],
     commandArgs: string[],
     description: string,
     { addFormat = true, ...runOptions }: JsonRunOptions = {},
   ): Promise<OutputType> {
-    let args: string[] = [];
-    if (addFormat) {
+    const args = [
       // Add format argument first, in case commandArgs contains positional parameters.
-      args = args.concat(["--format", "json"]);
-    }
-    args = args.concat(commandArgs);
+      ...(addFormat ? ["--format", "json"] : []),
+      ...commandArgs,
+    ];
     const result = await this.runCodeQlCliCommand(
       command,
       args,
@@ -922,7 +914,7 @@ export class CodeQLCliServer implements Disposable {
    * @param runOptions Options for running the command.
    * @returns The contents of the command's stdout, if the command succeeded.
    */
-  async runJsonCodeQlCliCommandWithAuthentication<OutputType>(
+  private async runJsonCodeQlCliCommandWithAuthentication<OutputType>(
     command: string[],
     commandArgs: string[],
     description: string,
@@ -1226,8 +1218,8 @@ export class CodeQLCliServer implements Disposable {
   }
 
   /**
-   * Gets the results from a bqrs.
-   * @param bqrsPath The path to the bqrs.
+   * Gets the results from a bqrs file.
+   * @param bqrsPath The path to the bqrs file.
    * @param resultSet The result set to get.
    * @param options Optional BqrsDecodeOptions arguments
    */
@@ -1240,30 +1232,62 @@ export class CodeQLCliServer implements Disposable {
       `--entities=${entities.join(",")}`,
       "--result-set",
       resultSet,
-    ]
-      .concat(pageSize ? ["--rows", pageSize.toString()] : [])
-      .concat(offset ? ["--start-at", offset.toString()] : [])
-      .concat([bqrsPath]);
-    return await this.runJsonCodeQlCliCommand<DecodedBqrsChunk>(
+      ...(pageSize ? ["--rows", pageSize.toString()] : []),
+      ...(offset ? ["--start-at", offset.toString()] : []),
+      bqrsPath,
+    ];
+    return this.runJsonCodeQlCliCommand<DecodedBqrsChunk>(
       ["bqrs", "decode"],
       subcommandArgs,
       "Reading bqrs data",
     );
   }
 
   /**
-   * Gets all results from a bqrs.
-   * @param bqrsPath The path to the bqrs.
+   * Gets all results from a bqrs file.
+   * @param bqrsPath The path to the bqrs file.
    */
   async bqrsDecodeAll(bqrsPath: string): Promise<DecodedBqrs> {
-    return await this.runJsonCodeQlCliCommand<DecodedBqrs>(
+    return this.runJsonCodeQlCliCommand<DecodedBqrs>(
       ["bqrs", "decode"],
       [bqrsPath],
       "Reading all bqrs data",
     );
   }
 
-  async runInterpretCommand(
+  /** Gets the difference between two bqrs files. */
+  async bqrsDiff(
+    bqrsPath1: string,
+    bqrsPath2: string,
+    options?: BqrsDiffOptions,
+  ): Promise<{
+    uniquePath1: string;
+    uniquePath2: string;
+    path: string;
+    cleanup: () => Promise<void>;
+  }> {
+    const { path, cleanup } = await dir({ unsafeCleanup: true });
+    const uniquePath1 = join(path, "left.bqrs");
+    const uniquePath2 = join(path, "right.bqrs");
+    await this.runCodeQlCliCommand(
+      ["bqrs", "diff"],
+      [
+        "--left",
+        uniquePath1,
+        "--right",
+        uniquePath2,
+        ...(options?.retainResultSets
+          ? ["--retain-result-sets", options.retainResultSets.join(",")]
+          : []),
+        bqrsPath1,
+        bqrsPath2,
+      ],
+      "Diffing bqrs files",
+    );
+    return { uniquePath1, uniquePath2, path, cleanup };
+  }
+
+  private async runInterpretCommand(
     format: string,
     additonalArgs: string[],
     metadata: QueryMetadata,
@@ -1278,21 +1302,22 @@ export class CodeQLCliServer implements Disposable {
       format,
       // Forward all of the query metadata.
       ...Object.entries(metadata).map(([key, value]) => `-t=${key}=${value}`),
-    ].concat(additonalArgs);
-    if (sourceInfo !== undefined) {
-      args.push(
-        "--source-archive",
-        sourceInfo.sourceArchive,
-        "--source-location-prefix",
-        sourceInfo.sourceLocationPrefix,
-      );
-    }
-
-    args.push("--threads", this.cliConfig.numberThreads.toString());
-
-    args.push("--max-paths", this.cliConfig.maxPaths.toString());
+      ...additonalArgs,
+      ...(sourceInfo !== undefined
+        ? [
+            "--source-archive",
+            sourceInfo.sourceArchive,
+            "--source-location-prefix",
+            sourceInfo.sourceLocationPrefix,
+          ]
+        : []),
+      "--threads",
+      this.cliConfig.numberThreads.toString(),
+      "--max-paths",
+      this.cliConfig.maxPaths.toString(),
+      resultsPath,
+    ];
 
-    args.push(resultsPath);
     await this.runCodeQlCliCommand(
       ["bqrs", "interpret"],
       args,
@@ -1797,7 +1822,7 @@ export class CodeQLCliServer implements Disposable {
  * Spawns a child server process using the CodeQL CLI
  * and attaches listeners to it.
  *
- * @param config The configuration containing the path to the CLI.
+ * @param codeqlPath The configuration containing the path to the CLI.
  * @param name Name of the server being started, to be shown in log and error messages.
  * @param command The `codeql` command to be run, provided as an array of command/subcommand names.
  * @param commandArgs The arguments to pass to the `codeql` command.
@@ -1823,9 +1848,9 @@ export function spawnServer(
   // Start the server process.
   const base = codeqlPath;
   const argsString = args.join(" ");
-  if (progressReporter !== undefined) {
-    progressReporter.report({ message: `Starting ${name}` });
-  }
+
+  progressReporter?.report({ message: `Starting ${name}` });
+
   void logger.log(`Starting ${name} using CodeQL CLI: ${base} ${argsString}`);
   const child = spawnChildProcess(base, args);
   if (!child || !child.pid) {
@@ -1859,9 +1884,8 @@ export function spawnServer(
     child.stdout.on("data", stdoutListener);
   }
 
-  if (progressReporter !== undefined) {
-    progressReporter.report({ message: `Started ${name}` });
-  }
+  progressReporter?.report({ message: `Started ${name}` });
+
   void logger.log(`${name} started on PID: ${child.pid}`);
   return child;
 }

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

@@ -19,7 +19,7 @@ export async function findLanguage(
   cliServer: CodeQLCliServer,
   queryUri: Uri | undefined,
 ): Promise<QueryLanguage | undefined> {
-  const uri = queryUri || window.activeTextEditor?.document.uri;
+  const uri = queryUri ?? window.activeTextEditor?.document.uri;
   if (uri !== undefined) {
     try {
       const queryInfo = await cliServer.resolveQueryByLanguage(

extensions/ql-vscode/src/common/bqrs-raw-results-mapper.ts

@@ -27,20 +27,13 @@ export function bqrsToResultSet(
   schema: BqrsResultSetSchema,
   chunk: DecodedBqrsChunk,
 ): RawResultSet {
-  const name = schema.name;
-  const totalRowCount = schema.rows;
-
-  const columns = schema.columns.map(mapColumn);
-
-  const rows = chunk.tuples.map(
-    (tuple): Row => tuple.map((cell): CellValue => mapCellValue(cell)),
-  );
-
   const resultSet: RawResultSet = {
-    name,
-    totalRowCount,
-    columns,
-    rows,
+    name: schema.name,
+    totalRowCount: schema.rows,
+    columns: schema.columns.map(mapColumn),
+    rows: chunk.tuples.map(
+      (tuple): Row => tuple.map((cell): CellValue => mapCellValue(cell)),
+    ),
   };
 
   if (chunk.next) {