-
Notifications
You must be signed in to change notification settings - Fork 504
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 password_hash return type #2260
Conversation
This isn't great. If the inputs are correct, users shouldn't be forced to handle |
After some try, password_hash returns null
This won't be an easy dynamicReturnTypeExtension...
I understand, but currently PHP 7.4 users are already forced to handle When I looked at
So even without dynamicReturnTypeExtension it would be more useful (and correct ?) for
while I can let
for the original Would you be ok with this first step ? |
I'd just do benevolent union of string|false|null and call it a day. |
Nice idea. Done :) |
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.
You're closing an issue with this PR - needs a regression test.
public function testBug5978(): void | ||
{ | ||
if (PHP_VERSION_ID >= 80000) { | ||
$this->markTestSkipped('Test does not run on PHP >= 8.0.'); |
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.
Instead, you could assert different errors on PHP 7.x vs. 8.x.
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, that's better
Thank you! |
Cf https://3v4l.org/DMv87
Closes phpstan/phpstan#5978