Change Target to have a similar interface to dict[t] and `dict.ge…

…t(t)` (pantsbuild#9340)

### Problem

As described in pantsbuild#9337, we need a mechanism to express that some rules optionally may use Fields when registered on the target type, but don't actually need them to operate.

For example, with `./v2 test`, the only Field we absolutely need for Pytest is `PythonTestsSources`. It's an added bonus if the target type has `Timeout` and `Coverage`. Even if they don't have those fields registered, we can still meaningfully return a `TestResult`. We simply can't use the added features of `--run-coverage` and `--pytest-timeouts`.

When these rules are using targets without those optional fields, we typically will want to use a default value for the `Field` when not registered. Typically, we will want that default to be the same as `Field(raw_value=None)`. Sometimes, we will want to be able to override that default value.

### Solution

Align `Target` with how you access a dictionary:

1. `tgt[Timeout]` will get the field instance if registered on the target type, else raise `KeyError`.
2. `tgt.get(Timeout)` will get the field instance if registered on the target type, else return `Timeout(raw_value=None)`.
3. `tgt.get(Timeout, default_raw_value=100)` will get the field instance if registered on the target type, else return `Timeout(raw_value=100)`.

Conceptually, it makes sense to think of `Target` similarly to a dictionary. We define a `Target` as *a unique combination of fields that are valid together*.  Tangibly, the dictionary maps `Type[Field] -> Field`.

### Result

Accessing fields on `Target` aligns more with Python conventions.

--

We can now more accurately reflect in rules what fields we actually must have vs. would like to have.

A crucial benefit of this is that it makes it much easier for core Pants devs to add new `Fields` to pre-existing target types and to consume those fields in core Pants without forcing all plugin authors to implement that field.

For example, we may want to add the field `RequirementsConstraints` to `PythonBinary` one day and use that in `python_create_binary.py` if we have that field registered, but still be able to operate on target types without it. This PR provides a mechanism for us to not only add that new field, but express in `python_create_binary.py` that we would like to use that field if it's available, without breaking any plugin authors.

src/python/pants/backend/python/rules/python_create_binary.py

@@ -28,20 +28,18 @@ class PythonBinaryFields:
     sources: PythonBinarySources
     entry_point: EntryPoint
 
-    # TODO: consume the other PythonBinary fields like `ZipSafe`. Consider making those fields
-    #  optional. We _need_ PythonBinarySources and EntryPoint to work properly. If your target
-    #  type also has ZipSafe, AlwaysWriteCache, etc, then we can do some additional things as an
-    #  extra bonus. Consider adding `Target.maybe_get()` to facilitate this.
+    # TODO: consume the other PythonBinary fields like `ZipSafe` and `AlwaysWriteCache`. These are
+    #  optional fields. If your target type has them registered, we can do extra meaningful things;
+    #  if you don't have them on your target type, we can still operate so long as you have the
+    #  required fields. Use `Target.get()` in the `create()` method.
 
     @staticmethod
     def is_valid_target(tgt: Target) -> bool:
         return tgt.has_fields([EntryPoint, PythonBinarySources])
 
     @classmethod
     def create(cls, tgt: Target) -> "PythonBinaryFields":
-        return cls(
-            tgt.address, sources=tgt.get(PythonBinarySources), entry_point=tgt.get(EntryPoint)
-        )
+        return cls(tgt.address, sources=tgt[PythonBinarySources], entry_point=tgt[EntryPoint])
 
 
 @rule

src/python/pants/engine/target.py

@@ -330,35 +330,61 @@ def _find_registered_field_subclass(
         return cast(Optional[Type[_F]], subclass)
 
     @final
-    def get(self, field: Type[_F]) -> _F:
+    def _maybe_get(self, field: Type[_F]) -> Optional[_F]:
+        result = self.field_values.get(field, None)
+        if result is not None:
+            return cast(_F, result)
+        field_subclass = self._find_registered_field_subclass(
+            field, registered_fields=self.field_types
+        )
+        if field_subclass is not None:
+            return cast(_F, self.field_values[field_subclass])
+        return None
+
+    @final
+    def __getitem__(self, field: Type[_F]) -> _F:
+        """Get the requested `Field` instance belonging to this target.
+
+        If the `Field` is not registered on this `Target` type, this method will raise a
+        `KeyError`. To avoid this, you should first call `tgt.has_field()` or `tgt.has_fields()`
+        to ensure that the field is registered, or, alternatively, use `Target.get()`.
+
+        See the docstring for `Target.get()` for how this method handles subclasses of the
+        requested Field and for tips on how to use the returned value.
+        """
+        result = self._maybe_get(field)
+        if result is not None:
+            return result
+        raise KeyError(
+            f"The target `{self}` does not have a field `{field.__name__}`. Before calling "
+            f"`my_tgt[{field.__name__}]`, call `my_tgt.has_field({field.__name__})` to "
+            f"filter out any irrelevant Targets or call `my_tgt.get({field.__name__})` to use the "
+            f"default Field value."
+        )
+
+    @final
+    def get(self, field: Type[_F], *, default_raw_value: Optional[Any] = None) -> _F:
         """Get the requested `Field` instance belonging to this target.
 
         This will return an instance of the requested field type, e.g. an instance of
         `Compatibility`, `Sources`, `EntryPoint`, etc. Usually, you will want to grab the
         `Field`'s inner value, e.g. `tgt.get(Compatibility).value`. (For `AsyncField`s, you would
         call `await Get[SourcesResult](SourcesRequest, tgt.get(Sources).request)`).
 
-        If the `Field` is not registered on this `Target` type, this method will raise a
-        `KeyError`. To avoid this, you should first call `tgt.has_field()` or `tgt.has_fields()`
-        to ensure that the field is registered.
-
         This works with subclasses of `Field`s. For example, if you subclass `Sources` to define a
         custom subclass `PythonSources`, both `python_tgt.get(PythonSources)` and
         `python_tgt.get(Sources)` will return the same `PythonSources` instance.
+
+        If the `Field` is not registered on this `Target` type, this will return an instance of
+        the requested Field by using `default_raw_value` to create the instance. Alternatively,
+        first call `tgt.has_field()` or `tgt.has_fields()` to ensure that the field is registered,
+        or, alternatively, use indexing (e.g. `tgt[Compatibility]`) to raise a KeyError when the
+        field is not registered.
         """
-        result = self.field_values.get(field, None)
+        result = self._maybe_get(field)
         if result is not None:
-            return cast(_F, result)
-        field_subclass = self._find_registered_field_subclass(
-            field, registered_fields=self.field_types
-        )
-        if field_subclass is not None:
-            return cast(_F, self.field_values[field_subclass])
-        raise KeyError(
-            f"The target `{self}` does not have a field `{field}`. Before calling "
-            f"`my_tgt.get({field.__name__})`, call `my_tgt.has_field({field.__name__})` to "
-            "filter out any irrelevant Targets."
-        )
+            return result
+        return field(raw_value=default_raw_value, address=self.address)
 
     @final
     @classmethod

src/python/pants/engine/target_test.py

@@ -37,6 +37,11 @@ def hydrate(self, raw_value: Optional[Iterable[str]], *, address: Address) -> Tu
         return tuple(raw_value)
 
 
+class UnrelatedField(BoolField):
+    alias: ClassVar = "unrelated"
+    default: ClassVar = False
+
+
 class HaskellSources(AsyncField):
     alias: ClassVar = "sources"
     sanitized_raw_value: Optional[Tuple[str, ...]]
@@ -87,75 +92,50 @@ def test_invalid_fields_rejected() -> None:
     assert "Unrecognized field `invalid_field=True`" in str(exc)
 
 
-def test_get_primitive_field() -> None:
+def test_get_field() -> None:
     extensions = ("GhcExistentialQuantification",)
-    assert (
-        HaskellTarget({HaskellGhcExtensions.alias: extensions}, address=Address.parse(":lib"))
-        .get(HaskellGhcExtensions)
-        .value
-        == extensions
-    )
-
-    # Default value
-    assert (
-        HaskellTarget({}, address=Address.parse(":default")).get(HaskellGhcExtensions).value == ()
-    )
+    tgt = HaskellTarget({HaskellGhcExtensions.alias: extensions}, address=Address.parse(":lib"))
 
+    assert tgt[HaskellGhcExtensions].value == extensions
+    assert tgt.get(HaskellGhcExtensions).value == extensions
+    assert tgt.get(HaskellGhcExtensions, default_raw_value=["GhcDefault"]).value == extensions
 
-def test_get_async_field() -> None:
-    def hydrate_field(
-        *, raw_source_files: List[str], hydrated_source_files: Tuple[str, ...]
-    ) -> HaskellSourcesResult:
-        sources_field = HaskellTarget(
-            {HaskellSources.alias: raw_source_files}, address=Address.parse(":lib")
-        ).get(HaskellSources)
-        result: HaskellSourcesResult = run_rule(
-            hydrate_haskell_sources,
-            rule_args=[HaskellSourcesRequest(sources_field)],
-            mock_gets=[
-                MockGet(
-                    product_type=Snapshot,
-                    subject_type=PathGlobs,
-                    mock=lambda _: Snapshot(
-                        directory_digest=EMPTY_DIRECTORY_DIGEST,
-                        files=hydrated_source_files,
-                        dirs=(),
-                    ),
-                )
-            ],
-        )
-        return result
+    # Default field value. This happens when the field is registered on the target type, but the
+    # user does not explicitly set the field in the BUILD file.
+    #
+    # NB: `default_raw_value` is not used in this case - that parameter is solely used when
+    # the field is not registered on the target type. To override the default field value, either
+    # subclass the Field and create a new target, or, in your call site, interpret the result and
+    # and apply your default.
+    default_field_tgt = HaskellTarget({}, address=Address.parse(":default"))
+    assert default_field_tgt[HaskellGhcExtensions].value == ()
+    assert default_field_tgt.get(HaskellGhcExtensions).value == ()
+    assert default_field_tgt.get(HaskellGhcExtensions, default_raw_value=["GhcDefault"]).value == ()
+    # Example of a call site applying its own default value instead of the field's default value.
+    assert default_field_tgt[HaskellGhcExtensions].value or 123 == 123
+
+    # Field is not registered on the target.
+    with pytest.raises(KeyError) as exc:
+        default_field_tgt[UnrelatedField]
+    assert UnrelatedField.__name__ in str(exc)
+    assert default_field_tgt.get(UnrelatedField).value == UnrelatedField.default
+    assert default_field_tgt.get(
+        UnrelatedField, default_raw_value=not UnrelatedField.default
+    ).value == (not UnrelatedField.default)
 
-    # Normal field
-    expected_files = ("monad.hs", "abstract_art.hs", "abstract_algebra.hs")
-    assert (
-        hydrate_field(
-            raw_source_files=["monad.hs", "abstract_*.hs"], hydrated_source_files=expected_files
-        ).snapshot.files
-        == expected_files
-    )
 
-    # Test `raw_value` gets sanitized/validated eagerly
-    with pytest.raises(ValueError) as exc:
-        HaskellTarget({HaskellSources.alias: [0, 1, 2]}, address=Address.parse(":lib")).get(
-            HaskellSources
+def test_primitive_field_hydration_is_eager() -> None:
+    with pytest.raises(TargetDefinitionException) as exc:
+        HaskellTarget(
+            {HaskellGhcExtensions.alias: ["GhcExistentialQuantification", "DoesNotStartWithGhc"]},
+            address=Address.parse(":bad_extension"),
         )
-    assert "Not all elements of the iterable have type" in str(exc)
-
-    # Test post-hydration validation
-    with pytest.raises(ValueError) as exc:
-        hydrate_field(raw_source_files=["*.js"], hydrated_source_files=("not_haskell.js",))
-    assert "Received non-Haskell sources" in str(exc)
-    assert "//:lib" in str(exc)
+    assert "must be prefixed by `Ghc`" in str(exc)
+    assert "//:bad_extension" in str(exc)
 
 
 def test_has_fields() -> None:
-    class UnrelatedField(BoolField):
-        alias: ClassVar = "unrelated"
-        default: ClassVar = False
-
     empty_union_membership = UnionMembership({})
-
     tgt = HaskellTarget({}, address=Address.parse(":lib"))
     assert tgt.has_fields([]) is True
     assert HaskellTarget.class_has_fields([], union_membership=empty_union_membership) is True
@@ -193,14 +173,49 @@ class UnrelatedField(BoolField):
     )
 
 
-def test_primitive_field_hydration_is_eager() -> None:
-    with pytest.raises(TargetDefinitionException) as exc:
-        HaskellTarget(
-            {HaskellGhcExtensions.alias: ["GhcExistentialQuantification", "DoesNotStartWithGhc"]},
-            address=Address.parse(":bad_extension"),
+def test_async_field() -> None:
+    def hydrate_field(
+        *, raw_source_files: List[str], hydrated_source_files: Tuple[str, ...]
+    ) -> HaskellSourcesResult:
+        sources_field = HaskellTarget(
+            {HaskellSources.alias: raw_source_files}, address=Address.parse(":lib")
+        )[HaskellSources]
+        result: HaskellSourcesResult = run_rule(
+            hydrate_haskell_sources,
+            rule_args=[HaskellSourcesRequest(sources_field)],
+            mock_gets=[
+                MockGet(
+                    product_type=Snapshot,
+                    subject_type=PathGlobs,
+                    mock=lambda _: Snapshot(
+                        directory_digest=EMPTY_DIRECTORY_DIGEST,
+                        files=hydrated_source_files,
+                        dirs=(),
+                    ),
+                )
+            ],
         )
-    assert "must be prefixed by `Ghc`" in str(exc)
-    assert "//:bad_extension" in str(exc)
+        return result
+
+    # Normal field
+    expected_files = ("monad.hs", "abstract_art.hs", "abstract_algebra.hs")
+    assert (
+        hydrate_field(
+            raw_source_files=["monad.hs", "abstract_*.hs"], hydrated_source_files=expected_files
+        ).snapshot.files
+        == expected_files
+    )
+
+    # Test that `raw_value` gets sanitized/validated eagerly.
+    with pytest.raises(ValueError) as exc:
+        HaskellTarget({HaskellSources.alias: [0, 1, 2]}, address=Address.parse(":lib"))
+    assert "Not all elements of the iterable have type" in str(exc)
+
+    # Test post-hydration validation.
+    with pytest.raises(ValueError) as exc:
+        hydrate_field(raw_source_files=["*.js"], hydrated_source_files=("not_haskell.js",))
+    assert "Received non-Haskell sources" in str(exc)
+    assert "//:lib" in str(exc)
 
 
 def test_add_custom_fields() -> None:
@@ -220,12 +235,12 @@ class CustomField(BoolField):
     assert tgt.has_field(CustomField) is True
     assert HaskellTarget.class_has_field(CustomField, union_membership=union_membership) is True
 
-    assert tgt.get(CustomField).value is True
+    assert tgt[CustomField].value is True
 
     default_tgt = HaskellTarget(
         {}, address=Address.parse(":default"), union_membership=union_membership
     )
-    assert default_tgt.get(CustomField).value is False
+    assert default_tgt[CustomField].value is False
 
 
 def test_override_preexisting_field_via_new_target() -> None:
@@ -280,15 +295,15 @@ class CustomHaskellTarget(Target):
     assert normal_tgt.has_field(HaskellGhcExtensions) is True
     assert normal_tgt.has_field(CustomHaskellGhcExtensions) is False
 
-    assert custom_tgt.get(HaskellGhcExtensions) == custom_tgt.get(CustomHaskellGhcExtensions)
-    assert custom_tgt.get(HaskellGhcExtensions).value == (
+    assert custom_tgt[HaskellGhcExtensions] == custom_tgt[CustomHaskellGhcExtensions]
+    assert custom_tgt[HaskellGhcExtensions].value == (
         "GhcNormalExtension",
         *CustomHaskellGhcExtensions.default_extensions,
     )
 
     # Check custom default value
     assert (
-        CustomHaskellTarget({}, address=Address.parse(":default")).get(HaskellGhcExtensions).value
+        CustomHaskellTarget({}, address=Address.parse(":default"))[HaskellGhcExtensions].value
         == CustomHaskellGhcExtensions.default_extensions
     )