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

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

Closed
wants to merge 1 commit into from

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Jan 10, 2023

Q A
Bug fix? yes
License MIT

This fixes another type issues (will be covered by phpstan when we ship #1446 and following PRs)

@goetas
Copy link
Collaborator

goetas commented Jan 10, 2023

@dgafka ?

@dgafka
Copy link
Contributor

dgafka commented Jan 11, 2023

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
Copy link
Contributor Author

simPod commented Jan 11, 2023

@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
Copy link
Contributor

dgafka commented Jan 12, 2023

This can be closed now #1462 :)

@goetas goetas closed this Jan 20, 2023
@simPod simPod deleted the type branch January 20, 2023 09:09
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.

3 participants