Skip to content

string cast might throw exception#4555

Open
canvural wants to merge 2 commits intophpstan:2.1.xfrom
canvural:bug-13806
Open

string cast might throw exception#4555
canvural wants to merge 2 commits intophpstan:2.1.xfrom
canvural:bug-13806

Conversation

@canvural
Copy link
Contributor

@canvural canvural marked this pull request as ready for review November 14, 2025 16:39
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@VincentLanglet
Copy link
Contributor

Does it also solve phpstan/phpstan#5952 ?
You could add a test for this issue :)

]);
}

public function testBug13806(): void
Copy link
Contributor

@staabm staabm Jan 22, 2026

Choose a reason for hiding this comment

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

I wonder why this test (and/or its result) does not depend on php version

$toStringMethod = $scope->getMethodReflection($exprType, '__toString');
if ($toStringMethod !== null) {
if ($this->phpVersion->throwsOnStringCast()) {
if ($toStringMethod->getThrowType() !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice, almost perfect 😊 But I think we're missing a few things:

  1. It should support @throws void as a means to mark "this does not throw exceptions".
  2. It should respect $this->implicitThrows. When throwType is null, if this property is true, it means an implicit throw point should be added. If this property is false, it means that __toString does not throw an exception.

There's already code in NodeScopeResolver handling these things - getMethodThrowPoint. Should be possible to reuse it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. It already supports that, no? The test case reports dead catch on line 16.
  2. How can I test that? Or just re-using getMethodThrowPoint is enough?

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.

PHPStan assumes string conversions cannot throw exceptions

5 participants