vimeo / psalm

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

Annotation to support singleton pattern. #7952

Open rzvc opened 2 years ago

rzvc commented 2 years ago

I can't find a way to indicate to Psalm that a method/function will always return the same object:

class Foo
{
    private static ?Foo $instance = null;
    public ?string $str = 'asdf';

    private function __construct()
    {
    }

    public static function getInstance() : self
    {
        if (is_null(self::$instance))
                self::$instance = new Foo();

        return self::$instance;
    }
}

if (is_null(Foo::getInstance()->str))
    return;

echo trim(Foo::getInstance()->str);     // Complains about $str possibly being null.

$foo = Foo::getInstance();

if (is_null($foo->str))
    return;

echo trim($foo->str);     // As expected, it doesn't complain here.

https://psalm.dev/r/55b5ff1a69

Something like @psalm-singleton on the method's or function's docblock would be nice.

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

I found these snippets:

https://psalm.dev/r/55b5ff1a69 ```php str)) return; echo trim(Foo::getInstance()->str); // Complains about $str possibly being null. $foo = Foo::getInstance(); if (is_null($foo->str)) return; echo trim($foo->str); // As expected, it doesn't complain here. ``` ``` Psalm output (using commit f960d71): ERROR: PossiblyNullArgument - 23:11 - Argument 1 of trim cannot be null, possibly null value provided ```
orklah commented 2 years ago

Even if Psalm understands it's dealing with a singleton, it won't help you.

Any statement between your is_null check and your trim could potentially mutate your instance and change $str value...

rzvc commented 2 years ago

That can be said about any object, and it would apply to the scenario where the object is used directly as well, but that one works. If Psalm would be aware of the singleton pattern, it would behave the same in both cases.