Convert ClassMetadata::getFieldValue/setFieldValue from @method to real interface methods#513
Conversation
| * | ||
| * @param T $entity | ||
| * | ||
| * @phpstan-ignore generics.variance (T in parameter position is intentional: the method only makes sense for the mapped entity type) |
There was a problem hiding this comment.
T in parameter position requires the generic to be invariant. So we should indeed change it.
There was a problem hiding this comment.
The parameter $object is typed as object rather than T. The interface declares @template-covariant T of object, meaning ClassMetadata<Cat> is a subtype of ClassMetadata<Animal>. Covariance requires T to appear only in return (output) positions. Using @param T $object would place T in a parameter position, violating that constraint.
I can't change to @template T, it's creating more issues with the child classes.
GromNaN
force-pushed
the
make-get-set-field-value-real-methods
branch
2 times, most recently
from
bf96d1c to
2349c76
Compare
GromNaN
left a comment
GromNaN
left a comment
There was a problem hiding this comment.
Now I understand how covariance works for this class.
| /** | ||
| * Gets the value of the given field of the given object. | ||
| * | ||
| * @throws InvalidArgumentException if the object is not supported, or the field is not mapped. |
There was a problem hiding this comment.
I also added @throws InvalidArgumentException to document the contract when $object is not of the expected type or $field is not mapped. Currently the ORM and ODM implementations don't throw an exception, but that would be an improvement when upgrading to persistence v5
| * | ||
| * @param T $entity | ||
| * | ||
| * @phpstan-ignore generics.variance (T in parameter position is intentional: the method only makes sense for the mapped entity type) |
There was a problem hiding this comment.
The parameter $object is typed as object rather than T. The interface declares @template-covariant T of object, meaning ClassMetadata<Cat> is a subtype of ClassMetadata<Animal>. Covariance requires T to appear only in return (output) positions. Using @param T $object would place T in a parameter position, violating that constraint.
I can't change to @template T, it's creating more issues with the child classes.
…al interface methods
GromNaN
force-pushed
the
make-get-set-field-value-real-methods
branch
from
2349c76 to
e67b1d6
Compare
3 participants
Closes #451
@param T $entityis used on both methods even thoughTis declared covariant. This is intentional( the methods only make sense for the mapped entity type) but it violates strict generic variance rules. PHPStan warnings are suppressed with@phpstan-ignore generics.variance.Open question: should
@template-covariant Tbe relaxed to@template T(invariant) to model this more accurately? That would be a breaking change for downstream type annotations.