From 4aeefb549526d031b221cf62f0ba56d7e610695f Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Tue, 26 May 2020 10:11:34 -0700 Subject: [PATCH] Review feedback and test fix. # Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] --- .../pants/backend/python/lint/isort/rules.py | 2 +- .../python/lint/python_fmt_integration_test.py | 10 +++++----- src/python/pants/core/goals/fmt.py | 7 +++++-- src/python/pants/core/goals/lint.py | 14 +++++++------- src/python/pants/core/goals/lint_test.py | 16 ++++++++-------- .../core/goals/{style.py => style_request.py} | 3 ++- 6 files changed, 28 insertions(+), 24 deletions(-) rename src/python/pants/core/goals/{style.py => style_request.py} (90%) diff --git a/src/python/pants/backend/python/lint/isort/rules.py b/src/python/pants/backend/python/lint/isort/rules.py index 79687bd219b..1861231624e 100644 --- a/src/python/pants/backend/python/lint/isort/rules.py +++ b/src/python/pants/backend/python/lint/isort/rules.py @@ -43,7 +43,7 @@ class IsortFieldSet(FieldSetWithOrigin): sources: PythonSources -class IsortRequest(FmtRequest): +class IsortRequest(FmtRequest, LintRequest): field_set_type = IsortFieldSet diff --git a/src/python/pants/backend/python/lint/python_fmt_integration_test.py b/src/python/pants/backend/python/lint/python_fmt_integration_test.py index ef26683ccda..941ce11bc35 100644 --- a/src/python/pants/backend/python/lint/python_fmt_integration_test.py +++ b/src/python/pants/backend/python/lint/python_fmt_integration_test.py @@ -3,9 +3,9 @@ from typing import List, Optional -from pants.backend.python.lint.black.rules import BlackFieldSets +from pants.backend.python.lint.black.rules import BlackRequest from pants.backend.python.lint.black.rules import rules as black_rules -from pants.backend.python.lint.isort.rules import IsortFieldSets +from pants.backend.python.lint.isort.rules import IsortRequest from pants.backend.python.lint.isort.rules import rules as isort_rules from pants.backend.python.lint.python_fmt import PythonFmtTargets, format_python_target from pants.backend.python.target_types import PythonLibrary @@ -29,8 +29,8 @@ def rules(cls): *black_rules(), *isort_rules(), RootRule(PythonFmtTargets), - RootRule(BlackFieldSets), - RootRule(IsortFieldSets), + RootRule(BlackRequest), + RootRule(IsortRequest), ) def run_black_and_isort( @@ -93,7 +93,7 @@ def test_skipped_formatter(self) -> None: def test_no_changes(self) -> None: source = FileContent( - "test/skipped.py", content=b"from animals import dog, cat\n\nprint('hello')\n", + "test/target.py", content=b'from animals import cat, dog\n\nprint("hello")\n', ) results = self.run_black_and_isort([source], name="different_file") assert results.output == self.get_digest([source]) diff --git a/src/python/pants/core/goals/fmt.py b/src/python/pants/core/goals/fmt.py index 1e2db3d3f64..b6f8cc97add 100644 --- a/src/python/pants/core/goals/fmt.py +++ b/src/python/pants/core/goals/fmt.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from typing import ClassVar, Iterable, List, Tuple, Type -from pants.core.goals.style import StyleRequest +from pants.core.goals.style_request import StyleRequest from pants.core.util_rules.filter_empty_sources import TargetsWithSources, TargetsWithSourcesRequest from pants.engine.console import Console from pants.engine.fs import EMPTY_DIGEST, Digest, DirectoryToMaterialize, MergeDigests, Workspace @@ -58,7 +58,10 @@ def did_change(self) -> bool: @union class FmtRequest(StyleRequest): - """A union for StyleRequests that should be formattable.""" + """A union for StyleRequests that should be formattable. + + Subclass and install a member of this type to provide a formatter. + """ @union diff --git a/src/python/pants/core/goals/lint.py b/src/python/pants/core/goals/lint.py index 9b431201a68..d2e56a46c88 100644 --- a/src/python/pants/core/goals/lint.py +++ b/src/python/pants/core/goals/lint.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from typing import Iterable -from pants.core.goals.style import StyleRequest +from pants.core.goals.style_request import StyleRequest from pants.core.util_rules.filter_empty_sources import ( FieldSetsWithSources, FieldSetsWithSourcesRequest, @@ -47,7 +47,10 @@ def prep_output(s: bytes) -> str: @union class LintRequest(StyleRequest): - """A union for StyleRequests that should be lintable.""" + """A union for StyleRequests that should be lintable. + + Subclass and install a member of this type to provide a linter. + """ class LintOptions(GoalSubsystem): @@ -101,16 +104,13 @@ async def lint( ) for lint_request_type in union_membership[LintRequest] ) - lint_requests_with_sources: Iterable[FieldSetsWithSources] = await MultiGet( + field_sets_with_sources: Iterable[FieldSetsWithSources] = await MultiGet( Get[FieldSetsWithSources](FieldSetsWithSourcesRequest(lint_request.field_sets)) for lint_request in lint_requests ) - # NB: We must convert back the generic FieldSetsWithSources objects back into their - # corresponding LintRequest, e.g. back to IsortFieldSets, in order for the union rule to - # work. valid_lint_requests: Iterable[StyleRequest] = tuple( lint_request_cls(lint_request) - for lint_request_cls, lint_request in zip(lint_request_types, lint_requests_with_sources) + for lint_request_cls, lint_request in zip(lint_request_types, field_sets_with_sources) if lint_request ) diff --git a/src/python/pants/core/goals/lint_test.py b/src/python/pants/core/goals/lint_test.py index 7d9db9efb25..ecc68936dda 100644 --- a/src/python/pants/core/goals/lint_test.py +++ b/src/python/pants/core/goals/lint_test.py @@ -127,13 +127,13 @@ def make_target_with_origin(address: Optional[Address] = None) -> TargetWithOrig @staticmethod def run_lint_rule( *, - field_set_collection_types: List[Type[LintRequest]], + lint_request_types: List[Type[LintRequest]], targets: List[TargetWithOrigin], per_target_caching: bool, include_sources: bool = True, ) -> Tuple[int, str]: console = MockConsole(use_colors=False) - union_membership = UnionMembership({LintRequest: field_set_collection_types}) + union_membership = UnionMembership({LintRequest: lint_request_types}) result: Lint = run_rule( lint, rule_args=[ @@ -164,7 +164,7 @@ def run_lint_rule( def test_empty_target_noops(self) -> None: def assert_noops(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[FailingFieldSets], + lint_request_types=[FailingFieldSets], targets=[self.make_target_with_origin()], per_target_caching=per_target_caching, include_sources=False, @@ -178,7 +178,7 @@ def assert_noops(per_target_caching: bool) -> None: def test_invalid_target_noops(self) -> None: def assert_noops(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[InvalidFieldSets], + lint_request_types=[InvalidFieldSets], targets=[self.make_target_with_origin()], per_target_caching=per_target_caching, ) @@ -194,7 +194,7 @@ def test_single_target_with_one_linter(self) -> None: def assert_expected(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[FailingFieldSets], + lint_request_types=[FailingFieldSets], targets=[target_with_origin], per_target_caching=per_target_caching, ) @@ -215,7 +215,7 @@ def test_single_target_with_multiple_linters(self) -> None: def assert_expected(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[SuccessfulFieldSets, FailingFieldSets], + lint_request_types=[SuccessfulFieldSets, FailingFieldSets], targets=[target_with_origin], per_target_caching=per_target_caching, ) @@ -239,7 +239,7 @@ def test_multiple_targets_with_one_linter(self) -> None: def get_stderr(*, per_target_caching: bool) -> str: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[ConditionallySucceedsFieldSets], + lint_request_types=[ConditionallySucceedsFieldSets], targets=[ self.make_target_with_origin(good_address), self.make_target_with_origin(bad_address), @@ -272,7 +272,7 @@ def test_multiple_targets_with_multiple_linters(self) -> None: def get_stderr(*, per_target_caching: bool) -> str: exit_code, stderr = self.run_lint_rule( - field_set_collection_types=[ConditionallySucceedsFieldSets, SuccessfulFieldSets,], + lint_request_types=[ConditionallySucceedsFieldSets, SuccessfulFieldSets,], targets=[ self.make_target_with_origin(good_address), self.make_target_with_origin(bad_address), diff --git a/src/python/pants/core/goals/style.py b/src/python/pants/core/goals/style_request.py similarity index 90% rename from src/python/pants/core/goals/style.py rename to src/python/pants/core/goals/style_request.py index 41ea2f97caa..64c64347c2b 100644 --- a/src/python/pants/core/goals/style.py +++ b/src/python/pants/core/goals/style_request.py @@ -18,7 +18,8 @@ class StyleRequest(Generic[_FS], metaclass=ABCMeta): """A request to style a collection of `FieldSet`s. - Should be subclassed for a particular style engine. + Should be subclassed for a particular style engine in order to support autoformatting or + linting. """ field_set_type: ClassVar[Type[_FS]]