From 575c85f8d09499287ebb35dbb7793f2a3caa2902 Mon Sep 17 00:00:00 2001
From: Daniel Moran <danxmoran@gmail.com>
Date: Wed, 12 Apr 2023 14:05:04 -0400
Subject: [PATCH] Fix `repr` for `Field`s that don't define `default`.
 (Cherry-pick of #18719) (#18723)

Found while working on #18566

The code for `Field` declares that subclasses must define one of
`default` and `required`, and leaves `default` without a value. However
its implementation of `__repr__` assumes that `default` will always be
defined. This can cause errors like:
```
AttributeError: 'RequiredField' object has no attribute 'default'
```
In various places, i.e. when stringifying a `FieldSet` or when
converting a `Target` to a `FieldSet` via `field_set_type.create(tgt)`.
---
 src/python/pants/engine/target.py      | 20 ++++++++++++--------
 src/python/pants/engine/target_test.py |  4 +++-
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/python/pants/engine/target.py b/src/python/pants/engine/target.py
index e885d86a59e..6f713f957e4 100644
--- a/src/python/pants/engine/target.py
+++ b/src/python/pants/engine/target.py
@@ -169,10 +169,10 @@ def _check_deprecated(cls, raw_value: Optional[Any], address: Address) -> None:
         )
 
     def __repr__(self) -> str:
-        return (
-            f"{self.__class__}(alias={repr(self.alias)}, value={repr(self.value)}, "
-            f"default={repr(self.default)})"
-        )
+        params = [f"alias={self.alias!r}", f"value={self.value!r}"]
+        if hasattr(self, "default"):
+            params.append(f"default={self.default!r}")
+        return f"{self.__class__}({', '.join(params)})"
 
     def __str__(self) -> str:
         return f"{self.alias}={self.value}"
@@ -246,10 +246,14 @@ def __init__(self, raw_value: Optional[Any], address: Address) -> None:
             self.address = address
 
     def __repr__(self) -> str:
-        return (
-            f"{self.__class__}(alias={repr(self.alias)}, address={self.address}, "
-            f"value={repr(self.value)}, default={repr(self.default)})"
-        )
+        params = [
+            f"alias={self.alias!r}",
+            f"address={self.address}",
+            f"value={self.value!r}",
+        ]
+        if hasattr(self, "default"):
+            params.append(f"default={self.default!r}")
+        return f"{self.__class__}({', '.join(params)})"
 
     def __hash__(self) -> int:
         return hash((self.__class__, self.value, self.address))
diff --git a/src/python/pants/engine/target_test.py b/src/python/pants/engine/target_test.py
index d32a99dd221..ee96702f75a 100644
--- a/src/python/pants/engine/target_test.py
+++ b/src/python/pants/engine/target_test.py
@@ -400,7 +400,7 @@ class CustomFortranTarget(Target):
 
 
 def test_required_field() -> None:
-    class RequiredField(StringField):
+    class RequiredField(Field):
         alias = "field"
         required = True
 
@@ -409,6 +409,8 @@ class RequiredTarget(Target):
         core_fields = (RequiredField,)
 
     address = Address("", target_name="lib")
+    # No errors getting the repr
+    assert repr(RequiredField("present", address))
 
     # No errors when defined
     RequiredTarget({"field": "present"}, address)