Update getReference phpdoc

[Florian-B]

Fix of the PHPDoc for the getReference method to allow code editor such as PhpStorm (and probably others) to correctly understand that the return of the method is an instance of $class and not any object.

Without this fix, PhpStorm cannot autocomplete the methods of the object retrieved by the getReference method, and with this proposal, it works.

[greg0ire]

Is psalm-return taken into account by PHPStorm? I'm asking because I don't think having just the information that this method returns T or any object is good enough to propose auto-completion.

[Florian-B]

No, PhpStorm does not take the psalm-return annotation into account.

However, I can confirm that adding T|object in the @return annotation enables auto-completion.
It must certainly interprets the psalm-param annotation, which indicates that T is an instance of a specific object.

Additionally, I think we can migrate the @psalm-param class-string<T>|null $class annotation to the native @param class-string<T>|null $class annotation. What do you think?

[greg0ire]

Ok, so let's say T in that instance is … YourClass. Why does PHPStorm enable auto-completion on YourClass|object? It is equivalent to just object, isn't it?

[Florian-B]

I'm not sure I fully understand your last message.

If we just put @return object, it refers to the primitive type Object, which can correspond to any object.

Whereas T refers to the type that is created by the "generics" brought with the @template, indicating that T is an instance of $class, so the correct return type is T.

However, we have to keep T|object because the parameter $class is not required at the moment.

[greg0ire]

I know what T refers to, I know what object refers to as well, but it seems, you and Phpstorm disagree with me and Psalm/PHPStan as to what T|object means. I don't think it means "instance of T and instance of object". To me, that's T&object, which… is just T. Likewise, T|object can IMO be simplified to just object.

[Florian-B]

I don't claim to be an expert on this, but it fix the auto-completion in PHPStorm (I don't have colleagues using VSCode or other popular code editor to test this fix).

Here are some resources:

• https://phpstan.org/blog/generics-in-php-using-phpdocs#class-names
• https://phpstan.org/blog/generics-by-examples#accept-a-class-string-and-return-the-object-of-that-type
• https://psalm.dev/docs/annotating_code/templated_annotations/

We have a very large project (around 6000 getReferences to migrate) and I think it's a shame to have to add the $class parameter (which, by the way, is a very good idea) without having a type-hint in return.

IMO when $class becomes required, the return type would be @return T.

[greg0ire]

I'm not an expert on PHPStorm either, I don't use it, that's why I'm asking why it behaves like that, I don't know. I recall in the distant past, Collection|User[] was used at some point to mean Collection<User> because IDEs only understood | and not &, but would expect this to be fixed by now. So IMO you should file 2 PHPStorm bugs:

1. It should no longer interpret | as &
2. It should add support for @psalm-return ($class is null ? object : T)

BTW, maybe it already understands that syntax, but ignores it because it's @psalm-return and not return? Can you check?

[Florian-B]

I've just tried several combinations (with only @return or only @psalm-return) but nothing changes 😞

However @return T&object works very well.

Here's a new proposal for phpdoc :

    /**
     * Loads an object using stored reference named by $name
     *
     * @param class-string<T>|null $class
     *
     * @return T&object
     * @psalm-return ($class is null ? object : T)
     *
     * @throws OutOfBoundsException - if repository does not exist.
     *
     * @template T of object
     */

Note that the @psalm-param has been changed to @param because I don't think it's necessarily specific to psalm.

What do you think?

[greg0ire]

I think T&object is completely wrong. For instance, when $class is null, there is no guarantee to get T.

[Florian-B]

Yes, I completely agree with you ! T&object is wrong when $class is null.

I took some time to test and I realize that PHPStorm seems to have trouble interpreting the conditional return of the psalm-return annotation, especially with generics (if I replace T with bool in the psalm-return, it understands bool|object).

I'll open an issue with PHPStorm after my vacation.

Do you think @return T|object is acceptable or not ? This annotation is not entirely true because it lacks precision, but it is not wrong because the method return either an instance of T or an object for which we cannot determine the type when the $class parameter is not provided.