phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.96k forks source link

[BUG]: Redundant generic return annotations #16661

Open ZanMaticPratnemer opened 2 weeks ago

ZanMaticPratnemer commented 2 weeks ago

Describe the bug Return annotations for some methods in the Model class say that the method returns T|ModelInterface<T>, where T is defined as @template T of static so the same as @template T of Model. Since Model implements ModelInterface, the return type of ModelInterface<T> is redundant and can cause problems with static analyzers if you were to try to call a method implemented in the child class of Model on the return value from methods that have this redundant type specified - for example findFirst().

To Reproduce In the base model class of my application:

    /**
     * @return null|T
     */
    public static function get(string $id)
    {
        $ret = static::findFirst([
            'id = :id:',
            'bind' => [
                'id' => $id,
            ],
        ]);

        if ($ret instanceof Row)
        {
            throw new \LogicException("get() found multiple entries for id {$id} on model " . static::class, 500);
        }

        return $ret;
    }

image Now I could specify that the get() method returns null|T|ModelInterface<T>, but that causes problems down the line, when I try to call model specific methods on the result of get().

This is just an example for the findFirst() method, but couldn't every method that returns a ModelInterface instance be annotated to return T instead since T in this scope always implements ModelInterface?

I think that the correct annotation for this specific method should be the following:

/**
 * @return T|\Phalcon\Mvc\Model\Row|null
 */

Expected behavior There should be no static analysis error in this code.

Details