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 589 forks source link

fix: do not try to tokenize doc comment when there's none #1460

Closed simPod closed 1 year ago

simPod commented 1 year ago
Q A
Bug fix? yes
License MIT

This fixes another type issues (will be covered by phpstan when we ship https://github.com/schmittjoh/serializer/pull/1446 and following PRs)

goetas commented 1 year ago

@dgafka ?

dgafka commented 1 year ago

Thank @simPod for rising the PR. From my understanding this solves scenario where there is no constructor and we went to the path for checking that. Is this correct?

If that's the case, this is totally valid scenario that having test case for would secure us for future refactors. If you're unsure how to write such test, please take a look on testInferTypeForConstructorPropertyPromotion as a reference :)

simPod commented 1 year ago

@dgafka TBH I did not investigate in what scenario it fails, I just considered it a "compile" error because of invalid types.

I had no time to investigate what code is not supported exactly and had to hot fix it production with my fork so I don't have a reproducible.

I just knew that here ->getConstructor()->getDocComment() can occur a NPE and getDocComment returns string|false so I handled it.

dgafka commented 1 year ago

This can be closed now #1462 :)