update-build-files: add support for Buildifier formatter

docs/notes/2.23.x.md

@@ -26,6 +26,16 @@ from the local or remote cache, or be memoized.
 Knowing where the test results come from may be useful when evaluating the efficiency of the caching strategy and 
 the nature of the changes in the source code that may lead to frequent cache invalidations.
 
+### update-build-files goal
+
+`buildifier` was added to the list of supported formatters that can be used to format the BUILD files. 
+It may be helpful if your organization is migrating from Bazel and wants to keep the style of the BUILD files 
+consistent or if for any other reason you may want to adopt the formatting style that is enforced by `buildifier`.  
+
+The `buildifier` can be used on its own, but it can also be used in pair with a Python formatter, such as `black` 
+or `ruff`. For instance, you could first run `buildifier` to sort the target fields alphabetically, 
+and then run `black` to keep the style consistent with the rest of the Python code.
+
 ### Remote caching/execution
 
 The deprecation for the `[GLOBAL].remote_auth_bearer_token_path` option has expired. Use [the `[GLOBAL].remote_auth_bearer_token = "@/path/to/file"` option](https://www.pantsbuild.org/2.23/reference/global-options#remote_oauth_bearer_token) instead.

src/python/pants/backend/BUILD

@@ -36,6 +36,8 @@ __dependents_rules__(
             "[/build_files/fmt/black/register.py]",
             "[/build_files/fmt/ruff/register.py]",
             "[/build_files/fmt/yapf/register.py]",
+            "[/build_files/fmt/buildifier/rules.py]",
+            "[/build_files/fmt/buildifier/subsystem.py]",
             "[/python/lint/black/rules.py]",
             "[/python/lint/black/subsystem.py]",
             "[/python/lint/ruff/format/rules.py]",

src/python/pants/backend/build_files/fmt/buildifier/rules.py

@@ -3,7 +3,7 @@
 
 from pants.backend.build_files.fmt.base import FmtBuildFilesRequest
 from pants.backend.build_files.fmt.buildifier.subsystem import Buildifier
-from pants.core.goals.fmt import FmtResult
+from pants.core.goals.fmt import AbstractFmtRequest, FmtResult
 from pants.core.util_rules.external_tool import download_external_tool
 from pants.engine.internals.native_engine import MergeDigests
 from pants.engine.intrinsics import merge_digests_request_to_digest
@@ -32,7 +32,30 @@ async def buildfier_fmt(
                 argv=[buildifier_tool.exe, "-type=build", *request.files],
                 input_digest=input_digest,
                 output_files=request.files,
-                description=f"Run buildifier on {pluralize(len(request.files), 'file')}.",
+                description=f"Run {Buildifier.options_scope} on {pluralize(len(request.files), 'file')}.",
+                level=LogLevel.DEBUG,
+            )
+        ),
+    )
+    return await FmtResult.create(request, result)
+
+
+# Note - this function is kept separate because it is invoked from update_build_files.py, but
+# not as a rule.
+async def _run_buildifier_fmt(
+    request: AbstractFmtRequest.Batch, buildifier: Buildifier, platform: Platform
+) -> FmtResult:
+    buildifier_tool = await download_external_tool(buildifier.get_request(platform))
+    input_digest = await merge_digests_request_to_digest(
+        MergeDigests((request.snapshot.digest, buildifier_tool.digest))
+    )
+    result = await fallible_to_exec_result_or_raise(
+        **implicitly(
+            Process(
+                argv=[buildifier_tool.exe, "-type=build", *request.files],
+                input_digest=input_digest,
+                output_files=request.files,
+                description=f"Run {Buildifier.options_scope} on {pluralize(len(request.files), 'file')}.",
                 level=LogLevel.DEBUG,
             )
         ),

src/python/pants/core/goals/update_build_files.py

@@ -18,6 +18,8 @@
 from pants.backend.build_files.fix.deprecations import renamed_fields_rules, renamed_targets_rules
 from pants.backend.build_files.fix.deprecations.base import FixedBUILDFile
 from pants.backend.build_files.fmt.black.register import BlackRequest
+from pants.backend.build_files.fmt.buildifier.rules import BuildifierRequest, _run_buildifier_fmt
+from pants.backend.build_files.fmt.buildifier.subsystem import Buildifier
 from pants.backend.build_files.fmt.ruff.register import RuffRequest
 from pants.backend.build_files.fmt.yapf.register import YapfRequest
 from pants.backend.python.goals import lockfile
@@ -47,6 +49,7 @@
 from pants.engine.goal import Goal, GoalSubsystem
 from pants.engine.internals.build_files import BuildFileOptions
 from pants.engine.internals.parser import ParseError
+from pants.engine.platform import Platform
 from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule, rule
 from pants.engine.unions import UnionMembership, UnionRule, union
 from pants.option.option_types import BoolOption, EnumOption
@@ -73,6 +76,7 @@ class Formatter(Enum):
     YAPF = "yapf"
     BLACK = "black"
     RUFF = "ruff"
+    BUILDIFIER = "buildifier"
 
 
 @union(in_scope_types=[EnvironmentName])
@@ -223,6 +227,7 @@ async def update_build_files(
         Formatter.BLACK: FormatWithBlackRequest,
         Formatter.YAPF: FormatWithYapfRequest,
         Formatter.RUFF: FormatWithRuffRequest,
+        Formatter.BUILDIFIER: FormatWithBuildifierRequest,
     }
     chosen_formatter_request_class = formatter_to_request_class.get(
         update_build_files_subsystem.formatter
@@ -418,6 +423,37 @@ async def format_build_file_with_ruff(
     return RewrittenBuildFile(request.path, build_lines, change_descriptions=change_descriptions)
 
 
+# ------------------------------------------------------------------------------------------
+# Buildifier formatter fixer
+# ------------------------------------------------------------------------------------------
+
+
+class FormatWithBuildifierRequest(RewrittenBuildFileRequest):
+    pass
+
+
+@rule
+async def format_build_file_with_buildifier(
+    request: FormatWithBuildifierRequest, buildifier: Buildifier, platform: Platform
+) -> RewrittenBuildFile:
+    input_snapshot = await Get(Snapshot, CreateDigest([request.to_file_content()]))
+    result = await _run_buildifier_fmt(
+        request=BuildifierRequest.Batch(
+            tool_name=Buildifier.options_scope,
+            elements=input_snapshot.files,
+            partition_metadata=None,
+            snapshot=input_snapshot,
+        ),
+        buildifier=buildifier,
+        platform=platform,
+    )
+    output_content = await Get(DigestContents, Digest, result.output.digest)
+    formatted_build_file_content = next(fc for fc in output_content if fc.path == request.path)
+    build_lines = tuple(formatted_build_file_content.content.decode("utf-8").splitlines())
+    change_descriptions = (f"Format with {Buildifier.name}",) if result.did_change else ()
+    return RewrittenBuildFile(request.path, build_lines, change_descriptions=change_descriptions)
+
+
 # ------------------------------------------------------------------------------------------
 # Rename deprecated target types fixer
 # ------------------------------------------------------------------------------------------
@@ -488,4 +524,5 @@ def rules():
         UnionRule(RewrittenBuildFileRequest, FormatWithBlackRequest),
         UnionRule(RewrittenBuildFileRequest, FormatWithYapfRequest),
         UnionRule(RewrittenBuildFileRequest, FormatWithRuffRequest),
+        UnionRule(RewrittenBuildFileRequest, FormatWithBuildifierRequest),
     )

src/python/pants/core/goals/update_build_files_test.py

@@ -9,6 +9,7 @@
 
 import pytest
 
+from pants.backend.build_files.fmt.buildifier.subsystem import Buildifier
 from pants.backend.python.lint.black.subsystem import Black
 from pants.backend.python.lint.ruff.subsystem import Ruff
 from pants.backend.python.lint.yapf.subsystem import Yapf
@@ -23,13 +24,15 @@
 )
 from pants.core.goals.update_build_files import (
     FormatWithBlackRequest,
+    FormatWithBuildifierRequest,
     FormatWithRuffRequest,
     FormatWithYapfRequest,
     RewrittenBuildFile,
     RewrittenBuildFileRequest,
     UpdateBuildFilesGoal,
     UpdateBuildFilesSubsystem,
     format_build_file_with_black,
+    format_build_file_with_buildifier,
     format_build_file_with_ruff,
     format_build_file_with_yapf,
     update_build_files,
@@ -336,6 +339,57 @@ def test_ruff_fixer_noops() -> None:
     assert build == 'target(name="t")\n'
 
 
+# ------------------------------------------------------------------------------------------
+# Buildifier formatter fixer
+# ------------------------------------------------------------------------------------------
+
+
+def run_buildifier(
+    build_content: str, *, extra_args: list[str] | None = None
+) -> tuple[GoalRuleResult, str]:
+    """Returns the Goal's result and contents of the BUILD file after execution."""
+    rule_runner = RuleRunner(
+        rules=(
+            format_build_file_with_buildifier,
+            update_build_files,
+            *config_files.rules(),
+            *pex.rules(),
+            *Buildifier.rules(),
+            *UpdateBuildFilesSubsystem.rules(),
+            UnionRule(RewrittenBuildFileRequest, FormatWithBuildifierRequest),
+        ),
+        target_types=[GenericTarget],
+    )
+    rule_runner.write_files({"BUILD": build_content})
+    goal_result = rule_runner.run_goal_rule(
+        UpdateBuildFilesGoal,
+        args=[f"--update-build-files-formatter={Buildifier.options_scope}", "::"],
+        global_args=extra_args or (),
+        env_inherit=BLACK_ENV_INHERIT,
+    )
+    rewritten_build = Path(rule_runner.build_root, "BUILD").read_text()
+    return goal_result, rewritten_build
+
+
+def test_buildifier_fixer_fixes() -> None:
+    result, build = run_buildifier("target(name='t')")
+    assert result.exit_code == 0
+    assert result.stdout == dedent(
+        f"""\
+        Updated BUILD:
+          - Format with {Buildifier.name}
+        """
+    )
+    assert build == 'target(name = "t")\n'
+
+
+def test_buildifier_fixer_noops() -> None:
+    result, build = run_buildifier('target(name = "t")\n')
+    assert result.exit_code == 0
+    assert not result.stdout
+    assert build == 'target(name = "t")\n'
+
+
 # ------------------------------------------------------------------------------------------
 # Yapf formatter fixer
 # ------------------------------------------------------------------------------------------