Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the PhpDocExtractor in addition to the ReflectionExtractor. #125

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nick-potts
Copy link
Contributor

I've added the PhpDocExtractor in addition to the ReflectionExtractor which allows you to use the phpdoc to decode data properly. That also works in tandem with the array denormalizer (which I've added to the verbs.php). Its typically default behaviour to include the PhpDocExtractor when the https://symfony.com/doc/current/components/property_info.html#phpdocextractor

I also fixed a bug with the NormalizeToPropertiesAndClassName. If the required parameters count is 0 and there's data, the Arr::has( will return true causing it to fail for no reason. I found that because the DTO class wouldn't serialise for me. I believe the allows us to store a serializable class as a property test isn't actually testing anything, otherwise it should've caught it - I wasn't sure so I didn't delete it.

I also believe now the deserialiser is more robust, the BackedEnum code can be removed and I wrote a test to try prove to myself its working fine - but do as you please with that.

Copy link
Contributor Author

@nick-potts nick-potts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@inxilpro
Copy link
Contributor

@nick-potts thanks for your patience on this one. I was having a hard time understanding the scope of this PR. Would you mind adding a test that fails when the array denormalizer is missing to demonstrate the value of adding that?

Copy link
Contributor

@inxilpro inxilpro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment.

@nick-potts
Copy link
Contributor Author

The initial request I made had a bunch of changes that were later removed so the initial comment doesn't make too much sense - the latest is purely stuff that allows the PhpDoc Extractor to work.

The test that was added fails without the array denormaliser - the big benefit from the phpdoc extractor is that it can deserialise arrays using just the phpdoc. From the DTO it tests against, you can see how this works.

/** @var DTO[] $dtos */
public array $dtos = [new DTO]

Without the PhpDoc extractor or the array denormaliser, this property wouldn't get deserialised properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants