orm icon indicating copy to clipboard operation
orm copied to clipboard

🐛 Possible incorrect phpDoc in EntityProviderInterface

Open KorDum opened this issue 2 years ago • 2 comments

No duplicates 🥲.

  • [X] I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

There is this code in the Cycle\ORM\Service\EntityProviderInterface:

interface EntityProviderInterface
{
    /**
     * Get/load entity by unique key/value pair.
     *
     * @template TEntity
     *
     * @param class-string<TEntity>|string $role Entity role or class name.
     * @param array $scope KV pair to locate the model, currently only support one pair.
     *
     * @psalm-return ($role is class-string ? TEntity : object)|null
     */
    public function get(string $role, array $scope, bool $load = true): ?object;
}

But phpstan doesn't understand correct because @param class-string<TEntity>|string $role

Correct example: https://phpstan.org/r/0eea0c19-41b8-46a0-bbab-caaeb6ae3a76

Hovewer Psalm requires just such a phpDocas it already in code. Or is there something I don't know.

Now I am getting the following error in phpstan:

Method Foo::get() should return Bar but returns object.

Version

ORM 2.3.4
PHP 8.2

KorDum avatar Sep 15 '23 12:09 KorDum

I agree. It may be replaced with

@return ($role is class-string<TEntity> ? TEntity|null : object|null)

Feel free to make a PR. Or it may do @msmakouz but later.

roxblnfk avatar Sep 15 '23 18:09 roxblnfk

It seems that this phpDoc is also not suitable. You can check - now both static analyzers produce something crazy.

See example (from real code):

Psalm: ERROR: NoValue - app/Model/Notification/Entity/NotificationRepository.php:40:9 - All possible types for this return were invalidated - This may be dead code (see https://psalm.dev/179) return $entity ?? throw new DomainException(); ERROR: DocblockTypeContradiction - app/Model/Notification/Entity/NotificationRepository.php:40:16 - Docblock-defined type null for $entity is always null (see https://psalm.dev/155) return $entity ?? throw new DomainException();

PhpStan: Method App\Model\Notification\Entity\NotificationRepository::get() should return App\Model\Notification\Entity\Notification but returns object.

KorDum avatar Sep 16 '23 08:09 KorDum