Consider @var annotation for type inference when deserializing

schmittjoh / serializer

Library for (de-)serializing data of any complexity (supports JSON, and XML)

http://jmsyst.com/libs/serializer

MIT License

2.32k stars 585 forks source link

Consider @var annotation for type inference when deserializing #893

Open goetas opened 6 years ago

goetas commented 6 years ago

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes

goetas commented 6 years ago

Not really fun of it, I prefer explicit types.

Closing. If someone has valid points, i'm ready to re-consider it.

Majkl578 commented 6 years ago

:-1: for closing

It's a great usability feature.

bdsl commented 5 years ago

I was about to make the same feature request as this. @var string seems just as explicit as @Serializer\Type("string"), and @var has the advantage that many different tools will read it. As it is I always put both annotations in. Most of the time we want the types set by the deserialise function to match types understood by IDEs, static analysis tools and documentation generating tools.

To make this more useful it would be good to understand use statements for namespace resolution, and if #1090 is implemented nullable types.

Majkl578 commented 5 years ago

Explicit @Type will be even more annoying with PHP 7.4's typed properties.

goetas commented 5 years ago

When typed properties will land in PHP, the @var annotation will be obsolete, then inferring the type from the property reflection will make perfectly sense. In that case will be happy to accept a PR with that feature.

bdsl commented 5 years ago

Even in 7.4 @var won't be entirely obsolete, because people use it with types that are not understood by the PHP engine - e.g. @var array<\DateTimeInterface>, @var int|string, @var mixed etc. The first of these is also supported by the existing @Type annotation.

But I might be interested in making that PR for 7.4 typed properties support some time a bit nearer to 7.4 release date.

Majkl578 commented 5 years ago

@goetas Not really sure what you mean by your comment - @var is already substituing absense of the typed properties and therefore supporting it makes sense already.

Majkl578 commented 5 years ago

Also agreed with @bdsl - 7.4 won't make @var obsolete by far - no union types, no generics.

goetas commented 5 years ago

Alright, I see that this feature has people that could use it. There are still many questions to be answered on how to implement it, some are:

should this be considered only when using the annotation driver?
if no, what happens for those that have the annotation driver disabled
should the argumentation used to close https://github.com/schmittjoh/JMSSerializerBundle/pull/427 be valid (at least partially even in this case?)
implementing this only for the types definitions as supported in php 7.4 should be more straightforward

Majkl578 commented 5 years ago

should this be considered only when using the annotation driver?

No, Doctine Annotations are not a parser for phpDoc, they're not compatible.

if no, what happens for those that have the annotation driver disabled

N/A, this feature is independent on the annotation driver.

goetas commented 5 years ago

implemented as separate driver, sounds a good idea to me!

magnetik commented 4 years ago

I'm not sure : is inferring types from PHP 7.4 types properties in the scope of this issue? Or is this covered elsewhere?

Tobion commented 3 years ago

This has been implemented for both var annotations as well as property types in #1214 and #1192. So this can be closed.