-
-
Notifications
You must be signed in to change notification settings - Fork 643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge validate
goal with lint
goal
#14102
Changes from 2 commits
796094c
d32b136
ba4244d
8f941c7
c9cc709
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,13 +6,13 @@ | |
import itertools | ||
import logging | ||
from dataclasses import dataclass | ||
from typing import Any, Iterable, cast | ||
from typing import Any, ClassVar, Iterable, cast | ||
|
||
from pants.core.goals.style_request import StyleRequest, style_batch_size_help, write_reports | ||
from pants.core.util_rules.distdir import DistDir | ||
from pants.engine.console import Console | ||
from pants.engine.engine_aware import EngineAwareReturnType | ||
from pants.engine.fs import EMPTY_DIGEST, Digest, Workspace | ||
from pants.engine.engine_aware import EngineAwareParameter, EngineAwareReturnType | ||
from pants.engine.fs import EMPTY_DIGEST, Digest, SpecsSnapshot, Workspace | ||
from pants.engine.goal import Goal, GoalSubsystem | ||
from pants.engine.process import FallibleProcessResult | ||
from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule | ||
|
@@ -134,6 +134,19 @@ class LintRequest(StyleRequest): | |
""" | ||
|
||
|
||
@union | ||
@dataclass(frozen=True) | ||
class LintFilesRequest(EngineAwareParameter): | ||
"""The entry point for linters that do not use targets.""" | ||
|
||
name: ClassVar[str] | ||
|
||
file_paths: tuple[str, ...] | ||
|
||
def debug_hint(self) -> str: | ||
return self.name | ||
|
||
|
||
# If a user wants linter reports to show up in dist/ they must ensure that the reports | ||
# are written under this directory. E.g., | ||
# ./pants --flake8-args="--output-file=reports/report.txt" lint <target> | ||
|
@@ -199,43 +212,63 @@ async def lint( | |
console: Console, | ||
workspace: Workspace, | ||
targets: Targets, | ||
specs_snapshot: SpecsSnapshot, | ||
lint_subsystem: LintSubsystem, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commenting in the wrong place, but: is it possible to update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been a problem for |
||
union_membership: UnionMembership, | ||
dist_dir: DistDir, | ||
) -> Lint: | ||
request_types = cast("Iterable[type[LintRequest]]", union_membership[LintRequest]) | ||
requests = tuple( | ||
target_request_types = cast("Iterable[type[LintRequest]]", union_membership[LintRequest]) | ||
target_requests = tuple( | ||
request_type( | ||
request_type.field_set_type.create(target) | ||
for target in targets | ||
if request_type.field_set_type.is_applicable(target) | ||
) | ||
for request_type in request_types | ||
for request_type in target_request_types | ||
) | ||
file_requests = tuple( | ||
request_type(specs_snapshot.snapshot.files) | ||
for request_type in union_membership[LintFilesRequest] | ||
) | ||
|
||
if lint_subsystem.per_file_caching: | ||
all_batch_results = await MultiGet( | ||
Get(LintResults, LintRequest, request.__class__([field_set])) | ||
for request in requests | ||
if request.field_sets | ||
for field_set in request.field_sets | ||
) | ||
all_requests = [ | ||
*( | ||
Get(LintResults, LintRequest, request.__class__([field_set])) | ||
for request in target_requests | ||
if request.field_sets | ||
for field_set in request.field_sets | ||
), | ||
*( | ||
Get(LintResults, LintFilesRequest, request.__class__((fp,))) | ||
for request in file_requests | ||
for fp in request.file_paths | ||
), | ||
] | ||
else: | ||
|
||
def address_str(fs: FieldSet) -> str: | ||
return fs.address.spec | ||
|
||
all_batch_results = await MultiGet( | ||
Get(LintResults, LintRequest, request.__class__(field_set_batch)) | ||
for request in 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, | ||
) | ||
) | ||
all_requests = [ | ||
*( | ||
Get(LintResults, LintRequest, 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, LintFilesRequest, request) for request in file_requests), | ||
] | ||
|
||
all_batch_results = cast( | ||
"tuple[LintResults, ...]", | ||
await MultiGet(all_requests), # type: ignore[arg-type] | ||
) | ||
|
||
def key_fn(results: LintResults): | ||
return results.linter_name | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth referring to a ticket in the help as part of the explanation of the gotchas? Improving the
--changed
behavior is possibly related to #13757 (i.e., the need to refactorSpecs
calculation to move it into a single call to the engine).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the most relevant thing is my design doc from November. Should I reference that? Fwit, that remains one of the most pressing projects I personally want to work on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine as is. We've always had these gotchas since adding
validate
in 2020.