Calling methods from a catch inside a try/catch/finally block

[Pnoexz]

Bug report

I'm getting a false positive for "variable might not be defined" when catching an application-level exception in a try/catch/finally block. It's very similar to #3302 but in their case, they weren't calling any function, just assigning a value to $a. Not only it can't detect public methods from the exception, it also can't access methods from the same class.

Code snippet that reproduces the problem

https://phpstan.org/r/9fa72d5e-9236-4d55-a931-fdea130a6aeb (reproduction steps in the comments).

This is only an issue if there is a finally block, as can be seen in this playground: https://phpstan.org/r/32dd9fd1-5aa9-4f92-b519-586e85a21193

Expected output

This isn't an actual issue, so it shouldn't be reported.

Did PHPStan help you today? Did it make you happy in any way?

Absolutely it did, as it has been for many years now. Thanks to you, I was able to put a number to what every developer in the project already knew: our codebase sucks. Now we are getting the much needed time to fix them all. We fixed all of them up to level 2, fixed the critical ones at level 5 (and baselined the not-so-critical ones), and gave up on the rest for now (around 8500, and counting). As a good note, all new projects start at 8 from the very beginning now.

[ondrejmirtes]

Hi, thank you for your kind words! :)

About your issue: PHPStan is trying to be really correct here. It accounts for the possibility that $e->getMessage(); or $this->getMessage(); can throw an exception. In which case the $message variable is really going to be undefined in the finally block.

In case of your own method, you can add @throws void there: https://phpstan.org/r/f98b3a77-0b3f-4f95-86c5-289d66ca8a9c

Alternatively, you can set implicitThrows: false in your configuration. See here what it does: https://phpstan.org/blog/bring-your-exceptions-under-control#what-does-absent-%40throws-above-a-function-mean%3F

The actual bug here is that Exception::getMessage() can never throw an exception because it's a final method. I fixed it here: phpstan/phpstan-src@f5e88ae

But for your own methods you need to use the technique described above.

[Pnoexz]

You are absolutely right: https://3v4l.org/gSYkX. I've learned that silencing the errors is usually a sign of bad design; we know our own method doesn't throw exceptions, but that won't stand the test of time, adds unnecessary maintenance, and increases the learning curve of the entire project. It's just easier to refactor our code.

Thanks for the quick reply and fix

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.