chore: Promisify execFile (#384)

`await` is generally more readable than explicit use of promises.
Also, this works towards flipping `strictNullChecks` to true, because
TypeScript complains a bit less about the promisified `execFile` than
about explicit uses of callbacks and promises.

While at it, this commit also moves the `getBazelInfo` function from
`debugger/client.ts` over to the `BazelInfo` class, where it can be more
easily reused. This has the nice side-effect, that also this `info` call
will now respect any `startupOptions` potentially configured by the
user.

Author: vogelsgesang

src/bazel/bazel_info.ts

@@ -13,32 +13,50 @@
 // limitations under the License.
 
 import * as child_process from "child_process";
+import * as util from "util";
 
 import { BazelCommand } from "./bazel_command";
 
+const execFile = util.promisify(child_process.execFile);
+
 /** Provides a promise-based API around the `bazel info` command. */
 export class BazelInfo extends BazelCommand {
   /**
-   * Runs `bazel info <key>` and returns the output.
+   * Gets the info for a single key by running `bazel info <key>`.
    *
    * @param key The info key to query.
    * @returns The output of `bazel info <key>`.
    */
-  public async run(key: string): Promise<string> {
-    return new Promise((resolve, reject) => {
-      child_process.execFile(
-        this.bazelExecutable,
-        this.execArgs([key]),
-        { cwd: this.workingDirectory },
-        (error: Error, stdout: string) => {
-          if (error) {
-            reject(error);
-          } else {
-            resolve(stdout.trim());
-          }
-        },
-      );
+  public async getOne(key: string): Promise<string> {
+    const execResult = await execFile(
+      this.bazelExecutable,
+      this.execArgs([key]),
+      {
+        cwd: this.workingDirectory,
+      },
+    );
+    return execResult.stdout.trim();
+  }
+
+  /**
+   * Runs `bazel info` and returns the output.
+   *
+   * @returns All `bazel info` entries
+   */
+  public async getAll(): Promise<Map<string, string>> {
+    const execResult = await execFile(this.bazelExecutable, this.execArgs([]), {
+      cwd: this.workingDirectory,
     });
+    const keyValues = new Map<string, string>();
+    const lines = execResult.stdout.trim().split("\n");
+    for (const line of lines) {
+      // Windows paths can have >1 ':', so can't use line.split(":", 2)
+      const splitterIndex = line.indexOf(":");
+      const key = line.substring(0, splitterIndex);
+      const value = line.substring(splitterIndex + 1);
+      keyValues.set(key.trim(), value.trim());
+    }
+    return keyValues;
   }
 
   protected bazelCommand(): string {

src/bazel/bazel_query.ts

@@ -16,12 +16,15 @@ import * as child_process from "child_process";
 import * as crypto from "crypto";
 import * as os from "os";
 import * as path from "path";
+import * as util from "util";
 import * as vscode from "vscode";
 
 import { blaze_query } from "../protos";
 import { BazelCommand } from "./bazel_command";
 import { getBazelWorkspaceFolder } from "./bazel_utils";
 
+const execFile = util.promisify(child_process.execFile);
+
 const protoOutputOptions = [
   "--proto:output_rule_attrs=''",
   "--noproto:rule_inputs_and_outputs",
@@ -106,14 +109,12 @@ export class BazelQuery extends BazelCommand {
    * Executes the command and returns a promise for the binary contents of
    * standard output.
    *
-   * @param query The query to execute.
-   * @param options
-   * @param options.ignoresErrors `true` if errors from executing the query
+   * @param options The options to pass to `bazel query`
+   * @param ignoresErrors `true` if errors from executing the query
    * should be ignored.
-   * @returns A promise that is resolved with the contents of the process's
-   * standard output, or rejected if the command fails.
+   * @returns The contents of the process's standard output
    */
-  protected run(
+  protected async run(
     options: string[],
     { ignoresErrors = false }: { ignoresErrors?: boolean } = {},
   ): Promise<Buffer> {
@@ -142,30 +143,27 @@ export class BazelQuery extends BazelCommand {
         `--output_base=${queryOutputBase}`,
       ]);
     }
-    return new Promise((resolve, reject) => {
-      const execOptions: child_process.ExecFileOptionsWithBufferEncoding = {
-        cwd: this.workingDirectory,
-        // A null encoding causes the callback below to receive binary data as a
-        // Buffer instead of text data as strings.
-        encoding: null,
-        maxBuffer: Number.MAX_SAFE_INTEGER,
-      };
-      child_process.execFile(
-        this.bazelExecutable,
-        this.execArgs(options, additionalStartupOptions),
-        execOptions,
-        (error: Error, stdout: Buffer) => {
-          if (error) {
-            if (ignoresErrors) {
-              resolve(new Buffer(0));
-            } else {
-              reject(error);
-            }
-          } else {
-            resolve(stdout);
-          }
-        },
-      );
-    });
+    const execOptions: child_process.ExecFileOptionsWithBufferEncoding = {
+      cwd: this.workingDirectory,
+      // A null encoding causes the callback below to receive binary data as a
+      // Buffer instead of text data as strings.
+      encoding: null,
+      maxBuffer: Number.MAX_SAFE_INTEGER,
+    };
+    try {
+      return (
+        await execFile(
+          this.bazelExecutable,
+          this.execArgs(options, additionalStartupOptions),
+          execOptions,
+        )
+      ).stdout;
+    } catch (e) {
+      if (ignoresErrors) {
+        return Buffer.alloc(0);
+      } else {
+        throw e;
+      }
+    }
   }
 }

src/buildifier/buildifier.ts

@@ -14,10 +14,18 @@
 
 import * as child_process from "child_process";
 import * as path from "path";
+import * as util from "util";
 import * as vscode from "vscode";
+
 import { IBuildifierResult, IBuildifierWarning } from "./buildifier_result";
 import { getDefaultBazelExecutablePath } from "../extension/configuration";
 
+const execFile = util.promisify(child_process.execFile);
+type PromiseExecFileException = child_process.ExecFileException & {
+  stdout: string;
+  stderr: string;
+};
+
 /** Whether to warn about lint findings or fix them. */
 export type BuildifierLintMode = "fix" | "warn";
 
@@ -196,52 +204,48 @@ export function getDefaultBuildifierJsonConfigPath(): string {
  * @param acceptNonSevereErrors If true, syntax/lint exit codes will not be
  * treated as severe tool errors.
  */
-export function executeBuildifier(
+export async function executeBuildifier(
   fileContent: string,
   args: string[],
   acceptNonSevereErrors: boolean,
 ): Promise<{ stdout: string; stderr: string }> {
-  return new Promise((resolve, reject) => {
-    // Determine the executable
-    let executable = getDefaultBuildifierExecutablePath();
-    const buildifierConfigJsonPath = getDefaultBuildifierJsonConfigPath();
-    if (buildifierConfigJsonPath.length !== 0) {
-      args = ["--config", buildifierConfigJsonPath, ...args];
-    }
-    // Paths starting with an `@` are referring to Bazel targets
-    if (executable.startsWith("@")) {
-      const targetName = executable;
-      executable = getDefaultBazelExecutablePath();
-      args = ["run", targetName, "--", ...args];
-    }
-    const execOptions = {
-      maxBuffer: Number.MAX_SAFE_INTEGER,
-      // Use the workspace folder as CWD, thereby allowing relative
-      // paths. See #329
-      cwd: vscode.workspace.workspaceFolders?.[0]?.uri?.fsPath,
-    };
+  // Determine the executable
+  let executable = getDefaultBuildifierExecutablePath();
+  const buildifierConfigJsonPath = getDefaultBuildifierJsonConfigPath();
+  if (buildifierConfigJsonPath.length !== 0) {
+    args = ["--config", buildifierConfigJsonPath, ...args];
+  }
+  // Paths starting with an `@` are referring to Bazel targets
+  if (executable.startsWith("@")) {
+    const targetName = executable;
+    executable = getDefaultBazelExecutablePath();
+    args = ["run", targetName, "--", ...args];
+  }
+  const execOptions = {
+    maxBuffer: Number.MAX_SAFE_INTEGER,
+    // Use the workspace folder as CWD, thereby allowing relative
+    // paths. See #329
+    cwd: vscode.workspace.workspaceFolders?.[0]?.uri?.fsPath,
+  };
 
-    // Start buildifier
-    const process = child_process.execFile(
-      executable,
-      args,
-      execOptions,
-      (error, stdout, stderr) => {
-        if (
-          !error ||
-          (acceptNonSevereErrors && shouldTreatBuildifierErrorAsSuccess(error))
-        ) {
-          resolve({ stdout, stderr });
-        } else {
-          reject(error);
-        }
-      },
-    );
-    // Write the file being linted/formatted to stdin and close the stream so
-    // that the buildifier process continues.
-    process.stdin.write(fileContent);
-    process.stdin.end();
-  });
+  // Start buildifier
+  const process = execFile(executable, args, execOptions);
+
+  // Write the file being linted/formatted to stdin and close the stream so
+  // that the buildifier process continues.
+  process.child.stdin?.write(fileContent);
+  process.child.stdin?.end();
+
+  try {
+    return await process;
+  } catch (e) {
+    const error = e as PromiseExecFileException;
+    if (acceptNonSevereErrors && shouldTreatBuildifierErrorAsSuccess(error)) {
+      return { stdout: error.stdout, stderr: error.stderr };
+    } else {
+      throw error;
+    }
+  }
 }
 
 /**

src/debug-adapter/client.ts

@@ -32,6 +32,7 @@ import { DebugProtocol } from "vscode-debugprotocol";
 import { skylark_debugging } from "../protos";
 import { BazelDebugConnection } from "./connection";
 import { Handles } from "./handles";
+import { BazelInfo } from "../bazel/bazel_info";
 
 /**
  * Returns a {@code number} equivalent to the given {@code number} or
@@ -166,7 +167,7 @@ class BazelDebugSession extends DebugSession {
     const verbose = args.verbose || false;
 
     const bazelExecutable = this.bazelExecutable(args);
-    this.bazelInfo = await this.getBazelInfo(bazelExecutable, args.cwd);
+    this.bazelInfo = await new BazelInfo(bazelExecutable, args.cwd).getAll();
 
     const fullArgs = args.bazelStartupOptions
       .concat([
@@ -531,48 +532,6 @@ class BazelDebugSession extends DebugSession {
     return bazelExecutable;
   }
 
-  /**
-   * Invokes {@code bazel info} and returns the information in a map.
-   *
-   * @param bazelExecutable The name/path of the Bazel executable.
-   * @param cwd The working directory in which Bazel should be launched.
-   */
-  private getBazelInfo(
-    bazelExecutable: string,
-    cwd: string,
-  ): Promise<Map<string, string>> {
-    return new Promise((resolve, reject) => {
-      const execOptions = {
-        cwd,
-        // The maximum amount of data allowed on stdout. 500KB should be plenty
-        // of `bazel info`, but if this becomes problematic we can switch to the
-        // event-based `child_process` APIs instead.
-        maxBuffer: 500 * 1024,
-      };
-      child_process.execFile(
-        bazelExecutable,
-        ["info"],
-        execOptions,
-        (error: Error, stdout: string) => {
-          if (error) {
-            reject(error);
-          } else {
-            const keyValues = new Map<string, string>();
-            const lines = stdout.trim().split("\n");
-            for (const line of lines) {
-              // Windows paths can have >1 ':', so can't use line.split(":", 2)
-              const splitterIndex = line.indexOf(":");
-              const key = line.substring(0, splitterIndex);
-              const value = line.substring(splitterIndex + 1);
-              keyValues.set(key.trim(), value.trim());
-            }
-            resolve(keyValues);
-          }
-        },
-      );
-    });
-  }
-
   /**
    * Launches the Bazel process to be debugged.
    *

src/extension/command_variables.ts

@@ -87,7 +87,7 @@ async function bazelGetTargetOutput(
   const outputPath = await new BazelInfo(
     getDefaultBazelExecutablePath(),
     workspaceInfo.bazelWorkspacePath,
-  ).run("output_path");
+  ).getOne("output_path");
   const outputs = await new BazelCQuery(
     getDefaultBazelExecutablePath(),
     workspaceInfo.bazelWorkspacePath,
@@ -122,7 +122,7 @@ async function bazelInfo(key: string): Promise<string> {
   return new BazelInfo(
     getDefaultBazelExecutablePath(),
     workspaceInfo.bazelWorkspacePath,
-  ).run(key);
+  ).getOne(key);
 }
 
 /**