Skip to content

Commit

Permalink
Add yapf Python formatter (#12317)
Browse files Browse the repository at this point in the history
This PR adds support for [YAPF](https://github.com/google/yapf) Python formatter via a new plugin.
The `yapf` plugin code is based on the code adapted from the `isort` plugin with a few concepts borrowed from the `black` one.

Its behaviour in principle is identical to what one would expect from the `black` and `isort` -- it's used in the `lint` and `fmt` goals (unless skipped with the `--yapf-skip` flag), has support for passing a config file explicitly via the `--yapf-config` flag, and can receive the additional arguments to be passed to the `yapf` command via the `--yapf-args.` 

A few details that are either important or are specific to `yapf:`

* When run, `yapf` exits with exit code `0` if when no changes were necessary and non-zero otherwise. 
* When the `lint` rule runs, the flag `--diff` is used (corresponding to the `--check` flag for `black`) and it shows the difference between the original and formatted code in the terminal.

[ci skip-rust]
  • Loading branch information
alexey-tereshenkov-oxb authored Jul 15, 2021
1 parent 1e9fd75 commit 61e54a1
Show file tree
Hide file tree
Showing 8 changed files with 462 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/python/pants/backend/python/lint/yapf/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

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

"""Autoformatter for Python.
See https://www.pantsbuild.org/docs/python-linters-and-formatters and
https://github.com/google/yapf .
"""

from pants.backend.python.lint import python_fmt
from pants.backend.python.lint.yapf import rules as yapf_rules
from pants.backend.python.lint.yapf import skip_field


def rules():
return (*yapf_rules.rules(), *python_fmt.rules(), *skip_field.rules())
150 changes: 150 additions & 0 deletions src/python/pants/backend/python/lint/yapf/rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from dataclasses import dataclass
from typing import Tuple

from pants.backend.python.lint.python_fmt import PythonFmtRequest
from pants.backend.python.lint.yapf.skip_field import SkipYapfField
from pants.backend.python.lint.yapf.subsystem import Yapf
from pants.backend.python.target_types import PythonSources
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, PexRequirements, VenvPex, VenvPexProcess
from pants.core.goals.fmt import FmtResult
from pants.core.goals.lint import LintRequest, LintResult, LintResults
from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
from pants.engine.fs import Digest, MergeDigests
from pants.engine.process import FallibleProcessResult, Process, ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import FieldSet, Target
from pants.engine.unions import UnionRule
from pants.util.logging import LogLevel
from pants.util.strutil import pluralize


@dataclass(frozen=True)
class YapfFieldSet(FieldSet):
required_fields = (PythonSources,)

sources: PythonSources

@classmethod
def opt_out(cls, tgt: Target) -> bool:
return tgt.get(SkipYapfField).value


class YapfRequest(PythonFmtRequest, LintRequest):
field_set_type = YapfFieldSet


@dataclass(frozen=True)
class SetupRequest:
request: YapfRequest
check_only: bool


@dataclass(frozen=True)
class Setup:
process: Process
original_digest: Digest


def generate_argv(source_files: SourceFiles, yapf: Yapf, check_only: bool) -> Tuple[str, ...]:
args = [*yapf.args]
if check_only:
# If "--diff" is passed, yapf returns zero when no changes were necessary and
# non-zero otherwise
args.append("--diff")
else:
# The "--in-place" flag makes yapf to actually reformat files
args.append("--in-place")
if yapf.config:
args.extend(["--style", yapf.config])
args.extend(source_files.files)
return tuple(args)


@rule(level=LogLevel.DEBUG)
async def setup_yapf(setup_request: SetupRequest, yapf: Yapf) -> Setup:
yapf_pex_get = Get(
VenvPex,
PexRequest(
output_filename="yapf.pex",
internal_only=True,
requirements=PexRequirements(yapf.all_requirements),
interpreter_constraints=InterpreterConstraints(yapf.interpreter_constraints),
main=yapf.main,
),
)
source_files_get = Get(
SourceFiles,
SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets),
)
source_files, yapf_pex = await MultiGet(source_files_get, yapf_pex_get)

source_files_snapshot = (
source_files.snapshot
if setup_request.request.prior_formatter_result is None
else setup_request.request.prior_formatter_result
)

config_files = await Get(
ConfigFiles, ConfigFilesRequest, yapf.config_request(source_files_snapshot.dirs)
)

input_digest = await Get(
Digest, MergeDigests((source_files_snapshot.digest, config_files.snapshot.digest))
)

process = await Get(
Process,
VenvPexProcess(
yapf_pex,
argv=generate_argv(
source_files,
yapf,
check_only=setup_request.check_only,
),
input_digest=input_digest,
output_files=source_files_snapshot.files,
description=f"Run yapf on {pluralize(len(setup_request.request.field_sets), 'file')}.",
level=LogLevel.DEBUG,
),
)
return Setup(process, original_digest=source_files_snapshot.digest)


@rule(desc="Format with yapf", level=LogLevel.DEBUG)
async def yapf_fmt(request: YapfRequest, yapf: Yapf) -> FmtResult:
if yapf.skip:
return FmtResult.skip(formatter_name="yapf")
setup = await Get(Setup, SetupRequest(request, check_only=False))
result = await Get(ProcessResult, Process, setup.process)
return FmtResult.from_process_result(
result,
original_digest=setup.original_digest,
formatter_name="yapf",
)


@rule(desc="Lint with yapf", level=LogLevel.DEBUG)
async def yapf_lint(request: YapfRequest, yapf: Yapf) -> LintResults:
if yapf.skip:
return LintResults([], linter_name="yapf")
setup = await Get(Setup, SetupRequest(request, check_only=True))
result = await Get(FallibleProcessResult, Process, setup.process)
return LintResults(
[LintResult.from_fallible_process_result(result)],
linter_name="yapf",
)


def rules():
return [
*collect_rules(),
UnionRule(PythonFmtRequest, YapfRequest),
UnionRule(LintRequest, YapfRequest),
*pex.rules(),
]
166 changes: 166 additions & 0 deletions src/python/pants/backend/python/lint/yapf/rules_integration_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
# Copyright 2021 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.python.lint.yapf.rules import YapfFieldSet, YapfRequest
from pants.backend.python.lint.yapf.rules import rules as yapf_rules
from pants.backend.python.target_types import PythonLibrary
from pants.core.goals.fmt import FmtResult
from pants.core.goals.lint import LintResult, LintResults
from pants.core.util_rules import config_files, source_files
from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
from pants.engine.addresses import Address
from pants.engine.fs import CreateDigest, Digest, FileContent
from pants.engine.target import Target
from pants.testutil.rule_runner import QueryRule, RuleRunner


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*yapf_rules(),
*source_files.rules(),
*config_files.rules(),
QueryRule(LintResults, (YapfRequest,)),
QueryRule(FmtResult, (YapfRequest,)),
QueryRule(SourceFiles, (SourceFilesRequest,)),
],
target_types=[PythonLibrary],
)


GOOD_FILE = "data = []\n"
BAD_FILE = "data = { 'a':11,'b':22 }\n"
FIXED_BAD_FILE = "data = {'a': 11, 'b': 22}\n"

# Note the indentation is 6 spaces; after formatting it should
# become 2 or 4 spaces depending on the configuration
NEEDS_CONFIG_FILE = "def func():\n return 42\n"
FIXED_NEEDS_CONFIG_FILE_INDENT2 = "def func():\n return 42\n"
FIXED_NEEDS_CONFIG_FILE_INDENT4 = "def func():\n return 42\n"


def run_yapf(
rule_runner: RuleRunner,
targets: list[Target],
*,
extra_args: list[str] | None = None,
) -> tuple[tuple[LintResult, ...], FmtResult]:
rule_runner.set_options(
["--backend-packages=pants.backend.python.lint.yapf", *(extra_args or ())],
env_inherit={"PATH", "PYENV_ROOT", "HOME"},
)
field_sets = [YapfFieldSet.create(tgt) for tgt in targets]
lint_results = rule_runner.request(LintResults, [YapfRequest(field_sets)])
input_sources = rule_runner.request(
SourceFiles,
[
SourceFilesRequest(field_set.sources for field_set in field_sets),
],
)
fmt_result = rule_runner.request(
FmtResult,
[
YapfRequest(field_sets, prior_formatter_result=input_sources.snapshot),
],
)
return lint_results.results, fmt_result


def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
files = [FileContent(path, content.encode()) for path, content in source_files.items()]
return rule_runner.request(Digest, [CreateDigest(files)])


def test_passing_source(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_yapf(rule_runner, [tgt])
assert len(lint_results) == 1
assert lint_results[0].exit_code == 0
assert lint_results[0].stderr == ""
assert fmt_result.output == get_digest(rule_runner, {"f.py": GOOD_FILE})
assert fmt_result.did_change is False


def test_failing_source(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_yapf(rule_runner, [tgt])
assert len(lint_results) == 1
assert lint_results[0].exit_code == 1
assert all(msg in lint_results[0].stdout for msg in ("reformatted", "original"))
assert fmt_result.skipped is False
assert fmt_result.output == get_digest(rule_runner, {"f.py": FIXED_BAD_FILE})
assert fmt_result.did_change is True


def test_multiple_targets(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{"good.py": GOOD_FILE, "bad.py": BAD_FILE, "BUILD": "python_library(name='t')"}
)
tgts = [
rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")),
rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")),
]
lint_results, fmt_result = run_yapf(rule_runner, tgts)
assert len(lint_results) == 1
assert lint_results[0].exit_code == 1
assert all(msg in lint_results[0].stdout for msg in ("reformatted", "original", "bad.py"))
assert "good.py" not in lint_results[0].stdout
assert fmt_result.output == get_digest(
rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE}
)
assert fmt_result.did_change is True


@pytest.mark.parametrize(
"path,section,extra_args",
(
(".style.yapf", "style", []),
("custom.style", "style", ["--yapf-config=custom.style"]),
),
)
def test_config_file(
rule_runner: RuleRunner, path: str, section: str, extra_args: list[str]
) -> None:
rule_runner.write_files(
{
"f.py": NEEDS_CONFIG_FILE,
"BUILD": "python_library(name='t', interpreter_constraints=['==3.9.*'])",
path: f"[{section}]\nindent_width = 2\n",
}
)
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_yapf(rule_runner, [tgt], extra_args=extra_args)
assert len(lint_results) == 1
assert lint_results[0].exit_code == 1
assert all(msg in lint_results[0].stdout for msg in ("reformatted", "original", "f.py"))
assert fmt_result.output == get_digest(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE_INDENT2})
assert fmt_result.did_change is True


def test_passthrough_args(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": NEEDS_CONFIG_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_yapf(
rule_runner, [tgt], extra_args=["--yapf-args=--style='{indent_width: 4}'"]
)
assert len(lint_results) == 1
assert lint_results[0].exit_code == 1
assert all(msg in lint_results[0].stdout for msg in ("reformatted", "original", "f.py"))
assert fmt_result.output == get_digest(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE_INDENT4})
assert fmt_result.did_change is True


def test_skip(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_yapf(rule_runner, [tgt], extra_args=["--yapf-skip"])
assert not lint_results
assert fmt_result.skipped is True
assert fmt_result.did_change is False
18 changes: 18 additions & 0 deletions src/python/pants/backend/python/lint/yapf/skip_field.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.python.target_types import PythonLibrary, PythonTests
from pants.engine.target import BoolField


class SkipYapfField(BoolField):
alias = "skip_yapf"
default = False
help = "If true, don't run yapf on this target's code."


def rules():
return [
PythonLibrary.register_plugin_field(SkipYapfField),
PythonTests.register_plugin_field(SkipYapfField),
]
Loading

0 comments on commit 61e54a1

Please sign in to comment.