Add ThrowableReturnTypeExtension

[herndlm]

Closes phpstan/phpstan#6001

Based on feedback of the first approach in #767 (comment)

[herndlm]

I'm still unsure when it comes to tests in phpstan - is this assert test enough?

[ondrejmirtes]

Yes, thank you!

[Seldaek]

Are you sure this is the best way to handle this? And if it is only PDOException could we not return string only for that class, and leave getCode as an int for other classes?

I am asking because as per https://phpstan.org/r/610c7193-7cba-46eb-be15-b9f09c91728f it is a bit weird IMO that a clear int case is seen as string|int, which forced me to do composer/composer@84f8fda to fix these errors https://github.com/composer/composer/actions/runs/1638602292

[ondrejmirtes]

@Seldaek We've had Exception::getCode() and Throwable::getCode() as only int in the past but it was wrong in many cases.

We haven't tried Exception::getCode()and Throwable::getCode() to be int|string, PDOException::getCode() as string and any other subclass to be int but I think it's certain there's going to be userland exceptions that use string as well...

[Seldaek]

I tried redefining getCode to add an int return type but it's final so I couldn't, so I'm not sure what those userland exceptions would be? Exceptions in extensions perhaps..

[ondrejmirtes]

@Seldaek You can do this crazy thing: https://3v4l.org/vcWJg

[Seldaek]

OK, although I would argue that is totally voiding the warranty and shouldn't mean PHPStan covers your ass with a mixed return type. I suspect many things can be broken in similar ways or with reflection or who knows what..

[Seldaek]

https://www.php.net/manual/en/exception.getcode.php clearly documents it as returning an int, so IMO an exception for PDO as it's insane but part of core makes sense, but everything else should be int, and those people doing insane shit can ignore their errors instead of forcing everyone else to do so.

[ondrejmirtes]

Alright, I'm eager to try my suggestion from #795 (comment) too: 1181717

Let's see what the integration tests and the feedback will say :)

[ondrejmirtes]

Try out 1.3.2 :) https://github.com/phpstan/phpstan/releases/tag/1.3.2

[Seldaek]

Thanks, looks good to me now, and sorry for the rant :D

[Seldaek]

composer/composer@508ac05 🎉

[Seldaek]

Ahh well now I'm left with https://github.com/composer/composer/runs/4736711738?check_suite_focus=true which makes sense of course.. There's no way to tell PHPStan to narrow down the return type for getCode is there? Redefining with @method did not seem to help.

[ondrejmirtes]

Yeah, this might be actually solved for good, until someone notices that some exception from a popular framework like Symfony or Doctrine also misuses this :D

The fact that @method doesn't work here surprised me, I'm gonna look into it.

[Seldaek]

At least adding this to SolverProblemsException's class docblock did not help, maybe I did it wrong tho:

 * @method self::ERROR_DEPENDENCY_RESOLUTION_FAILED getCode()

[Seldaek]

Maybe its effect is negated by the ThrowableReturnTypeExtension?

[ondrejmirtes]

Good guess, that was it: e04cc8d

[Seldaek]

Nice :) Thanks a bunch

[ondrejmirtes]

Enjoy 1.3.3 :) https://github.com/phpstan/phpstan/releases/tag/1.3.3

[Seldaek]

It builds 👍🏻 composer/composer@64d39a9