vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.5k stars 655 forks source link

Issue when testing code that falsely claims to throw #1093

Closed aidantwoods closed 5 years ago

aidantwoods commented 5 years ago

Took a little while to narrow down the cause for this one :p

Using the following src/test.php file where we claim to throw, but actually don't:

<?php

final class Foo
{
    /**
     * @throws \TypeError
     */
    public static function notReallyThrowing(int $a): string
    {
        if ($a > 0) {
            return '';
        }

        return (string) $a;
    }

    public function test(): string
    {
        try {
            return self::notReallyThrowing(2);
        } catch (\Throwable $E) {
            return '';
        }
    }
}

with

<?xml version="1.0"?>
<psalm
    checkForThrowsDocblock="true"
>
    <projectFiles>
        <directory name="src" />
    </projectFiles>
</psalm>

The following occurs:

$ vendor/bin/psalm
Scanning files...
Analyzing files...
PHP Fatal error:  Uncaught InvalidArgumentException: Could not get class storage for typeerror in /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Provider/ClassLikeStorageProvider.php:40
Stack trace:
#0 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/ClassLikes.php(413): Psalm\Internal\Provider\ClassLikeStorageProvider->get('typeerror')
#1 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Codebase.php(582): Psalm\Internal\Codebase\ClassLikes->classExtends('typeerror', 'Throwable')
#2 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php(210): Psalm\Codebase->classExtendsOrImplements('TypeError', 'Throwable')
#3 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(227): Psalm\Internal\Analyzer\Statements\Block\TryAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Stmt\TryCatch), Object(Psalm\Context))
#4 /Users/Aidan/tmp/psalm-test/vendor/ in /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Provider/ClassLikeStorageProvider.php on line 40

If we instead actually throw in some case:

<?php

final class Foo
{
    /**
     * @throws \TypeError
     */
    public static function actuallyThrowing(int $a): string
    {
        if ($a > 0) {
            throw new \TypeError;
        }

        return (string) $a;
    }

    public function test(): string
    {
        try {
            return self::actuallyThrowing(2);
        } catch (\Throwable $E) {
            return '';
        }
    }
}

then everything goes as expected:

$ vendor/bin/psalm
Scanning files...
Analyzing files...

------------------------------
No errors found!
------------------------------

Checks took 0.19 seconds and used 31.554MB of memory
Psalm was able to infer types for 100.000% of analyzed code (1 file)
muglug commented 5 years ago

Thanks for taking the time to reproduce!

aidantwoods commented 5 years ago

Thanks for taking the time to reproduce!

No worries :) That was a super fast fix, bug wasn't even open for 10 minutes!

aidantwoods commented 5 years ago

Hmm, so it seems that this issue re-occurs when the function claiming to throw isn't present in the scan directory but is instead found via the autoloader. e.g. using the same config as above, but also running composer require paragonie/paseto (which happens to include one of these functions that claims to throw without explicitly throwing), then if we test this file with psalm:

<?php

use ParagonIE\Paseto\{Keys\SymmetricKey, Protocol\Version2};

final class Foo
{
    public function test(string $encoded): void
    {
        try {
            $Key = SymmetricKey::fromEncodedString($encoded, new Version2);
        } catch (\Throwable $E) {
            return;
        }
    }
}

we again see

$ vendor/bin/psalm
Scanning files...
Analyzing files...
PHP Fatal error:  Uncaught InvalidArgumentException: Could not get class storage for typeerror in /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Provider/ClassLikeStorageProvider.php:40
Stack trace:
#0 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Codebase/ClassLikes.php(413): Psalm\Internal\Provider\ClassLikeStorageProvider->get('typeerror')
#1 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Codebase.php(582): Psalm\Internal\Codebase\ClassLikes->classExtends('typeerror', 'Throwable')
#2 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Block/TryAnalyzer.php(210): Psalm\Codebase->classExtendsOrImplements('TypeError', 'Throwable')
#3 /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/StatementsAnalyzer.php(227): Psalm\Internal\Analyzer\Statements\Block\TryAnalyzer::analyze(Object(Psalm\Internal\Analyzer\StatementsAnalyzer), Object(PhpParser\Node\Stmt\TryCatch), Object(Psalm\Context))
#4 /Users/Aidan/tmp/psalm-test/vendor/ in /Users/Aidan/tmp/psalm-test/vendor/vimeo/psalm/src/Psalm/Internal/Provider/ClassLikeStorageProvider.php on line 40

on 3.0.2.

muglug commented 5 years ago

Hmm I was unable to repro in a bare repo with that file and those deps - are you v. sure you're on latest?

aidantwoods commented 5 years ago

Definitely on latest (3.0.2). You definitely have the checkForThrowsDocblock check enabled right? (Also important that nothing in the scanned directory references \TypeError in docblocks or otherwise—first time psalm sees it has to be via the autoloaded file for this to occur).

Let me know if you still can't repro and I'll try put together a more isolated example.

muglug commented 5 years ago

You definitely have the checkForThrowsDocblock check enabled right

UHH ok one sec

muglug commented 5 years ago

Still cannot reproduce :/

muglug commented 5 years ago

here's what I was using (minus the vendor dir)

psalm-test.zip

aidantwoods commented 5 years ago

Hmm okay this is very weird, only occurred on the second run. Seems that vendor/bin/psalm --clear-cache will cause it to behave fine for a single run again.

screenshot 2018-11-29 at 06 38 47
muglug commented 5 years ago

Sorry! Fixed that in 21f4a2c

aidantwoods commented 5 years ago

Thanks for taking the time to investigate 😄