Extend the Visitor Interface

[wouterj]

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

We're using static analysis to analyse and improve our code. However, the VisitorInterface of the JMS Serializer seems to be missing many methods that are considered to be part of the "visitor API":

• visitNull($data, array $type)
• visitString($data, array $type)
• visitBoolean(bool $data, array $type)
• visitInteger(int $data, array $type)
• visitDouble(float $data, array $type)
• visitArray($data, array $type)
• startVisitingObject(ClassMetadata $metadata, object $object, array $type)
• visitProperty(PropertyMetadata $metadata, $v)
• endVisitingObject(ClassMetadata $metadata, object $data, array $type)

These methods exists in all 4 visitors in this bundle. Furthermore, they are recommended in the UPGRADE guide to be used to add items in a serialization listeners, meaning they should be part of some public visitor API imho.

Is there a way these methods can be introduced to the interface?

[goetas]

the mentioned interface was meant only for few usecases (and that is why is makred as internal), the SerializationVisitorInterface and DeserializationVisitorInterface are the one that should be used more often. TypeHandlers should typehint one of the two interfaces said above, event listeners were supposed to check the return type of $event->getVisitor() and use it as one of the two interfaces above.

In the previous phrase "use it", if we were in java would be "cast it".

[wouterj]

Ah, I see. So modifying the VisitorInterface seems not to be a good solution (hard to keep BC and there already are better - more specific - visitor interfaces).

Maybe it's better to overwrite the getVisitor() method in each Event to specify the more specific interface (e.g. SerializationVisitorInterface) as return type. That would remove the need to check the return type in a listener and make the events more type safe. Unfortunately this is only possible since PHP 7.4, so feel free to close this issue (to avoid stale issues).

[goetas]

Maybe it's better to overwrite the getVisitor() method in each Event to specify the more specific interface (e.g. SerializationVisitorInterface) as return type. That would remove the need to check the return type in a listener and make the events more type safe. Unfortunately this is only possible since PHP 7.4,

that was exactly the propose of the interface, but at that time was not allowed by php. I prefer to keep open this issue and solve it as soon as dropping php 7.2 and 7.3 will become a good option.