-
Notifications
You must be signed in to change notification settings - Fork 660
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
Mark throwable methods as pure #3171
Conversation
This works probably because it is not parsed by PHP but by something more tolerant, but let's make it more valid (the final access type in the signature is not valid) PHP anyway, that will raise fewer eyebrows.
Some of the failing tests look like they are cause by my changes, I have to look into that. |
Exception is the only possible class implementation of Throwable, and all of its methods except __toString() are final. See https://github.com/php/php-src/blob/ca006e54e30bc5ac96f35af1ed7fd73a8c422cf0/Zend/zend_exceptions.stub.php#L3-L25 Closes vimeo#3170
a96fe6a
to
4fee755
Compare
Thanks! Looks like the failures were just due to my earlier bad merge |
*/ | ||
public final function getCode(){} | ||
public final function getCode(): int {} |
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.
getCode()
can return string
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.
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.
Yes. PDO throw exceptions with string codes.
https://3v4l.org/lP7cb
https://www.php.net/manual/en/exception.getcode.php
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.
Indeed. WTF… a PR is on its way.
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.
See fix here: #3175
No description provided.