-
Notifications
You must be signed in to change notification settings - Fork 545
Create PhpParserExpr.stub #4503
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
| $parameters = null; | ||
| $singleParametersAcceptor = null; | ||
| if (count($parametersAcceptors) === 1) { | ||
| if (!array_is_list($args)) { |
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.
doing a runtime check, to not change the phpdoc on a @api class
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.
Well this is worse because suddenly you're throwing an exception for a valid input advertised by the signature.
If it's not a list, let's make it a list.
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.
Great idea.
It might even be worth creating a new rule which errors in my wrong code example?
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.
Not sure how to express the logic you'd want...
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.
Error on conditions which always lead to unchecked exceptions, when the conditions narrows a parameter type.
But you are right, maybe too specific
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.
I'd merge this if you call array_values here instead 😊
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.
was not at the computer until now. fix incoming ;).
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.
I'm also outside the entire time 😂
|
This pull request has been marked as ready for review. |
|
Thank you! |
fixes "~140 offset might not exist" errors