diff --git a/src/python/pants/backend/codegen/protobuf/python/rules.py b/src/python/pants/backend/codegen/protobuf/python/rules.py index 4312c54a786..945080379e3 100644 --- a/src/python/pants/backend/codegen/protobuf/python/rules.py +++ b/src/python/pants/backend/codegen/protobuf/python/rules.py @@ -4,7 +4,7 @@ from pants.backend.codegen.protobuf.protoc import Protoc from pants.backend.codegen.protobuf.target_types import ProtobufSources from pants.backend.python.target_types import PythonSources -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFilesRequest from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest from pants.core.util_rules.strip_source_roots import StrippedSourceFiles from pants.engine.addresses import Addresses @@ -47,34 +47,29 @@ async def generate_python_from_protobuf( # actually generate those dependencies; it only needs to look at their .proto files to work # with imports. transitive_targets = await Get(TransitiveTargets, Addresses([request.protocol_target.address])) - all_sources_request = Get( - SourceFiles, - AllSourceFilesRequest( + # NB: By stripping the source roots, we avoid having to set the value `--proto_path` + # for Protobuf imports to be discoverable. + all_stripped_sources_request = Get( + StrippedSourceFiles, + SourceFilesRequest( (tgt.get(Sources) for tgt in transitive_targets.closure), for_sources_types=(ProtobufSources,), ), ) - unstripped_target_sources_request = Get( - SourceFiles, AllSourceFilesRequest([request.protocol_target[ProtobufSources]]), + target_stripped_sources_request = Get( + StrippedSourceFiles, SourceFilesRequest([request.protocol_target[ProtobufSources]]), ) ( downloaded_protoc_binary, create_output_dir_result, - all_sources_unstripped, - target_sources_unstripped, + all_sources_stripped, + target_sources_stripped, ) = await MultiGet( download_protoc_request, create_output_dir_request, - all_sources_request, - unstripped_target_sources_request, - ) - - # NB: By stripping the source roots, we avoid having to set the value `--proto_path` - # for Protobuf imports to be discoverable. - all_sources_stripped, target_sources_stripped = await MultiGet( - Get(StrippedSourceFiles, SourceFiles, all_sources_unstripped), - Get(StrippedSourceFiles, SourceFiles, target_sources_unstripped), + all_stripped_sources_request, + target_stripped_sources_request, ) input_digest = await Get( diff --git a/src/python/pants/backend/python/dependency_inference/module_mapper.py b/src/python/pants/backend/python/dependency_inference/module_mapper.py index b3e166ccf24..d121ae30b74 100644 --- a/src/python/pants/backend/python/dependency_inference/module_mapper.py +++ b/src/python/pants/backend/python/dependency_inference/module_mapper.py @@ -7,7 +7,7 @@ from pants.backend.python.target_types import PythonRequirementsField, PythonSources from pants.base.specs import AddressSpecs, DescendantAddresses -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest +from pants.core.util_rules.determine_source_files import SourceFilesRequest from pants.core.util_rules.strip_source_roots import StrippedSourceFiles from pants.engine.addresses import Address from pants.engine.rules import Get, MultiGet, collect_rules, rule @@ -73,7 +73,7 @@ async def map_first_party_modules_to_addresses() -> FirstPartyModuleToAddressMap for tgt in candidate_explicit_targets ) stripped_sources_per_explicit_target = await MultiGet( - Get(StrippedSourceFiles, AllSourceFilesRequest([tgt[PythonSources]])) + Get(StrippedSourceFiles, SourceFilesRequest([tgt[PythonSources]])) for tgt in candidate_explicit_targets ) diff --git a/src/python/pants/backend/python/dependency_inference/rules.py b/src/python/pants/backend/python/dependency_inference/rules.py index acebd9d3be6..4bcb9094081 100644 --- a/src/python/pants/backend/python/dependency_inference/rules.py +++ b/src/python/pants/backend/python/dependency_inference/rules.py @@ -12,7 +12,7 @@ from pants.backend.python.rules import ancestor_files from pants.backend.python.rules.ancestor_files import AncestorFiles, AncestorFilesRequest from pants.backend.python.target_types import PythonSources, PythonTestsSources -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest +from pants.core.util_rules.determine_source_files import SourceFilesRequest from pants.core.util_rules.strip_source_roots import StrippedSourceFiles from pants.engine.fs import Digest, DigestContents from pants.engine.internals.graph import Owners, OwnersRequest @@ -89,9 +89,7 @@ async def infer_python_dependencies( if not python_inference.imports: return InferredDependencies() - stripped_sources = await Get( - StrippedSourceFiles, AllSourceFilesRequest([request.sources_field]) - ) + stripped_sources = await Get(StrippedSourceFiles, SourceFilesRequest([request.sources_field])) modules = tuple( PythonModule.create_from_stripped_path(PurePath(fp)) for fp in stripped_sources.snapshot.files diff --git a/src/python/pants/backend/python/lint/bandit/rules.py b/src/python/pants/backend/python/lint/bandit/rules.py index 5ee14c9bfec..d7a50709ad9 100644 --- a/src/python/pants/backend/python/lint/bandit/rules.py +++ b/src/python/pants/backend/python/lint/bandit/rules.py @@ -22,11 +22,7 @@ LintSubsystem, ) from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import ( Digest, DigestSubset, @@ -37,14 +33,14 @@ ) from pants.engine.process import FallibleProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import FieldSetWithOrigin +from pants.engine.target import FieldSet from pants.engine.unions import UnionRule from pants.python.python_setup import PythonSetup from pants.util.strutil import pluralize @dataclass(frozen=True) -class BanditFieldSet(FieldSetWithOrigin): +class BanditFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -62,7 +58,7 @@ class BanditPartition: def generate_args( - *, specified_source_files: SourceFiles, bandit: Bandit, output_file: Optional[str] + *, source_files: SourceFiles, bandit: Bandit, output_file: Optional[str] ) -> Tuple[str, ...]: args = [] if bandit.config is not None: @@ -70,7 +66,7 @@ def generate_args( if output_file: args.append(f"--output={output_file}") args.extend(bandit.args) - args.extend(specified_source_files.files) + args.extend(source_files.files) return tuple(args) @@ -100,26 +96,16 @@ async def bandit_lint_partition( ), ) - all_source_files_request = Get( - SourceFiles, AllSourceFilesRequest(field_set.sources for field_set in partition.field_sets) - ) - specified_source_files_request = Get( - SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in partition.field_sets - ), + source_files_request = Get( + SourceFiles, SourceFilesRequest(field_set.sources for field_set in partition.field_sets) ) - requirements_pex, config_digest, all_source_files, specified_source_files = await MultiGet( - requirements_pex_request, - config_digest_request, - all_source_files_request, - specified_source_files_request, + requirements_pex, config_digest, source_files = await MultiGet( + requirements_pex_request, config_digest_request, source_files_request ) input_digest = await Get( - Digest, - MergeDigests((all_source_files.snapshot.digest, requirements_pex.digest, config_digest)), + Digest, MergeDigests((source_files.snapshot.digest, requirements_pex.digest, config_digest)) ) address_references = ", ".join( @@ -129,7 +115,7 @@ async def bandit_lint_partition( lint_subsystem.reports_dir / "bandit_report.txt" if lint_subsystem.reports_dir else None ) args = generate_args( - specified_source_files=specified_source_files, + source_files=source_files, bandit=bandit, output_file=report_path.name if report_path else None, ) diff --git a/src/python/pants/backend/python/lint/bandit/rules_integration_test.py b/src/python/pants/backend/python/lint/bandit/rules_integration_test.py index 219f046b119..db1d4304b27 100644 --- a/src/python/pants/backend/python/lint/bandit/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/bandit/rules_integration_test.py @@ -6,12 +6,11 @@ from pants.backend.python.lint.bandit.rules import BanditFieldSet, BanditRequest from pants.backend.python.lint.bandit.rules import rules as bandit_rules from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.core.goals.lint import LintResults from pants.engine.addresses import Address from pants.engine.fs import DigestContents, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin +from pants.engine.target import Target from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.interpreter_selection_utils import skip_unless_python27_and_python3_present @@ -29,26 +28,19 @@ class BanditIntegrationTest(ExternalToolTestBase): def rules(cls): return (*super().rules(), *bandit_rules(), RootRule(BanditRequest)) - def make_target_with_origin( - self, - source_files: List[FileContent], - *, - interpreter_constraints: Optional[str] = None, - origin: Optional[OriginSpec] = None, - ) -> TargetWithOrigin: + def make_target( + self, source_files: List[FileContent], *, interpreter_constraints: Optional[str] = None + ) -> Target: for source_file in source_files: self.create_file(source_file.path, source_file.content.decode()) - target = PythonLibrary( + return PythonLibrary( {PythonInterpreterCompatibility.alias: interpreter_constraints}, address=Address.parse(":target"), ) - if origin is None: - origin = SingleAddress(directory="", name="target") - return TargetWithOrigin(target, origin) def run_bandit( self, - targets: List[TargetWithOrigin], + targets: List[Target], *, config: Optional[str] = None, passthrough_args: Optional[str] = None, @@ -74,7 +66,7 @@ def run_bandit( ) def test_passing_source(self) -> None: - target = self.make_target_with_origin([self.good_source]) + target = self.make_target([self.good_source]) result = self.run_bandit([target]) assert len(result) == 1 assert result[0].exit_code == 0 @@ -82,7 +74,7 @@ def test_passing_source(self) -> None: assert result[0].results_file is None def test_failing_source(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_bandit([target]) assert len(result) == 1 assert result[0].exit_code == 1 @@ -90,7 +82,7 @@ def test_failing_source(self) -> None: assert result[0].results_file is None def test_mixed_sources(self) -> None: - target = self.make_target_with_origin([self.good_source, self.bad_source]) + target = self.make_target([self.good_source, self.bad_source]) result = self.run_bandit([target]) assert len(result) == 1 assert result[0].exit_code == 1 @@ -100,8 +92,8 @@ def test_mixed_sources(self) -> None: def test_multiple_targets(self) -> None: targets = [ - self.make_target_with_origin([self.good_source]), - self.make_target_with_origin([self.bad_source]), + self.make_target([self.good_source]), + self.make_target([self.bad_source]), ] result = self.run_bandit(targets) assert len(result) == 1 @@ -110,20 +102,9 @@ def test_multiple_targets(self) -> None: assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout assert result[0].results_file is None - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], - origin=FilesystemLiteralSpec(self.good_source.path), - ) - result = self.run_bandit([target]) - assert len(result) == 1 - assert result[0].exit_code == 0 - assert "No issues identified." in result[0].stdout - assert result[0].results_file is None - @skip_unless_python27_and_python3_present def test_uses_correct_python_version(self) -> None: - py2_target = self.make_target_with_origin( + py2_target = self.make_target( [self.py3_only_source], interpreter_constraints="CPython==2.7.*" ) py2_result = self.run_bandit([py2_target]) @@ -131,7 +112,7 @@ def test_uses_correct_python_version(self) -> None: assert py2_result[0].exit_code == 0 assert "py3.py (syntax error while parsing AST from file)" in py2_result[0].stdout - py3_target = self.make_target_with_origin( + py3_target = self.make_target( [self.py3_only_source], interpreter_constraints="CPython>=3.6" ) py3_result = self.run_bandit([py3_target]) @@ -152,7 +133,7 @@ def test_uses_correct_python_version(self) -> None: assert "No issues identified." in batched_py3_result.stdout def test_respects_config_file(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_bandit([target], config="skips: ['B303']\n") assert len(result) == 1 assert result[0].exit_code == 0 @@ -160,7 +141,7 @@ def test_respects_config_file(self) -> None: assert result[0].results_file is None def test_respects_passthrough_args(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_bandit([target], passthrough_args="--skip B303") assert len(result) == 1 assert result[0].exit_code == 0 @@ -168,12 +149,12 @@ def test_respects_passthrough_args(self) -> None: assert result[0].results_file is None def test_skip(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_bandit([target], skip=True) assert not result def test_3rdparty_plugin(self) -> None: - target = self.make_target_with_origin( + target = self.make_target( [FileContent("bad.py", b"aws_key = 'JalrXUtnFEMI/K7MDENG/bPxRfiCYzEXAMPLEKEY'\n")], # NB: `bandit-aws` does not currently work with Python 3.8. See # https://github.com/pantsbuild/pants/issues/10545. @@ -188,7 +169,7 @@ def test_3rdparty_plugin(self) -> None: assert result[0].results_file is None def test_output_file(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_bandit([target], additional_args=["--lint-reports-dir='.'"]) assert len(result) == 1 assert result[0].exit_code == 1 diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py index 6ddc96a1414..2575c347db7 100644 --- a/src/python/pants/backend/python/lint/black/rules.py +++ b/src/python/pants/backend/python/lint/black/rules.py @@ -20,21 +20,17 @@ from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) -from pants.engine.fs import EMPTY_SNAPSHOT, Digest, GlobMatchErrorBehavior, MergeDigests, PathGlobs +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest +from pants.engine.fs import Digest, GlobMatchErrorBehavior, MergeDigests, PathGlobs from pants.engine.process import FallibleProcessResult, Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import FieldSetWithOrigin +from pants.engine.target import FieldSet from pants.engine.unions import UnionRule from pants.util.strutil import pluralize @dataclass(frozen=True) -class BlackFieldSet(FieldSetWithOrigin): +class BlackFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -56,9 +52,7 @@ class Setup: original_digest: Digest -def generate_args( - *, specified_source_files: SourceFiles, black: Black, check_only: bool, -) -> Tuple[str, ...]: +def generate_args(*, source_files: SourceFiles, black: Black, check_only: bool,) -> Tuple[str, ...]: args = [] if check_only: args.append("--check") @@ -70,8 +64,8 @@ def generate_args( # Black to run over everything recursively under the directory of our target, as Black should # only touch files directly specified. We can use `--include` to ensure that Black only # operates on the files we actually care about. - args.extend(["--include", "|".join(re.escape(f) for f in specified_source_files.files)]) - args.extend(PurePath(f).parent.as_posix() for f in specified_source_files.files) + args.extend(["--include", "|".join(re.escape(f) for f in source_files.files)]) + args.extend(PurePath(f).parent.as_posix() for f in source_files.files) return tuple(args) @@ -96,32 +90,23 @@ async def setup(setup_request: SetupRequest, black: Black) -> Setup: ), ) - all_source_files_request = Get( + source_files_request = Get( SourceFiles, - AllSourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), - ) - specified_source_files_request = Get( - SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in setup_request.request.field_sets - ), + SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), ) - requests = requirements_pex_request, config_digest_request, specified_source_files_request - all_source_files, requirements_pex, config_digest, specified_source_files = ( - await MultiGet(all_source_files_request, *requests) - if setup_request.request.prior_formatter_result is None - else (SourceFiles(EMPTY_SNAPSHOT, ()), *await MultiGet(*requests)) + source_files, requirements_pex, config_digest = await MultiGet( + source_files_request, requirements_pex_request, config_digest_request ) - all_source_files_snapshot = ( - all_source_files.snapshot + source_files_snapshot = ( + source_files.snapshot if setup_request.request.prior_formatter_result is None else setup_request.request.prior_formatter_result ) input_digest = await Get( Digest, - MergeDigests((all_source_files_snapshot.digest, requirements_pex.digest, config_digest)), + MergeDigests((source_files_snapshot.digest, requirements_pex.digest, config_digest)), ) address_references = ", ".join( @@ -133,19 +118,17 @@ async def setup(setup_request: SetupRequest, black: Black) -> Setup: PexProcess( requirements_pex, argv=generate_args( - specified_source_files=specified_source_files, - black=black, - check_only=setup_request.check_only, + source_files=source_files, black=black, check_only=setup_request.check_only ), input_digest=input_digest, - output_files=all_source_files_snapshot.files, + output_files=source_files_snapshot.files, description=( f"Run Black on {pluralize(len(setup_request.request.field_sets), 'target')}: " f"{address_references}." ), ), ) - return Setup(process, original_digest=all_source_files_snapshot.digest) + return Setup(process, original_digest=source_files_snapshot.digest) @rule(desc="Format using Black") 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 4e156465653..63d222a78cc 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 @@ -6,14 +6,13 @@ from pants.backend.python.lint.black.rules import BlackFieldSet, BlackRequest from pants.backend.python.lint.black.rules import rules as black_rules from pants.backend.python.target_types import PythonLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintResults -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin +from pants.engine.target import Target from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.option.util import create_options_bootstrapper @@ -32,19 +31,14 @@ class BlackIntegrationTest(ExternalToolTestBase): def rules(cls): return (*super().rules(), *black_rules(), RootRule(BlackRequest)) - def make_target_with_origin( - self, source_files: List[FileContent], *, origin: Optional[OriginSpec] = None, - ) -> TargetWithOrigin: + def make_target(self, source_files: List[FileContent]) -> Target: for source_file in source_files: self.create_file(f"{source_file.path}", source_file.content.decode()) - target = PythonLibrary({}, address=Address.parse(":target")) - if origin is None: - origin = SingleAddress(directory="", name="target") - return TargetWithOrigin(target, origin) + return PythonLibrary({}, address=Address.parse(":target")) def run_black( self, - targets: List[TargetWithOrigin], + targets: List[Target], *, config: Optional[str] = None, passthrough_args: Optional[str] = None, @@ -66,7 +60,7 @@ def run_black( input_sources = self.request_single_product( SourceFiles, Params( - AllSourceFilesRequest(field_set.sources for field_set in field_sets), + SourceFilesRequest(field_set.sources for field_set in field_sets), options_bootstrapper, ), ) @@ -83,7 +77,7 @@ def get_digest(self, source_files: List[FileContent]) -> Digest: return self.request_single_product(Digest, CreateDigest(source_files)) def test_passing_source(self) -> None: - target = self.make_target_with_origin([self.good_source]) + target = self.make_target([self.good_source]) lint_results, fmt_result = self.run_black([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 0 @@ -93,7 +87,7 @@ def test_passing_source(self) -> None: assert fmt_result.did_change is False def test_failing_source(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) lint_results, fmt_result = self.run_black([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 1 @@ -103,7 +97,7 @@ def test_failing_source(self) -> None: assert fmt_result.did_change is True def test_mixed_sources(self) -> None: - target = self.make_target_with_origin([self.good_source, self.bad_source]) + target = self.make_target([self.good_source, self.bad_source]) lint_results, fmt_result = self.run_black([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 1 @@ -116,8 +110,8 @@ def test_mixed_sources(self) -> None: def test_multiple_targets(self) -> None: targets = [ - self.make_target_with_origin([self.good_source]), - self.make_target_with_origin([self.bad_source]), + self.make_target([self.good_source]), + self.make_target([self.bad_source]), ] lint_results, fmt_result = self.run_black(targets) assert len(lint_results) == 1 @@ -129,20 +123,8 @@ def test_multiple_targets(self) -> None: assert fmt_result.output == self.get_digest([self.good_source, self.fixed_bad_source]) assert fmt_result.did_change is True - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], origin=FilesystemLiteralSpec(self.good_source.path) - ) - lint_results, fmt_result = self.run_black([target]) - 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 == self.get_digest([self.good_source, self.bad_source]) - assert fmt_result.did_change is False - def test_respects_config_file(self) -> None: - target = self.make_target_with_origin([self.needs_config_source]) + target = self.make_target([self.needs_config_source]) lint_results, fmt_result = self.run_black( [target], config="[tool.black]\nskip-string-normalization = 'true'\n" ) @@ -154,7 +136,7 @@ def test_respects_config_file(self) -> None: assert fmt_result.did_change is False def test_respects_passthrough_args(self) -> None: - target = self.make_target_with_origin([self.needs_config_source]) + target = self.make_target([self.needs_config_source]) lint_results, fmt_result = self.run_black( [target], passthrough_args="--skip-string-normalization", ) @@ -166,7 +148,7 @@ def test_respects_passthrough_args(self) -> None: assert fmt_result.did_change is False def test_skip(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) lint_results, fmt_result = self.run_black([target], skip=True) assert not lint_results assert fmt_result == FmtResult.noop() diff --git a/src/python/pants/backend/python/lint/docformatter/rules.py b/src/python/pants/backend/python/lint/docformatter/rules.py index 47966059e9e..69527ad6aaf 100644 --- a/src/python/pants/backend/python/lint/docformatter/rules.py +++ b/src/python/pants/backend/python/lint/docformatter/rules.py @@ -18,21 +18,17 @@ from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) -from pants.engine.fs import EMPTY_SNAPSHOT, Digest, MergeDigests +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest +from pants.engine.fs import Digest, MergeDigests from pants.engine.process import FallibleProcessResult, Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import FieldSetWithOrigin +from pants.engine.target import FieldSet from pants.engine.unions import UnionRule from pants.util.strutil import pluralize @dataclass(frozen=True) -class DocformatterFieldSet(FieldSetWithOrigin): +class DocformatterFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -55,13 +51,9 @@ class Setup: def generate_args( - *, specified_source_files: SourceFiles, docformatter: Docformatter, check_only: bool, + *, source_files: SourceFiles, docformatter: Docformatter, check_only: bool, ) -> Tuple[str, ...]: - return ( - "--check" if check_only else "--in-place", - *docformatter.args, - *specified_source_files.files, - ) + return ("--check" if check_only else "--in-place", *docformatter.args, *source_files.files) @rule @@ -76,31 +68,21 @@ async def setup(setup_request: SetupRequest, docformatter: Docformatter) -> Setu ), ) - all_source_files_request = Get( - SourceFiles, - AllSourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), - ) - specified_source_files_request = Get( + source_files_request = Get( SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in setup_request.request.field_sets - ), + SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), ) - requests = requirements_pex_request, specified_source_files_request - all_source_files, requirements_pex, specified_source_files = ( - await MultiGet(all_source_files_request, *requests) - if setup_request.request.prior_formatter_result is None - else (SourceFiles(EMPTY_SNAPSHOT, ()), *await MultiGet(*requests)) - ) - all_source_files_snapshot = ( - all_source_files.snapshot + source_files, requirements_pex = await MultiGet(source_files_request, requirements_pex_request) + + source_files_snapshot = ( + source_files.snapshot if setup_request.request.prior_formatter_result is None else setup_request.request.prior_formatter_result ) input_digest = await Get( - Digest, MergeDigests((all_source_files_snapshot.digest, requirements_pex.digest)) + Digest, MergeDigests((source_files_snapshot.digest, requirements_pex.digest)) ) address_references = ", ".join( @@ -112,19 +94,19 @@ async def setup(setup_request: SetupRequest, docformatter: Docformatter) -> Setu PexProcess( requirements_pex, argv=generate_args( - specified_source_files=specified_source_files, + source_files=source_files, docformatter=docformatter, check_only=setup_request.check_only, ), input_digest=input_digest, - output_files=all_source_files_snapshot.files, + output_files=source_files_snapshot.files, description=( f"Run Docformatter on {pluralize(len(setup_request.request.field_sets), 'target')}: " f"{address_references}." ), ), ) - return Setup(process, original_digest=all_source_files_snapshot.digest) + return Setup(process, original_digest=source_files_snapshot.digest) @rule(desc="Format Python docstrings with docformatter") 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 0f2334e6968..15191b88843 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 @@ -6,14 +6,13 @@ from pants.backend.python.lint.docformatter.rules import DocformatterFieldSet, DocformatterRequest from pants.backend.python.lint.docformatter.rules import rules as docformatter_rules from pants.backend.python.target_types import PythonLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintResults -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin +from pants.engine.target import Target from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.option.util import create_options_bootstrapper @@ -29,22 +28,13 @@ class DocformatterIntegrationTest(ExternalToolTestBase): def rules(cls): return (*super().rules(), *docformatter_rules(), RootRule(DocformatterRequest)) - def make_target_with_origin( - self, source_files: List[FileContent], *, origin: Optional[OriginSpec] = None, - ) -> TargetWithOrigin: + def make_target(self, source_files: List[FileContent]) -> Target: for source_file in source_files: self.create_file(f"{source_file.path}", source_file.content.decode()) - target = PythonLibrary({}, address=Address.parse(":target")) - if origin is None: - origin = SingleAddress(directory="", name="target") - return TargetWithOrigin(target, origin) + return PythonLibrary({}, address=Address.parse(":target")) def run_docformatter( - self, - targets: List[TargetWithOrigin], - *, - passthrough_args: Optional[str] = None, - skip: bool = False, + self, targets: List[Target], *, passthrough_args: Optional[str] = None, skip: bool = False, ) -> Tuple[LintResults, FmtResult]: args = ["--backend-packages=pants.backend.python.lint.docformatter"] if passthrough_args: @@ -59,7 +49,7 @@ def run_docformatter( input_sources = self.request_single_product( SourceFiles, Params( - AllSourceFilesRequest(field_set.sources for field_set in field_sets), + SourceFilesRequest(field_set.sources for field_set in field_sets), options_bootstrapper, ), ) @@ -76,7 +66,7 @@ def get_digest(self, source_files: List[FileContent]) -> Digest: return self.request_single_product(Digest, CreateDigest(source_files)) def test_passing_source(self) -> None: - target = self.make_target_with_origin([self.good_source]) + target = self.make_target([self.good_source]) lint_results, fmt_result = self.run_docformatter([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 0 @@ -85,7 +75,7 @@ def test_passing_source(self) -> None: assert fmt_result.did_change is False def test_failing_source(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) lint_results, fmt_result = self.run_docformatter([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 3 @@ -94,7 +84,7 @@ def test_failing_source(self) -> None: assert fmt_result.did_change is True def test_mixed_sources(self) -> None: - target = self.make_target_with_origin([self.good_source, self.bad_source]) + target = self.make_target([self.good_source, self.bad_source]) lint_results, fmt_result = self.run_docformatter([target]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 3 @@ -104,8 +94,8 @@ def test_mixed_sources(self) -> None: def test_multiple_targets(self) -> None: targets = [ - self.make_target_with_origin([self.good_source]), - self.make_target_with_origin([self.bad_source]), + self.make_target([self.good_source]), + self.make_target([self.bad_source]), ] lint_results, fmt_result = self.run_docformatter(targets) assert len(lint_results) == 1 @@ -114,23 +104,12 @@ def test_multiple_targets(self) -> None: assert fmt_result.output == self.get_digest([self.good_source, self.fixed_bad_source]) assert fmt_result.did_change is True - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], origin=FilesystemLiteralSpec(self.good_source.path) - ) - lint_results, fmt_result = self.run_docformatter([target]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stderr == "" - assert fmt_result.output == self.get_digest([self.good_source, self.bad_source]) - assert fmt_result.did_change is False - def test_respects_passthrough_args(self) -> None: needs_config = FileContent( path="needs_config.py", content=b'"""\nOne line docstring acting like it\'s multiline.\n"""\n', ) - target = self.make_target_with_origin([needs_config]) + target = self.make_target([needs_config]) lint_results, fmt_result = self.run_docformatter( [target], passthrough_args="--make-summary-multi-line" ) @@ -141,7 +120,7 @@ def test_respects_passthrough_args(self) -> None: assert fmt_result.did_change is False def test_skip(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) lint_results, fmt_result = self.run_docformatter([target], skip=True) assert not lint_results assert fmt_result == FmtResult.noop() diff --git a/src/python/pants/backend/python/lint/flake8/rules.py b/src/python/pants/backend/python/lint/flake8/rules.py index 49db854c5a0..6cc122a3829 100644 --- a/src/python/pants/backend/python/lint/flake8/rules.py +++ b/src/python/pants/backend/python/lint/flake8/rules.py @@ -22,11 +22,7 @@ LintSubsystem, ) from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import ( Digest, DigestSubset, @@ -37,14 +33,14 @@ ) from pants.engine.process import FallibleProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import FieldSetWithOrigin +from pants.engine.target import FieldSet from pants.engine.unions import UnionRule from pants.python.python_setup import PythonSetup from pants.util.strutil import pluralize @dataclass(frozen=True) -class Flake8FieldSet(FieldSetWithOrigin): +class Flake8FieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -62,7 +58,7 @@ class Flake8Partition: def generate_args( - *, specified_source_files: SourceFiles, flake8: Flake8, output_file: Optional[str] + *, source_files: SourceFiles, flake8: Flake8, output_file: Optional[str] ) -> Tuple[str, ...]: args = [] if flake8.config: @@ -70,7 +66,7 @@ def generate_args( if output_file: args.append(f"--output-file={output_file}") args.extend(flake8.args) - args.extend(specified_source_files.files) + args.extend(source_files.files) return tuple(args) @@ -100,26 +96,17 @@ async def flake8_lint_partition( ), ) - all_source_files_request = Get( - SourceFiles, AllSourceFilesRequest(field_set.sources for field_set in partition.field_sets) - ) - specified_source_files_request = Get( - SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in partition.field_sets - ), + source_files_request = Get( + SourceFiles, SourceFilesRequest(field_set.sources for field_set in partition.field_sets) ) - requirements_pex, config_digest, all_source_files, specified_source_files = await MultiGet( - requirements_pex_request, - config_digest_request, - all_source_files_request, - specified_source_files_request, + requirements_pex, config_digest, source_files = await MultiGet( + requirements_pex_request, config_digest_request, source_files_request ) input_digest = await Get( Digest, - MergeDigests((all_source_files.snapshot.digest, requirements_pex.digest, config_digest)), + MergeDigests((source_files.snapshot.digest, requirements_pex.digest, config_digest)), ) address_references = ", ".join( @@ -134,7 +121,7 @@ async def flake8_lint_partition( PexProcess( requirements_pex, argv=generate_args( - specified_source_files=specified_source_files, + source_files=source_files, flake8=flake8, output_file=report_path.name if report_path else None, ), diff --git a/src/python/pants/backend/python/lint/flake8/rules_integration_test.py b/src/python/pants/backend/python/lint/flake8/rules_integration_test.py index f983d1d2c1f..4439e6eae1d 100644 --- a/src/python/pants/backend/python/lint/flake8/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/flake8/rules_integration_test.py @@ -6,12 +6,11 @@ from pants.backend.python.lint.flake8.rules import Flake8FieldSet, Flake8Request from pants.backend.python.lint.flake8.rules import rules as flake8_rules from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.core.goals.lint import LintResults from pants.engine.addresses import Address from pants.engine.fs import DigestContents, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin +from pants.engine.target import Target from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.interpreter_selection_utils import skip_unless_python27_and_python3_present @@ -28,26 +27,19 @@ class Flake8IntegrationTest(ExternalToolTestBase): def rules(cls): return (*super().rules(), *flake8_rules(), RootRule(Flake8Request)) - def make_target_with_origin( - self, - source_files: List[FileContent], - *, - interpreter_constraints: Optional[str] = None, - origin: Optional[OriginSpec] = None, - ) -> TargetWithOrigin: + def make_target( + self, source_files: List[FileContent], *, interpreter_constraints: Optional[str] = None + ) -> Target: for source_file in source_files: self.create_file(source_file.path, source_file.content.decode()) - target = PythonLibrary( + return PythonLibrary( {PythonInterpreterCompatibility.alias: interpreter_constraints}, address=Address.parse(":target"), ) - if origin is None: - origin = SingleAddress(directory="test", name="target") - return TargetWithOrigin(target, origin) def run_flake8( self, - targets: List[TargetWithOrigin], + targets: List[Target], *, config: Optional[str] = None, passthrough_args: Optional[str] = None, @@ -73,7 +65,7 @@ def run_flake8( ) def test_passing_source(self) -> None: - target = self.make_target_with_origin([self.good_source]) + target = self.make_target([self.good_source]) result = self.run_flake8([target]) assert len(result) == 1 assert result[0].exit_code == 0 @@ -81,7 +73,7 @@ def test_passing_source(self) -> None: assert result[0].results_file is None def test_failing_source(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_flake8([target]) assert len(result) == 1 assert result[0].exit_code == 1 @@ -89,7 +81,7 @@ def test_failing_source(self) -> None: assert result[0].results_file is None def test_mixed_sources(self) -> None: - target = self.make_target_with_origin([self.good_source, self.bad_source]) + target = self.make_target([self.good_source, self.bad_source]) result = self.run_flake8([target]) assert len(result) == 1 assert result[0].exit_code == 1 @@ -98,8 +90,8 @@ def test_mixed_sources(self) -> None: def test_multiple_targets(self) -> None: targets = [ - self.make_target_with_origin([self.good_source]), - self.make_target_with_origin([self.bad_source]), + self.make_target([self.good_source]), + self.make_target([self.bad_source]), ] result = self.run_flake8(targets) assert len(result) == 1 @@ -107,18 +99,9 @@ def test_multiple_targets(self) -> None: assert "good.py" not in result[0].stdout assert "bad.py:1:1: F401" in result[0].stdout - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], origin=FilesystemLiteralSpec(self.good_source.path) - ) - result = self.run_flake8([target]) - assert len(result) == 1 - assert result[0].exit_code == 0 - assert result[0].stdout.strip() == "" - @skip_unless_python27_and_python3_present def test_uses_correct_python_version(self) -> None: - py2_target = self.make_target_with_origin( + py2_target = self.make_target( [self.py3_only_source], interpreter_constraints="CPython==2.7.*" ) py2_result = self.run_flake8([py2_target]) @@ -126,7 +109,7 @@ def test_uses_correct_python_version(self) -> None: assert py2_result[0].exit_code == 1 assert "py3.py:1:8: E999 SyntaxError" in py2_result[0].stdout - py3_target = self.make_target_with_origin( + py3_target = self.make_target( [self.py3_only_source], interpreter_constraints="CPython>=3.6" ) py3_result = self.run_flake8([py3_target]) @@ -147,28 +130,26 @@ def test_uses_correct_python_version(self) -> None: assert batched_py3_result.stdout.strip() == "" def test_respects_config_file(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_flake8([target], config="[flake8]\nignore = F401\n") assert len(result) == 1 assert result[0].exit_code == 0 assert result[0].stdout.strip() == "" def test_respects_passthrough_args(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_flake8([target], passthrough_args="--ignore=F401") assert len(result) == 1 assert result[0].exit_code == 0 assert result[0].stdout.strip() == "" def test_skip(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_flake8([target], skip=True) assert not result def test_3rdparty_plugin(self) -> None: - target = self.make_target_with_origin( - [FileContent("bad.py", b"'constant' and 'constant2'\n")] - ) + target = self.make_target([FileContent("bad.py", b"'constant' and 'constant2'\n")]) result = self.run_flake8( [target], additional_args=["--flake8-extra-requirements=flake8-pantsbuild>=2.0,<3"] ) @@ -177,7 +158,7 @@ def test_3rdparty_plugin(self) -> None: assert "bad.py:1:1: PB11" in result[0].stdout def test_output_file(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_flake8([target], additional_args=["--lint-reports-dir='.'"]) assert len(result) == 1 assert result[0].exit_code == 1 diff --git a/src/python/pants/backend/python/lint/isort/rules.py b/src/python/pants/backend/python/lint/isort/rules.py index c8e19ab7162..a8dd53cf6f1 100644 --- a/src/python/pants/backend/python/lint/isort/rules.py +++ b/src/python/pants/backend/python/lint/isort/rules.py @@ -18,13 +18,8 @@ from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import ( - EMPTY_SNAPSHOT, Digest, GlobExpansionConjunction, GlobMatchErrorBehavior, @@ -33,13 +28,13 @@ ) from pants.engine.process import FallibleProcessResult, Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import FieldSetWithOrigin +from pants.engine.target import FieldSet from pants.engine.unions import UnionRule from pants.util.strutil import pluralize @dataclass(frozen=True) -class IsortFieldSet(FieldSetWithOrigin): +class IsortFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -61,16 +56,14 @@ class Setup: original_digest: Digest -def generate_args( - *, specified_source_files: SourceFiles, isort: Isort, check_only: bool, -) -> Tuple[str, ...]: +def generate_args(*, source_files: SourceFiles, isort: Isort, check_only: bool) -> Tuple[str, ...]: # NB: isort auto-discovers config files. There is no way to hardcode them via command line # flags. So long as the files are in the Pex's input files, isort will use the config. args = [] if check_only: args.append("--check-only") args.extend(isort.args) - args.extend(specified_source_files.files) + args.extend(source_files.files) return tuple(args) @@ -96,36 +89,23 @@ async def setup(setup_request: SetupRequest, isort: Isort) -> Setup: ), ) - all_source_files_request = Get( - SourceFiles, - AllSourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), - ) - specified_source_files_request = Get( + source_files_request = Get( SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in setup_request.request.field_sets - ), + SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), ) - requests = ( - requirements_pex_request, - config_digest_request, - specified_source_files_request, - ) - all_source_files, requirements_pex, config_digest, specified_source_files = ( - await MultiGet(all_source_files_request, *requests) - if setup_request.request.prior_formatter_result is None - else (SourceFiles(EMPTY_SNAPSHOT, ()), *await MultiGet(*requests)) + source_files, requirements_pex, config_digest = await MultiGet( + source_files_request, requirements_pex_request, config_digest_request ) - all_source_files_snapshot = ( - all_source_files.snapshot + source_files_snapshot = ( + source_files.snapshot if setup_request.request.prior_formatter_result is None else setup_request.request.prior_formatter_result ) input_digest = await Get( Digest, - MergeDigests((all_source_files_snapshot.digest, requirements_pex.digest, config_digest)), + MergeDigests((source_files_snapshot.digest, requirements_pex.digest, config_digest)), ) address_references = ", ".join( @@ -137,19 +117,17 @@ async def setup(setup_request: SetupRequest, isort: Isort) -> Setup: PexProcess( requirements_pex, argv=generate_args( - specified_source_files=specified_source_files, - isort=isort, - check_only=setup_request.check_only, + source_files=source_files, isort=isort, check_only=setup_request.check_only ), input_digest=input_digest, - output_files=all_source_files_snapshot.files, + output_files=source_files_snapshot.files, description=( f"Run isort on {pluralize(len(setup_request.request.field_sets), 'target')}: " f"{address_references}." ), ), ) - return Setup(process, original_digest=all_source_files_snapshot.digest) + return Setup(process, original_digest=source_files_snapshot.digest) @rule(desc="Format using isort") 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 63bdb6c6d52..521c9dd652a 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 @@ -6,14 +6,13 @@ from pants.backend.python.lint.isort.rules import IsortFieldSet, IsortRequest from pants.backend.python.lint.isort.rules import rules as isort_rules from pants.backend.python.target_types import PythonLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.core.goals.fmt import FmtResult from pants.core.goals.lint import LintResults -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin +from pants.engine.target import Target from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.option.util import create_options_bootstrapper @@ -39,19 +38,14 @@ class IsortIntegrationTest(ExternalToolTestBase): def rules(cls): return (*super().rules(), *isort_rules(), RootRule(IsortRequest)) - def make_target_with_origin( - self, source_files: List[FileContent], *, origin: Optional[OriginSpec] = None, - ) -> TargetWithOrigin: + def make_target_with_origin(self, source_files: List[FileContent]) -> Target: for source_file in source_files: self.create_file(f"{source_file.path}", source_file.content.decode()) - target = PythonLibrary({}, address=Address.parse(":target")) - if origin is None: - origin = SingleAddress(directory="", name="target") - return TargetWithOrigin(target, origin) + return PythonLibrary({}, address=Address.parse(":target")) def run_isort( self, - targets: List[TargetWithOrigin], + targets: List[Target], *, config: Optional[str] = None, passthrough_args: Optional[str] = None, @@ -73,7 +67,7 @@ def run_isort( input_sources = self.request_single_product( SourceFiles, Params( - AllSourceFilesRequest(field_set.sources for field_set in field_sets), + SourceFilesRequest(field_set.sources for field_set in field_sets), options_bootstrapper, ), ) @@ -137,18 +131,6 @@ def test_multiple_targets(self) -> None: assert fmt_result.output == self.get_digest([self.good_source, self.fixed_bad_source]) assert fmt_result.did_change is True - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], origin=FilesystemLiteralSpec(self.good_source.path) - ) - lint_results, fmt_result = self.run_isort([target]) - assert len(lint_results) == 1 - assert lint_results[0].exit_code == 0 - assert lint_results[0].stdout == "" - assert fmt_result.stdout == "" - assert fmt_result.output == self.get_digest([self.good_source, self.bad_source]) - assert fmt_result.did_change is False - def test_respects_config_file(self) -> None: target = self.make_target_with_origin([self.needs_config_source]) lint_results, fmt_result = self.run_isort( diff --git a/src/python/pants/backend/python/lint/pylint/rules.py b/src/python/pants/backend/python/lint/pylint/rules.py index f5047ff8547..a992874ffad 100644 --- a/src/python/pants/backend/python/lint/pylint/rules.py +++ b/src/python/pants/backend/python/lint/pylint/rules.py @@ -27,7 +27,7 @@ ) from pants.core.goals.lint import LintRequest, LintResult, LintResults from pants.core.util_rules import determine_source_files, strip_source_roots -from pants.core.util_rules.determine_source_files import SourceFiles, SpecifiedSourceFilesRequest +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address, Addresses from pants.engine.fs import ( EMPTY_DIGEST, @@ -42,7 +42,7 @@ from pants.engine.target import ( Dependencies, DependenciesRequest, - FieldSetWithOrigin, + FieldSet, Target, Targets, TransitiveTargets, @@ -54,7 +54,7 @@ @dataclass(frozen=True) -class PylintFieldSet(FieldSetWithOrigin): +class PylintFieldSet(FieldSet): required_fields = (PythonSources,) sources: PythonSources @@ -95,12 +95,12 @@ class PylintRequest(LintRequest): field_set_type = PylintFieldSet -def generate_args(*, specified_source_files: SourceFiles, pylint: Pylint) -> Tuple[str, ...]: +def generate_args(*, source_files: SourceFiles, pylint: Pylint) -> Tuple[str, ...]: args = [] if pylint.config is not None: args.append(f"--rcfile={pylint.config}") args.extend(pylint.args) - args.extend(specified_source_files.files) + args.extend(source_files.files) return tuple(args) @@ -164,11 +164,8 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L prepare_python_sources_request = Get( PythonSourceFiles, PythonSourceFilesRequest(partition.targets_with_dependencies), ) - specified_source_files_request = Get( - SourceFiles, - SpecifiedSourceFilesRequest( - (field_set.sources, field_set.origin) for field_set in partition.field_sets - ), + field_set_sources_request = Get( + SourceFiles, SourceFilesRequest(field_set.sources for field_set in partition.field_sets), ) ( @@ -178,7 +175,7 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L config_digest, prepared_plugin_sources, prepared_python_sources, - specified_source_files, + field_set_sources, ) = await MultiGet( pylint_pex_request, requirements_pex_request, @@ -186,7 +183,7 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L config_digest_request, prepare_plugin_sources_request, prepare_python_sources_request, - specified_source_files_request, + field_set_sources_request, ) prefixed_plugin_sources = ( @@ -229,7 +226,7 @@ async def pylint_lint_partition(partition: PylintPartition, pylint: Pylint) -> L FallibleProcessResult, PexProcess( pylint_runner_pex, - argv=generate_args(specified_source_files=specified_source_files, pylint=pylint), + argv=generate_args(source_files=field_set_sources, pylint=pylint), input_digest=input_digest, extra_env={"PEX_EXTRA_SYS_PATH": ":".join(pythonpath)}, description=( diff --git a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py index 265f4016087..4153b593a51 100644 --- a/src/python/pants/backend/python/lint/pylint/rules_integration_test.py +++ b/src/python/pants/backend/python/lint/pylint/rules_integration_test.py @@ -9,13 +9,12 @@ from pants.backend.python.lint.pylint.rules import PylintFieldSet, PylintRequest from pants.backend.python.lint.pylint.rules import rules as pylint_rules from pants.backend.python.target_types import PythonLibrary, PythonRequirementLibrary -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.goals.lint import LintResults from pants.engine.addresses import Address from pants.engine.fs import FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin, WrappedTarget +from pants.engine.target import Target, WrappedTarget from pants.python.python_requirement import PythonRequirement from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase @@ -42,22 +41,17 @@ def target_types(cls): @classmethod def rules(cls): - return ( - *super().rules(), - *pylint_rules(), - RootRule(PylintRequest), - ) + return (*super().rules(), *pylint_rules(), RootRule(PylintRequest)) - def make_target_with_origin( + def make_target( self, source_files: List[FileContent], *, package: Optional[str] = None, name: str = "target", interpreter_constraints: Optional[str] = None, - origin: Optional[OriginSpec] = None, dependencies: Optional[List[Address]] = None, - ) -> TargetWithOrigin: + ) -> Target: if not package: package = self.package for source_file in source_files: @@ -76,16 +70,11 @@ def make_target_with_origin( """ ), ) - target = self.request_single_product( - WrappedTarget, Address(package, target_name=name) - ).target - if origin is None: - origin = SingleAddress(directory=package, name=name) - return TargetWithOrigin(target, origin) + return self.request_single_product(WrappedTarget, Address(package, target_name=name)).target def run_pylint( self, - targets: List[TargetWithOrigin], + targets: List[Target], *, config: Optional[str] = None, passthrough_args: Optional[str] = None, @@ -114,21 +103,21 @@ def run_pylint( ) def test_passing_source(self) -> None: - target = self.make_target_with_origin([self.good_source]) + target = self.make_target([self.good_source]) result = self.run_pylint([target]) assert len(result) == 1 assert result[0].exit_code == 0 assert "Your code has been rated at 10.00/10" in result[0].stdout.strip() def test_failing_source(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_pylint([target]) assert len(result) == 1 assert result[0].exit_code == PYLINT_FAILURE_RETURN_CODE assert f"{self.package}/bad.py:2:0: C0103" in result[0].stdout def test_mixed_sources(self) -> None: - target = self.make_target_with_origin([self.good_source, self.bad_source]) + target = self.make_target([self.good_source, self.bad_source]) result = self.run_pylint([target]) assert len(result) == 1 assert result[0].exit_code == PYLINT_FAILURE_RETURN_CODE @@ -137,8 +126,8 @@ def test_mixed_sources(self) -> None: def test_multiple_targets(self) -> None: targets = [ - self.make_target_with_origin([self.good_source], name="t1"), - self.make_target_with_origin([self.bad_source], name="t2"), + self.make_target([self.good_source], name="t1"), + self.make_target([self.bad_source], name="t2"), ] result = self.run_pylint(targets) assert len(result) == 1 @@ -146,22 +135,13 @@ def test_multiple_targets(self) -> None: assert f"{self.package}/good.py" not in result[0].stdout assert f"{self.package}/bad.py:2:0: C0103" in result[0].stdout - def test_precise_file_args(self) -> None: - target = self.make_target_with_origin( - [self.good_source, self.bad_source], origin=FilesystemLiteralSpec(self.good_source.path) - ) - result = self.run_pylint([target]) - assert len(result) == 1 - assert result[0].exit_code == 0 - assert "Your code has been rated at 10.00/10" in result[0].stdout.strip() - @skip_unless_python27_and_python3_present def test_uses_correct_python_version(self) -> None: py2_args = [ "--pylint-version=pylint<2", "--pylint-extra-requirements=['setuptools<45', 'isort>=4.3.21,<4.4']", ] - py2_target = self.make_target_with_origin( + py2_target = self.make_target( [self.py3_only_source], name="py2", interpreter_constraints="CPython==2.7.*" ) py2_result = self.run_pylint([py2_target], additional_args=py2_args) @@ -169,7 +149,7 @@ def test_uses_correct_python_version(self) -> None: assert py2_result[0].exit_code == 2 assert "invalid syntax (, line 2) (syntax-error)" in py2_result[0].stdout - py3_target = self.make_target_with_origin( + py3_target = self.make_target( [self.py3_only_source], name="py3", # NB: Avoid Python 3.8+ for this test due to issues with asteroid/ast. @@ -192,14 +172,14 @@ def test_uses_correct_python_version(self) -> None: assert "Your code has been rated at 10.00/10" in batched_py3_result.stdout.strip() def test_respects_config_file(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_pylint([target], config="[pylint]\ndisable = C0103\n") assert len(result) == 1 assert result[0].exit_code == 0 assert "Your code has been rated at 10.00/10" in result[0].stdout.strip() def test_respects_passthrough_args(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_pylint([target], passthrough_args="--disable=C0103") assert len(result) == 1 assert result[0].exit_code == 0 @@ -257,7 +237,7 @@ def test_includes_direct_dependencies(self) -> None: print(green(THIS_VARIABLE_EXISTS)) """ ) - target = self.make_target_with_origin( + target = self.make_target( source_files=[FileContent(f"{self.package}/target.py", source_content.encode())], dependencies=[ Address(self.package, target_name="direct_dep"), @@ -271,7 +251,7 @@ def test_includes_direct_dependencies(self) -> None: assert "Your code has been rated at 10.00/10" in result[0].stdout.strip() def test_skip(self) -> None: - target = self.make_target_with_origin([self.bad_source]) + target = self.make_target([self.bad_source]) result = self.run_pylint([target], skip=True) assert not result @@ -289,8 +269,8 @@ def test_pep420_namespace_packages(self) -> None: ).encode(), ) targets = [ - self.make_target_with_origin([self.good_source]), - self.make_target_with_origin( + self.make_target([self.good_source]), + self.make_target( [test_fc], package="tests/python/project", dependencies=[Address.parse(f"{self.package}:target")], @@ -316,7 +296,7 @@ def test_plugin(self): self.assertEqual(True, True) """ ) - target = self.make_target_with_origin( + target = self.make_target( [FileContent(f"{self.package}/thirdparty_plugin.py", source_content.encode())] ) result = self.run_pylint( @@ -405,7 +385,7 @@ def register(linter): load-plugins=print_plugin """ ) - target = self.make_target_with_origin( + target = self.make_target( [FileContent(f"{self.package}/source_plugin.py", b"'''Docstring.'''\nprint()\n")] ) result = self.run_pylint( diff --git a/src/python/pants/backend/python/lint/python_fmt.py b/src/python/pants/backend/python/lint/python_fmt.py index dcab1386c28..e253b2feb3d 100644 --- a/src/python/pants/backend/python/lint/python_fmt.py +++ b/src/python/pants/backend/python/lint/python_fmt.py @@ -7,7 +7,7 @@ from pants.backend.python.target_types import PythonSources from pants.core.goals.fmt import FmtResult, LanguageFmtResults, LanguageFmtTargets from pants.core.goals.style_request import StyleRequest -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.fs import Digest, Snapshot from pants.engine.rules import Get, collect_rules, rule from pants.engine.unions import UnionMembership, UnionRule, union @@ -27,13 +27,9 @@ class PythonFmtRequest(StyleRequest): async def format_python_target( python_fmt_targets: PythonFmtTargets, union_membership: UnionMembership ) -> LanguageFmtResults: - targets_with_origins = python_fmt_targets.targets_with_origins original_sources = await Get( SourceFiles, - AllSourceFilesRequest( - target_with_origin.target[PythonSources] - for target_with_origin in python_fmt_targets.targets_with_origins - ), + SourceFilesRequest(target[PythonSources] for target in python_fmt_targets.targets), ) prior_formatter_result = original_sources.snapshot @@ -47,8 +43,8 @@ async def format_python_target( PythonFmtRequest, fmt_request_type( ( - fmt_request_type.field_set_type.create(target_with_origin) - for target_with_origin in targets_with_origins + fmt_request_type.field_set_type.create(target) + for target in python_fmt_targets.targets ), prior_formatter_result=prior_formatter_result, ), 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 f733474fb5c..6e355aa8e2c 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 @@ -9,12 +9,11 @@ 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 -from pants.base.specs import SingleAddress from pants.core.goals.fmt import LanguageFmtResults from pants.engine.addresses import Address from pants.engine.fs import CreateDigest, Digest, FileContent from pants.engine.rules import RootRule -from pants.engine.target import TargetsWithOrigins, TargetWithOrigin +from pants.engine.target import Targets from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase from pants.testutil.option.util import create_options_bootstrapper @@ -38,9 +37,9 @@ def run_black_and_isort( ) -> LanguageFmtResults: for source_file in source_files: self.create_file(source_file.path, source_file.content.decode()) - target = PythonLibrary({}, address=Address.parse(f"test:{name}")) - origin = SingleAddress(directory="test", name=name) - targets = PythonFmtTargets(TargetsWithOrigins([TargetWithOrigin(target, origin)])) + targets = PythonFmtTargets( + Targets([PythonLibrary({}, address=Address.parse(f"test:{name}"))]) + ) args = [ "--backend-packages=['pants.backend.python.lint.black', 'pants.backend.python.lint.isort']", *(extra_args or []), diff --git a/src/python/pants/backend/python/rules/pytest_runner.py b/src/python/pants/backend/python/rules/pytest_runner.py index 77beed8a98b..22f9e8a352f 100644 --- a/src/python/pants/backend/python/rules/pytest_runner.py +++ b/src/python/pants/backend/python/rules/pytest_runner.py @@ -29,7 +29,7 @@ PythonTestsTimeout, ) from pants.core.goals.test import TestDebugRequest, TestFieldSet, TestResult, TestSubsystem -from pants.core.util_rules.determine_source_files import SourceFiles, SpecifiedSourceFilesRequest +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Addresses from pants.engine.fs import AddPrefix, Digest, DigestSubset, MergeDigests, PathGlobs, Snapshot from pants.engine.internals.uuid import UUIDRequest @@ -152,22 +152,20 @@ async def setup_pytest_for_target( # Get the file names for the test_target so that we can specify to Pytest precisely which files # to test, rather than using auto-discovery. - specified_source_files_request = Get( - SourceFiles, SpecifiedSourceFilesRequest([(field_set.sources, field_set.origin)]) - ) + field_set_source_files_request = Get(SourceFiles, SourceFilesRequest([field_set.sources])) ( pytest_pex, requirements_pex, test_runner_pex, prepared_sources, - specified_source_files, + field_set_source_files, ) = await MultiGet( pytest_pex_request, requirements_pex_request, test_runner_pex_request, prepared_sources_request, - specified_source_files_request, + field_set_source_files_request, ) input_digest = await Get( @@ -192,7 +190,7 @@ async def setup_pytest_for_target( ] return TestTargetSetup( test_runner_pex=test_runner_pex, - args=(*pytest.options.args, *coverage_args, *specified_source_files.files), + args=(*pytest.options.args, *coverage_args, *field_set_source_files.files), input_digest=input_digest, source_roots=prepared_sources.source_roots, timeout_seconds=field_set.timeout.calculate_from_global_options(pytest), diff --git a/src/python/pants/backend/python/rules/pytest_runner_integration_test.py b/src/python/pants/backend/python/rules/pytest_runner_integration_test.py index c3c4545fd5b..e9fc8d64fec 100644 --- a/src/python/pants/backend/python/rules/pytest_runner_integration_test.py +++ b/src/python/pants/backend/python/rules/pytest_runner_integration_test.py @@ -12,7 +12,6 @@ from pants.backend.python.rules.coverage import create_coverage_config from pants.backend.python.rules.pytest_runner import PythonTestFieldSet from pants.backend.python.target_types import PythonLibrary, PythonRequirementLibrary, PythonTests -from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.goals.test import Status, TestDebugRequest, TestResult from pants.core.util_rules import determine_source_files, strip_source_roots @@ -20,7 +19,6 @@ from pants.engine.fs import DigestContents, FileContent from pants.engine.process import InteractiveRunner from pants.engine.rules import RootRule -from pants.engine.target import TargetWithOrigin from pants.python.python_requirement import PythonRequirement from pants.testutil.engine.util import Params from pants.testutil.external_tool_test_base import ExternalToolTestBase @@ -136,7 +134,6 @@ def run_pytest( self, *, passthrough_args: Optional[str] = None, - origin: Optional[OriginSpec] = None, junit_xml_dir: Optional[str] = None, use_coverage: bool = False, execution_slot_var: Optional[str] = None, @@ -157,13 +154,11 @@ def run_pytest( if execution_slot_var: args.append(f"--pytest-execution-slot-var={execution_slot_var}") - options_bootstrapper = create_options_bootstrapper(args=args) - address = Address(self.package, target_name="target") - if origin is None: - origin = SingleAddress(directory=address.spec_path, name=address.target_name) - tgt = PythonTests({}, address=address) params = Params( - PythonTestFieldSet.create(TargetWithOrigin(tgt, origin)), options_bootstrapper + PythonTestFieldSet.create( + PythonTests({}, address=Address(self.package, target_name="target")) + ), + create_options_bootstrapper(args=args), ) test_result = self.request_single_product(TestResult, params) debug_request = self.request_single_product(TestDebugRequest, params) @@ -193,14 +188,6 @@ def test_mixed_sources(self) -> None: assert f"{self.package}/test_good.py ." in result.stdout assert f"{self.package}/test_bad.py F" in result.stdout - def test_precise_file_args(self) -> None: - self.create_python_test_target([self.good_source, self.bad_source]) - file_arg = FilesystemLiteralSpec(PurePath(self.package, self.good_source.path).as_posix()) - result = self.run_pytest(origin=file_arg) - assert result.status == Status.SUCCESS - assert f"{self.package}/test_good.py ." in result.stdout - assert f"{self.package}/test_bad.py F" not in result.stdout - def test_absolute_import(self) -> None: self.create_python_library([self.library_source]) source = FileContent( diff --git a/src/python/pants/backend/python/rules/python_sources.py b/src/python/pants/backend/python/rules/python_sources.py index 8eef0daa63b..19199cf0a32 100644 --- a/src/python/pants/backend/python/rules/python_sources.py +++ b/src/python/pants/backend/python/rules/python_sources.py @@ -9,7 +9,7 @@ from pants.backend.python.target_types import PythonSources from pants.core.target_types import FilesSources, ResourcesSources from pants.core.util_rules import determine_source_files -from pants.core.util_rules.determine_source_files import AllSourceFilesRequest, SourceFiles +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.core.util_rules.strip_source_roots import StrippedSourceFiles from pants.core.util_rules.strip_source_roots import rules as strip_source_roots_rules from pants.engine.fs import MergeDigests, Snapshot @@ -81,7 +81,7 @@ async def prepare_python_sources( ) -> PythonSourceFiles: sources = await Get( SourceFiles, - AllSourceFilesRequest( + SourceFilesRequest( (tgt.get(Sources) for tgt in request.targets), for_sources_types=request.valid_sources_types, enable_codegen=True, diff --git a/src/python/pants/base/specs.py b/src/python/pants/base/specs.py index 6c000d4ebfb..6a4709347fc 100644 --- a/src/python/pants/base/specs.py +++ b/src/python/pants/base/specs.py @@ -323,23 +323,12 @@ class FilesystemSpec(Spec, metaclass=ABCMeta): pass -class FilesystemResolvedSpec(FilesystemSpec, metaclass=ABCMeta): - @property - @abstractmethod - def resolved_files(self) -> Tuple[str, ...]: - """The literal files this spec refers to after resolving all globs and excludes.""" - - @dataclass(frozen=True) -class FilesystemLiteralSpec(FilesystemResolvedSpec): +class FilesystemLiteralSpec(FilesystemSpec): """A literal file name, e.g. `foo.py`.""" file: str - @property - def resolved_files(self) -> Tuple[str, ...]: - return (self.file,) - def to_spec_string(self) -> str: return self.file @@ -354,46 +343,6 @@ def to_spec_string(self) -> str: return self.glob -@dataclass(frozen=True) -class FilesystemResolvedGlobSpec(FilesystemGlobSpec, FilesystemResolvedSpec): - """A spec with resolved globs. - - For example, `*.py` may resolve to `('f1.py', 'f2.py', '__init__.py')`. - """ - - files: Tuple[str, ...] - - @property - def resolved_files(self) -> Tuple[str, ...]: - return self.files - - -@dataclass(frozen=True) -class FilesystemMergedSpec(FilesystemResolvedSpec): - """Represents multiple FilesystemSpecs belonging to the same target.""" - - globs: Tuple[str, ...] - files: Tuple[str, ...] - - @classmethod - def create( - cls, original_specs: Iterable[Union[FilesystemLiteralSpec, FilesystemResolvedGlobSpec]] - ) -> "FilesystemMergedSpec": - globs: List[str] = [] - files: List[str] = [] - for spec in original_specs: - globs.append(spec.to_spec_string()) - files.extend(spec.resolved_files) - return cls(globs=tuple(sorted(globs)), files=tuple(sorted(files))) - - @property - def resolved_files(self) -> Tuple[str, ...]: - return self.files - - def to_spec_string(self) -> str: - return ", ".join(self.globs) - - @dataclass(frozen=True) class FilesystemIgnoreSpec(FilesystemSpec): """A spec to ignore certain files or globs.""" @@ -449,10 +398,6 @@ def to_path_globs(self, glob_match_error_behavior: GlobMatchErrorBehavior) -> Pa return self._generate_path_globs((*self.includes, *self.ignores), glob_match_error_behavior) -class AmbiguousSpecs(Exception): - pass - - @dataclass(frozen=True) class Specs: address_specs: AddressSpecs @@ -462,6 +407,3 @@ class Specs: def provided(self) -> bool: """Did the user provide specs?""" return bool(self.address_specs) or bool(self.filesystem_specs) - - -OriginSpec = Union[AddressSpec, FilesystemResolvedSpec] diff --git a/src/python/pants/core/goals/fmt.py b/src/python/pants/core/goals/fmt.py index f47790ca4d1..d0a23033f41 100644 --- a/src/python/pants/core/goals/fmt.py +++ b/src/python/pants/core/goals/fmt.py @@ -11,7 +11,7 @@ from pants.engine.goal import Goal, GoalSubsystem from pants.engine.process import ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule -from pants.engine.target import Field, Target, TargetsWithOrigins +from pants.engine.target import Field, Target, Targets from pants.engine.unions import UnionMembership, union from pants.util.strutil import strip_v2_chroot_path @@ -66,7 +66,7 @@ class LanguageFmtTargets: required_fields: ClassVar[Tuple[Type[Field], ...]] - targets_with_origins: TargetsWithOrigins + targets: Targets @classmethod def belongs_to_language(cls, tgt: Target) -> bool: @@ -130,7 +130,7 @@ class Fmt(Goal): @goal_rule async def fmt( console: Console, - targets_with_origins: TargetsWithOrigins, + targets: Targets, fmt_subsystem: FmtSubsystem, workspace: Workspace, union_membership: UnionMembership, @@ -138,22 +138,16 @@ async def fmt( language_target_collection_types = union_membership[LanguageFmtTargets] language_target_collections: Iterable[LanguageFmtTargets] = tuple( language_target_collection_type( - TargetsWithOrigins( - target_with_origin - for target_with_origin in targets_with_origins - if language_target_collection_type.belongs_to_language(target_with_origin.target) + Targets( + target + for target in targets + if language_target_collection_type.belongs_to_language(target) ) ) for language_target_collection_type in language_target_collection_types ) targets_with_sources: Iterable[TargetsWithSources] = await MultiGet( - Get( - TargetsWithSources, - TargetsWithSourcesRequest( - target_with_origin.target - for target_with_origin in language_target_collection.targets_with_origins - ), - ) + Get(TargetsWithSources, TargetsWithSourcesRequest(language_target_collection.targets),) for language_target_collection in language_target_collections ) # NB: We must convert back the generic TargetsWithSources objects back into their @@ -161,10 +155,10 @@ async def fmt( # rule to work. valid_language_target_collections: Iterable[LanguageFmtTargets] = tuple( language_target_collection_cls( - TargetsWithOrigins( - target_with_origin - for target_with_origin in language_target_collection.targets_with_origins - if target_with_origin.target in language_targets_with_sources + Targets( + target + for target in language_target_collection.targets + if target in language_targets_with_sources ) ) for language_target_collection_cls, language_target_collection, language_targets_with_sources in zip( @@ -178,10 +172,10 @@ async def fmt( Get( LanguageFmtResults, LanguageFmtTargets, - language_target_collection.__class__(TargetsWithOrigins([target_with_origin])), + language_target_collection.__class__(Targets([target])), ) for language_target_collection in valid_language_target_collections - for target_with_origin in language_target_collection.targets_with_origins + for target in language_target_collection.targets ) else: per_language_results = await MultiGet( diff --git a/src/python/pants/core/goals/fmt_test.py b/src/python/pants/core/goals/fmt_test.py index 184dd707389..7f5d0e153e6 100644 --- a/src/python/pants/core/goals/fmt_test.py +++ b/src/python/pants/core/goals/fmt_test.py @@ -6,7 +6,6 @@ from textwrap import dedent from typing import ClassVar, Iterable, List, Optional, Type, cast -from pants.base.specs import SingleAddress from pants.core.goals.fmt import ( Fmt, FmtResult, @@ -18,7 +17,7 @@ from pants.core.util_rules.filter_empty_sources import TargetsWithSources, TargetsWithSourcesRequest from pants.engine.addresses import Address from pants.engine.fs import EMPTY_DIGEST, Digest, FileContent, MergeDigests, Workspace -from pants.engine.target import Sources, Target, TargetsWithOrigins, TargetWithOrigin +from pants.engine.target import Sources, Target, Targets from pants.engine.unions import UnionMembership from pants.testutil.engine.util import MockConsole, MockGet, create_goal_subsystem, run_rule from pants.testutil.test_base import TestBase @@ -55,9 +54,7 @@ def stdout(_: Iterable[Address]) -> str: pass def language_fmt_results(self, result_digest: Digest) -> LanguageFmtResults: - addresses = [ - target_with_origin.target.address for target_with_origin in self.targets_with_origins - ] + addresses = [target.address for target in self.targets] return LanguageFmtResults( ( FmtResult( @@ -113,21 +110,16 @@ def setUp(self) -> None: ).digest @staticmethod - def make_target_with_origin( + def make_target( address: Optional[Address] = None, *, target_cls: Type[Target] = FortranTarget - ) -> TargetWithOrigin: - if address is None: - address = Address.parse(":tests") - return TargetWithOrigin( - target_cls({}, address=address), - origin=SingleAddress(directory=address.spec_path, name=address.target_name), - ) + ) -> Target: + return target_cls({}, address=address or Address.parse(":tests")) def run_fmt_rule( self, *, language_target_collection_types: List[Type[LanguageFmtTargets]], - targets: List[TargetWithOrigin], + targets: List[Target], result_digest: Digest, per_target_caching: bool, include_sources: bool = True, @@ -138,7 +130,7 @@ def run_fmt_rule( fmt, rule_args=[ console, - TargetsWithOrigins(targets), + Targets(targets), create_goal_subsystem(FmtSubsystem, per_target_caching=per_target_caching), Workspace(self.scheduler), union_membership, @@ -182,7 +174,7 @@ def test_empty_target_noops(self) -> None: def assert_noops(*, per_target_caching: bool) -> None: stderr = self.run_fmt_rule( language_target_collection_types=[FortranTargets], - targets=[self.make_target_with_origin()], + targets=[self.make_target()], result_digest=self.fortran_digest, per_target_caching=per_target_caching, include_sources=False, @@ -197,7 +189,7 @@ def test_invalid_target_noops(self) -> None: def assert_noops(*, per_target_caching: bool) -> None: stderr = self.run_fmt_rule( language_target_collection_types=[InvalidTargets], - targets=[self.make_target_with_origin()], + targets=[self.make_target()], result_digest=self.fortran_digest, per_target_caching=per_target_caching, ) @@ -209,7 +201,7 @@ def assert_noops(*, per_target_caching: bool) -> None: def test_single_language_with_single_target(self) -> None: address = Address.parse(":tests") - target_with_origin = self.make_target_with_origin(address) + target_with_origin = self.make_target(address) def assert_expected(*, per_target_caching: bool) -> None: stderr = self.run_fmt_rule( @@ -235,7 +227,7 @@ def test_single_language_with_multiple_targets(self) -> None: def get_stderr(*, per_target_caching: bool) -> str: stderr = self.run_fmt_rule( language_target_collection_types=[FortranTargets], - targets=[self.make_target_with_origin(addr) for addr in addresses], + targets=[self.make_target(addr) for addr in addresses], result_digest=self.fortran_digest, per_target_caching=per_target_caching, ) @@ -266,8 +258,8 @@ def assert_expected(*, per_target_caching: bool) -> None: stderr = self.run_fmt_rule( language_target_collection_types=[FortranTargets, SmalltalkTargets], targets=[ - self.make_target_with_origin(fortran_address, target_cls=FortranTarget), - self.make_target_with_origin(smalltalk_address, target_cls=SmalltalkTarget), + self.make_target(fortran_address, target_cls=FortranTarget), + self.make_target(smalltalk_address, target_cls=SmalltalkTarget), ], result_digest=self.merged_digest, per_target_caching=per_target_caching, @@ -291,12 +283,10 @@ def test_multiple_languages_with_multiple_targets(self) -> None: smalltalk_addresses = [Address.parse(":py1"), Address.parse(":py2")] fortran_targets = [ - self.make_target_with_origin(addr, target_cls=FortranTarget) - for addr in fortran_addresses + self.make_target(addr, target_cls=FortranTarget) for addr in fortran_addresses ] smalltalk_targets = [ - self.make_target_with_origin(addr, target_cls=SmalltalkTarget) - for addr in smalltalk_addresses + self.make_target(addr, target_cls=SmalltalkTarget) for addr in smalltalk_addresses ] def get_stderr(*, per_target_caching: bool) -> str: diff --git a/src/python/pants/core/goals/lint.py b/src/python/pants/core/goals/lint.py index 293b993316f..3e25b6639a1 100644 --- a/src/python/pants/core/goals/lint.py +++ b/src/python/pants/core/goals/lint.py @@ -17,7 +17,7 @@ from pants.engine.goal import Goal, GoalSubsystem from pants.engine.process import FallibleProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule -from pants.engine.target import TargetsWithOrigins +from pants.engine.target import Targets from pants.engine.unions import UnionMembership, union from pants.util.strutil import strip_v2_chroot_path @@ -132,16 +132,16 @@ class Lint(Goal): async def lint( console: Console, workspace: Workspace, - targets_with_origins: TargetsWithOrigins, + targets: Targets, lint_subsystem: LintSubsystem, union_membership: UnionMembership, ) -> Lint: request_types = union_membership[LintRequest] requests: Iterable[StyleRequest] = tuple( request_type( - request_type.field_set_type.create(target_with_origin) - for target_with_origin in targets_with_origins - if request_type.field_set_type.is_valid(target_with_origin.target) + request_type.field_set_type.create(target) + for target in targets + if request_type.field_set_type.is_valid(target) ) for request_type in request_types ) diff --git a/src/python/pants/core/goals/lint_test.py b/src/python/pants/core/goals/lint_test.py index c51f26924e8..0042757a62b 100644 --- a/src/python/pants/core/goals/lint_test.py +++ b/src/python/pants/core/goals/lint_test.py @@ -5,7 +5,6 @@ from textwrap import dedent from typing import ClassVar, Iterable, List, Optional, Tuple, Type -from pants.base.specs import SingleAddress from pants.core.goals.lint import Lint, LintRequest, LintResult, LintResults, LintSubsystem, lint from pants.core.util_rules.filter_empty_sources import ( FieldSetsWithSources, @@ -13,13 +12,7 @@ ) from pants.engine.addresses import Address from pants.engine.fs import Workspace -from pants.engine.target import ( - FieldSetWithOrigin, - Sources, - Target, - TargetsWithOrigins, - TargetWithOrigin, -) +from pants.engine.target import FieldSet, Sources, Target, Targets from pants.engine.unions import UnionMembership from pants.testutil.engine.util import MockConsole, MockGet, create_goal_subsystem, run_rule from pants.testutil.test_base import TestBase @@ -30,7 +23,7 @@ class MockTarget(Target): core_fields = (Sources,) -class MockLinterFieldSet(FieldSetWithOrigin): +class MockLinterFieldSet(FieldSet): required_fields = (Sources,) @@ -125,19 +118,14 @@ def stdout(addresses: Iterable[Address]) -> str: class LintTest(TestBase): @staticmethod - def make_target_with_origin(address: Optional[Address] = None) -> TargetWithOrigin: - if address is None: - address = Address.parse(":tests") - return TargetWithOrigin( - MockTarget({}, address=address), - origin=SingleAddress(directory=address.spec_path, name=address.target_name), - ) + def make_target(address: Optional[Address] = None) -> Target: + return MockTarget({}, address=address or Address.parse(":tests")) def run_lint_rule( self, *, lint_request_types: List[Type[LintRequest]], - targets: List[TargetWithOrigin], + targets: List[Target], per_target_caching: bool, include_sources: bool = True, ) -> Tuple[int, str]: @@ -149,7 +137,7 @@ def run_lint_rule( rule_args=[ console, workspace, - TargetsWithOrigins(targets), + Targets(targets), create_goal_subsystem(LintSubsystem, per_target_caching=per_target_caching), union_membership, ], @@ -176,7 +164,7 @@ def test_empty_target_noops(self) -> None: def assert_noops(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( lint_request_types=[FailingRequest], - targets=[self.make_target_with_origin()], + targets=[self.make_target()], per_target_caching=per_target_caching, include_sources=False, ) @@ -190,7 +178,7 @@ def test_invalid_target_noops(self) -> None: def assert_noops(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( lint_request_types=[InvalidRequest], - targets=[self.make_target_with_origin()], + targets=[self.make_target()], per_target_caching=per_target_caching, ) assert exit_code == 0 @@ -201,7 +189,7 @@ def assert_noops(per_target_caching: bool) -> None: def test_single_target_with_one_linter(self) -> None: address = Address.parse(":tests") - target_with_origin = self.make_target_with_origin(address) + target_with_origin = self.make_target(address) def assert_expected(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( @@ -222,7 +210,7 @@ def assert_expected(per_target_caching: bool) -> None: def test_single_target_with_multiple_linters(self) -> None: address = Address.parse(":tests") - target_with_origin = self.make_target_with_origin(address) + target_with_origin = self.make_target(address) def assert_expected(per_target_caching: bool) -> None: exit_code, stderr = self.run_lint_rule( @@ -251,10 +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( lint_request_types=[ConditionallySucceedsRequest], - targets=[ - self.make_target_with_origin(good_address), - self.make_target_with_origin(bad_address), - ], + targets=[self.make_target(good_address), self.make_target(bad_address),], per_target_caching=per_target_caching, ) assert exit_code == ConditionallySucceedsRequest.exit_code([bad_address]) @@ -284,10 +269,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( lint_request_types=[ConditionallySucceedsRequest, SuccessfulRequest], - targets=[ - self.make_target_with_origin(good_address), - self.make_target_with_origin(bad_address), - ], + targets=[self.make_target(good_address), self.make_target(bad_address),], per_target_caching=per_target_caching, ) assert exit_code == ConditionallySucceedsRequest.exit_code([bad_address]) diff --git a/src/python/pants/core/goals/test.py b/src/python/pants/core/goals/test.py index 86701f9b1e7..27f0d7ed3b4 100644 --- a/src/python/pants/core/goals/test.py +++ b/src/python/pants/core/goals/test.py @@ -23,7 +23,7 @@ from pants.engine.process import FallibleProcessResult, InteractiveProcess, InteractiveRunner from pants.engine.rules import Get, MultiGet, collect_rules, goal_rule, rule from pants.engine.target import ( - FieldSetWithOrigin, + FieldSet, Sources, TargetsToValidFieldSets, TargetsToValidFieldSetsRequest, @@ -104,7 +104,7 @@ class TestDebugRequest: @union -class TestFieldSet(FieldSetWithOrigin, metaclass=ABCMeta): +class TestFieldSet(FieldSet, metaclass=ABCMeta): """The fields necessary to run tests on a target.""" sources: Sources diff --git a/src/python/pants/core/goals/test_test.py b/src/python/pants/core/goals/test_test.py index b2e68655d70..13e4365d4c6 100644 --- a/src/python/pants/core/goals/test_test.py +++ b/src/python/pants/core/goals/test_test.py @@ -159,7 +159,7 @@ def mock_find_valid_field_sets( return TargetsToValidFieldSets({}) return TargetsToValidFieldSets( { - tgt_with_origin: [field_set.create(tgt_with_origin)] + tgt_with_origin: [field_set.create(tgt_with_origin.target)] for tgt_with_origin in targets } ) diff --git a/src/python/pants/core/util_rules/determine_source_files.py b/src/python/pants/core/util_rules/determine_source_files.py index 27e51ab8dac..337c2f9469b 100644 --- a/src/python/pants/core/util_rules/determine_source_files.py +++ b/src/python/pants/core/util_rules/determine_source_files.py @@ -2,12 +2,10 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from typing import Iterable, Set, Tuple, Type, Union +from typing import Iterable, Set, Tuple, Type -from pants.base.specs import AddressSpec, OriginSpec from pants.core.target_types import FilesSources -from pants.engine.addresses import Address -from pants.engine.fs import DigestSubset, MergeDigests, PathGlobs, Snapshot +from pants.engine.fs import MergeDigests, Snapshot from pants.engine.rules import Get, MultiGet, RootRule, collect_rules, rule from pants.engine.target import HydratedSources, HydrateSourcesRequest from pants.engine.target import Sources as SourcesField @@ -35,7 +33,7 @@ def files(self) -> Tuple[str, ...]: @frozen_after_init @dataclass(unsafe_hash=True) -class AllSourceFilesRequest: +class SourceFilesRequest: sources_fields: Tuple[SourcesField, ...] for_sources_types: Tuple[Type[SourcesField], ...] enable_codegen: bool @@ -52,41 +50,8 @@ def __init__( self.enable_codegen = enable_codegen -@frozen_after_init -@dataclass(unsafe_hash=True) -class SpecifiedSourceFilesRequest: - sources_fields_with_origins: Tuple[Tuple[SourcesField, OriginSpec], ...] - for_sources_types: Tuple[Type[SourcesField], ...] - enable_codegen: bool - - def __init__( - self, - sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]], - *, - for_sources_types: Iterable[Type[SourcesField]] = (SourcesField,), - enable_codegen: bool = False, - ) -> None: - self.sources_fields_with_origins = tuple(sources_fields_with_origins) - self.for_sources_types = tuple(for_sources_types) - self.enable_codegen = enable_codegen - - -def calculate_specified_sources( - sources_snapshot: Snapshot, address: Address, origin: OriginSpec -) -> Union[Snapshot, DigestSubset]: - # AddressSpecs simply use the entire `sources` field. If it's a generated subtarget, we also - # know we're as precise as we can get (1 file), so use the whole snapshot. - if isinstance(origin, AddressSpec) or not address.is_base_target: - return sources_snapshot - # NB: we ensure that `precise_files_specified` is a subset of the original `sources` field. - # It's possible when given a glob filesystem spec that the spec will have - # resolved files not belonging to this target - those must be filtered out. - precise_files_specified = set(sources_snapshot.files).intersection(origin.resolved_files) - return DigestSubset(sources_snapshot.digest, PathGlobs(sorted(precise_files_specified))) - - @rule -async def determine_all_source_files(request: AllSourceFilesRequest) -> SourceFiles: +async def determine_source_files(request: SourceFilesRequest) -> SourceFiles: """Merge all `Sources` fields into one Snapshot.""" unrooted_files: Set[str] = set() all_hydrated_sources = await MultiGet( @@ -112,58 +77,5 @@ async def determine_all_source_files(request: AllSourceFilesRequest) -> SourceFi return SourceFiles(result, tuple(sorted(unrooted_files))) -@rule -async def determine_specified_source_files(request: SpecifiedSourceFilesRequest) -> SourceFiles: - """Determine the specified `sources` for targets. - - Possibly finding a subset of the original `sources` fields if the user supplied file arguments. - """ - all_unrooted_files: Set[str] = set() - all_hydrated_sources = await MultiGet( - Get( - HydratedSources, - HydrateSourcesRequest( - sources_field_with_origin[0], - for_sources_types=request.for_sources_types, - enable_codegen=request.enable_codegen, - ), - ) - for sources_field_with_origin in request.sources_fields_with_origins - ) - - full_snapshots = {} - digest_subset_requests = {} - for hydrated_sources, sources_field_with_origin in zip( - all_hydrated_sources, request.sources_fields_with_origins - ): - sources_field, origin = sources_field_with_origin - if isinstance(sources_field, FilesSources): - all_unrooted_files.update(hydrated_sources.snapshot.files) - if not hydrated_sources.snapshot.files: - continue - specified_sources = calculate_specified_sources( - hydrated_sources.snapshot, sources_field.address, origin - ) - if isinstance(specified_sources, Snapshot): - full_snapshots[sources_field] = specified_sources - else: - digest_subset_requests[sources_field] = specified_sources - - snapshot_subsets: Tuple[Snapshot, ...] = () - if digest_subset_requests: - snapshot_subsets = await MultiGet( - Get(Snapshot, DigestSubset, request) for request in digest_subset_requests.values() - ) - - all_snapshots: Iterable[Snapshot] = (*full_snapshots.values(), *snapshot_subsets) - result = await Get(Snapshot, MergeDigests(snapshot.digest for snapshot in all_snapshots)) - unrooted_files = all_unrooted_files.intersection(result.files) - return SourceFiles(result, tuple(sorted(unrooted_files))) - - def rules(): - return [ - *collect_rules(), - RootRule(AllSourceFilesRequest), - RootRule(SpecifiedSourceFilesRequest), - ] + return [*collect_rules(), RootRule(SourceFilesRequest)] diff --git a/src/python/pants/core/util_rules/determine_source_files_test.py b/src/python/pants/core/util_rules/determine_source_files_test.py index c0f132721c8..17ea46024d0 100644 --- a/src/python/pants/core/util_rules/determine_source_files_test.py +++ b/src/python/pants/core/util_rules/determine_source_files_test.py @@ -1,24 +1,12 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). +import itertools from pathlib import PurePath -from typing import Iterable, List, NamedTuple, Optional, Tuple, Type +from typing import Iterable, List, NamedTuple, Type -from pants.base.specs import ( - AscendantAddresses, - DescendantAddresses, - FilesystemLiteralSpec, - FilesystemResolvedGlobSpec, - OriginSpec, - SiblingAddresses, - SingleAddress, -) from pants.core.target_types import FilesSources -from pants.core.util_rules.determine_source_files import ( - AllSourceFilesRequest, - SourceFiles, - SpecifiedSourceFilesRequest, -) +from pants.core.util_rules.determine_source_files import SourceFiles, SourceFilesRequest from pants.core.util_rules.determine_source_files import rules as determine_source_files_rules from pants.engine.addresses import Address from pants.engine.target import Sources as SourcesField @@ -32,227 +20,74 @@ class TargetSources(NamedTuple): source_files: List[str] @property - def source_file_absolute_paths(self) -> List[str]: + def full_paths(self) -> List[str]: return [PurePath(self.source_root, name).as_posix() for name in self.source_files] SOURCES1 = TargetSources("src/python", ["s1.py", "s2.py", "s3.py"]) SOURCES2 = TargetSources("tests/python", ["t1.py", "t2.java"]) SOURCES3 = TargetSources("src/java", ["j1.java", "j2.java"]) -SOURCES4 = TargetSources("src/python", ["README.md"]) class DetermineSourceFilesTest(TestBase): @classmethod def rules(cls): - return ( - *super().rules(), - *determine_source_files_rules(), - ) + return (*super().rules(), *determine_source_files_rules()) - def mock_sources_field_with_origin( + def mock_sources_field( self, sources: TargetSources, *, - origin: Optional[OriginSpec] = None, include_sources: bool = True, sources_field_cls: Type[SourcesField] = SourcesField, - ) -> Tuple[SourcesField, OriginSpec]: + ) -> SourcesField: sources_field = sources_field_cls( sources.source_files if include_sources else [], address=Address.parse(f"{sources.source_root}:lib"), ) self.create_files(path=sources.source_root, files=sources.source_files) - if origin is None: - origin = SiblingAddresses(sources.source_root) - return sources_field, origin + return sources_field - def _do_get_all_source_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]], - ) -> SourceFiles: - request = AllSourceFilesRequest( - ( - sources_field_with_origin[0] - for sources_field_with_origin in sources_fields_with_origins - ), - ) - return self.request_single_product( - SourceFiles, - Params( - request, - create_options_bootstrapper( - args=[ - "--source-root-patterns=src/python", - "--source-root-patterns=src/java", - "--source-root-patterns=tests/python", - ] - ), - ), + def assert_sources_resolved( + self, + sources_fields: Iterable[SourcesField], + *, + expected: Iterable[TargetSources], + expected_unrooted: Iterable[str] = (), + ) -> None: + result = self.request_single_product( + SourceFiles, Params(SourceFilesRequest(sources_fields), create_options_bootstrapper()), ) - - def get_all_source_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]], - ) -> List[str]: - return sorted(self._do_get_all_source_files(sources_fields_with_origins).snapshot.files) - - def get_all_unrooted_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]] - ): - return list(self._do_get_all_source_files(sources_fields_with_origins).unrooted_files) - - def _do_get_specified_source_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]], - ) -> SourceFiles: - request = SpecifiedSourceFilesRequest(sources_fields_with_origins,) - return self.request_single_product( - SourceFiles, - Params( - request, - create_options_bootstrapper( - args=[ - "--source-root-patterns=src/python", - "--source-root-patterns=src/java", - "--source-root-patterns=tests/python", - ] - ), - ), + assert list(result.snapshot.files) == sorted( + set(itertools.chain.from_iterable(sources.full_paths for sources in expected)) ) - - def get_specified_source_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]], - ) -> List[str]: - return sorted( - self._do_get_specified_source_files(sources_fields_with_origins).snapshot.files - ) - - def get_specified_unrooted_files( - self, sources_fields_with_origins: Iterable[Tuple[SourcesField, OriginSpec]] - ): - return list(self._do_get_specified_source_files(sources_fields_with_origins).unrooted_files) + assert list(result.unrooted_files) == sorted(expected_unrooted) def test_address_specs(self) -> None: - sources_field1 = self.mock_sources_field_with_origin( - SOURCES1, origin=SingleAddress(directory=SOURCES1.source_root, name="lib") - ) - sources_field2 = self.mock_sources_field_with_origin( - SOURCES2, origin=SiblingAddresses(SOURCES2.source_root) - ) - sources_field3 = self.mock_sources_field_with_origin( - SOURCES3, origin=DescendantAddresses(SOURCES3.source_root) - ) - sources_field4 = self.mock_sources_field_with_origin( - SOURCES1, origin=AscendantAddresses(SOURCES1.source_root) - ) + sources_field1 = self.mock_sources_field(SOURCES1) + sources_field2 = self.mock_sources_field(SOURCES2) + sources_field3 = self.mock_sources_field(SOURCES3) + sources_field4 = self.mock_sources_field(SOURCES1) - def assert_all_source_files_resolved( - sources_field_with_origin: Tuple[SourcesField, OriginSpec], sources: TargetSources - ) -> None: - expected = sources.source_file_absolute_paths - assert self.get_all_source_files([sources_field_with_origin]) == expected - assert self.get_specified_source_files([sources_field_with_origin]) == expected + self.assert_sources_resolved([sources_field1], expected=[SOURCES1]) + self.assert_sources_resolved([sources_field2], expected=[SOURCES2]) + self.assert_sources_resolved([sources_field3], expected=[SOURCES3]) + self.assert_sources_resolved([sources_field4], expected=[SOURCES1]) - assert_all_source_files_resolved(sources_field1, SOURCES1) - assert_all_source_files_resolved(sources_field2, SOURCES2) - assert_all_source_files_resolved(sources_field3, SOURCES3) - assert_all_source_files_resolved(sources_field4, SOURCES1) # NB: sources_field1 and sources_field4 refer to the same files. We should be able to # handle this gracefully. - combined_sources_fields = [sources_field1, sources_field2, sources_field3, sources_field4] - combined_expected = sorted( - [ - *SOURCES1.source_file_absolute_paths, - *SOURCES2.source_file_absolute_paths, - *SOURCES3.source_file_absolute_paths, - ] - ) - assert self.get_all_source_files(combined_sources_fields) == combined_expected - assert self.get_all_unrooted_files(combined_sources_fields) == [] - assert self.get_specified_source_files(combined_sources_fields) == combined_expected - assert self.get_specified_unrooted_files(combined_sources_fields) == [] - - def test_filesystem_specs(self) -> None: - # Literal file arg. - sources_field1_all_sources = SOURCES1.source_file_absolute_paths - sources_field1_slice = slice(0, 1) - sources_field1 = self.mock_sources_field_with_origin( - SOURCES1, origin=FilesystemLiteralSpec(sources_field1_all_sources[0]) - ) - - # Glob file arg that matches the entire `sources`. - sources_field2_all_sources = SOURCES2.source_file_absolute_paths - sources_field2_slice = slice(0, len(sources_field2_all_sources)) - sources_field2_origin = FilesystemResolvedGlobSpec( - f"{SOURCES2.source_root}/*.py", files=tuple(sources_field2_all_sources) - ) - sources_field2 = self.mock_sources_field_with_origin(SOURCES2, origin=sources_field2_origin) - - # Glob file arg that only matches a subset of the `sources` _and_ includes resolved - # files not owned by the target. - sources_field3_all_sources = SOURCES3.source_file_absolute_paths - sources_field3_slice = slice(0, 1) - sources_field3_origin = FilesystemResolvedGlobSpec( - f"{SOURCES3.source_root}/*.java", - files=tuple( - PurePath(SOURCES3.source_root, name).as_posix() - for name in [SOURCES3.source_files[0], "other_target.java", "j.tmp.java"] - ), - ) - sources_field3 = self.mock_sources_field_with_origin(SOURCES3, origin=sources_field3_origin) - - # FilesSources should appear in the "all_source_files" results but not in the - # "all_rooted_files" results. - sources_field4 = self.mock_sources_field_with_origin( - SOURCES4, sources_field_cls=FilesSources - ) - sources_field4_all_sources = SOURCES4.source_file_absolute_paths - - def assert_file_args_resolved( - sources_field_with_origin: Tuple[SourcesField, OriginSpec], - all_sources: List[str], - expected_slice: slice, - ) -> None: - assert self.get_all_source_files([sources_field_with_origin]) == all_sources - assert self.get_all_unrooted_files([sources_field_with_origin]) == [] - - assert ( - self.get_specified_source_files([sources_field_with_origin]) - == all_sources[expected_slice] - ) - assert self.get_specified_unrooted_files([sources_field_with_origin]) == [] - - assert_file_args_resolved(sources_field1, sources_field1_all_sources, sources_field1_slice) - assert_file_args_resolved(sources_field2, sources_field2_all_sources, sources_field2_slice) - assert_file_args_resolved(sources_field3, sources_field3_all_sources, sources_field3_slice) - - combined_sources_fields = [sources_field1, sources_field2, sources_field3, sources_field4] - assert self.get_all_source_files(combined_sources_fields) == sorted( - [ - *sources_field1_all_sources, - *sources_field2_all_sources, - *sources_field3_all_sources, - *sources_field4_all_sources, - ] - ) - assert self.get_all_unrooted_files(combined_sources_fields) == sorted( - [*sources_field4_all_sources] - ) - - assert self.get_specified_source_files(combined_sources_fields) == sorted( - [ - *sources_field1_all_sources[sources_field1_slice], - *sources_field2_all_sources[sources_field2_slice], - *sources_field3_all_sources[sources_field3_slice], - *sources_field4_all_sources, - ] + self.assert_sources_resolved( + [sources_field1, sources_field2, sources_field3, sources_field4], + expected=[SOURCES1, SOURCES2, SOURCES3], ) - assert self.get_specified_unrooted_files(combined_sources_fields) == sorted( - [*sources_field4_all_sources,] + def test_file_sources(self) -> None: + sources = TargetSources("src/python", ["README.md"]) + field = self.mock_sources_field(sources, sources_field_cls=FilesSources) + self.assert_sources_resolved( + [field], expected=[sources], expected_unrooted=sources.full_paths ) def test_gracefully_handle_no_sources(self) -> None: - sources_field = self.mock_sources_field_with_origin(SOURCES1, include_sources=False) - assert self.get_all_source_files([sources_field]) == [] - assert self.get_specified_source_files([sources_field]) == [] - assert self.get_all_unrooted_files([sources_field]) == [] - assert self.get_specified_unrooted_files([sources_field]) == [] + sources_field = self.mock_sources_field(SOURCES1, include_sources=False) + self.assert_sources_resolved([sources_field], expected=[]) diff --git a/src/python/pants/engine/addresses.py b/src/python/pants/engine/addresses.py index 437b6a7bafe..0871c16c004 100644 --- a/src/python/pants/engine/addresses.py +++ b/src/python/pants/engine/addresses.py @@ -5,7 +5,7 @@ from typing import Sequence from pants.base.exceptions import ResolveError -from pants.base.specs import OriginSpec +from pants.base.specs import Spec from pants.build_graph.address import Address as Address from pants.build_graph.address import AddressInput as AddressInput # noqa: F401: rexporting. from pants.build_graph.address import BuildFileAddress as BuildFileAddress @@ -35,7 +35,7 @@ class AddressWithOrigin: """An Address along with the cmd-line spec it was generated from.""" address: Address - origin: OriginSpec + origin: Spec class AddressesWithOrigins(Collection[AddressWithOrigin]): diff --git a/src/python/pants/engine/internals/graph.py b/src/python/pants/engine/internals/graph.py index b69953ceca8..241f14de11f 100644 --- a/src/python/pants/engine/internals/graph.py +++ b/src/python/pants/engine/internals/graph.py @@ -5,30 +5,16 @@ import itertools import logging import os.path -from collections import defaultdict from dataclasses import dataclass from pathlib import PurePath -from typing import ( - DefaultDict, - Dict, - Iterable, - List, - NamedTuple, - Optional, - Sequence, - Set, - Tuple, - Type, - Union, -) +from typing import Dict, Iterable, List, NamedTuple, Optional, Sequence, Set, Tuple, Type, Union from pants.base.exceptions import ResolveError from pants.base.specs import ( AddressSpecs, AscendantAddresses, FilesystemLiteralSpec, - FilesystemMergedSpec, - FilesystemResolvedGlobSpec, + FilesystemSpec, FilesystemSpecs, Specs, ) @@ -366,32 +352,27 @@ async def addresses_with_origins_from_filesystem_specs( """Find the owner(s) for each FilesystemSpec while preserving the original FilesystemSpec those owners come from. - When a file has only one owning target, we generate a subtarget from that owner whose source's - field only refers to that one file. Otherwise, we use all the original owning targets. - - This will merge FilesystemSpecs that come from the same owning target into a single - FilesystemMergedSpec. + Every returned address will be a generated subtarget, meaning that each address will have + exactly one file in its `sources` field. """ - pathglobs_per_include = ( - filesystem_specs.path_globs_for_spec( - spec, global_options.options.owners_not_found_behavior.to_glob_match_error_behavior(), + owners_not_found_behavior = global_options.options.owners_not_found_behavior + snapshot_per_include = await MultiGet( + Get( + Snapshot, + PathGlobs, + filesystem_specs.path_globs_for_spec( + spec, owners_not_found_behavior.to_glob_match_error_behavior() + ), ) for spec in filesystem_specs.includes ) - snapshot_per_include = await MultiGet( - Get(Snapshot, PathGlobs, pg) for pg in pathglobs_per_include - ) owners_per_include = await MultiGet( Get(Owners, OwnersRequest(sources=snapshot.files)) for snapshot in snapshot_per_include ) - addresses_to_specs: DefaultDict[ - Address, List[Union[FilesystemLiteralSpec, FilesystemResolvedGlobSpec]] - ] = defaultdict(list) - for spec, snapshot, owners in zip( - filesystem_specs.includes, snapshot_per_include, owners_per_include - ): + addresses_to_specs: Dict[Address, FilesystemSpec] = {} + for spec, owners in zip(filesystem_specs.includes, owners_per_include): if ( - global_options.options.owners_not_found_behavior != OwnersNotFoundBehavior.ignore + owners_not_found_behavior != OwnersNotFoundBehavior.ignore and isinstance(spec, FilesystemLiteralSpec) and not owners ): @@ -400,20 +381,17 @@ async def addresses_with_origins_from_filesystem_specs( global_options.options.owners_not_found_behavior, ignore_option="--owners-not-found-behavior=ignore", ) - # We preserve what literal files any globs resolved to. This allows downstream goals to be - # more precise in which files they operate on. - origin: Union[FilesystemLiteralSpec, FilesystemResolvedGlobSpec] = ( - spec - if isinstance(spec, FilesystemLiteralSpec) - else FilesystemResolvedGlobSpec(glob=spec.glob, files=snapshot.files) - ) for address in owners: - addresses_to_specs[address].append(origin) + # If the address is already covered by a prior spec, and that spec is a literal spec, + # then we do not overwrite it. The implication is that if the same address is resolved + # both by a glob spec and a literal spec, the literal spec will be used because it's + # more precise. + if isinstance(addresses_to_specs.get(address), FilesystemLiteralSpec): + continue + else: + addresses_to_specs[address] = spec return AddressesWithOrigins( - AddressWithOrigin( - address, specs[0] if len(specs) == 1 else FilesystemMergedSpec.create(specs) - ) - for address, specs in addresses_to_specs.items() + AddressWithOrigin(address, spec) for address, spec in addresses_to_specs.items() ) diff --git a/src/python/pants/engine/internals/graph_test.py b/src/python/pants/engine/internals/graph_test.py index fa1bc0ee6bb..077ddbb81c8 100644 --- a/src/python/pants/engine/internals/graph_test.py +++ b/src/python/pants/engine/internals/graph_test.py @@ -12,7 +12,6 @@ from pants.base.specs import ( FilesystemGlobSpec, FilesystemLiteralSpec, - FilesystemResolvedGlobSpec, FilesystemSpecs, SingleAddress, ) @@ -371,24 +370,36 @@ def test_filesystem_specs_literal_file(self) -> None: def test_filesystem_specs_glob(self) -> None: self.create_files("demo", ["f1.txt", "f2.txt"]) self.add_to_build_file("demo", "target(sources=['*.txt'])") + spec = FilesystemGlobSpec("demo/*.txt") result = self.request_single_product( - AddressesWithOrigins, - Params( - FilesystemSpecs([FilesystemGlobSpec("demo/*.txt")]), create_options_bootstrapper() - ), + AddressesWithOrigins, Params(FilesystemSpecs([spec]), create_options_bootstrapper()), + ) + assert result == AddressesWithOrigins( + [ + AddressWithOrigin( + Address("demo", relative_file_path="f1.txt", target_name="demo"), origin=spec + ), + AddressWithOrigin( + Address("demo", relative_file_path="f2.txt", target_name="demo"), origin=spec + ), + ] ) - expected_origin = FilesystemResolvedGlobSpec( - glob="demo/*.txt", files=("demo/f1.txt", "demo/f2.txt") + + # If a glob and a literal spec both resolve to the same file, the literal spec should be + # used as it's more precise. + literal_spec = FilesystemLiteralSpec("demo/f1.txt") + result = self.request_single_product( + AddressesWithOrigins, + Params(FilesystemSpecs([spec, literal_spec]), create_options_bootstrapper()), ) assert result == AddressesWithOrigins( [ AddressWithOrigin( Address("demo", relative_file_path="f1.txt", target_name="demo"), - origin=expected_origin, + origin=literal_spec, ), AddressWithOrigin( - Address("demo", relative_file_path="f2.txt", target_name="demo"), - origin=expected_origin, + Address("demo", relative_file_path="f2.txt", target_name="demo"), origin=spec ), ] ) diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py index 5364d8eb80c..4d9698c2456 100644 --- a/src/python/pants/engine/target.py +++ b/src/python/pants/engine/target.py @@ -25,7 +25,7 @@ from typing_extensions import final -from pants.base.specs import OriginSpec +from pants.base.specs import Spec from pants.engine.addresses import Address, assert_single_address from pants.engine.collection import Collection, DeduplicatedCollection from pants.engine.fs import Snapshot @@ -494,7 +494,7 @@ class WrappedTarget: @dataclass(frozen=True) class TargetWithOrigin: target: Target - origin: OriginSpec + origin: Spec class Targets(Collection[Target]): @@ -733,7 +733,7 @@ class FieldSetWithOrigin(_AbstractFieldSet, metaclass=ABCMeta): See FieldSet for documentation on how subclasses should use this base class. """ - origin: OriginSpec + origin: Spec @classmethod def create(cls: Type[_FSWO], target_with_origin: TargetWithOrigin) -> _FSWO: