Support constructor promotion with namespaced and union typehints #333
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).
|
| @@ -1535,62 +1535,105 @@ public static function getForLoopForIncrementVariable($stackPtr, $forLoops) | |||
| */ | |||
| public static function isConstructorPromotion(File $phpcsFile, $stackPtr) | |||
There was a problem hiding this comment.
@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.
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.
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