Add a psalm type for field mapping #9198
Conversation
|
@laryjulien thanks! Both Psalm and Phpstan now detect 7 issues with keys that are accessed without checking first if they might be null… for each of them, I see 3 possibilities:
I'm not sure what's best, but I think it mostly depends on whether the issue could actually happen or not. |
| * columnName?: string, | ||
| * columnDefinition?: string | ||
| * } | ||
| * @psalm-return FieldMapping |
There was a problem hiding this comment.
this is a BC break IMHO, previous type states that fieldName, type, scale, length, unique, nullable, and precision are always available, while the new type states that they might not exist in the returned array.
There was a problem hiding this comment.
Oh right, didn't catch that, why did you change this @laryjulien ?
There was a problem hiding this comment.
I have changed my PR to only use FieldMapping type in the initial context of issue.
I suggest to open an other PR to use this type in a wider context. Indeed, there are small variations on field mapping definition which appear to be more complex than I thought at first glance.
| @@ -1534,7 +1532,7 @@ private function validateAndCompleteTypedAssociationMapping(array $mapping): arr | |||
| /** | |||
| * Validates & completes the given field mapping. | |||
| * | |||
| * @psalm-param array<string, mixed> $mapping The field mapping to validate & complete. | |||
| * @psalm-param FieldMapping $mapping The field mapping to validate & complete. | |||
There was a problem hiding this comment.
This is actually the BC break, as the other method is private.
laryjulien
force-pushed
the
fix-fieldmapping-definition
branch
from
2002d46 to
444b5a5
Compare
Field mapping have different definitions in property definition and method return. As suggested in issue and to avoid further desynchronization, a psalm type has been created. Fixes doctrine#9193
laryjulien
force-pushed
the
fix-fieldmapping-definition
branch
from
444b5a5 to
5aba762
Compare
|
Thanks @laryjulien ! |
Field mapping have different definitions
in property definition and method return.
As suggested in issue and to avoid further desynchronization,
a psalm type has been created.
Fixes #9193