Fix phpstan/phpstan#5952: Allow throwing exceptions from __toString() not detected since level 4#5405
Conversation
- Added __toString() throw point tracking to echo statement handling in NodeScopeResolver - Reuses existing CastStringHandler pattern: checks PhpVersion::throwsOnStringCast() and MethodThrowPointHelper - New regression test in tests/PHPStan/Rules/Exceptions/data/bug-5952.php - The root cause was that echo $obj did not consider implicit __toString() calls when collecting throw points
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Rather than calling directly MethodThrowPointHelper can't we simulate a MethodCall ?
Address review feedback: process a synthetic MethodCall expression through processExprNode() instead of directly calling MethodThrowPointHelper. This leverages the existing MethodCallHandler infrastructure for __toString() throw point tracking in echo statements. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Remove the phpversion check, it's already done in the toString resolution.
Also add a test for echo on an union
- 123|Foo
- 123|Bar
- 123|Baz
The phpVersion->throwsOnStringCast() check is unnecessary because the toString resolution already handles the PHP version gate internally. Also adds regression tests for echo on union types (int|Foo, int|Bar, int|Baz) as requested in review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Both background tasks have already been handled — |
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
This fix for echo but I assume the same issue could occurs for some other situation where __toStirng is implicitely called. Is there a general fix ?
Extract ToStringThrowPointHelper to handle __toString() throw points
and impure points in a reusable way, and apply it to all places where
PHP implicitly calls __toString():
- echo (NodeScopeResolver)
- print (PrintHandler)
- string concatenation . (BinaryOpHandler)
- concat assignment .= (AssignOpHandler)
- string interpolation "$obj" / "{$obj}" (InterpolatedStringHandler)
- explicit (string) cast (CastStringHandler) - refactored to use helper
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
All clear - every background task has completed successfully. The review feedback has been fully addressed and pushed. |
2 participants
Summary
When
echo $objis used where$objhas a__toString()method that can throw, PHPStan incorrectly reported "Dead catch - Exception is never thrown in the try block." This is because the echo statement handler inNodeScopeResolverdid not track throw points from implicit__toString()calls.Changes
__toString()throw point tracking to the echo statement handler insrc/Analyser/NodeScopeResolver.phpCastStringHandler(explicit(string)cast): checksPhpVersion::throwsOnStringCast()(PHP >= 7.4) and usesMethodThrowPointHelper::getThrowPoint()to determine if the__toString()method can throwtests/PHPStan/Rules/Exceptions/data/bug-5952.phpRoot cause
The
echostatement handler inNodeScopeResolvercollected throw points from expression processing but did not account for PHP's implicit__toString()call when echoing an object. The explicit string cast handler (CastStringHandler) already had this logic, but it was not applied toechostatements.Test
The regression test covers:
echo $objwhere__toString()throws (implicitly via@throws voidabsence) - no dead catch errorecho $objwhere__toString()has@throws \Exception- no dead catch errorecho $objwhere__toString()has@throws void- dead catch correctly reportedecho 123(non-object) - dead catch correctly reportedFixes phpstan/phpstan#5952