-
Couldn't load subscription status.
- Fork 17
Support constructor promotion with namespaced and union typehints #333
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
Conversation
Ah, it's just not handling union types in PHP < 8. I need to figure out what phpcs under PHP 5 and 7 see when they look at a union type.
That works. psalm in the automated tests is failing because it's having trouble parsing the XML config file but I think it's unrelated to this PR (psalm passes locally).
Fatal error: Uncaught Psalm\Exception\ConfigException: Error parsing file /home/runner/work/phpcs-variable-analysis/phpcs-variable-analysis/ on line 9: Element '{https://getpsalm.org/schema/config}psalm': No matching global declaration available for the validation root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sirbrillig Just out of curiousity: why are you not using the File::getMethodParameters() to parse the function signature ? That would allow for just checking if the parameter has a 'property_visibility' index to know whether it is a promoted property...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I didn't know about File::getMethodParameters() 😅 That does sound much easier! TIL there's helpers like this inside the source code.
Related: I really need to get around to adding phpcsutils to this package; you've done a fantastic job with its documentation!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you didn't know about this one... you might also be interested in the File::getMemberProperties() (for declared, non constructor promoted, properties in a class)...
And yes, PHPCSUtils has those too and improved on them further + much more.
Happy to talk things through with you at some point if you think that would help. Unfortunately won't have any time in the foreseeable future to actually help you with the code changes.
Constructor property promotion supports full typehinting, but while this sniff handled a simple typehint like
stringor?string, it could not handle complex typehints likefalse|null|\App\Models\User.In this PR we update the handling to be able to better recognize a typehint.
Fixes #331