Skip to content

Commit

Permalink
Review feedback and test fix.
Browse files Browse the repository at this point in the history
# 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]
  • Loading branch information
stuhood committed May 26, 2020
1 parent 339b850 commit 4aeefb5
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 24 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/lint/isort/rules.py
Expand Up @@ -43,7 +43,7 @@ class IsortFieldSet(FieldSetWithOrigin):
sources: PythonSources


class IsortRequest(FmtRequest):
class IsortRequest(FmtRequest, LintRequest):
field_set_type = IsortFieldSet


Expand Down
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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])
Expand Down
7 changes: 5 additions & 2 deletions src/python/pants/core/goals/fmt.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions src/python/pants/core/goals/lint.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
)

Expand Down
16 changes: 8 additions & 8 deletions src/python/pants/core/goals/lint_test.py
Expand Up @@ -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=[
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
Expand Up @@ -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]]
Expand Down

0 comments on commit 4aeefb5

Please sign in to comment.