Support running buildifier through Bazel (#350)

Buildifier is frequently installed through Bazel as described on
https://github.com/bazelbuild/buildtools/blob/master/buildifier/README.md#setup-and-usage-via-bazel
such that it can be run through `bazel run`.

This commit allows the `bazel.buildifierExecutable` setting to refer
to a Bazel target. All paths starting with `@` are interpreted as Bazel
target names and are executed through `bazel run`. Bazel targets could
also start with `//` but we interpret those as normal file system paths.
If someone wants to run a target in the own workspace, they can simply
use, e.g., `@//:buildifier` instead of `//:buildifier`.

As a drive-by fix, I also fixed #329 since it was a one-line fix.

Fixes #185, #329

Co-authored-by: John Firebaugh <[email protected]>

package.json

@@ -118,7 +118,7 @@
                 "bazel.buildifierExecutable": {
                     "type": "string",
                     "default": "",
-                    "markdownDescription": "The name of the Buildifier executable. This may be an absolute path, or a simple name that will be searched for on the system path. If empty, \"buildifier\" on the system path will be used.\n\nBuildifier can be downloaded from https://github.com/bazelbuild/buildtools/releases.",
+                    "markdownDescription": "The name of the Buildifier executable. This may be an absolute path, or a simple name that will be searched for on the system path. Paths starting with `@` are interpreted as Bazel targets and are run via `bazel run`. If empty, \"buildifier\" on the system path will be used.\n\nBuildifier can be downloaded from https://github.com/bazelbuild/buildtools/releases.",
                     "scope": "machine-overridable"
                 },
                 "bazel.buildifierFixOnFormat": {

src/buildifier/buildifier.ts

@@ -16,6 +16,7 @@ import * as child_process from "child_process";
 import * as path from "path";
 import * as vscode from "vscode";
 import { IBuildifierResult, IBuildifierWarning } from "./buildifier_result";
+import { getDefaultBazelExecutablePath } from "../extension/configuration";
 
 /** Whether to warn about lint findings or fix them. */
 export type BuildifierLintMode = "fix" | "warn";
@@ -182,17 +183,30 @@ export function getDefaultBuildifierExecutablePath(): string {
  * @param acceptNonSevereErrors If true, syntax/lint exit codes will not be
  * treated as severe tool errors.
  */
-function executeBuildifier(
+export function executeBuildifier(
   fileContent: string,
   args: string[],
   acceptNonSevereErrors: boolean,
 ): Promise<{ stdout: string; stderr: string }> {
   return new Promise((resolve, reject) => {
+    // Determine the executable
+    let executable = getDefaultBuildifierExecutablePath();
+    // 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(
-      getDefaultBuildifierExecutablePath(),
+      executable,
       args,
       execOptions,
       (error, stdout, stderr) => {

src/buildifier/buildifier_availability.ts

@@ -12,11 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import * as child_process from "child_process";
 import * as vscode from "vscode";
 import * as which from "which";
 
-import { getDefaultBuildifierExecutablePath } from "./buildifier";
+import {
+  executeBuildifier,
+  getDefaultBuildifierExecutablePath,
+} from "./buildifier";
 
 /** The URL to load for buildifier's releases. */
 const BUILDTOOLS_RELEASES_URL =
@@ -31,36 +33,34 @@ const BUILDTOOLS_RELEASES_URL =
  */
 export async function checkBuildifierIsAvailable() {
   const buildifierExecutable = getDefaultBuildifierExecutablePath();
-  await which(buildifierExecutable)
-    .catch(async () => {
+  // Check if the program exists (in case it's an actual executable and not
+  // an target name starting with `@`).
+  if (!buildifierExecutable.startsWith("@")) {
+    try {
+      await which(buildifierExecutable);
+    } catch (e) {
       await showBuildifierDownloadPrompt("Buildifier was not found");
-    })
-    .then(() => {
-      // If we found it, make sure it's a compatible version by running
-      // buildifier on an empty input and see if it exits successfully and the
-      // output parses.
-      const process = child_process.execFile(
-        buildifierExecutable,
-        ["--format=json", "--mode=check"],
-        {},
-        (error: Error, stdout: string) => {
-          if (!error && stdout) {
-            try {
-              JSON.parse(stdout);
-              return;
-            } catch {
-              // Swallow the error; we'll display the prompt below.
-            }
-          }
-          // If no valid JSON back, we don't have a compatible version.
-          // eslint-disable-next-line @typescript-eslint/no-floating-promises
-          showBuildifierDownloadPrompt(
-            "Buildifier is too old (0.25.1 or higher is needed)",
-          );
-        },
-      );
-      process.stdin.end();
-    });
+      return;
+    }
+  }
+
+  // Make sure it's a compatible version by running
+  // buildifier on an empty input and see if it exits successfully and the
+  // output parses.
+  const { stdout } = await executeBuildifier(
+    "",
+    ["--format=json", "--mode=check"],
+    false,
+  );
+  try {
+    JSON.parse(stdout);
+  } catch {
+    // If we got no valid JSON back, we don't have a compatible version.
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    showBuildifierDownloadPrompt(
+      "Buildifier is too old (0.25.1 or higher is needed)",
+    );
+  }
 }
 
 /**