Skip to content

Fix phpstan/phpstan#5952: Allow throwing exceptions from __toString() not detected since level 4#5405

Merged
VincentLanglet merged 7 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-6rkqe4h
Apr 7, 2026
Merged

Fix phpstan/phpstan#5952: Allow throwing exceptions from __toString() not detected since level 4#5405
VincentLanglet merged 7 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-6rkqe4h

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When echo $obj is used where $obj has 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 in NodeScopeResolver did not track throw points from implicit __toString() calls.

Changes

  • Added __toString() throw point tracking to the echo statement handler in src/Analyser/NodeScopeResolver.php
  • The fix reuses the same pattern as CastStringHandler (explicit (string) cast): checks PhpVersion::throwsOnStringCast() (PHP >= 7.4) and uses MethodThrowPointHelper::getThrowPoint() to determine if the __toString() method can throw
  • Added regression test at tests/PHPStan/Rules/Exceptions/data/bug-5952.php

Root cause

The echo statement handler in NodeScopeResolver collected 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 to echo statements.

Test

The regression test covers:

  • echo $obj where __toString() throws (implicitly via @throws void absence) - no dead catch error
  • echo $obj where __toString() has @throws \Exception - no dead catch error
  • echo $obj where __toString() has @throws void - dead catch correctly reported
  • echo 123 (non-object) - dead catch correctly reported

Fixes phpstan/phpstan#5952

- 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
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

make tests also completed successfully. All tests pass, static analysis is clean, and the commit has been pushed. The review feedback has been fully addressed.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Both background tasks have already been handled — make tests and make phpstan both passed, and the commit was pushed. Nothing else to do here.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

All clear - every background task has completed successfully. The review feedback has been fully addressed and pushed.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than an array getToStringThrowAndImpurePoints could return an ExpressionResult. And the helper should be called ImplicitToStringCallHelper

…urn ExpressionResult

Rename the helper to better describe its purpose and change the return type
from array{list<InternalThrowPoint>, list<ImpurePoint>} to ExpressionResult
for a cleaner API at all call sites.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. The changes address VincentLanglet's review:

  1. Renamed ToStringThrowPointHelper to ImplicitToStringCallHelper - better describes its purpose
  2. Changed return type from array{list<InternalThrowPoint>, list<ImpurePoint>} to ExpressionResult - cleaner API, call sites now use $toStringResult->getThrowPoints() / $toStringResult->getImpurePoints() instead of array destructuring
  3. Renamed method from getToStringThrowAndImpurePoints to processImplicitToStringCall

All call sites updated, PHPStan passes clean, regression test still passes.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the lint errors

Class \PHPStan\Analyser\ExprHandler\Helper\ImplicitToStringCallHelper should not be referenced via a fully qualified name, but via a use statement.

@VincentLanglet VincentLanglet self-assigned this Apr 6, 2026
…qualified name

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Already handled — tests passed, commit pushed. Nothing more to do.

@VincentLanglet VincentLanglet requested a review from staabm April 6, 2026 09:19

$toStringResult = $this->implicitToStringCallHelper->processImplicitToStringCall($expr->expr, $scope);
$throwPoints = array_merge($throwPoints, $toStringResult->getThrowPoints());
$impurePoints = array_merge($impurePoints, $toStringResult->getImpurePoints());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this PR is not only about throw points, but also impure points.

it seems the impurity portion does not have tests

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both background tasks confirmed complete. Everything was already pushed — nothing more needed.

…add purity tests

The echo statement handler in NodeScopeResolver was not collecting impure points
from implicit __toString() calls or from sub-expression processing. Also adds
purity tests for all implicit __toString() call sites: echo, print, string
concatenation, concat assignment, and string interpolation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@VincentLanglet VincentLanglet requested a review from staabm April 6, 2026 10:27
@@ -861,19 +863,22 @@ public function processStmtNode(
} elseif ($stmt instanceof Echo_) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need more changes in NodeScopeResolver to reflect the changes made in ExprHandler classes with this PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All 11732 tests pass.

Summary: After analyzing the codebase, no additional changes are needed in NodeScopeResolver.php. Here's why:

  • The echo handler (line 872) already correctly uses ImplicitToStringCallHelper — this was added in this PR
  • All other implicit __toString() call sites are expressions (not statements), so they're handled by their respective ExprHandler classes:
    • PrintHandlerprint $obj
    • CastStringHandler(string) $obj
    • BinaryOpHandler'hello' . $obj
    • AssignOpHandler$x .= $obj
    • InterpolatedStringHandler"hello $obj"

The ImplicitToStringCallHelper is injected into NodeScopeResolver solely for the echo statement, since echo is a Stmt (not an Expr) and cannot have its own ExprHandler.

I was unable to post this reply as a PR comment due to GitHub token permissions. You may want to reply to staabm's comment manually with this explanation.

@VincentLanglet VincentLanglet merged commit 7ebe2f2 into phpstan:2.1.x Apr 7, 2026
659 checks passed
@VincentLanglet VincentLanglet deleted the create-pull-request/patch-6rkqe4h branch April 7, 2026 06:55
phpstan-bot added a commit to phpstan-bot/phpstan-src that referenced this pull request Apr 7, 2026
… not detected since level 4 (phpstan#5405)

Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants