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

Add buildifier as a BUILD file formatter #16573

Merged
merged 4 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 6 additions & 0 deletions src/python/pants/backend/build_files/fmt/buildifier/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()

python_tests(name="tests")
Empty file.
10 changes: 10 additions & 0 deletions src/python/pants/backend/build_files/fmt/buildifier/register.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.build_files.fmt.buildifier import rules as buildifier_rules


def rules():
return [
*buildifier_rules.rules(),
]
48 changes: 48 additions & 0 deletions src/python/pants/backend/build_files/fmt/buildifier/rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.build_files.fmt.buildifier.subsystem import Buildifier
from pants.core.goals.fmt import FmtResult, _FmtBuildFilesRequest
from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest
from pants.engine.internals.native_engine import Digest, MergeDigests, Snapshot
from pants.engine.internals.selectors import Get
from pants.engine.platform import Platform
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import collect_rules, rule
from pants.engine.unions import UnionRule
from pants.util.logging import LogLevel
from pants.util.strutil import pluralize


class BuildifierRequest(_FmtBuildFilesRequest):
name = "buildifier"


@rule(desc="Format with Buildifier", level=LogLevel.DEBUG)
async def buildfier_fmt(request: BuildifierRequest, buildifier: Buildifier) -> FmtResult:
buildifier_tool = await Get(
DownloadedExternalTool, ExternalToolRequest, buildifier.get_request(Platform.current)
)
input_digest = await Get(
Digest,
MergeDigests((request.snapshot.digest, buildifier_tool.digest)),
)
result = await Get(
ProcessResult,
Process(
argv=[buildifier_tool.exe, "-type=build", *request.snapshot.files],
input_digest=input_digest,
output_files=request.snapshot.files,
description=f"Run buildifier on {pluralize(len(request.snapshot.files), 'file')}.",
level=LogLevel.DEBUG,
),
)
output_snapshot = await Get(Snapshot, Digest, result.output_digest)
return FmtResult.create(request, result, output_snapshot)


def rules():
return [
*collect_rules(),
UnionRule(_FmtBuildFilesRequest, BuildifierRequest),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

from textwrap import dedent

import pytest

from pants.backend.build_files.fmt.buildifier.rules import BuildifierRequest
from pants.backend.build_files.fmt.buildifier.rules import rules as buildifier_rules
from pants.backend.codegen.protobuf.target_types import rules as target_types_rules
from pants.core.goals.fmt import FmtResult
from pants.core.util_rules import external_tool
from pants.engine.fs import CreateDigest, FileContent, PathGlobs
from pants.engine.internals.native_engine import Digest, Snapshot
from pants.testutil.rule_runner import QueryRule, RuleRunner


class Materials:
def __init__(self, **kwargs):
pass


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*buildifier_rules(),
*external_tool.rules(),
*target_types_rules(),
QueryRule(FmtResult, [BuildifierRequest]),
],
# NB: Objects are easier to test with
objects={"materials": Materials},
)


GOOD_FILE = dedent(
"""\
materials(
drywall = 40,
status = "paid",
studs = 200,
)
"""
)

BAD_FILE = dedent(
"""\
materials(status='paid', studs=200, drywall=40)
"""
)


def run_buildifier(rule_runner: RuleRunner) -> FmtResult:
rule_runner.set_options(
["--backend-packages=pants.backend.build_files.fmt.buildifier"],
env_inherit={"PATH", "PYENV_ROOT"},
)
snapshot = rule_runner.request(Snapshot, [PathGlobs(["**/BUILD"])])
fmt_result = rule_runner.request(FmtResult, [BuildifierRequest(snapshot)])
return fmt_result


def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
files = [FileContent(path, content.encode()) for path, content in source_files.items()]
digest = rule_runner.request(Digest, [CreateDigest(files)])
return rule_runner.request(Snapshot, [digest])
thejcannon marked this conversation as resolved.
Show resolved Hide resolved


def test_passing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"BUILD": GOOD_FILE})
fmt_result = run_buildifier(rule_runner)
assert fmt_result.output == get_snapshot(rule_runner, {"BUILD": GOOD_FILE})
assert fmt_result.did_change is False


def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"BUILD": BAD_FILE})
fmt_result = run_buildifier(rule_runner)
assert fmt_result.output == get_snapshot(rule_runner, {"BUILD": GOOD_FILE})
assert fmt_result.did_change is True


def test_multiple_files(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"good/BUILD": GOOD_FILE,
"bad/BUILD": BAD_FILE,
}
)
fmt_result = run_buildifier(rule_runner)
assert fmt_result.output == get_snapshot(
rule_runner, {"good/BUILD": GOOD_FILE, "bad/BUILD": GOOD_FILE}
)
assert fmt_result.did_change is True
46 changes: 46 additions & 0 deletions src/python/pants/backend/build_files/fmt/buildifier/subsystem.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.core.util_rules.external_tool import TemplatedExternalTool
from pants.option.option_types import ArgsListOption, SkipOption
from pants.util.strutil import softwrap


class Buildifier(TemplatedExternalTool):
options_scope = "buildifier"
name = "Buildifier"
help = softwrap(
"""
Buildifier is a tool for formatting BUILD files with a standard convention.
Pants supports running Buildifier on your Pants BUILD files for several reasons:
- You might like the style that buildifier uses.
- You might be incrementally adopting Pants from Bazel, and are already using buildifier.
Please note that there are differences from Bazel's BUILD files (which are Starlark) and
Pants' BUILD files (which are Python), so buildifier may issue a syntax error.
In practice, these errors should be rare. See https://bazel.build/rules/language#differences_with_python.
"""
)

default_version = "5.1.0"
default_known_versions = [
"5.1.0|macos_x86_64|c9378d9f4293fc38ec54a08fbc74e7a9d28914dae6891334401e59f38f6e65dc|7125968",
"5.1.0|macos_arm64 |745feb5ea96cb6ff39a76b2821c57591fd70b528325562486d47b5d08900e2e4|7334498",
"5.1.0|linux_x86_64|52bf6b102cb4f88464e197caac06d69793fa2b05f5ad50a7e7bf6fbd656648a3|7226100",
"5.1.0|linux_arm64 |917d599dbb040e63ae7a7e1adb710d2057811902fdc9e35cce925ebfd966eeb8|7171938",
]
default_url_template = (
"https://github.com/bazelbuild/buildtools/releases/download/{version}/buildifier-{platform}"
)
default_url_platform_mapping = {
"macos_arm64": "darwin-arm64",
"macos_x86_64": "darwin-amd64",
"linux_arm64": "linux-arm64",
"linux_x86_64": "linux-amd64",
}

skip = SkipOption("fmt")
args = ArgsListOption(example="-lint=fix")

# NB: buildifier doesn't (yet) support config files https://github.com/bazelbuild/buildtools/issues/479
1 change: 1 addition & 0 deletions src/python/pants/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ python_sources(
target(
thejcannon marked this conversation as resolved.
Show resolved Hide resolved
name="plugins",
dependencies=[
"src/python/pants/backend/build_files/fmt/buildifier",
"src/python/pants/backend/awslambda/python",
"src/python/pants/backend/codegen/protobuf/lint/buf",
"src/python/pants/backend/codegen/protobuf/python",
Expand Down