Can't pass-through array without further hydration, even with custom caster
bradjones1 opened this issue · 5 comments
I am using ObjectHydrator as a way to both do some basic sanity-checking on responses from third-party APIs (by way of hydration) and then using that value object to pass around.
In some cases I wish to cast complex data properties into a further value-object classes, but in other cases I want to preserve an array "as-is" without being cast. This works fine for a "reflection-only" approach where the type of the property in the constructor is simply array.
However, if you also have a docblock (as my code standard mandates) that simply says array for this same property, you raise an exception Unable to hydrate object: The\\Class\nCaused by: Unable to resolve item type for type: array
This is due to the code in \EventSauce\ObjectHydrator\NaivePropertyTypeResolver::extractItemType which assumes a docblock will always specify a type for all the members of the array in either phpdoc or phpstan format.
The workaround appears to be to use the phpstan style array<mixed> type annotation (mixed if you want maximum compatibility) but this is definitely a WTF.
I think perhaps we could add some extra special-casing to ::extractItemType to return array if $type === 'array'? Was this a design choice, though? In that case this is a documentation item?
I suppose we could also add additional language in the exception to hint you in the right direction, like "If you intend to retain an array without casting or explicitly specifying its members, change the docblock to array<int|integer|string|mixed>"
Went down this rabbit hole and determined that the docblock test coverage is still lacking (e.g., in addition to the recently fixed #50) and also that we can better support simple array docblock param types by inferring that as containing mixed members, and also allowing various variations of FQCNs in docblocks as well. Looks like no regressions in the rest of the test suite (at least from my local testing) so hopefully these changes can be confidently merged.
Hi @bradjones1 thanks for digging into this! A+ grade contribution :D I'll make sure to release the related PR today.
@frankdejonge Thanks for the prompt review and the compliment, it means a lot coming from you!
Could we also publish a release? Refs #49