Skip to content

Commit

Permalink
Fix repr for Fields that don't define default. (Cherry-pick of p…
Browse files Browse the repository at this point in the history
…antsbuild#18719) (pantsbuild#18723)

Found while working on pantsbuild#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)`.
  • Loading branch information
danxmoran authored Apr 12, 2023
1 parent a82b356 commit 575c85f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 9 deletions.
20 changes: 12 additions & 8 deletions src/python/pants/engine/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/engine/target_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ class CustomFortranTarget(Target):


def test_required_field() -> None:
class RequiredField(StringField):
class RequiredField(Field):
alias = "field"
required = True

Expand All @@ -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)
Expand Down

0 comments on commit 575c85f

Please sign in to comment.