Fix false positive dead catch for ReflectionMethod::invoke/invokeArgs#4985
Fix false positive dead catch for ReflectionMethod::invoke/invokeArgs#4985staabm merged 8 commits intophpstan:2.1.xfrom
Conversation
- Added ReflectionMethodInvokeMethodThrowTypeExtension to declare that ReflectionMethod::invoke(), ReflectionMethod::invokeArgs(), ReflectionFunction::invoke(), and ReflectionFunction::invokeArgs() can throw any Throwable, since they execute arbitrary user code - New regression test in tests/PHPStan/Rules/Exceptions/data/bug-7719.php - The root cause was that BetterReflection's adapter declared @throws ReflectionException on these methods, causing PHPStan to treat ReflectionException as the only possible thrown type Closes phpstan/phpstan#7719
…s never thrown in the try block.
|
This pull request has been marked as ready for review. |
VincentLanglet
left a comment
There was a problem hiding this comment.
Maybe it's better to have ondrej opinion about something hardcoded like this ? WDYT ?
since there is currently no way to differentiate implicity throw points via extension I think we can hardcode it (similar to how we hardcode things into TypeSpecifier which are not achievable via extenions). |
The other solution would be a dynamicThrowTypeExtention with explicitPoint which resolve the method called (so ReflectionMethod/Function needs to be generic of the name method/function) AND the args, in order to know whether it will throw an exception or not ; and which one. But that's way more work. |
Summary
PHPStan incorrectly reported "Dead catch - RuntimeException is never thrown in the try block" when catching exceptions around
ReflectionMethod::invoke()andReflectionMethod::invokeArgs()calls. These methods execute arbitrary user code via reflection and can throw any exception, not justReflectionException.Changes
src/Type/Php/ReflectionMethodInvokeMethodThrowTypeExtension.php— aDynamicMethodThrowTypeExtensionthat declaresReflectionMethod::invoke(),ReflectionMethod::invokeArgs(),ReflectionFunction::invoke(), andReflectionFunction::invokeArgs()can throwThrowabletests/PHPStan/Rules/Exceptions/data/bug-7719.phpcovering all four methodstestBug7719()toCatchWithUnthrownExceptionRuleTest.phpphpstan-baseline.neonwith one new entry for the extension fileRoot cause
The BetterReflection library's adapter class (
vendor/ondrejmirtes/better-reflection/src/Reflection/Adapter/ReflectionMethod.php) has@throws ReflectionExceptionannotations oninvoke()andinvokeArgs(). PHPStan reads these throw type annotations and concludes that onlyReflectionExceptioncan be thrown by these methods. However, in real PHP,ReflectionMethod::invoke()andinvokeArgs()execute arbitrary user code that can throw any exception. The fix adds a dynamic throw type extension that overrides this to returnThrowable, correctly indicating that any exception type is possible.Test
The regression test in
tests/PHPStan/Rules/Exceptions/data/bug-7719.phpcreates four scenarios:ReflectionMethod::invokeArgs()with a dynamic method name — catch RuntimeException should not be reported as deadReflectionMethod::invoke()with a dynamic method name — sameReflectionFunction::invokeArgs()with a dynamic function name — sameReflectionFunction::invoke()with a dynamic function name — sameAll four expect no errors (empty expected errors array).
Fixes phpstan/phpstan#7719
Closes phpstan/phpstan#9267