-
Notifications
You must be signed in to change notification settings - Fork 545
string cast might throw exception #4555
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
base: 2.1.x
Are you sure you want to change the base?
Conversation
src/Analyser/NodeScopeResolver.php
Outdated
| $exprType = $scope->getType($expr->expr); | ||
| $toStringMethod = $scope->getMethodReflection($exprType, '__toString'); | ||
| if ($toStringMethod !== null) { | ||
| if ($toStringMethod->getThrowType() !== null) { |
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.
the rfc is PHP 7.4+, meaning this logic should be gated via PHPVersion class.
note though, that NodeScopeResolver is undergoing a major refactoring atm, so this might conflict easily
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.
It won't conflict, I'm not doing any major changes in NodeScopeResolver that I'm not pushing.
But it's possible that the same change also needs to happen in PHPStan\Analyser\Generator namespace. In this case it doesn't because there's no StringCastHandler ported yet.
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 thought the minimum version PHPStan supports was 7.4 Is it 7.1?
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 are 2 things.
the php version PHPStan can analyze against, configured in the .neon config:
phpstan-src/conf/parametersSchema.neon
Line 103 in 5afb019
| schema(int(), min(70100), max(80599)), |
and the PHP runtime version you use to run PHPStan itself:
https://github.com/phpstan/phpstan/blob/5afb019da9f70ae5a2b4247a1830b5103423a325/composer.json#L7
| @@ -0,0 +1,20 @@ | |||
| <?php | |||
|
|
|||
| namespace Bug13806; | |||
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 feel like this lack of test,
It should add a test for
function doFoo(Stringable $myVariable): void
to check the implicit throw tag
and a test for
class MyStringVoid {
/** @throws void */
public function __toString() {
throw new \InvalidArgumentException();
}
}
to check in this case the catch is reported as dead catch
|
This pull request has been marked as ready for review. |
|
Does it also solve phpstan/phpstan#5952 ? |
closes phpstan/phpstan#13806