vimeo / psalm

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

False positive with PropertyNotSetInConstructor #5422

Open BenMorel opened 3 years ago

BenMorel commented 3 years ago

Consider the following code:

final class Uuid
{
    private string $value;

    public function __construct(string $value)
    {
        if (static::isValid($value)) {
            $this->value = 'foo';
            return;
        }

        throw new InvalidArgumentException('Provided UUid is invalid');
    }

    public static function isValid(string $value): bool
    {
        return preg_match('/whatever/', $value) === 1;
    }
}

Psalm reports:

ERROR: PropertyNotSetInConstructor - 7:20 - Property Uuid::$value is not defined in constructor of Uuid and in any private or final methods called in the constructor

I believe this is a false positive, as the constructor can only return when the value has been set.

Reproduce: https://psalm.dev/r/7a6e02239a

psalm-github-bot[bot] commented 3 years ago

I found these snippets:

https://psalm.dev/r/7a6e02239a ```php value = 'foo'; return; } throw new InvalidArgumentException('Provided UUid is invalid'); } public static function isValid(string $value): bool { return preg_match('/whatever/', $value) === 1; } } ``` ``` Psalm output (using commit d19088b): ERROR: PropertyNotSetInConstructor - 5:20 - Property Uuid::$value is not defined in constructor of Uuid and in any private or final methods called in the constructor ```
weirdan commented 3 years ago

Equivalent code with an inverted if is fine: https://psalm.dev/r/627a5cbd29

psalm-github-bot[bot] commented 3 years ago

I found these snippets:

https://psalm.dev/r/627a5cbd29 ```php value = 'foo'; } public static function isValid(string $value): bool { return preg_match('/whatever/', $value) === 1; } } ``` ``` Psalm output (using commit d19088b): No issues! ```
BenMorel commented 3 years ago

@weirdan Yep. It would be nice if it worked both ways!