Allow explicit dependencies to resolve ambiguities

src/python/pants/backend/python/target_types_test.py

@@ -25,6 +25,7 @@
     PythonRequirementsField,
     PythonRequirementTarget,
     PythonSourcesGeneratorTarget,
+    PythonSourceTarget,
     ResolvedPexEntryPoint,
     ResolvePexEntryPointRequest,
     ResolvePythonDistributionEntryPointsRequest,
@@ -41,6 +42,7 @@
 from pants.core.goals.generate_lockfiles import UnrecognizedResolveNamesError
 from pants.engine.addresses import Address
 from pants.engine.internals.graph import _TargetParametrizations, _TargetParametrizationsRequest
+from pants.engine.internals.parametrize import Parametrize
 from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.target import (
     InferredDependencies,
@@ -615,3 +617,92 @@ def gen_pex_binary_tgt(entry_point: str, tags: list[str] | None = None) -> PexBi
         gen_pex_binary_tgt("subdir.f.py", tags=["overridden"]),
         gen_pex_binary_tgt("subdir.f:main"),
     }
+
+
+def assert_inferred(
+    rule_runner: RuleRunner, address: Address, expected: list[Address], comment=""
+) -> None:
+    tgt = rule_runner.get_target(address)
+    inferred = rule_runner.request(
+        InferredDependencies,
+        [
+            InferPexBinaryEntryPointDependency(
+                PexBinaryEntryPointDependencyInferenceFieldSet.create(tgt)
+            )
+        ],
+    )
+    assert InferredDependencies(expected) == inferred, comment
+
+
+def test_20806() -> None:
+    rule_runner = RuleRunner(
+        rules=[
+            *target_types_rules.rules(),
+            *import_rules(),
+            *python_sources.rules(),
+            QueryRule(InferredDependencies, [InferPythonDistributionDependencies]),
+        ],
+        target_types=[
+            PythonDistribution,
+            PythonRequirementTarget,
+            PythonSourceTarget,
+            PexBinary,
+        ],
+        objects={"parametrize": Parametrize},
+    )
+
+    rule_runner.write_files(
+        {
+            "BUILD": dedent(
+                """\
+            python_source(name="src", source="example.py", tags=parametrize(a=["a"], b=["b"]))
+
+            pex_binary(name="ambiguous", entry_point="./example.py")
+
+            pex_binary(name="with-target", entry_point="./example.py:src@tags=a")
+
+            pex_binary(name="with-deps", entry_point="./example.py", dependencies=[":src@tags=a"])
+
+            pex_binary(name="with-deps-and-ignore", entry_point="./example.py", dependencies=[":src@tags=a", "!:src@tags=b"])
+
+            pex_binary(name="with-ignore-only", entry_point="./example.py", dependencies=[ "!:src@tags=b"])
+            """
+            ),
+            "example.py": "print('worked!')",
+        }
+    )
+
+    assert_inferred(
+        rule_runner,
+        Address(spec_path="", target_name="ambiguous"),
+        [],
+        comment="The ambiguous should not be disambiguated",
+    )
+
+    # assert_inferred(
+    #     rule_runner,
+    #     Address(spec_path="", target_name="with-target"),
+    #     [Address(spec_path="", target_name="src", parameters={"tags": "a"})],
+    #     comment="Using an explicit target is not standard (the entry is actually a Python entrypoint, not a Pants target)",
+    # )
+
+    assert_inferred(
+        rule_runner,
+        Address(spec_path="", target_name="with-deps"),
+        [Address(spec_path="", target_name="src", parameters={"tags": "a"})],
+        comment="Explicitly providing the dep should resolve the ambiguity",
+    )
+
+    assert_inferred(
+        rule_runner,
+        Address(spec_path="", target_name="with-ignore-only"),
+        [Address(spec_path="", target_name="src", parameters={"tags": "a"})],
+        comment="Ignoring the other ambiguous cases should disambiguate",
+    )
+
+    assert_inferred(
+        rule_runner,
+        Address(spec_path="", target_name="with-deps-and-ignore"),
+        [Address(spec_path="", target_name="src", parameters={"tags": "a"})],
+        comment="Ignoring the other ambiguous cases should disambiguate, even if one of the ambiguous ones is provided as a dep",
+    )

src/python/pants/engine/target.py

@@ -2626,7 +2626,7 @@ class ExplicitlyProvidedDependencies:
 
     @memoized_method
     def any_are_covered_by_includes(self, addresses: Iterable[Address]) -> bool:
-        """Return True if every address is in the explicitly provided includes.
+        """Return True if any address is in the explicitly provided includes.
 
         Note that if the input addresses are generated targets, they will still be marked as covered
         if their original target generator is in the explicitly provided includes.
@@ -2636,6 +2636,20 @@ def any_are_covered_by_includes(self, addresses: Iterable[Address]) -> bool:
             for addr in addresses
         )
 
+    @memoized_method
+    def covered_by_includes(self, addresses: frozenset[Address]) -> frozenset[Address]:
+        """Return only the addresses which are in the explicitly provided includes."""
+        out = set()
+        for addr in addresses:
+            if addr in self.includes:
+                out.add(addr)
+            maybe_tgt_generator = addr.maybe_convert_to_target_generator()
+            if maybe_tgt_generator in self.includes:
+                # TODO: do we include the maybe_tgt_generator or the addr?
+                out.add(maybe_tgt_generator)
+
+        return frozenset(out)
+
     @memoized_method
     def remaining_after_disambiguation(
         self, addresses: Iterable[Address], owners_must_be_ancestors: bool
@@ -2682,21 +2696,43 @@ def maybe_warn_of_ambiguous_dependency_inference(
         `!!`. If `owners_must_be_ancestors` is True, any addresses which are not ancestors of the
         target in question will also be ignored.
         """
-        if not ambiguous_addresses or self.any_are_covered_by_includes(ambiguous_addresses):
-            return
-        remaining = self.remaining_after_disambiguation(
+        if not ambiguous_addresses:
+            return None
+        remaining_after_ignores = self.remaining_after_disambiguation(
             ambiguous_addresses, owners_must_be_ancestors=owners_must_be_ancestors
         )
+        if len(remaining_after_ignores) == 1:
+            return
+
+        remaining_after_includes = self.covered_by_includes(remaining_after_ignores)
+        if len(remaining_after_includes) == 1:
+            return
+        elif len(remaining_after_includes) > 1:
+            # if multiple includes still own this target, we warn only about them, and not all targets
+            remaining = remaining_after_includes
+            suggestion = (
+                f"\n\nMultiple dependencies specified in the `dependencies` field of {original_address} "
+                f"own this {import_reference}. Try removing all of them except for one, "
+                "or ignore the ones you do not want by prefixing "
+                f"with `!` or `!!` so that one or no targets are left."
+            )
+        else:
+            # if none of the includes helped, we warn about all owners
+            remaining = remaining_after_ignores
+            suggestion = (
+                f"\n\nPlease explicitly include the dependency you want in the `dependencies` "
+                f"field of {original_address}, or ignore the ones you do not want by prefixing "
+                f"with `!` or `!!` so that one or no targets are left."
+            )
+
         if len(remaining) <= 1:
             return
         logger.warning(
             f"{context}, but Pants cannot safely infer a dependency because more than one target "
             f"owns this {import_reference}, so it is ambiguous which to use: "
             f"{sorted(addr.spec for addr in remaining)}."
-            f"\n\nPlease explicitly include the dependency you want in the `dependencies` "
-            f"field of {original_address}, or ignore the ones you do not want by prefixing "
-            f"with `!` or `!!` so that one or no targets are left."
-            f"\n\nAlternatively, you can remove the ambiguity by deleting/changing some of the "
+            + suggestion
+            + f"\n\nAlternatively, you can remove the ambiguity by deleting/changing some of the "
             f"targets so that only 1 target owns this {import_reference}. Refer to "
             f"{doc_url('docs/using-pants/troubleshooting-common-issues#import-errors-and-missing-dependencies')}."
         )
@@ -2710,12 +2746,17 @@ def disambiguated(
         `!!`. If `owners_must_be_ancestors` is True, any addresses which are not ancestors of the
         target in question will also be ignored.
         """
-        if not ambiguous_addresses or self.any_are_covered_by_includes(ambiguous_addresses):
+        if not ambiguous_addresses:
             return None
         remaining_after_ignores = self.remaining_after_disambiguation(
             ambiguous_addresses, owners_must_be_ancestors=owners_must_be_ancestors
         )
-        return list(remaining_after_ignores)[0] if len(remaining_after_ignores) == 1 else None
+        if len(remaining_after_ignores) == 1:
+            return list(remaining_after_ignores)[0]
+        only_explicitly_included = self.covered_by_includes(remaining_after_ignores)
+        if len(only_explicitly_included) == 1:
+            return list(only_explicitly_included)[0]
+        return None
 
 
 FS = TypeVar("FS", bound="FieldSet")

src/python/pants/engine/target_test.py

@@ -1309,16 +1309,15 @@ def get_disambiguated(
     assert get_disambiguated(all_addr, ignores=[addr_a.maybe_convert_to_target_generator()]) is None
     assert get_disambiguated(all_addr, ignores=all_addr) is None
     assert get_disambiguated([]) is None
-    # If any includes would disambiguate the ambiguous target, we don't consider disambiguating
-    # via excludes as the user has already explicitly disambiguated the module.
-    assert get_disambiguated(all_addr, ignores=[addr_a, addr_b], includes=[addr_a]) is None
+    # Verify that ignores take precedence over includes
+    assert get_disambiguated(all_addr, ignores=[addr_a, addr_b], includes=[addr_a]) is addr_c
     assert (
         get_disambiguated(
             ambiguous=all_addr,
             ignores=[addr_a, addr_b],
             includes=[addr_a.maybe_convert_to_target_generator()],
         )
-        is None
+        is addr_c
     )
 
     # You can also disambiguate via `owners_must_be_ancestors`.
@@ -1332,6 +1331,24 @@ def get_disambiguated(
         == addr_a
     )
 
+    # Verify that includes can resolve ambiguities
+    assert get_disambiguated(ambiguous=all_addr, includes=[addr_a]) is addr_a
+
+    # Verify that includes which do not resolve the ambiguity do not resolve the ambiguity
+    assert get_disambiguated(ambiguous=all_addr, includes=[addr_a, addr_b]) is None
+
+    # Verify that ignores works for parametrized targets
+    addr_param_a = Address(spec_path="", target_name="src", parameters={"tag": "a"})
+    addr_param_b = Address(spec_path="", target_name="src", parameters={"tag": "b"})
+    assert (
+        get_disambiguated(
+            ambiguous=[addr_param_a, addr_param_b],
+            includes=[addr_param_a],
+            ignores=[addr_param_b],
+        )
+        == addr_param_a
+    )
+
 
 def test_explicitly_provided_dependencies_maybe_warn_of_ambiguous_dependency_inference(
     caplog,