Incorrect Generic Typing for ModelManagerInterface #8158
Labels
Comments
|
This was already discussed in #6721 and #7066 is technically not the proper way to get another entity because you cannot be sure the entity is managed by the same entityManager behind the modelManager implementation. I would recommend to use Also, a method-level template creates other issues as https://phpstan.org/r/f359d9d9-b98f-475e-9ec8-205070d14d0a. Also, changing this, if it was decided as wanted, would be too little gain for too much work and there is not enought maintainer/contributor. So I'll close as not planned. Thanks for your understanding. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Environment
Sonata packages
show
Symfony packages
show
PHP version
Subject
Summary
IDEs and static analyzers think that
$authorin this code is a Blog, not an Author:Detailed explanation
Sonata\AdminBundle\Model\ModelManagerInterfacedeclares a class-level generic typeT:This type is an entity class used to restrict various parameters and return values of the class's methods. It is set when an admin's type is specified like so:
...which specifies
Sonata\AdminBundle\DependencyInjection\Admin\TaggedAdminInterface's generic typeT, which is used in its definition of the return value ofgetModelManager():That class-level type
Tfor ModelManagerInterface is used for a number of methods, despite those methods being usable for any entity classes.For example, in an admin that
@extends AbstractAdmin<Blog>, calling$this->getModelManager()->findOneBy(Author::class, ['id' => 1])is expected to return aBlog, but of course it returns anAuthor.Minimal repository with the bug
https://github.com/amacrobert-meq/sonata-generic-type-example
Steps to reproduce
@extends AbstractAdmin<X>whereXis the entity the admin is for)$someOtherEntity = $this->getModelManager()->findOneBy(Y::class, []);whereYis an entity other than the admin's entityvendor/bin/phpstanExpected results
PHPStan resolves with no errors.
Actual results
PHPStan errors with the following:
Suggested solution
The generic type
Ton ModelManagerInterface should not be class-level, but method-level.For example, this change would let IDEs and static analyzers know that the return value of
findBy(A::class, [])is an array ofA, without restricting the entity that is passed tofindBy().executeQuery()because it is not passed a $class, but that's fine (and correct) because we do not know the query result type anyway.LockInterface, which are informed by the implementation ofModelManagerInterface.<T>from anywhereModelManager<T>is used inTaggedAdminInterface.TfromModelManagerInterface's implementations insonata-project/doctrine-orm-admin-bundleandsonata-project/doctrine-mongodb-admin-bundleThe text was updated successfully, but these errors were encountered: