Skip to content

Commit

Permalink
Allow using Ruff to format BUILD files (pantsbuild#20411)
Browse files Browse the repository at this point in the history
Closes pantsbuild#20392.

In addition to `black` and `yapf`, we will now allow using `ruff` to
format build files. This PR contains changes to the `update-build-files`
goal and the `ruff` subsystem to enable this.
  • Loading branch information
krishnan-chandra authored Feb 8, 2024
1 parent 0df48c7 commit 3129951
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 28 deletions.
1 change: 1 addition & 0 deletions docs/docs/using-pants/key-concepts/backends.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ This list is also available via `pants backends --help`, which includes any addi
| :--------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------------------- |
| `pants.backend.build_files.fmt.black` | Enables autoformatting `BUILD` files using `black`. | |
| `pants.backend.build_files.fmt.buildifier` | Enables autoformatting `BUILD` files using `buildifier`. | |
| `pants.backend.build_files.fmt.ruff` | Enables autoformatting `BUILD` files using `ruff`. | |
| `pants.backend.build_files.fmt.yapf` | Enables autoformatting `BUILD` files using `yapf`. | |
| `pants.backend.awslambda.python` | Enables generating an AWS Lambda zip file from Python code. | [AWS Lambda](../../python/integrations/aws-lambda.mdx) |
| `pants.backend.codegen.protobuf.lint.buf` | Activate the Buf formatter and linter for Protocol Buffers. | [Protobuf](../../python/integrations/protobuf-and-grpc.mdx) |
Expand Down
3 changes: 3 additions & 0 deletions src/python/pants/backend/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ __dependents_rules__(
"[/build_files/fix/deprecations/renamed_fields_rules.py]",
"[/build_files/fix/deprecations/renamed_targets_rules.py]",
"[/build_files/fmt/black/register.py]",
"[/build_files/fmt/ruff/register.py]",
"[/build_files/fmt/yapf/register.py]",
"[/python/lint/black/rules.py]",
"[/python/lint/black/subsystem.py]",
"[/python/lint/ruff/rules.py]",
"[/python/lint/ruff/subsystem.py]",
"[/python/lint/yapf/rules.py]",
"[/python/lint/yapf/subsystem.py]",
"[src/python/pants/backend/python/goals/lockfile.py]",
Expand Down
6 changes: 6 additions & 0 deletions src/python/pants/backend/build_files/fmt/ruff/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()

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

from __future__ import annotations

import pytest

from pants.backend.build_files.fmt.ruff.register import RuffRequest
from pants.backend.build_files.fmt.ruff.register import rules as ruff_build_rules
from pants.backend.python.lint.ruff.rules import rules as ruff_fmt_rules
from pants.backend.python.lint.ruff.subsystem import Ruff
from pants.backend.python.lint.ruff.subsystem import rules as ruff_subsystem_rules
from pants.backend.python.target_types import PythonSourcesGeneratorTarget
from pants.core.goals.fmt import FmtResult
from pants.core.util_rules import config_files
from pants.engine.fs import PathGlobs
from pants.engine.internals.native_engine import Snapshot
from pants.testutil.python_interpreter_selection import all_major_minor_python_versions
from pants.testutil.rule_runner import QueryRule, RuleRunner


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*ruff_build_rules(),
*ruff_fmt_rules(),
*ruff_subsystem_rules(),
*config_files.rules(),
QueryRule(FmtResult, (RuffRequest.Batch,)),
],
target_types=[PythonSourcesGeneratorTarget],
)


def run_ruff(rule_runner: RuleRunner, *, extra_args: list[str] | None = None) -> FmtResult:
rule_runner.set_options(
["--backend-packages=pants.backend.build_files.fmt.ruff", *(extra_args or ())],
env_inherit={"PATH", "PYENV_ROOT", "HOME"},
)
snapshot = rule_runner.request(Snapshot, [PathGlobs(["**/BUILD"])])
fmt_result = rule_runner.request(
FmtResult,
[
RuffRequest.Batch("", snapshot.files, partition_metadata=None, snapshot=snapshot),
],
)
return fmt_result


@pytest.mark.platform_specific_behavior
@pytest.mark.parametrize(
"major_minor_interpreter",
all_major_minor_python_versions(Ruff.default_interpreter_constraints),
)
def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
rule_runner.write_files({"BUILD": 'python_sources(name="t")\n'})
interpreter_constraint = f"=={major_minor_interpreter}.*"
fmt_result = run_ruff(
rule_runner,
extra_args=[f"--ruff-interpreter-constraints=['{interpreter_constraint}']"],
)
assert "1 file left unchanged" in fmt_result.stdout
assert fmt_result.output == rule_runner.make_snapshot({"BUILD": 'python_sources(name="t")\n'})
assert fmt_result.did_change is False


def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"BUILD": "python_sources(name='t')\n"})
fmt_result = run_ruff(rule_runner)
assert "1 file reformatted" in fmt_result.stdout
assert fmt_result.output == rule_runner.make_snapshot({"BUILD": 'python_sources(name="t")\n'})
assert fmt_result.did_change is True


def test_multiple_files(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"good/BUILD": 'python_sources(name="t")\n',
"bad/BUILD": "python_sources(name='t')\n",
}
)
fmt_result = run_ruff(rule_runner)
assert "1 file reformatted, 1 file left unchanged" in fmt_result.stdout
assert fmt_result.output == rule_runner.make_snapshot(
{"good/BUILD": 'python_sources(name="t")\n', "bad/BUILD": 'python_sources(name="t")\n'}
)
assert fmt_result.did_change is True


@pytest.mark.parametrize(
"config_path,extra_args",
(["pyproject.toml", []], ["custom_config.toml", ["--ruff-config=custom_config.toml"]]),
)
def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list[str]) -> None:
# Force single-quote formatting to pass config and ensure there are no changes.
# Use the `tool.ruff` key in pyproject.toml, but don't include in custom config.
config_content = (
'[tool.ruff.format]\nquote-style = "single"\n'
if config_path == "pyproject.toml"
else '[format]\nquote-style = "single"\n'
)
rule_runner.write_files(
{
"BUILD": "python_sources(name='t')\n",
config_path: config_content,
}
)
before_file_content = rule_runner.read_file("BUILD")
fmt_result = run_ruff(rule_runner, extra_args=extra_args)
after_file_content = rule_runner.read_file("BUILD")

assert before_file_content == after_file_content
assert fmt_result.output == rule_runner.make_snapshot({"BUILD": "python_sources(name='t')\n"})
assert fmt_result.did_change is False
29 changes: 29 additions & 0 deletions src/python/pants/backend/build_files/fmt/ruff/register.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.build_files.fmt.base import FmtBuildFilesRequest
from pants.backend.python.lint.ruff import subsystem as ruff_subsystem
from pants.backend.python.lint.ruff.rules import _run_ruff_fmt
from pants.backend.python.lint.ruff.subsystem import Ruff
from pants.backend.python.subsystems.python_tool_base import get_lockfile_interpreter_constraints
from pants.core.goals.fmt import FmtResult
from pants.engine.rules import collect_rules, rule
from pants.util.logging import LogLevel


class RuffRequest(FmtBuildFilesRequest):
tool_subsystem = Ruff


@rule(desc="Format with Ruff", level=LogLevel.DEBUG)
async def ruff_fmt(request: RuffRequest.Batch, ruff: Ruff) -> FmtResult:
ruff_ics = await get_lockfile_interpreter_constraints(ruff)
return await _run_ruff_fmt(request, ruff, ruff_ics)


def rules():
return [
*collect_rules(),
*RuffRequest.rules(),
*ruff_subsystem.rules(),
]
48 changes: 33 additions & 15 deletions src/python/pants/backend/python/lint/ruff/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import Any, Tuple
from typing import Any, Optional, Tuple

from typing_extensions import assert_never

from pants.backend.python.lint.ruff.subsystem import Ruff, RuffFieldSet, RuffMode
from pants.backend.python.util_rules import pex
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess
from pants.core.goals.fix import FixResult, FixTargetsRequest
from pants.core.goals.fmt import FmtResult, FmtTargetsRequest
from pants.core.goals.fmt import AbstractFmtRequest, FmtResult, FmtTargetsRequest
from pants.core.goals.lint import LintResult, LintTargetsRequest
from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
from pants.core.util_rules.partitions import PartitionerType
Expand Down Expand Up @@ -76,14 +77,34 @@ def tool_id(self) -> str:
class _RunRuffRequest:
snapshot: Snapshot
mode: RuffMode
interpreter_constraints: Optional[InterpreterConstraints] = None


@rule(level=LogLevel.DEBUG)
async def run_ruff(
# Note - this function is kept separate because it is invoked from update_build_files.py, but
# not as a rule.
async def _run_ruff_fmt(
request: AbstractFmtRequest.Batch,
ruff: Ruff,
interpreter_constraints: Optional[InterpreterConstraints] = None,
) -> FmtResult:
run_ruff_request = _RunRuffRequest(
snapshot=request.snapshot,
mode=RuffMode.FORMAT,
interpreter_constraints=interpreter_constraints,
)
result = await _run_ruff(run_ruff_request, ruff)
return await FmtResult.create(request, result)


async def _run_ruff(
request: _RunRuffRequest,
ruff: Ruff,
) -> FallibleProcessResult:
ruff_pex_get = Get(VenvPex, PexRequest, ruff.to_pex_request())
ruff_pex_get = Get(
VenvPex,
PexRequest,
ruff.to_pex_request(interpreter_constraints=request.interpreter_constraints),
)

config_files_get = Get(
ConfigFiles, ConfigFilesRequest, ruff.config_request(request.snapshot.dirs)
Expand Down Expand Up @@ -129,31 +150,28 @@ async def run_ruff(

@rule(desc="Fix with `ruff check --fix`", level=LogLevel.DEBUG)
async def ruff_fix(request: RuffFixRequest.Batch, ruff: Ruff) -> FixResult:
result = await Get(
FallibleProcessResult, _RunRuffRequest(snapshot=request.snapshot, mode=RuffMode.FIX)
result = await _run_ruff(
_RunRuffRequest(snapshot=request.snapshot, mode=RuffMode.FIX),
ruff,
)
return await FixResult.create(request, result)


@rule(desc="Lint with `ruff check`", level=LogLevel.DEBUG)
async def ruff_lint(request: RuffLintRequest.Batch[RuffFieldSet, Any]) -> LintResult:
async def ruff_lint(request: RuffLintRequest.Batch[RuffFieldSet, Any], ruff: Ruff) -> LintResult:
source_files = await Get(
SourceFiles, SourceFilesRequest(field_set.source for field_set in request.elements)
)
result = await Get(
FallibleProcessResult,
result = await _run_ruff(
_RunRuffRequest(snapshot=source_files.snapshot, mode=RuffMode.LINT),
ruff,
)
return LintResult.create(request, result)


@rule(desc="Format with `ruff format`", level=LogLevel.DEBUG)
async def ruff_fmt(request: RuffFormatRequest.Batch, ruff: Ruff) -> FmtResult:
result = await Get(
FallibleProcessResult,
_RunRuffRequest(snapshot=request.snapshot, mode=RuffMode.FORMAT),
)
return await FmtResult.create(request, result)
return await _run_ruff_fmt(request, ruff)


def rules():
Expand Down
70 changes: 57 additions & 13 deletions src/python/pants/core/goals/update_build_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
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.ruff.register import RuffRequest
from pants.backend.build_files.fmt.yapf.register import YapfRequest
from pants.backend.python.goals import lockfile
from pants.backend.python.lint.black.rules import _run_black
from pants.backend.python.lint.black.subsystem import Black
from pants.backend.python.lint.ruff.rules import _run_ruff_fmt
from pants.backend.python.lint.ruff.subsystem import Ruff
from pants.backend.python.lint.yapf.rules import _run_yapf
from pants.backend.python.lint.yapf.subsystem import Yapf
from pants.backend.python.subsystems.python_tool_base import get_lockfile_interpreter_constraints
Expand Down Expand Up @@ -69,6 +72,7 @@ class RewrittenBuildFile:
class Formatter(Enum):
YAPF = "yapf"
BLACK = "black"
RUFF = "ruff"


@union(in_scope_types=[EnvironmentName])
Expand Down Expand Up @@ -139,12 +143,12 @@ def activated(cls, union_membership: UnionMembership) -> bool:
default=True,
help=softwrap(
"""
Format BUILD files using Black or Yapf.
Format BUILD files using Black, Ruff or Yapf.
Set `[black].args` / `[yapf].args`, `[black].config` / `[yapf].config` ,
and `[black].config_discovery` / `[yapf].config_discovery` to change
Black's or Yapf's behavior. Set
`[black].interpreter_constraints` / `[yapf].interpreter_constraints`
Set `[black].args` / `[ruff].args` / `[yapf].args`, `[black].config` / `[ruff].config`, `[yapf].config` ,
and `[black].config_discovery` / `[ruff].config_discovery`, `[yapf].config_discovery` to change
Black's, Ruff's, or Yapf's behavior. Set
`[black].interpreter_constraints` / `[ruff].interpreter_constraints` / `[yapf].interpreter_constraints`
and `[python].interpreter_search_path` to change which interpreter is
used to run the formatter.
"""
Expand Down Expand Up @@ -218,16 +222,21 @@ async def update_build_files(
)

rewrite_request_classes = []
formatter_to_request_class: dict[Formatter, type[RewrittenBuildFileRequest]] = {
Formatter.BLACK: FormatWithBlackRequest,
Formatter.YAPF: FormatWithYapfRequest,
Formatter.RUFF: FormatWithRuffRequest,
}
chosen_formatter_request_class = formatter_to_request_class.get(
update_build_files_subsystem.formatter
)
if not chosen_formatter_request_class:
raise ValueError(f"Unrecognized formatter: {update_build_files_subsystem.formatter}")

for request in union_membership[RewrittenBuildFileRequest]:
if issubclass(request, (FormatWithBlackRequest, FormatWithYapfRequest)):
is_chosen_formatter = issubclass(request, FormatWithBlackRequest) ^ (
update_build_files_subsystem.formatter == Formatter.YAPF
)
if update_build_files_subsystem.fmt and issubclass(request, chosen_formatter_request_class):
rewrite_request_classes.append(request)

if update_build_files_subsystem.fmt and is_chosen_formatter:
rewrite_request_classes.append(request)
else:
continue
if update_build_files_subsystem.fix_safe_deprecations or not issubclass(
request, DeprecationFixerRequest
):
Expand Down Expand Up @@ -367,6 +376,40 @@ async def format_build_file_with_black(
return RewrittenBuildFile(request.path, build_lines, change_descriptions=change_descriptions)


# ------------------------------------------------------------------------------------------
# Ruff formatter fixer
# ------------------------------------------------------------------------------------------


class FormatWithRuffRequest(RewrittenBuildFileRequest):
pass


@rule
async def format_build_file_with_ruff(
request: FormatWithRuffRequest, ruff: Ruff
) -> RewrittenBuildFile:
input_snapshot = await Get(Snapshot, CreateDigest([request.to_file_content()]))
ruff_ics = await get_lockfile_interpreter_constraints(ruff)
result = await _run_ruff_fmt(
RuffRequest.Batch(
Ruff.options_scope,
input_snapshot.files,
partition_metadata=None,
snapshot=input_snapshot,
),
ruff,
ruff_ics,
)
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 = ("Format with Ruff",) if result.did_change else ()

return RewrittenBuildFile(request.path, build_lines, change_descriptions=change_descriptions)


# ------------------------------------------------------------------------------------------
# Rename deprecated target types fixer
# ------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -436,4 +479,5 @@ def rules():
# after all our deprecation fixers.
UnionRule(RewrittenBuildFileRequest, FormatWithBlackRequest),
UnionRule(RewrittenBuildFileRequest, FormatWithYapfRequest),
UnionRule(RewrittenBuildFileRequest, FormatWithRuffRequest),
)
Loading

0 comments on commit 3129951

Please sign in to comment.