diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py index 1ad01e13f01..22625e4a3ee 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -9,7 +9,6 @@ ProtobufSourceField, ) from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.core.util_rules.system_binaries import ( @@ -21,7 +20,7 @@ from pants.engine.fs import Digest, MergeDigests from pants.engine.internals.native_engine import Snapshot from pants.engine.platform import Platform -from pants.engine.process import FallibleProcessResult, Process, ProcessResult +from pants.engine.process import 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 @@ -41,17 +40,11 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipBufFormatField).value -class BufFormatRequest(LintTargetsRequest, FmtRequest): +class BufFormatRequest(FmtRequest): field_set_type = BufFieldSet name = "buf-format" -@dataclass(frozen=True) -class SetupRequest: - request: BufFormatRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process @@ -59,7 +52,7 @@ class Setup: @rule(level=LogLevel.DEBUG) -async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Setup: +async def setup_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> Setup: diff_binary = await Get(DiffBinary, DiffBinaryRequest()) download_buf_get = Get( DownloadedExternalTool, ExternalToolRequest, buf.get_request(Platform.current) @@ -75,7 +68,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se ) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.sources for field_set in request.field_sets), ) downloaded_buf, binary_shims, source_files = await MultiGet( download_buf_get, binary_shims_get, source_files_get @@ -83,8 +76,8 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) input_digest = await Get( @@ -95,9 +88,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se argv = [ downloaded_buf.exe, "format", - # If linting, use `-d` to error with a diff and `--exit-code` to exit with a non-zero exit code if - # the file is not already formatted. Else, write the change with `-w`. - *(["-d", "--exit-code"] if setup_request.check_only else ["-w"]), + "-w", *buf.format_args, "--path", ",".join(source_files_snapshot.files), @@ -106,7 +97,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se argv=argv, input_digest=input_digest, output_files=source_files_snapshot.files, - description=f"Run buf format on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run buf format on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, env={ "PATH": binary_shims.bin_directory, @@ -119,7 +110,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se async def run_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> FmtResult: if buf.skip_format: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, BufFormatRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -131,18 +122,8 @@ async def run_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> FmtRes ) -@rule(desc="Lint with buf format", level=LogLevel.DEBUG) -async def run_buf_lint(request: BufFormatRequest, buf: BufSubsystem) -> LintResults: - if buf.skip_format: - return LintResults([], linter_name=request.name) - 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=request.name) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, BufFormatRequest), - UnionRule(LintTargetsRequest, BufFormatRequest), ] diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py index 905c48e74f1..d33aed54892 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py @@ -10,7 +10,6 @@ from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget from pants.backend.codegen.protobuf.target_types import rules as target_types_rules from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import config_files, external_tool, source_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address @@ -29,7 +28,6 @@ def rule_runner() -> RuleRunner: *external_tool.rules(), *source_files.rules(), *target_types_rules(), - QueryRule(LintResults, [BufFormatRequest]), QueryRule(FmtResult, [BufFormatRequest]), QueryRule(SourceFiles, [SourceFilesRequest]), ], @@ -46,7 +44,7 @@ def run_buf( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( [ "--backend-packages=pants.backend.codegen.protobuf.lint.buf", @@ -55,7 +53,6 @@ def run_buf( env_inherit={"PATH"}, ) field_sets = [BufFieldSet.create(tgt) for tgt in targets] - results = rule_runner.request(LintResults, [BufFormatRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -69,7 +66,7 @@ def run_buf( ], ) - return results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -81,11 +78,7 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stdout == "" - assert lint_results[0].stderr == "" + fmt_result = run_buf(rule_runner, [tgt]) assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) assert fmt_result.did_change is False @@ -94,10 +87,7 @@ def test_passing(rule_runner: RuleRunner) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 100 - assert "f.proto.orig" in lint_results[0].stdout + fmt_result = run_buf(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) assert fmt_result.did_change is True @@ -110,11 +100,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="good.proto")), rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.proto")), ] - lint_results, fmt_result = run_buf(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 100 - assert "bad.proto.orig" in lint_results[0].stdout - assert "good.proto" not in lint_results[0].stdout + fmt_result = run_buf(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.proto": GOOD_FILE, "bad.proto": GOOD_FILE} ) @@ -124,11 +110,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-args=--debug"]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stdout == "" - assert "DEBUG" in lint_results[0].stderr + fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-args=--debug"]) assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) assert fmt_result.did_change is False @@ -137,7 +119,6 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-skip"]) - assert not lint_results + fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/go/lint/gofmt/rules.py b/src/python/pants/backend/go/lint/gofmt/rules.py index 11d3f7a7f42..f1a64045852 100644 --- a/src/python/pants/backend/go/lint/gofmt/rules.py +++ b/src/python/pants/backend/go/lint/gofmt/rules.py @@ -3,7 +3,6 @@ from __future__ import annotations -import dataclasses import os.path from dataclasses import dataclass @@ -13,12 +12,11 @@ from pants.backend.go.subsystems.golang import GoRoot from pants.backend.go.target_types import GoPackageSourcesField from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest from pants.engine.internals.native_engine import Snapshot from pants.engine.internals.selectors import Get -from pants.engine.process import FallibleProcessResult, Process, ProcessResult +from pants.engine.process import Process, ProcessResult from pants.engine.rules import collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule @@ -42,12 +40,6 @@ class GofmtRequest(FmtRequest): name = GofmtSubsystem.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: GofmtRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process @@ -55,20 +47,20 @@ class Setup: @rule(level=LogLevel.DEBUG) -async def setup_gofmt(setup_request: SetupRequest, goroot: GoRoot) -> Setup: +async def setup_gofmt(request: GofmtRequest, goroot: GoRoot) -> Setup: source_files = await Get( SourceFiles, - SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.sources for field_set in request.field_sets), ) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) argv = ( os.path.join(goroot.path, "bin/gofmt"), - "-l" if setup_request.check_only else "-w", + "-w", *source_files_snapshot.files, ) process = Process( @@ -85,7 +77,7 @@ async def setup_gofmt(setup_request: SetupRequest, goroot: GoRoot) -> Setup: async def gofmt_fmt(request: GofmtRequest, gofmt: GofmtSubsystem) -> FmtResult: if gofmt.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, GofmtRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -97,28 +89,9 @@ async def gofmt_fmt(request: GofmtRequest, gofmt: GofmtSubsystem) -> FmtResult: ) -@rule(desc="Lint with gofmt", level=LogLevel.DEBUG) -async def gofmt_lint(request: GofmtRequest, gofmt: GofmtSubsystem) -> LintResults: - if gofmt.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - result = await Get(FallibleProcessResult, Process, setup.process) - lint_result = LintResult.from_fallible_process_result(result) - if lint_result.exit_code == 0 and lint_result.stdout.strip() != "": - # Note: gofmt returns success even if it would have reformatted the files. - # When this occurs, convert the LintResult into a failure. - lint_result = dataclasses.replace( - lint_result, - exit_code=1, - stdout=f"The following Go files require formatting:\n{lint_result.stdout}\n", - ) - return LintResults([lint_result], linter_name=request.name) - - def rules(): return [ *collect_rules(), *golang.rules(), UnionRule(FmtRequest, GofmtRequest), - UnionRule(LintTargetsRequest, GofmtRequest), ] diff --git a/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py b/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py index a41a35c62b5..d8a6bbec76c 100644 --- a/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py +++ b/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py @@ -21,7 +21,6 @@ third_party_pkg, ) from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import source_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address @@ -46,7 +45,6 @@ def rule_runner() -> RuleRunner: *build_pkg.rules(), *link.rules(), *assembly.rules(), - QueryRule(LintResults, (GofmtRequest,)), QueryRule(FmtResult, (GofmtRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -110,10 +108,9 @@ def run_gofmt( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options(extra_args or (), env_inherit={"PATH"}) field_sets = [GofmtFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [GofmtRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -126,7 +123,7 @@ def run_gofmt( GofmtRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -140,10 +137,7 @@ def test_passing(rule_runner: RuleRunner) -> None: {"f.go": GOOD_FILE, "go.mod": GO_MOD, "BUILD": "go_mod(name='mod')\ngo_package(name='pkg')"} ) tgt = rule_runner.get_target(Address("", target_name="pkg")) - lint_results, fmt_result = run_gofmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" + fmt_result = run_gofmt(rule_runner, [tgt]) assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.go": GOOD_FILE}) assert fmt_result.did_change is False @@ -154,10 +148,7 @@ def test_failing(rule_runner: RuleRunner) -> None: {"f.go": BAD_FILE, "go.mod": GO_MOD, "BUILD": "go_mod(name='mod')\ngo_package(name='pkg')"} ) tgt = rule_runner.get_target(Address("", target_name="pkg")) - lint_results, fmt_result = run_gofmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.go" in lint_results[0].stdout + fmt_result = run_gofmt(rule_runner, [tgt]) assert fmt_result.stderr == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.go": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -173,11 +164,7 @@ def test_mixed_sources(rule_runner: RuleRunner) -> None: } ) tgt = rule_runner.get_target(Address("", target_name="pkg")) - lint_results, fmt_result = run_gofmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "bad.go" in lint_results[0].stdout - assert "good.go" not in lint_results[0].stdout + fmt_result = run_gofmt(rule_runner, [tgt]) assert fmt_result.output == get_snapshot( rule_runner, {"good.go": GOOD_FILE, "bad.go": FIXED_BAD_FILE} ) @@ -196,11 +183,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: } ) tgts = [rule_runner.get_target(Address("good")), rule_runner.get_target(Address("bad"))] - lint_results, fmt_result = run_gofmt(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "bad/f.go" in lint_results[0].stdout - assert "good/f.go" not in lint_results[0].stdout + fmt_result = run_gofmt(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good/f.go": GOOD_FILE, "bad/f.go": FIXED_BAD_FILE} ) @@ -212,7 +195,6 @@ def test_skip(rule_runner: RuleRunner) -> None: {"f.go": BAD_FILE, "go.mod": GO_MOD, "BUILD": "go_mod(name='mod')\ngo_package(name='pkg')"} ) tgt = rule_runner.get_target(Address("", target_name="pkg")) - lint_results, fmt_result = run_gofmt(rule_runner, [tgt], extra_args=["--gofmt-skip"]) - assert not lint_results + fmt_result = run_gofmt(rule_runner, [tgt], extra_args=["--gofmt-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/java/lint/google_java_format/rules.py b/src/python/pants/backend/java/lint/google_java_format/rules.py index 9ee91e7377e..92ce2de5383 100644 --- a/src/python/pants/backend/java/lint/google_java_format/rules.py +++ b/src/python/pants/backend/java/lint/google_java_format/rules.py @@ -1,6 +1,5 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -import dataclasses import logging from dataclasses import dataclass @@ -9,12 +8,11 @@ from pants.backend.java.target_types import JavaSourceField from pants.core.goals.fmt import FmtRequest, FmtResult from pants.core.goals.generate_lockfiles import GenerateToolLockfileSentinel -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest from pants.engine.internals.native_engine import Snapshot from pants.engine.internals.selectors import Get, MultiGet -from pants.engine.process import FallibleProcessResult, ProcessResult +from pants.engine.process import ProcessResult from pants.engine.rules import collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule @@ -39,7 +37,7 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipGoogleJavaFormatField).value -class GoogleJavaFormatRequest(FmtRequest, LintTargetsRequest): +class GoogleJavaFormatRequest(FmtRequest): field_set_type = GoogleJavaFormatFieldSet name = GoogleJavaFormatSubsystem.options_scope @@ -48,12 +46,6 @@ class GoogleJavaFormatToolLockfileSentinel(GenerateToolLockfileSentinel): resolve_name = GoogleJavaFormatSubsystem.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: GoogleJavaFormatRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: JvmProcess @@ -62,7 +54,7 @@ class Setup: @rule(level=LogLevel.DEBUG) async def setup_google_java_format( - setup_request: SetupRequest, + request: GoogleJavaFormatRequest, tool: GoogleJavaFormatSubsystem, jdk: InternalJdk, ) -> Setup: @@ -73,15 +65,15 @@ async def setup_google_java_format( source_files, tool_classpath = await MultiGet( Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ), Get(ToolClasspath, ToolClasspathRequest(lockfile=lockfile_request)), ) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) toolcp_relpath = "__toolcp" @@ -103,8 +95,8 @@ async def setup_google_java_format( *maybe_java11_or_higher_options, "com.google.googlejavaformat.java.Main", *(["--aosp"] if tool.aosp else []), - "--dry-run" if setup_request.check_only else "--replace", - *source_files.files, + "--replace", + *source_files_snapshot.files, ] process = JvmProcess( @@ -115,7 +107,7 @@ async def setup_google_java_format( extra_immutable_input_digests=extra_immutable_input_digests, extra_nailgun_keys=extra_immutable_input_digests, output_files=source_files_snapshot.files, - description=f"Run Google Java Format on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run Google Java Format on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ) @@ -128,7 +120,7 @@ async def google_java_format_fmt( ) -> FmtResult: if tool.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, GoogleJavaFormatRequest, request) result = await Get(ProcessResult, JvmProcess, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -140,26 +132,6 @@ async def google_java_format_fmt( ) -@rule(desc="Lint with Google Java Format", level=LogLevel.DEBUG) -async def google_java_format_lint( - request: GoogleJavaFormatRequest, tool: GoogleJavaFormatSubsystem -) -> LintResults: - if tool.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - result = await Get(FallibleProcessResult, JvmProcess, setup.process) - lint_result = LintResult.from_fallible_process_result(result) - if lint_result.exit_code == 0 and lint_result.stdout.strip() != "": - # Note: The formetter returns success even if it would have reformatted the files. - # When this occurs, convert the LintResult into a failure. - lint_result = dataclasses.replace( - lint_result, - exit_code=1, - stdout=f"The following Java files require formatting:\n{lint_result.stdout}\n", - ) - return LintResults([lint_result], linter_name=request.name) - - @rule def generate_google_java_format_lockfile_request( _: GoogleJavaFormatToolLockfileSentinel, tool: GoogleJavaFormatSubsystem @@ -172,6 +144,5 @@ def rules(): *collect_rules(), *jvm_tool.rules(), UnionRule(FmtRequest, GoogleJavaFormatRequest), - UnionRule(LintTargetsRequest, GoogleJavaFormatRequest), UnionRule(GenerateToolLockfileSentinel, GoogleJavaFormatToolLockfileSentinel), ] diff --git a/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py b/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py index a8298ee9588..d86edc3d134 100644 --- a/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py +++ b/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py @@ -15,7 +15,6 @@ from pants.backend.java.target_types import rules as target_types_rules from pants.build_graph.address import Address from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import config_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import CreateDigest, Digest, FileContent @@ -45,7 +44,6 @@ def rule_runner() -> RuleRunner: *target_types_rules(), *gjf_fmt_rules.rules(), *skip_field.rules(), - QueryRule(LintResults, (GoogleJavaFormatRequest,)), QueryRule(FmtResult, (GoogleJavaFormatRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -83,11 +81,8 @@ def rule_runner() -> RuleRunner: """ -def run_google_java_format( - rule_runner: RuleRunner, targets: list[Target] -) -> tuple[tuple[LintResult, ...], FmtResult]: +def run_google_java_format(rule_runner: RuleRunner, targets: list[Target]) -> FmtResult: field_sets = [GoogleJavaFormatFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [GoogleJavaFormatRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -100,7 +95,7 @@ def run_google_java_format( GoogleJavaFormatRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -112,9 +107,7 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"Foo.java": GOOD_FILE, "BUILD": "java_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="Foo.java")) - lint_results, fmt_result = run_google_java_format(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 + fmt_result = run_google_java_format(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"Foo.java": GOOD_FILE}) assert fmt_result.did_change is False @@ -122,10 +115,7 @@ def test_passing(rule_runner: RuleRunner) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"Bar.java": BAD_FILE, "BUILD": "java_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="Bar.java")) - lint_results, fmt_result = run_google_java_format(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "The following Java files require formatting:\nBar.java\n\n" == lint_results[0].stdout + fmt_result = run_google_java_format(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"Bar.java": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -138,10 +128,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="Foo.java")), rule_runner.get_target(Address("", target_name="t", relative_file_path="Bar.java")), ] - lint_results, fmt_result = run_google_java_format(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "The following Java files require formatting:\nBar.java\n\n" == lint_results[0].stdout + fmt_result = run_google_java_format(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"Foo.java": GOOD_FILE, "Bar.java": FIXED_BAD_FILE} ) diff --git a/src/python/pants/backend/python/lint/autoflake/rules.py b/src/python/pants/backend/python/lint/autoflake/rules.py index 3f31f8c59eb..f4050116e22 100644 --- a/src/python/pants/backend/python/lint/autoflake/rules.py +++ b/src/python/pants/backend/python/lint/autoflake/rules.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import Tuple from pants.backend.python.lint.autoflake.skip_field import SkipAutoflakeField from pants.backend.python.lint.autoflake.subsystem import Autoflake @@ -10,12 +9,12 @@ from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest from pants.engine.internals.native_engine import Snapshot -from pants.engine.process import FallibleProcessResult, Process, ProcessResult -from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.internals.selectors import MultiGet +from pants.engine.process import Process, ProcessResult +from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule from pants.util.logging import LogLevel @@ -34,61 +33,46 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipAutoflakeField).value -class AutoflakeRequest(FmtRequest, LintTargetsRequest): +class AutoflakeRequest(FmtRequest): field_set_type = AutoflakeFieldSet name = Autoflake.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: AutoflakeRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process original_snapshot: Snapshot -def generate_argv( - source_files: SourceFiles, autoflake: Autoflake, *, check_only: bool -) -> Tuple[str, ...]: - args = [] - if check_only: - args.append("--check") - else: - args.append("--in-place") - args.append("--remove-all-unused-imports") - args.extend(autoflake.args) - args.extend(source_files.files) - return tuple(args) - - @rule(level=LogLevel.DEBUG) -async def setup_autoflake(setup_request: SetupRequest, autoflake: Autoflake) -> Setup: +async def setup_autoflake(request: AutoflakeRequest, autoflake: Autoflake) -> Setup: autoflake_pex_get = Get(VenvPex, PexRequest, autoflake.to_pex_request()) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ) source_files, autoflake_pex = await MultiGet(source_files_get, autoflake_pex_get) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) process = await Get( Process, VenvPexProcess( autoflake_pex, - argv=generate_argv(source_files, autoflake, check_only=setup_request.check_only), + argv=( + "--in-place", + "--remove-all-unused-imports", + *autoflake.args, + *source_files_snapshot.files, + ), input_digest=source_files_snapshot.digest, output_files=source_files_snapshot.files, - description=f"Run Autoflake on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run Autoflake on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ), ) @@ -99,7 +83,7 @@ async def setup_autoflake(setup_request: SetupRequest, autoflake: Autoflake) -> async def autoflake_fmt(request: AutoflakeRequest, autoflake: Autoflake) -> FmtResult: if autoflake.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, AutoflakeRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -111,32 +95,9 @@ async def autoflake_fmt(request: AutoflakeRequest, autoflake: Autoflake) -> FmtR ) -@rule(desc="Lint with autoflake", level=LogLevel.DEBUG) -async def autoflake_lint(request: AutoflakeRequest, autoflake: Autoflake) -> LintResults: - if autoflake.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - result = await Get(FallibleProcessResult, Process, setup.process) - - def strip_check_result(output: str) -> str: - return "\n".join(line for line in output.splitlines() if line != "No issues detected!") - - return LintResults( - [ - LintResult( - result.exit_code, - strip_check_result(result.stdout.decode()), - result.stderr.decode(), - ) - ], - linter_name=request.name, - ) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, AutoflakeRequest), - UnionRule(LintTargetsRequest, AutoflakeRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py b/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py index 8504064f4d3..8f2d7b4ca75 100644 --- a/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.lint.autoflake.subsystem import rules as autoflake_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget 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 @@ -32,7 +31,6 @@ def rule_runner() -> RuleRunner: *source_files.rules(), *config_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (AutoflakeRequest,)), QueryRule(FmtResult, (AutoflakeRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -50,13 +48,12 @@ def run_autoflake( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( ["--backend-packages=pants.backend.python.lint.autoflake", *(extra_args or ())], env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) field_sets = [AutoflakeFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [AutoflakeRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -69,7 +66,7 @@ def run_autoflake( AutoflakeRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -86,14 +83,11 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_autoflake( + fmt_result = run_autoflake( rule_runner, [tgt], extra_args=[f"--autoflake-interpreter-constraints=['=={major_minor_interpreter}.*']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE}) assert fmt_result.did_change is False @@ -102,10 +96,7 @@ def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) - def test_failing_source(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_autoflake(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.py: Unused imports/variables detected" in lint_results[0].stdout + fmt_result = run_autoflake(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -118,11 +109,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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_autoflake(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "bad.py: Unused imports/variables detected" in lint_results[0].stdout - assert "good.py" not in lint_results[0].stderr + fmt_result = run_autoflake(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE} ) @@ -132,8 +119,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_autoflake(rule_runner, [tgt], extra_args=["--autoflake-skip"]) - assert not lint_results + fmt_result = run_autoflake(rule_runner, [tgt], extra_args=["--autoflake-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False @@ -153,9 +139,8 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="good.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")), ] - lint_results, fmt_result = run_autoflake(rule_runner, good_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" and fmt_result.stdout == "" + fmt_result = run_autoflake(rule_runner, good_tgts) + assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "good.pyi": GOOD_FILE} ) @@ -165,12 +150,7 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")), ] - lint_results, fmt_result = run_autoflake(rule_runner, bad_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 1 - # Note that we can't be specific about the file in this output check. For some reason - # autoflake non-deterministically outputs only one the files that needed changes, not - # all of them. - assert "Unused imports/variables detected" in lint_results[0].stdout + fmt_result = run_autoflake(rule_runner, bad_tgts) assert fmt_result.output == get_snapshot( rule_runner, {"bad.py": FIXED_BAD_FILE, "bad.pyi": FIXED_BAD_FILE} ) diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py index 4b57604c8c4..75ffa4d2799 100644 --- a/src/python/pants/backend/python/lint/black/rules.py +++ b/src/python/pants/backend/python/lint/black/rules.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import Tuple from pants.backend.python.lint.black.skip_field import SkipBlackField from pants.backend.python.lint.black.subsystem import Black @@ -12,13 +11,13 @@ 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.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest 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.internals.native_engine import Snapshot -from pants.engine.process import FallibleProcessResult, Process, ProcessResult -from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.internals.selectors import MultiGet +from pants.engine.process import Process, ProcessResult +from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule from pants.util.logging import LogLevel @@ -37,46 +36,26 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipBlackField).value -class BlackRequest(FmtRequest, LintTargetsRequest): +class BlackRequest(FmtRequest): field_set_type = BlackFieldSet name = Black.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: BlackRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process original_snapshot: Snapshot -def generate_argv(source_files: SourceFiles, black: Black, *, check_only: bool) -> Tuple[str, ...]: - args = [] - if check_only: - args.append("--check") - if black.config: - args.extend(["--config", black.config]) - args.extend(["-W", "{pants_concurrency}"]) - args.extend(black.args) - args.extend(source_files.files) - return tuple(args) - - @rule(level=LogLevel.DEBUG) -async def setup_black( - setup_request: SetupRequest, black: Black, python_setup: PythonSetup -) -> Setup: +async def setup_black(request: BlackRequest, black: Black, python_setup: PythonSetup) -> Setup: # Black requires 3.6+ but uses the typed-ast library to work with 2.7, 3.4, 3.5, 3.6, and 3.7. # However, typed-ast does not understand 3.8+, so instead we must run Black with Python 3.8+ # when relevant. We only do this if if <3.8 can't be used, as we don't want a loose requirement # like `>=3.6` to result in requiring Python 3.8, which would error if 3.8 is not installed on # the machine. all_interpreter_constraints = InterpreterConstraints.create_from_compatibility_fields( - (field_set.interpreter_constraints for field_set in setup_request.request.field_sets), + (field_set.interpreter_constraints for field_set in request.field_sets), python_setup, ) tool_interpreter_constraints = ( @@ -98,14 +77,14 @@ async def setup_black( source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ) source_files, black_pex = await MultiGet(source_files_get, black_pex_get) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) config_files = await Get( @@ -119,11 +98,17 @@ async def setup_black( Process, VenvPexProcess( black_pex, - argv=generate_argv(source_files, black, check_only=setup_request.check_only), + argv=( + *(("--config", black.config) if black.config else ()), + "-W", + "{pants_concurrency}", + *black.args, + *source_files_snapshot.files, + ), input_digest=input_digest, output_files=source_files_snapshot.files, - concurrency_available=len(setup_request.request.field_sets), - description=f"Run Black on {pluralize(len(setup_request.request.field_sets), 'file')}.", + concurrency_available=len(request.field_sets), + description=f"Run Black on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ), ) @@ -134,7 +119,7 @@ async def setup_black( async def black_fmt(request: BlackRequest, black: Black) -> FmtResult: if black.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, BlackRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -146,22 +131,9 @@ async def black_fmt(request: BlackRequest, black: Black) -> FmtResult: ) -@rule(desc="Lint with Black", level=LogLevel.DEBUG) -async def black_lint(request: BlackRequest, black: Black) -> LintResults: - if black.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - result = await Get(FallibleProcessResult, Process, setup.process) - return LintResults( - [LintResult.from_fallible_process_result(result, strip_chroot_path=True)], - linter_name=request.name, - ) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, BlackRequest), - UnionRule(LintTargetsRequest, BlackRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/black/rules_integration_test.py b/src/python/pants/backend/python/lint/black/rules_integration_test.py index 2fe03561a67..3d0990784ff 100644 --- a/src/python/pants/backend/python/lint/black/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/black/rules_integration_test.py @@ -14,7 +14,6 @@ from pants.backend.python.lint.black.subsystem import rules as black_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget 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 @@ -38,7 +37,6 @@ def rule_runner() -> RuleRunner: *source_files.rules(), *config_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (BlackRequest,)), QueryRule(FmtResult, (BlackRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -54,7 +52,7 @@ def rule_runner() -> RuleRunner: def run_black( rule_runner: RuleRunner, targets: list[Target], *, extra_args: list[str] | None = None -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( ["--backend-packages=pants.backend.python.lint.black", *(extra_args or ())], # We propagate LANG and LC_ALL to satisfy click, which black depends upon. Without this we @@ -73,7 +71,6 @@ def run_black( env_inherit={"PATH", "PYENV_ROOT", "HOME", "LANG", "LC_ALL"}, ) field_sets = [BlackFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [BlackRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -86,7 +83,7 @@ def run_black( BlackRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -106,14 +103,11 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: interpreter_constraint = ( ">=3.6.2,<3.7" if major_minor_interpreter == "3.6" else f"=={major_minor_interpreter}.*" ) - lint_results, fmt_result = run_black( + fmt_result = run_black( rule_runner, [tgt], extra_args=[f"--black-interpreter-constraints=['{interpreter_constraint}']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert "1 file would be left unchanged" in lint_results[0].stderr assert "1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE}) assert fmt_result.did_change is False @@ -122,10 +116,7 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "1 file would be reformatted" in lint_results[0].stderr + fmt_result = run_black(rule_runner, [tgt]) assert "1 file reformatted" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -139,10 +130,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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_black(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "1 file would be reformatted, 1 file would be left unchanged" in lint_results[0].stderr + fmt_result = run_black(rule_runner, tgts) assert "1 file reformatted, 1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE} @@ -163,10 +151,7 @@ def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list } ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black(rule_runner, [tgt], extra_args=extra_args) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert "1 file would be left unchanged" in lint_results[0].stderr + fmt_result = run_black(rule_runner, [tgt], extra_args=extra_args) assert "1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": NEEDS_CONFIG_FILE}) assert fmt_result.did_change is False @@ -175,12 +160,9 @@ def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": NEEDS_CONFIG_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black( + fmt_result = run_black( rule_runner, [tgt], extra_args=["--black-args='--skip-string-normalization'"] ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert "1 file would be left unchanged" in lint_results[0].stderr assert "1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": NEEDS_CONFIG_FILE}) assert fmt_result.did_change is False @@ -189,8 +171,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black(rule_runner, [tgt], extra_args=["--black-skip"]) - assert not lint_results + fmt_result = run_black(rule_runner, [tgt], extra_args=["--black-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False @@ -216,10 +197,7 @@ class Foo: {"f.py": content, "BUILD": "python_sources(name='t', interpreter_constraints=['>=3.8'])"} ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert "1 file would be left unchanged" in lint_results[0].stderr + fmt_result = run_black(rule_runner, [tgt]) assert "1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": content}) assert fmt_result.did_change is False @@ -240,10 +218,7 @@ def replaced(x: bool) -> str: {"f.py": content, "BUILD": "python_sources(name='t', interpreter_constraints=['>=3.9'])"} ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_black(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert "1 file would be left unchanged" in lint_results[0].stderr + fmt_result = run_black(rule_runner, [tgt]) assert "1 file left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot(rule_runner, {"f.py": content}) assert fmt_result.did_change is False @@ -264,12 +239,8 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="good.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")), ] - lint_results, fmt_result = run_black(rule_runner, good_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 0 - assert ( - "2 files would be left unchanged" in lint_results[0].stderr - and "2 files left unchanged" in fmt_result.stderr - ) + fmt_result = run_black(rule_runner, good_tgts) + assert "2 files left unchanged" in fmt_result.stderr assert fmt_result.output == get_snapshot( rule_runner, {"good.pyi": GOOD_FILE, "good.py": GOOD_FILE} ) @@ -279,12 +250,8 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")), ] - lint_results, fmt_result = run_black(rule_runner, bad_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 1 - assert ( - "2 files would be reformatted" in lint_results[0].stderr - and "2 files reformatted" in fmt_result.stderr - ) + fmt_result = run_black(rule_runner, bad_tgts) + assert "2 files reformatted" in fmt_result.stderr assert fmt_result.output == get_snapshot( rule_runner, {"bad.pyi": FIXED_BAD_FILE, "bad.py": FIXED_BAD_FILE} ) diff --git a/src/python/pants/backend/python/lint/docformatter/rules.py b/src/python/pants/backend/python/lint/docformatter/rules.py index 2e3e497716f..65ab81fa61a 100644 --- a/src/python/pants/backend/python/lint/docformatter/rules.py +++ b/src/python/pants/backend/python/lint/docformatter/rules.py @@ -10,12 +10,12 @@ from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest from pants.engine.internals.native_engine import Snapshot -from pants.engine.process import FallibleProcessResult, Process, ProcessResult -from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.internals.selectors import MultiGet +from pants.engine.process import Process, ProcessResult +from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule from pants.util.logging import LogLevel @@ -33,17 +33,11 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipDocformatterField).value -class DocformatterRequest(FmtRequest, LintTargetsRequest): +class DocformatterRequest(FmtRequest): field_set_type = DocformatterFieldSet name = Docformatter.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: DocformatterRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process @@ -57,35 +51,31 @@ def generate_args( @rule(level=LogLevel.DEBUG) -async def setup_docformatter(setup_request: SetupRequest, docformatter: Docformatter) -> Setup: - +async def setup_docformatter(request: DocformatterRequest, docformatter: Docformatter) -> Setup: docformatter_pex_get = Get(VenvPex, PexRequest, docformatter.to_pex_request()) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ) source_files, docformatter_pex = await MultiGet(source_files_get, docformatter_pex_get) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) - process = await Get( Process, VenvPexProcess( docformatter_pex, - argv=generate_args( - source_files=source_files, - docformatter=docformatter, - check_only=setup_request.check_only, + argv=( + "--in-place", + *docformatter.args, + *source_files_snapshot.files, ), input_digest=source_files_snapshot.digest, output_files=source_files_snapshot.files, - description=( - f"Run Docformatter on {pluralize(len(setup_request.request.field_sets), 'file')}." - ), + description=(f"Run Docformatter on {pluralize(len(request.field_sets), 'file')}."), level=LogLevel.DEBUG, ), ) @@ -96,7 +86,7 @@ async def setup_docformatter(setup_request: SetupRequest, docformatter: Docforma async def docformatter_fmt(request: DocformatterRequest, docformatter: Docformatter) -> FmtResult: if docformatter.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, DocformatterRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -108,21 +98,9 @@ async def docformatter_fmt(request: DocformatterRequest, docformatter: Docformat ) -@rule(desc="Lint with docformatter", level=LogLevel.DEBUG) -async def docformatter_lint( - request: DocformatterRequest, docformatter: Docformatter -) -> LintResults: - if docformatter.skip: - return LintResults([], linter_name=request.name) - 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=request.name) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, DocformatterRequest), - UnionRule(LintTargetsRequest, DocformatterRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py b/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py index f535dc83e76..d8ec4686abe 100644 --- a/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/docformatter/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.lint.docformatter.subsystem import rules as docformatter_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import source_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address @@ -31,7 +30,6 @@ def rule_runner() -> RuleRunner: *docformatter_subsystem_rules(), *source_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (DocformatterRequest,)), QueryRule(FmtResult, (DocformatterRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -46,13 +44,12 @@ def rule_runner() -> RuleRunner: def run_docformatter( rule_runner: RuleRunner, targets: list[Target], *, extra_args: list[str] | None = None -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( ["--backend-packages=pants.backend.python.lint.docformatter", *(extra_args or ())], env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) field_sets = [DocformatterFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [DocformatterRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -65,7 +62,7 @@ def run_docformatter( DocformatterRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -82,14 +79,11 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_docformatter( + fmt_result = run_docformatter( rule_runner, [tgt], extra_args=[f"--docformatter-interpreter-constraints=['=={major_minor_interpreter}.*']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE}) assert fmt_result.did_change is False @@ -97,10 +91,7 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_docformatter(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 3 - assert lint_results[0].stderr.strip() == "f.py" + fmt_result = run_docformatter(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -113,10 +104,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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_docformatter(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 3 - assert lint_results[0].stderr.strip() == "bad.py" + fmt_result = run_docformatter(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE} ) @@ -127,14 +115,11 @@ def test_respects_passthrough_args(rule_runner: RuleRunner) -> None: content = '"""\nOne line docstring acting like it\'s multiline.\n"""\n' rule_runner.write_files({"f.py": content, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_docformatter( + fmt_result = run_docformatter( rule_runner, [tgt], extra_args=["--docformatter-args='--make-summary-multi-line'"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": content}) assert fmt_result.did_change is False @@ -142,10 +127,7 @@ def test_respects_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_docformatter( - rule_runner, [tgt], extra_args=["--docformatter-skip"] - ) - assert not lint_results + fmt_result = run_docformatter(rule_runner, [tgt], extra_args=["--docformatter-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False @@ -156,15 +138,11 @@ def test_stub_files(rule_runner: RuleRunner) -> None: ) good_tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="good.pyi")) - lint_results, fmt_result = run_docformatter(rule_runner, [good_tgt]) - assert len(lint_results) == 1 and lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" and fmt_result.stdout == "" + fmt_result = run_docformatter(rule_runner, [good_tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"good.pyi": GOOD_FILE}) assert not fmt_result.did_change bad_tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.pyi")) - lint_results, fmt_result = run_docformatter(rule_runner, [bad_tgt]) - assert len(lint_results) == 1 and lint_results[0].exit_code == 3 - assert "bad.pyi" in lint_results[0].stderr + fmt_result = run_docformatter(rule_runner, [bad_tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"bad.pyi": FIXED_BAD_FILE}) assert fmt_result.did_change diff --git a/src/python/pants/backend/python/lint/isort/rules.py b/src/python/pants/backend/python/lint/isort/rules.py index 53a3d7a9d2c..845f4d89bfb 100644 --- a/src/python/pants/backend/python/lint/isort/rules.py +++ b/src/python/pants/backend/python/lint/isort/rules.py @@ -1,5 +1,6 @@ # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations from dataclasses import dataclass from typing import Tuple @@ -10,12 +11,11 @@ from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import PexRequest, PexResolveInfo, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest 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.internals.native_engine import Snapshot -from pants.engine.process import FallibleProcessResult, Process, ProcessResult +from pants.engine.process import 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 @@ -34,17 +34,11 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipIsortField).value -class IsortRequest(FmtRequest, LintTargetsRequest): +class IsortRequest(FmtRequest): field_set_type = IsortFieldSet name = Isort.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: IsortRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process @@ -52,11 +46,9 @@ class Setup: def generate_argv( - source_files: SourceFiles, isort: Isort, *, is_isort5: bool, check_only: bool + source_files: tuple[str, ...], isort: Isort, *, is_isort5: bool ) -> Tuple[str, ...]: args = [*isort.args] - if check_only: - args.append("--check-only") if is_isort5 and len(isort.config) == 1: explicitly_configured_config_args = [ arg @@ -72,23 +64,23 @@ def generate_argv( # `[isort].config` to be a string rather than list[str] option. if not explicitly_configured_config_args: args.append(f"--settings={isort.config[0]}") - args.extend(source_files.files) + args.extend(source_files) return tuple(args) @rule(level=LogLevel.DEBUG) -async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup: +async def setup_isort(request: IsortRequest, isort: Isort) -> Setup: isort_pex_get = Get(VenvPex, PexRequest, isort.to_pex_request()) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ) source_files, isort_pex = await MultiGet(source_files_get, isort_pex_get) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) config_files = await Get( @@ -112,12 +104,10 @@ async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup: Process, VenvPexProcess( isort_pex, - argv=generate_argv( - source_files, isort, is_isort5=is_isort5, check_only=setup_request.check_only - ), + argv=generate_argv(source_files_snapshot.files, isort, is_isort5=is_isort5), input_digest=input_digest, output_files=source_files_snapshot.files, - description=f"Run isort on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run isort on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ), ) @@ -128,7 +118,7 @@ async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup: async def isort_fmt(request: IsortRequest, isort: Isort) -> FmtResult: if isort.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, IsortRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -140,22 +130,9 @@ async def isort_fmt(request: IsortRequest, isort: Isort) -> FmtResult: ) -@rule(desc="Lint with isort", level=LogLevel.DEBUG) -async def isort_lint(request: IsortRequest, isort: Isort) -> LintResults: - if isort.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - result = await Get(FallibleProcessResult, Process, setup.process) - return LintResults( - [LintResult.from_fallible_process_result(result, strip_chroot_path=True)], - linter_name=request.name, - ) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, IsortRequest), - UnionRule(LintTargetsRequest, IsortRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/isort/rules_integration_test.py b/src/python/pants/backend/python/lint/isort/rules_integration_test.py index cb336662a96..33c107ebe7d 100644 --- a/src/python/pants/backend/python/lint/isort/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/isort/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.lint.isort.subsystem import rules as isort_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget 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 @@ -32,7 +31,6 @@ def rule_runner() -> RuleRunner: *source_files.rules(), *config_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (IsortRequest,)), QueryRule(FmtResult, (IsortRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -54,13 +52,12 @@ def run_isort( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( ["--backend-packages=pants.backend.python.lint.isort", *(extra_args or ())], env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) field_sets = [IsortFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [IsortRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -73,7 +70,7 @@ def run_isort( IsortRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -90,14 +87,11 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_isort( + fmt_result = run_isort( rule_runner, [tgt], extra_args=[f"--isort-interpreter-constraints=['=={major_minor_interpreter}.*']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE}) assert fmt_result.did_change is False @@ -106,10 +100,7 @@ def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) - def test_failing_source(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_isort(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.py Imports are incorrectly sorted" in lint_results[0].stderr + fmt_result = run_isort(rule_runner, [tgt]) assert fmt_result.stdout == "Fixing f.py\n" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -123,11 +114,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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_isort(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "bad.py Imports are incorrectly sorted" in lint_results[0].stderr - assert "good.py" not in lint_results[0].stderr + fmt_result = run_isort(rule_runner, tgts) assert "Fixing bad.py\n" == fmt_result.stdout assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE} @@ -147,10 +134,7 @@ def test_config_file(rule_runner: RuleRunner, path: str, extra_args: list[str]) } ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_isort(rule_runner, [tgt], extra_args=extra_args) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.py Imports are incorrectly sorted" in lint_results[0].stderr + fmt_result = run_isort(rule_runner, [tgt], extra_args=extra_args) assert fmt_result.stdout == "Fixing f.py\n" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE}) assert fmt_result.did_change is True @@ -159,12 +143,7 @@ def test_config_file(rule_runner: RuleRunner, path: str, extra_args: list[str]) def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": NEEDS_CONFIG_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_isort( - rule_runner, [tgt], extra_args=["--isort-args='--combine-as'"] - ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.py Imports are incorrectly sorted" in lint_results[0].stderr + fmt_result = run_isort(rule_runner, [tgt], extra_args=["--isort-args='--combine-as'"]) assert fmt_result.stdout == "Fixing f.py\n" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE}) assert fmt_result.did_change is True @@ -173,8 +152,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_isort(rule_runner, [tgt], extra_args=["--isort-skip"]) - assert not lint_results + fmt_result = run_isort(rule_runner, [tgt], extra_args=["--isort-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False @@ -194,9 +172,7 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="good.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")), ] - lint_results, fmt_result = run_isort(rule_runner, good_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" and fmt_result.stdout == "" + fmt_result = run_isort(rule_runner, good_tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "good.pyi": GOOD_FILE} ) @@ -206,9 +182,7 @@ def test_stub_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.pyi")), rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")), ] - lint_results, fmt_result = run_isort(rule_runner, bad_tgts) - assert len(lint_results) == 1 and lint_results[0].exit_code == 1 - assert "bad.pyi Imports are incorrectly sorted" in lint_results[0].stderr + fmt_result = run_isort(rule_runner, bad_tgts) assert fmt_result.stdout == "Fixing bad.py\nFixing bad.pyi\n" assert fmt_result.output == get_snapshot( rule_runner, {"bad.py": FIXED_BAD_FILE, "bad.pyi": FIXED_BAD_FILE} diff --git a/src/python/pants/backend/python/lint/pyupgrade/rules.py b/src/python/pants/backend/python/lint/pyupgrade/rules.py index 2ac5e588ce9..21bdb640ea5 100644 --- a/src/python/pants/backend/python/lint/pyupgrade/rules.py +++ b/src/python/pants/backend/python/lint/pyupgrade/rules.py @@ -10,12 +10,12 @@ from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest from pants.engine.internals.native_engine import Snapshot +from pants.engine.internals.selectors import MultiGet from pants.engine.process import FallibleProcessResult -from pants.engine.rules import Get, MultiGet, collect_rules, rule +from pants.engine.rules import Get, collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule from pants.util.logging import LogLevel @@ -33,7 +33,7 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipPyUpgradeField).value -class PyUpgradeRequest(FmtRequest, LintTargetsRequest): +class PyUpgradeRequest(FmtRequest): field_set_type = PyUpgradeFieldSet name = PyUpgrade.options_scope @@ -88,20 +88,9 @@ async def pyupgrade_fmt(result: PyUpgradeResult, pyupgrade: PyUpgrade) -> FmtRes ) -@rule(desc="Lint with pyupgrade", level=LogLevel.DEBUG) -async def pyupgrade_lint(result: PyUpgradeResult, pyupgrade: PyUpgrade) -> LintResults: - if pyupgrade.skip: - return LintResults([], linter_name=PyUpgradeRequest.name) - return LintResults( - [LintResult.from_fallible_process_result(result.process_result)], - linter_name=PyUpgradeRequest.name, - ) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, PyUpgradeRequest), - UnionRule(LintTargetsRequest, PyUpgradeRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py b/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py index 5d9901c55fc..f45c00f415f 100644 --- a/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.lint.pyupgrade.subsystem import rules as pyupgrade_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget 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 @@ -32,7 +31,6 @@ def rule_runner() -> RuleRunner: *source_files.rules(), *config_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (PyUpgradeRequest,)), QueryRule(FmtResult, (PyUpgradeRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -56,7 +54,7 @@ def run_pyupgrade( *, extra_args: list[str] | None = None, pyupgrade_arg: str = "--py36-plus", -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( [ "--backend-packages=pants.backend.python.lint.pyupgrade", @@ -66,7 +64,6 @@ def run_pyupgrade( env_inherit={"PATH", "PYENV_ROOT", "HOME"}, ) field_sets = [PyUpgradeFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [PyUpgradeRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -79,7 +76,7 @@ def run_pyupgrade( PyUpgradeRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -96,14 +93,11 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: rule_runner.write_files({"f.py": PY_36_GOOD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_pyupgrade( + fmt_result = run_pyupgrade( rule_runner, [tgt], extra_args=[f"--pyupgrade-interpreter-constraints=['=={major_minor_interpreter}.*']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": PY_36_GOOD_FILE}) assert fmt_result.did_change is False @@ -111,10 +105,7 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": PY_36_BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_pyupgrade(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "Rewriting f.py" in lint_results[0].stderr + fmt_result = run_pyupgrade(rule_runner, [tgt]) assert fmt_result.skipped is False assert fmt_result.output == get_snapshot(rule_runner, {"f.py": PY_36_FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -128,12 +119,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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_pyupgrade(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "Rewriting bad.py" in lint_results[0].stderr - assert "good.py" not in lint_results[0].stderr - assert "good.py" not in lint_results[0].stdout + fmt_result = run_pyupgrade(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.py": PY_36_GOOD_FILE, "bad.py": PY_36_FIXED_BAD_FILE} ) @@ -147,14 +133,11 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: tgt = rule_runner.get_target( Address("", target_name="t", relative_file_path="some_file_name.py") ) - lint_results, fmt_result = run_pyupgrade( + fmt_result = run_pyupgrade( rule_runner, [tgt], pyupgrade_arg="--py38-plus", ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "Rewriting some_file_name.py" in lint_results[0].stderr assert fmt_result.output == get_snapshot( rule_runner, {"some_file_name.py": PY_38_FIXED_BAD_FILE} ) @@ -164,7 +147,6 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": PY_36_BAD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_pyupgrade(rule_runner, [tgt], extra_args=["--pyupgrade-skip"]) - assert not lint_results + fmt_result = run_pyupgrade(rule_runner, [tgt], extra_args=["--pyupgrade-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/python/lint/yapf/rules.py b/src/python/pants/backend/python/lint/yapf/rules.py index a7b1152c617..e3f2b712ac3 100644 --- a/src/python/pants/backend/python/lint/yapf/rules.py +++ b/src/python/pants/backend/python/lint/yapf/rules.py @@ -2,7 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import Tuple from pants.backend.python.lint.yapf.skip_field import SkipYapfField from pants.backend.python.lint.yapf.subsystem import Yapf @@ -10,12 +9,11 @@ from pants.backend.python.util_rules import pex from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest 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.internals.native_engine import Snapshot -from pants.engine.process import FallibleProcessResult, Process, ProcessResult +from pants.engine.process import 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 @@ -34,57 +32,35 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipYapfField).value -class YapfRequest(FmtRequest, LintTargetsRequest): +class YapfRequest(FmtRequest): field_set_type = YapfFieldSet name = Yapf.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: YapfRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process original_snapshot: Snapshot -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: +async def setup_yapf(request: YapfRequest, yapf: Yapf) -> Setup: yapf_pex_get = Get(VenvPex, PexRequest, yapf.to_pex_request()) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in 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 + if request.prior_formatter_result is None + else 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)) ) @@ -93,14 +69,15 @@ async def setup_yapf(setup_request: SetupRequest, yapf: Yapf) -> Setup: Process, VenvPexProcess( yapf_pex, - argv=generate_argv( - source_files, - yapf, - check_only=setup_request.check_only, + argv=( + *yapf.args, + "--in-place", + *(("--style", yapf.config) if yapf.config else ()), + *source_files_snapshot.files, ), input_digest=input_digest, output_files=source_files_snapshot.files, - description=f"Run yapf on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run yapf on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ), ) @@ -111,7 +88,7 @@ async def setup_yapf(setup_request: SetupRequest, yapf: Yapf) -> Setup: async def yapf_fmt(request: YapfRequest, yapf: Yapf) -> FmtResult: if yapf.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, YapfRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -123,22 +100,9 @@ async def yapf_fmt(request: YapfRequest, yapf: Yapf) -> FmtResult: ) -@rule(desc="Lint with yapf", level=LogLevel.DEBUG) -async def yapf_lint(request: YapfRequest, yapf: Yapf) -> LintResults: - if yapf.skip: - return LintResults([], linter_name=request.name) - 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=request.name, - ) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, YapfRequest), - UnionRule(LintTargetsRequest, YapfRequest), *pex.rules(), ] diff --git a/src/python/pants/backend/python/lint/yapf/rules_integration_test.py b/src/python/pants/backend/python/lint/yapf/rules_integration_test.py index 2b8de7df60c..5cdf3a409e2 100644 --- a/src/python/pants/backend/python/lint/yapf/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/yapf/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.lint.yapf.subsystem import rules as yapf_subsystem_rules from pants.backend.python.target_types import PythonSourcesGeneratorTarget 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 @@ -32,7 +31,6 @@ def rule_runner() -> RuleRunner: *source_files.rules(), *config_files.rules(), *target_types_rules.rules(), - QueryRule(LintResults, (YapfRequest,)), QueryRule(FmtResult, (YapfRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -56,13 +54,12 @@ def run_yapf( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> 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, [ @@ -75,7 +72,7 @@ def run_yapf( YapfRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -92,14 +89,11 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_yapf( + fmt_result = run_yapf( rule_runner, [tgt], extra_args=[f"--yapf-interpreter-constraints=['=={major_minor_interpreter}.*']"], ) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE}) assert fmt_result.did_change is False @@ -107,10 +101,7 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(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")) + fmt_result = run_yapf(rule_runner, [tgt]) assert fmt_result.skipped is False assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -124,11 +115,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: 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 + fmt_result = run_yapf(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE} ) @@ -153,10 +140,7 @@ def test_config_file( } ) 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")) + fmt_result = run_yapf(rule_runner, [tgt], extra_args=extra_args) assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE_INDENT2}) assert fmt_result.did_change is True @@ -164,12 +148,9 @@ def test_config_file( def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": NEEDS_CONFIG_FILE, "BUILD": "python_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py")) - lint_results, fmt_result = run_yapf( + 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_snapshot(rule_runner, {"f.py": FIXED_NEEDS_CONFIG_FILE_INDENT4}) assert fmt_result.did_change is True @@ -177,7 +158,6 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(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 + fmt_result = run_yapf(rule_runner, [tgt], extra_args=["--yapf-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/scala/lint/scalafmt/rules.py b/src/python/pants/backend/scala/lint/scalafmt/rules.py index 046ae8ee3cb..989ba70c987 100644 --- a/src/python/pants/backend/scala/lint/scalafmt/rules.py +++ b/src/python/pants/backend/scala/lint/scalafmt/rules.py @@ -13,7 +13,6 @@ from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior from pants.core.goals.fmt import FmtRequest, FmtResult from pants.core.goals.generate_lockfiles import GenerateToolLockfileSentinel -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.goals.tailor import group_by_dir from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import ( @@ -25,7 +24,7 @@ Snapshot, ) from pants.engine.internals.selectors import Get, MultiGet -from pants.engine.process import FallibleProcessResult, ProcessResult +from pants.engine.process import ProcessResult from pants.engine.rules import collect_rules, rule from pants.engine.target import FieldSet, Target from pants.engine.unions import UnionRule @@ -51,7 +50,7 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipScalafmtField).value -class ScalafmtRequest(FmtRequest, LintTargetsRequest): +class ScalafmtRequest(FmtRequest): field_set_type = ScalafmtFieldSet name = ScalafmtSubsystem.options_scope @@ -60,12 +59,6 @@ class ScalafmtToolLockfileSentinel(GenerateToolLockfileSentinel): resolve_name = ScalafmtSubsystem.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: ScalafmtRequest - check_only: bool - - @dataclass(frozen=True) class Partition: process: JvmProcess @@ -96,7 +89,6 @@ class SetupScalafmtPartition: extra_immutable_input_digests: FrozenDict[str, Digest] config_file: str files: tuple[str, ...] - check_only: bool def find_nearest_ancestor_file(files: set[str], dir: str, config_file: str) -> str | None: @@ -170,10 +162,6 @@ async def setup_scalafmt_partition( f"--config={request.config_file}", "--non-interactive", ] - if request.check_only: - args.append("--list") - else: - args.append("--quiet") args.extend(request.files) process = JvmProcess( @@ -194,7 +182,7 @@ async def setup_scalafmt_partition( @rule(level=LogLevel.DEBUG) async def setup_scalafmt( - setup_request: SetupRequest, + request: ScalafmtRequest, tool: ScalafmtSubsystem, ) -> Setup: toolcp_relpath = "__toolcp" @@ -203,15 +191,15 @@ async def setup_scalafmt( source_files, tool_classpath = await MultiGet( Get( SourceFiles, - SourceFilesRequest(field_set.source for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.source for field_set in request.field_sets), ), Get(ToolClasspath, ToolClasspathRequest(lockfile=lockfile_request)), ) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) config_files = await Get( @@ -243,7 +231,6 @@ async def setup_scalafmt( extra_immutable_input_digests=FrozenDict(extra_immutable_input_digests), config_file=config_file, files=tuple(sorted(files)), - check_only=setup_request.check_only, ), ) for config_file, files in source_files_by_config_file.items() @@ -256,7 +243,7 @@ async def setup_scalafmt( async def scalafmt_fmt(request: ScalafmtRequest, tool: ScalafmtSubsystem) -> FmtResult: if tool.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, ScalafmtRequest, request) results = await MultiGet( Get(ProcessResult, JvmProcess, partition.process) for partition in setup.partitions ) @@ -293,21 +280,6 @@ def format(description: str, output) -> str: return fmt_result -@rule(desc="Lint with scalafmt", level=LogLevel.DEBUG) -async def scalafmt_lint(request: ScalafmtRequest, tool: ScalafmtSubsystem) -> LintResults: - if tool.skip: - return LintResults([], linter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=True)) - results = await MultiGet( - Get(FallibleProcessResult, JvmProcess, partition.process) for partition in setup.partitions - ) - lint_results = [ - LintResult.from_fallible_process_result(result, partition_description=partition.description) - for result, partition in zip(results, setup.partitions) - ] - return LintResults(lint_results, linter_name=request.name) - - @rule def generate_scalafmt_lockfile_request( _: ScalafmtToolLockfileSentinel, tool: ScalafmtSubsystem @@ -320,6 +292,5 @@ def rules(): *collect_rules(), *lockfile.rules(), UnionRule(FmtRequest, ScalafmtRequest), - UnionRule(LintTargetsRequest, ScalafmtRequest), UnionRule(GenerateToolLockfileSentinel, ScalafmtToolLockfileSentinel), ] diff --git a/src/python/pants/backend/scala/lint/scalafmt/rules_integration_test.py b/src/python/pants/backend/scala/lint/scalafmt/rules_integration_test.py index d1672889493..35ce4bd5066 100644 --- a/src/python/pants/backend/scala/lint/scalafmt/rules_integration_test.py +++ b/src/python/pants/backend/scala/lint/scalafmt/rules_integration_test.py @@ -20,7 +20,6 @@ from pants.backend.scala.target_types import ScalaSourcesGeneratorTarget, ScalaSourceTarget from pants.build_graph.address import Address 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.external_tool import rules as external_tool_rules from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest @@ -51,7 +50,6 @@ def rule_runner() -> RuleRunner: *target_types.rules(), *scalafmt_rules(), *skip_field.rules(), - QueryRule(LintResults, (ScalafmtRequest,)), QueryRule(FmtResult, (ScalafmtRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), QueryRule(Snapshot, (PathGlobs,)), @@ -103,11 +101,8 @@ def rule_runner() -> RuleRunner: """ -def run_scalafmt( - rule_runner: RuleRunner, targets: list[Target] -) -> tuple[tuple[LintResult, ...], FmtResult]: +def run_scalafmt(rule_runner: RuleRunner, targets: list[Target]) -> FmtResult: field_sets = [ScalafmtFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [ScalafmtRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -120,7 +115,7 @@ def run_scalafmt( ScalafmtRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -138,9 +133,7 @@ def test_passing(rule_runner: RuleRunner) -> None: } ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="Foo.scala")) - lint_results, fmt_result = run_scalafmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 + fmt_result = run_scalafmt(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"Foo.scala": GOOD_FILE}) assert fmt_result.did_change is False @@ -154,10 +147,7 @@ def test_failing(rule_runner: RuleRunner) -> None: } ) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="Bar.scala")) - lint_results, fmt_result = run_scalafmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "Bar.scala\n" == lint_results[0].stdout + fmt_result = run_scalafmt(rule_runner, [tgt]) assert fmt_result.output == get_snapshot(rule_runner, {"Bar.scala": FIXED_BAD_FILE}) assert fmt_result.did_change is True @@ -175,10 +165,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="Foo.scala")), rule_runner.get_target(Address("", target_name="t", relative_file_path="Bar.scala")), ] - lint_results, fmt_result = run_scalafmt(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "Bar.scala\n" == lint_results[0].stdout + fmt_result = run_scalafmt(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"Foo.scala": GOOD_FILE, "Bar.scala": FIXED_BAD_FILE} ) @@ -207,8 +194,7 @@ def test_multiple_config_files(rule_runner: RuleRunner) -> None: Address("foo/bar", target_name="bar", relative_file_path="Bar.scala") ), ] - lint_results, fmt_result = run_scalafmt(rule_runner, tgts) - assert len(lint_results) == 2 + fmt_result = run_scalafmt(rule_runner, tgts) assert fmt_result.output == get_snapshot( rule_runner, {"foo/Foo.scala": GOOD_FILE, "foo/bar/Bar.scala": FIXED_BAD_FILE_INDENT_4} ) diff --git a/src/python/pants/backend/shell/lint/shfmt/rules.py b/src/python/pants/backend/shell/lint/shfmt/rules.py index 59ee9e12303..1757c420d55 100644 --- a/src/python/pants/backend/shell/lint/shfmt/rules.py +++ b/src/python/pants/backend/shell/lint/shfmt/rules.py @@ -7,14 +7,13 @@ from pants.backend.shell.lint.shfmt.subsystem import Shfmt from pants.backend.shell.target_types import ShellSourceField from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest, MergeDigests from pants.engine.internals.native_engine import Snapshot from pants.engine.platform import Platform -from pants.engine.process import FallibleProcessResult, Process, ProcessResult +from pants.engine.process import 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 @@ -33,17 +32,11 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipShfmtField).value -class ShfmtRequest(FmtRequest, LintTargetsRequest): +class ShfmtRequest(FmtRequest): field_set_type = ShfmtFieldSet name = Shfmt.options_scope -@dataclass(frozen=True) -class SetupRequest: - request: ShfmtRequest - check_only: bool - - @dataclass(frozen=True) class Setup: process: Process @@ -51,20 +44,20 @@ class Setup: @rule(level=LogLevel.DEBUG) -async def setup_shfmt(setup_request: SetupRequest, shfmt: Shfmt) -> Setup: +async def setup_shfmt(request: ShfmtRequest, shfmt: Shfmt) -> Setup: download_shfmt_get = Get( DownloadedExternalTool, ExternalToolRequest, shfmt.get_request(Platform.current) ) source_files_get = Get( SourceFiles, - SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), + SourceFilesRequest(field_set.sources for field_set in request.field_sets), ) downloaded_shfmt, source_files = await MultiGet(download_shfmt_get, source_files_get) source_files_snapshot = ( source_files.snapshot - if setup_request.request.prior_formatter_result is None - else setup_request.request.prior_formatter_result + if request.prior_formatter_result is None + else request.prior_formatter_result ) config_files = await Get( @@ -80,9 +73,8 @@ async def setup_shfmt(setup_request: SetupRequest, shfmt: Shfmt) -> Setup: argv = [ downloaded_shfmt.exe, - # If linting, use `-d` to error with a diff. Else, write the change with `-w` and list - # what was changed with `-l`. - *(["-d"] if setup_request.check_only else ["-l", "-w"]), + "-l", + "-w", *shfmt.args, *source_files_snapshot.files, ] @@ -90,7 +82,7 @@ async def setup_shfmt(setup_request: SetupRequest, shfmt: Shfmt) -> Setup: argv=argv, input_digest=input_digest, output_files=source_files_snapshot.files, - description=f"Run shfmt on {pluralize(len(setup_request.request.field_sets), 'file')}.", + description=f"Run shfmt on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ) return Setup(process, original_snapshot=source_files_snapshot) @@ -100,7 +92,7 @@ async def setup_shfmt(setup_request: SetupRequest, shfmt: Shfmt) -> Setup: async def shfmt_fmt(request: ShfmtRequest, shfmt: Shfmt) -> FmtResult: if shfmt.skip: return FmtResult.skip(formatter_name=request.name) - setup = await Get(Setup, SetupRequest(request, check_only=False)) + setup = await Get(Setup, ShfmtRequest, request) result = await Get(ProcessResult, Process, setup.process) output_snapshot = await Get(Snapshot, Digest, result.output_digest) return FmtResult( @@ -112,18 +104,8 @@ async def shfmt_fmt(request: ShfmtRequest, shfmt: Shfmt) -> FmtResult: ) -@rule(desc="Lint with shfmt", level=LogLevel.DEBUG) -async def shfmt_lint(request: ShfmtRequest, shfmt: Shfmt) -> LintResults: - if shfmt.skip: - return LintResults([], linter_name=request.name) - 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=request.name) - - def rules(): return [ *collect_rules(), UnionRule(FmtRequest, ShfmtRequest), - UnionRule(LintTargetsRequest, ShfmtRequest), ] diff --git a/src/python/pants/backend/shell/lint/shfmt/rules_integration_test.py b/src/python/pants/backend/shell/lint/shfmt/rules_integration_test.py index 2702f1d52cb..a44e2c5fa03 100644 --- a/src/python/pants/backend/shell/lint/shfmt/rules_integration_test.py +++ b/src/python/pants/backend/shell/lint/shfmt/rules_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.shell.target_types import ShellSourcesGeneratorTarget from pants.backend.shell.target_types import rules as target_types_rules from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import config_files, external_tool, source_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address @@ -31,7 +30,6 @@ def rule_runner() -> RuleRunner: *external_tool.rules(), *source_files.rules(), *target_types_rules(), - QueryRule(LintResults, [ShfmtRequest]), QueryRule(FmtResult, [ShfmtRequest]), QueryRule(SourceFiles, [SourceFilesRequest]), ], @@ -74,13 +72,12 @@ def run_shfmt( targets: list[Target], *, extra_args: list[str] | None = None, -) -> tuple[tuple[LintResult, ...], FmtResult]: +) -> FmtResult: rule_runner.set_options( ["--backend-packages=pants.backend.shell.lint.shfmt", *(extra_args or ())], env_inherit={"PATH"}, ) field_sets = [ShfmtFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [ShfmtRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -93,7 +90,7 @@ def run_shfmt( ShfmtRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: @@ -105,10 +102,7 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps def test_passing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.sh": GOOD_FILE, "BUILD": "shell_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh")) - lint_results, fmt_result = run_shfmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" + fmt_result = run_shfmt(rule_runner, [tgt]) assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, {"f.sh": GOOD_FILE}) assert fmt_result.did_change is False @@ -117,10 +111,7 @@ def test_passing(rule_runner: RuleRunner) -> None: def test_failing(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.sh": BAD_FILE, "BUILD": "shell_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh")) - lint_results, fmt_result = run_shfmt(rule_runner, [tgt]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.sh.orig" in lint_results[0].stdout + fmt_result = run_shfmt(rule_runner, [tgt]) assert fmt_result.stdout == "f.sh\n" assert fmt_result.output == get_snapshot(rule_runner, {"f.sh": GOOD_FILE}) assert fmt_result.did_change is True @@ -134,11 +125,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("", target_name="t", relative_file_path="good.sh")), rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.sh")), ] - lint_results, fmt_result = run_shfmt(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "bad.sh.orig" in lint_results[0].stdout - assert "good.sh" not in lint_results[0].stdout + fmt_result = run_shfmt(rule_runner, tgts) assert "bad.sh\n" == fmt_result.stdout assert fmt_result.output == get_snapshot( rule_runner, {"good.sh": GOOD_FILE, "bad.sh": GOOD_FILE} @@ -160,11 +147,7 @@ def test_config_files(rule_runner: RuleRunner) -> None: rule_runner.get_target(Address("a", relative_file_path="f.sh")), rule_runner.get_target(Address("b", relative_file_path="f.sh")), ] - lint_results, fmt_result = run_shfmt(rule_runner, tgts) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "a/f.sh.orig" in lint_results[0].stdout - assert "b/f.sh.orig" not in lint_results[0].stdout + fmt_result = run_shfmt(rule_runner, tgts) assert fmt_result.stdout == "a/f.sh\n" assert fmt_result.output == get_snapshot( rule_runner, {"a/f.sh": FIXED_NEEDS_CONFIG_FILE, "b/f.sh": NEEDS_CONFIG_FILE} @@ -175,10 +158,7 @@ def test_config_files(rule_runner: RuleRunner) -> None: def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.sh": NEEDS_CONFIG_FILE, "BUILD": "shell_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh")) - lint_results, fmt_result = run_shfmt(rule_runner, [tgt], extra_args=["--shfmt-args=-ci"]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 - assert "f.sh.orig" in lint_results[0].stdout + fmt_result = run_shfmt(rule_runner, [tgt], extra_args=["--shfmt-args=-ci"]) assert fmt_result.stdout == "f.sh\n" assert fmt_result.output == get_snapshot(rule_runner, {"f.sh": FIXED_NEEDS_CONFIG_FILE}) assert fmt_result.did_change is True @@ -187,7 +167,6 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.sh": BAD_FILE, "BUILD": "shell_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh")) - lint_results, fmt_result = run_shfmt(rule_runner, [tgt], extra_args=["--shfmt-skip"]) - assert not lint_results + fmt_result = run_shfmt(rule_runner, [tgt], extra_args=["--shfmt-skip"]) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/terraform/lint/tffmt/tffmt.py b/src/python/pants/backend/terraform/lint/tffmt/tffmt.py index 596c00e4868..2ee62824d8b 100644 --- a/src/python/pants/backend/terraform/lint/tffmt/tffmt.py +++ b/src/python/pants/backend/terraform/lint/tffmt/tffmt.py @@ -8,17 +8,15 @@ from pants.backend.terraform.tool import TerraformProcess from pants.backend.terraform.tool import rules as tool_rules from pants.core.goals.fmt import FmtRequest, FmtResult -from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest from pants.core.util_rules import external_tool from pants.engine.fs import Digest, MergeDigests from pants.engine.internals.native_engine import Snapshot from pants.engine.internals.selectors import Get, MultiGet -from pants.engine.process import FallibleProcessResult, ProcessResult +from pants.engine.process import ProcessResult from pants.engine.rules import collect_rules, rule from pants.engine.unions import UnionRule from pants.option.option_types import SkipOption from pants.option.subsystem import Subsystem -from pants.util.logging import LogLevel logger = logging.getLogger(__name__) @@ -78,24 +76,10 @@ def format(directory, output): return fmt_result -@rule(desc="Lint with `terraform fmt`", level=LogLevel.DEBUG) -async def tffmt_lint(request: TffmtRequest, tffmt: TfFmtSubsystem) -> LintResults: - if tffmt.skip: - return LintResults([], linter_name=request.name) - setup = await Get(StyleSetup, StyleSetupRequest(request, ("fmt", "-check"))) - results = await MultiGet( - Get(FallibleProcessResult, TerraformProcess, process) - for _, (process, _) in setup.directory_to_process.items() - ) - lint_results = [LintResult.from_fallible_process_result(result) for result in results] - return LintResults(lint_results, linter_name=request.name) - - def rules(): return [ *collect_rules(), *external_tool.rules(), *tool_rules(), - UnionRule(LintTargetsRequest, TffmtRequest), UnionRule(FmtRequest, TffmtRequest), ] diff --git a/src/python/pants/backend/terraform/lint/tffmt/tffmt_integration_test.py b/src/python/pants/backend/terraform/lint/tffmt/tffmt_integration_test.py index 29e106eb107..c92bf1cefba 100644 --- a/src/python/pants/backend/terraform/lint/tffmt/tffmt_integration_test.py +++ b/src/python/pants/backend/terraform/lint/tffmt/tffmt_integration_test.py @@ -1,7 +1,7 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). import textwrap -from typing import List, Sequence, Tuple +from typing import List import pytest @@ -10,7 +10,6 @@ from pants.backend.terraform.lint.tffmt.tffmt import TffmtRequest from pants.backend.terraform.target_types import TerraformFieldSet, TerraformModuleTarget from pants.core.goals.fmt import FmtResult -from pants.core.goals.lint import LintResult, LintResults from pants.core.util_rules import external_tool, source_files from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address @@ -30,7 +29,6 @@ def rule_runner() -> RuleRunner: *tool.rules(), *style.rules(), *source_files.rules(), - QueryRule(LintResults, (TffmtRequest,)), QueryRule(FmtResult, (TffmtRequest,)), QueryRule(SourceFiles, (SourceFilesRequest,)), ], @@ -96,7 +94,7 @@ def run_tffmt( targets: List[Target], *, skip: bool = False, -) -> Tuple[Sequence[LintResult], FmtResult]: +) -> FmtResult: args = [ "--backend-packages=pants.backend.experimental.terraform", "--backend-packages=pants.backend.experimental.terraform.lint.tffmt", @@ -105,7 +103,6 @@ def run_tffmt( args.append("--terraform-fmt-skip") rule_runner.set_options(args) field_sets = [TerraformFieldSet.create(tgt) for tgt in targets] - lint_results = rule_runner.request(LintResults, [TffmtRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -118,7 +115,7 @@ def run_tffmt( TffmtRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) - return lint_results.results, fmt_result + return fmt_result def get_content(rule_runner: RuleRunner, digest: Digest) -> DigestContents: @@ -132,10 +129,7 @@ def get_snapshot(rule_runner: RuleRunner, source_files: List[FileContent]) -> Sn def test_passing_source(rule_runner: RuleRunner) -> None: target = make_target(rule_runner, [GOOD_SOURCE]) - lint_results, fmt_result = run_tffmt(rule_runner, [target]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" + fmt_result = run_tffmt(rule_runner, [target]) assert fmt_result.stdout == "" assert fmt_result.output == get_snapshot(rule_runner, [GOOD_SOURCE]) assert fmt_result.did_change is False @@ -143,12 +137,9 @@ def test_passing_source(rule_runner: RuleRunner) -> None: def test_failing_source(rule_runner: RuleRunner) -> None: target = make_target(rule_runner, [BAD_SOURCE]) - lint_results, fmt_result = run_tffmt(rule_runner, [target]) + fmt_result = run_tffmt(rule_runner, [target]) contents = get_content(rule_runner, fmt_result.output.digest) print(f">>>{contents[0].content.decode()}<<<") - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 3 - assert "bad.tf" in lint_results[0].stdout assert fmt_result.stderr == "" assert fmt_result.output == get_snapshot(rule_runner, [FIXED_BAD_SOURCE]) assert fmt_result.did_change is True @@ -156,11 +147,7 @@ def test_failing_source(rule_runner: RuleRunner) -> None: def test_mixed_sources(rule_runner: RuleRunner) -> None: target = make_target(rule_runner, [GOOD_SOURCE, BAD_SOURCE]) - lint_results, fmt_result = run_tffmt(rule_runner, [target]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 3 - assert "bad.tf" in lint_results[0].stdout - assert "good.tf" not in lint_results[0].stdout + fmt_result = run_tffmt(rule_runner, [target]) assert fmt_result.output == get_snapshot(rule_runner, [GOOD_SOURCE, FIXED_BAD_SOURCE]) assert fmt_result.did_change is True @@ -170,18 +157,13 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: make_target(rule_runner, [GOOD_SOURCE], target_name="tgt_good"), make_target(rule_runner, [BAD_SOURCE], target_name="tgt_bad"), ] - lint_results, fmt_result = run_tffmt(rule_runner, targets) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 3 - assert "bad.tf" in lint_results[0].stdout - assert "good.tf" not in lint_results[0].stdout + fmt_result = run_tffmt(rule_runner, targets) assert fmt_result.output == get_snapshot(rule_runner, [GOOD_SOURCE, FIXED_BAD_SOURCE]) assert fmt_result.did_change is True def test_skip(rule_runner: RuleRunner) -> None: target = make_target(rule_runner, [BAD_SOURCE]) - lint_results, fmt_result = run_tffmt(rule_runner, [target], skip=True) - assert not lint_results + fmt_result = run_tffmt(rule_runner, [target], skip=True) assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/core/goals/fmt.py b/src/python/pants/core/goals/fmt.py index 4ae5abced17..d9a0aa2cc1c 100644 --- a/src/python/pants/core/goals/fmt.py +++ b/src/python/pants/core/goals/fmt.py @@ -4,6 +4,7 @@ from __future__ import annotations import itertools +import logging from collections import defaultdict from dataclasses import dataclass from typing import TypeVar @@ -17,7 +18,7 @@ from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.console import Console from pants.engine.engine_aware import EngineAwareReturnType -from pants.engine.fs import Digest, MergeDigests, Snapshot, Workspace +from pants.engine.fs import Digest, MergeDigests, Snapshot, SnapshotDiff, Workspace from pants.engine.goal import Goal, GoalSubsystem from pants.engine.internals.native_engine import EMPTY_SNAPSHOT from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule, rule @@ -29,6 +30,8 @@ _F = TypeVar("_F", bound="FmtResult") +logger = logging.getLogger(__name__) + @dataclass(frozen=True) class FmtResult(EngineAwareReturnType): @@ -38,6 +41,15 @@ class FmtResult(EngineAwareReturnType): stderr: str formatter_name: str + def __post_init__(self): + # NB: We debug log stdout/stderr because `message` doesn't log it. + log = f"Output from {self.formatter_name}" + if self.stdout: + log += f"\n{self.stdout}" + if self.stderr: + log += f"\n{self.stderr}" + logger.debug(log) + @classmethod def skip(cls: type[_F], *, formatter_name: str) -> _F: return cls( @@ -70,13 +82,20 @@ def message(self) -> str | None: if self.skipped: return f"{self.formatter_name} skipped." message = "made changes." if self.did_change else "made no changes." - output = "" - if self.stdout: - output += f"\n{self.stdout}" - if self.stderr: - output += f"\n{self.stderr}" - if output: - output = f"{output.rstrip()}\n\n" + + # NB: Instead of printing out `stdout` and `stderr`, we just print a list of files which + # changed. We do this for two reasons: + # 1. This is run as part of both `fmt` and `lint`, and we want consistent output between both + # 2. Different formatters have different stdout/stderr. This way is consistent across all + # formatters. + if self.did_change: + output = "".join( + f"\n {file}" + for file in SnapshotDiff.from_snapshots(self.input, self.output).changed_files + ) + else: + output = "" + return f"{self.formatter_name} {message}{output}" def cacheable(self) -> bool: diff --git a/src/python/pants/core/goals/fmt_test.py b/src/python/pants/core/goals/fmt_test.py index aa465b70598..689580da25e 100644 --- a/src/python/pants/core/goals/fmt_test.py +++ b/src/python/pants/core/goals/fmt_test.py @@ -3,6 +3,7 @@ from __future__ import annotations +import logging from dataclasses import dataclass from pathlib import Path from textwrap import dedent @@ -186,7 +187,8 @@ def test_streaming_output_skip() -> None: assert result.message() == "formatter skipped." -def test_streaming_output_changed() -> None: +def test_streaming_output_changed(caplog) -> None: + caplog.set_level(logging.DEBUG) changed_digest = Digest(EMPTY_DIGEST.fingerprint, 2) changed_snapshot = Snapshot._unsafe_create(changed_digest, [], []) result = FmtResult( @@ -197,17 +199,14 @@ def test_streaming_output_changed() -> None: formatter_name="formatter", ) assert result.level() == LogLevel.WARN - assert result.message() == dedent( - """\ - formatter made changes. - stdout - stderr - - """ - ) + assert result.message() == "formatter made changes." + assert ["Output from formatter\nstdout\nstderr"] == [ + rec.message for rec in caplog.records if rec.levelno == logging.DEBUG + ] -def test_streaming_output_not_changed() -> None: +def test_streaming_output_not_changed(caplog) -> None: + caplog.set_level(logging.DEBUG) result = FmtResult( input=EMPTY_SNAPSHOT, output=EMPTY_SNAPSHOT, @@ -216,11 +215,7 @@ def test_streaming_output_not_changed() -> None: formatter_name="formatter", ) assert result.level() == LogLevel.INFO - assert result.message() == dedent( - """\ - formatter made no changes. - stdout - stderr - - """ - ) + assert result.message() == "formatter made no changes." + assert ["Output from formatter\nstdout\nstderr"] == [ + rec.message for rec in caplog.records if rec.levelno == logging.DEBUG + ] diff --git a/src/python/pants/core/goals/lint.py b/src/python/pants/core/goals/lint.py index ed07e4df957..deae26a4141 100644 --- a/src/python/pants/core/goals/lint.py +++ b/src/python/pants/core/goals/lint.py @@ -6,8 +6,9 @@ import itertools import logging from dataclasses import dataclass -from typing import Any, ClassVar, Iterable, Type, cast +from typing import Any, ClassVar, Iterable, Iterator, Type, cast +from pants.core.goals.fmt import FmtRequest, FmtResult from pants.core.goals.style_request import ( StyleRequest, determine_specified_tool_names, @@ -36,7 +37,9 @@ class AmbiguousRequestNamesError(Exception): def __init__( - self, ambiguous_name: str, requests: set[Type[LintTargetsRequest] | Type[LintFilesRequest]] + self, + ambiguous_name: str, + requests: set[Type[StyleRequest] | Type[LintFilesRequest]], ): request_names = { f"{request_target.__module__}.{request_target.__qualname__}" @@ -152,7 +155,7 @@ def cacheable(self) -> bool: @union class LintTargetsRequest(StyleRequest): - """AThe entry point for linters that need targets.""" + """The entry point for linters that need targets.""" @union @@ -199,7 +202,7 @@ class Lint(Goal): def _check_ambiguous_request_names( - *requests: Type[LintFilesRequest] | Type[LintTargetsRequest], + *requests: Type[LintFilesRequest] | Type[StyleRequest], ) -> None: for name, request_group in itertools.groupby(requests, key=lambda target: target.name): request_group_set = set(request_group) @@ -218,9 +221,14 @@ async def lint( union_membership: UnionMembership, dist_dir: DistDir, ) -> Lint: - target_request_types = cast( - "Iterable[type[LintTargetsRequest]]", union_membership[LintTargetsRequest] + lint_target_request_types = cast( + "Iterable[type[LintTargetsRequest]]", union_membership.get(LintTargetsRequest) ) + fmt_target_request_types = cast("Iterable[type[FmtRequest]]", union_membership.get(FmtRequest)) + target_request_types = [ + *lint_target_request_types, + *fmt_target_request_types, + ] file_request_types = union_membership[LintFilesRequest] _check_ambiguous_request_names(*target_request_types, *file_request_types) @@ -243,6 +251,29 @@ async def lint( for request_type in target_request_types ) + def address_str(fs: FieldSet) -> str: + return fs.address.spec + + def batch(field_sets: Iterable[FieldSet]) -> Iterator[list[FieldSet]]: + return partition_sequentially( + field_sets, + key=address_str, + size_target=lint_subsystem.batch_size, + size_max=4 * lint_subsystem.batch_size, + ) + + lint_target_requests = ( + request.__class__(field_set_batch) + for request in target_requests + if isinstance(request, LintTargetsRequest) and request.field_sets + for field_set_batch in batch(request.field_sets) + ) + fmt_requests = ( + request.__class__(field_set_batch) + for request in target_requests + if isinstance(request, FmtRequest) and request.field_sets + for field_set_batch in batch(request.field_sets) + ) file_requests = ( tuple( request_type(specs_snapshot.snapshot.files) @@ -253,46 +284,46 @@ async def lint( else () ) - def address_str(fs: FieldSet) -> str: - return fs.address.spec - all_requests = [ - *( - Get(LintResults, LintTargetsRequest, request.__class__(field_set_batch)) - for request in target_requests - if request.field_sets - for field_set_batch in partition_sequentially( - request.field_sets, - key=address_str, - size_target=lint_subsystem.batch_size, - size_max=4 * lint_subsystem.batch_size, - ) - ), + *(Get(LintResults, LintTargetsRequest, request) for request in lint_target_requests), + *(Get(FmtResult, FmtRequest, request) for request in fmt_requests), *(Get(LintResults, LintFilesRequest, request) for request in file_requests), ] all_batch_results = cast( - "tuple[LintResults, ...]", + "tuple[LintResults | FmtResult, ...]", await MultiGet(all_requests), # type: ignore[arg-type] ) - def key_fn(results: LintResults): + def key_fn(results: LintResults | FmtResult): + if isinstance(results, FmtResult): + return results.formatter_name return results.linter_name # NB: We must pre-sort the data for itertools.groupby() to work properly. sorted_all_batch_results = sorted(all_batch_results, key=key_fn) + + def coerce_to_lintresult(batch_results: LintResults | FmtResult) -> tuple[LintResult, ...]: + if isinstance(batch_results, FmtResult): + return ( + LintResult( + 1 if batch_results.did_change else 0, + batch_results.stdout, + batch_results.stderr, + ), + ) + return batch_results.results + # We consolidate all results for each linter into a single `LintResults`. all_results = tuple( sorted( ( LintResults( itertools.chain.from_iterable( - batch_results.results for batch_results in all_linter_results + coerce_to_lintresult(batch_results) for batch_results in results ), linter_name=linter_name, ) - for linter_name, all_linter_results in itertools.groupby( - sorted_all_batch_results, key=key_fn - ) + for linter_name, results in itertools.groupby(sorted_all_batch_results, key=key_fn) ), key=key_fn, ) diff --git a/src/python/pants/core/goals/lint_test.py b/src/python/pants/core/goals/lint_test.py index f085f62cedd..baa5a2b9e55 100644 --- a/src/python/pants/core/goals/lint_test.py +++ b/src/python/pants/core/goals/lint_test.py @@ -10,6 +10,7 @@ import pytest +from pants.core.goals.fmt import FmtRequest, FmtResult from pants.core.goals.lint import ( AmbiguousRequestNamesError, Lint, @@ -23,6 +24,7 @@ from pants.core.util_rules.distdir import DistDir from pants.engine.addresses import Address from pants.engine.fs import SpecsSnapshot, Workspace +from pants.engine.internals.native_engine import EMPTY_DIGEST, EMPTY_SNAPSHOT, Digest, Snapshot from pants.engine.target import FieldSet, MultipleSourcesField, Target, Targets from pants.engine.unions import UnionMembership from pants.testutil.option_util import create_goal_subsystem @@ -116,6 +118,28 @@ def lint_results(self) -> LintResults: return LintResults([LintResult(0, "", "")], linter_name=self.name) +class MockFmtRequest(FmtRequest): + field_set_type = MockLinterFieldSet + + +class SuccessfulFormatter(MockFmtRequest): + name = "SuccessfulFormatter" + + @property + def fmt_result(self) -> FmtResult: + return FmtResult(EMPTY_SNAPSHOT, EMPTY_SNAPSHOT, "", "", formatter_name=self.name) + + +class FailingFormatter(MockFmtRequest): + name = "FailingFormatter" + + @property + def fmt_result(self) -> FmtResult: + before = EMPTY_SNAPSHOT + after = Snapshot._unsafe_create(Digest(EMPTY_DIGEST.fingerprint, 2), [], []) + return FmtResult(before, after, "", "", formatter_name=self.name) + + @pytest.fixture def rule_runner() -> RuleRunner: return RuleRunner() @@ -129,6 +153,7 @@ def run_lint_rule( rule_runner: RuleRunner, *, lint_request_types: Sequence[Type[LintTargetsRequest]], + fmt_request_types: Sequence[Type[FmtRequest]] = [], targets: list[Target], run_files_linter: bool = False, batch_size: int = 128, @@ -138,6 +163,7 @@ def run_lint_rule( { LintTargetsRequest: lint_request_types, LintFilesRequest: [MockFilesRequest] if run_files_linter else [], + FmtRequest: fmt_request_types, } ) lint_subsystem = create_goal_subsystem( @@ -169,6 +195,11 @@ def run_lint_rule( input_type=LintFilesRequest, mock=lambda mock_request: mock_request.lint_results, ), + MockGet( + output_type=FmtResult, + input_type=FmtRequest, + mock=lambda mock_request: mock_request.fmt_result, + ), ], union_membership=union_membership, ) @@ -194,17 +225,22 @@ def test_summary(rule_runner: RuleRunner) -> None: good_address = Address("", target_name="good") bad_address = Address("", target_name="bad") - request_types = [ + lint_request_types = [ ConditionallySucceedsRequest, FailingRequest, SkippedRequest, SuccessfulRequest, ] + fmt_request_types = [ + SuccessfulFormatter, + FailingFormatter, + ] targets = [make_target(good_address), make_target(bad_address)] exit_code, stderr = run_lint_rule( rule_runner, - lint_request_types=request_types, + lint_request_types=lint_request_types, + fmt_request_types=fmt_request_types, targets=targets, run_files_linter=True, ) @@ -213,22 +249,26 @@ def test_summary(rule_runner: RuleRunner) -> None: """\ ✕ ConditionallySucceedsLinter failed. + ✕ FailingFormatter failed. ✕ FailingLinter failed. ✓ FilesLinter succeeded. + ✓ SuccessfulFormatter succeeded. ✓ SuccessfulLinter succeeded. """ ) exit_code, stderr = run_lint_rule( rule_runner, - lint_request_types=request_types, + lint_request_types=lint_request_types, + fmt_request_types=fmt_request_types, targets=targets, run_files_linter=True, - only=[FailingRequest.name, MockFilesRequest.name], + only=[FailingRequest.name, MockFilesRequest.name, FailingFormatter.name], ) assert stderr == dedent( """\ + ✕ FailingFormatter failed. ✕ FailingLinter failed. ✓ FilesLinter succeeded. """