Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support running buildifier through Bazel #350

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
18 changes: 16 additions & 2 deletions src/buildifier/buildifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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) => {
Expand Down
62 changes: 31 additions & 31 deletions src/buildifier/buildifier_availability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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)",
);
}
}

/**
Expand Down