search

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

feat: support unions in arrays #1552

Open simPod opened 3 months ago

simPod commented 3 months ago
Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
scyzoryck commented 3 months ago

@idbentley can you help with code review, please? :)

scyzoryck commented 3 months ago

@simPod I think we need also E2E test of deserialisation - I think it should already work for simple types :)

idbentley commented 3 months ago

My main feedback is that this PR seems rather incomplete.

The changes to the @Type lexer/parser may support the simplest array<A|B> test case, but I can think of some edge cases that would cause that type parsing to fail. The @Type typer lexer/parser currently has no union support, so this adds support for array<A|B> but there's no support for just A|B.

The only tests seem to be of the docblockparser (which is good), but no tests of any serialization/deserialization, no tests of the Type annotation lexer changes.

What is the expected behaviour of A|B[] in a docblock type?

Overall, I think this is a great feature, but needs some extra help.