phpstan / phpstan

PHP Static Analysis Tool - discover bugs in your code without running it!
https://phpstan.org/
MIT License
12.91k stars 879 forks source link

False positive: Dead catch #10315

Open Geolim4 opened 10 months ago

Geolim4 commented 10 months ago

Bug report

I'm working on a new version of my library and I have an abstract class that contain this method:

\Phpfastcache\Core\Pool\DriverPoolAbstractTrait

    /**
     * @param ExtendedCacheItemInterface ...$items
     * @return array<array<string, mixed>>
     * @throws PhpfastcacheUnsupportedMethodException
     */
    protected function driverReadMultiple(ExtendedCacheItemInterface ...$items): array
    {
        throw new PhpfastcacheUnsupportedMethodException();
    }

And I do have multiple child class that extends the abstract class, but for compatibility reason some child class override the driverReadMultiple method, and some aren't.

But Phpstan still returns me this error:

 ------ --------------------------------------------------------------------------------------------------------------- 
  Line   Phpfastcache\Core\Pool\CacheItemPoolTrait.php (in context of class Phpfastcache\Drivers\Redis\DriverAbstract)
 ------ ---------------------------------------------------------------------------------------------------------------
  138    Dead catch - Phpfastcache\Exceptions\PhpfastcacheUnsupportedMethodException is never thrown in the try block.
 ------ ---------------------------------------------------------------------------------------------------------------

\Phpfastcache\Core\Pool\CacheItemPoolTrait This is the "guilty" code:

                try {
                    $driverArrays = $this->driverReadMultiple(...$items);
                } catch (PhpfastcacheUnsupportedMethodException) {
                    /**
                     * Fallback for drivers that does not yet implement driverReadMultiple() method.
                     */
                    $driverArrays = array_combine(
                        array_map(fn($item) => $item->getKey(), $items),
                        array_map(fn($item) => $this->driverRead($item), $items)
                    );
                }

At the moment of this call, from a static analysis point of view, the code is not aware of the child implementing it.

I tried to reproduce it with Phpstan playground, but I think its too specific to Phpfastcache architecture :(

Code snippet that reproduces the problem

No response

Expected output

No error

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

It helps me a lot since years thanks again :smi

mergeable[bot] commented 10 months ago

This bug report is missing a link to reproduction at phpstan.org/try.

It will most likely be closed after manual review.

VincentLanglet commented 10 months ago

It shouldn't that hard to create a reproducer...

If I understood correctly the phpfastcache code, it should be something like https://phpstan.org/r/78c48681-6423-4652-a7f3-0f46c516505b

Geolim4 commented 10 months ago

It shouldn't that hard to create a reproducer...

If I understood correctly the phpfastcache code, it should be something like https://phpstan.org/r/78c48681-6423-4652-a7f3-0f46c516505b

Yeah, exactly, I struggled trying to reproduce it for an hour, but I gave up after being unable to reproduce the behavior. Thanks

phpstan-bot commented 1 month ago

@VincentLanglet @Geolim4 After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.0 – 8.3 (1 error)
+PHP 8.1 – 8.3 (1 error)
 ==========

 12: Dead catch - Exception is never thrown in the try block.

-PHP 7.2 – 7.4 (3 errors)
+PHP 8.0 (4 errors)
 ==========

+39: Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".
+39: Method Foo2::doThings() has invalid return type never.
 12: Dead catch - Exception is never thrown in the try block.
+11: Method Foo2::driverReadMultiple() should return array but returns never.
+
+PHP 7.2 – 7.4 (6 errors)
+==========
+
+39: Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".
+39: Method Foo2::doThings() has invalid return type never.
+12: Dead catch - Exception is never thrown in the try block.
 12: Non-capturing catch is supported only on PHP 8.0 and later.
+11: Method Foo2::driverReadMultiple() should return array but returns never.
 12: Non-capturing catch is supported only on PHP 8.0 and later.
Full report PHP 8.1 – 8.3 (1 error) ----------- | Line | Error | |---|---| | 12 | `Dead catch - Exception is never thrown in the try block.` | PHP 8.0 (4 errors) ----------- | Line | Error | |---|---| | 39 | `Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".` | | 39 | `Method Foo2::doThings() has invalid return type never.` | | 12 | `Dead catch - Exception is never thrown in the try block.` | | 11 | `Method Foo2::driverReadMultiple() should return array but returns never.` | PHP 7.2 – 7.4 (6 errors) ----------- | Line | Error | |---|---| | 39 | `Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".` | | 39 | `Method Foo2::doThings() has invalid return type never.` | | 12 | `Dead catch - Exception is never thrown in the try block.` | | 12 | `Non-capturing catch is supported only on PHP 8.0 and later.` | | 11 | `Method Foo2::driverReadMultiple() should return array but returns never.` | | 12 | `Non-capturing catch is supported only on PHP 8.0 and later.` |
phpstan-bot commented 2 weeks ago

@VincentLanglet @Geolim4 After the latest push in 2.0.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.0 – 8.3 (1 error)
+PHP 8.1 – 8.3 (1 error)
 ==========

 12: Dead catch - Exception is never thrown in the try block.

-PHP 7.2 – 7.4 (3 errors)
+PHP 8.0 (4 errors)
 ==========

+39: Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".
+39: Method Foo2::doThings() has invalid return type never.
 12: Dead catch - Exception is never thrown in the try block.
+11: Method Foo2::driverReadMultiple() should return array<mixed> but returns never.
+
+PHP 7.2 – 7.4 (6 errors)
+==========
+
+39: Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".
+39: Method Foo2::doThings() has invalid return type never.
+12: Dead catch - Exception is never thrown in the try block.
 12: Non-capturing catch is supported only on PHP 8.0 and later.
+11: Method Foo2::driverReadMultiple() should return array<mixed> but returns never.
 12: Non-capturing catch is supported only on PHP 8.0 and later.
Full report PHP 8.1 – 8.3 (1 error) ----------- | Line | Error | |---|---| | 12 | `Dead catch - Exception is never thrown in the try block.` | PHP 8.0 (4 errors) ----------- | Line | Error | |---|---| | 39 | `Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".` | | 39 | `Method Foo2::doThings() has invalid return type never.` | | 12 | `Dead catch - Exception is never thrown in the try block.` | | 11 | `Method Foo2::driverReadMultiple() should return array but returns never.` | PHP 7.2 – 7.4 (6 errors) ----------- | Line | Error | |---|---| | 39 | `Tip: Method Foo2::doThings() always throws an exception, it should have return type "never".` | | 39 | `Method Foo2::doThings() has invalid return type never.` | | 12 | `Dead catch - Exception is never thrown in the try block.` | | 12 | `Non-capturing catch is supported only on PHP 8.0 and later.` | | 11 | `Method Foo2::driverReadMultiple() should return array but returns never.` | | 12 | `Non-capturing catch is supported only on PHP 8.0 and later.` |