phpstan / phpstan-doctrine

Doctrine extensions for PHPStan
MIT License
593 stars 97 forks source link

Repository type not matched when set to a property #21

Closed soullivaneuh closed 5 years ago

soullivaneuh commented 6 years ago

In a nutshell, this PHP class:

use Doctrine\ORM\Decorator\EntityManagerDecorator;
use PowerDNSBundle\Doctrine\ORM\PowerDNSEntityManager;
use PowerDNSBundle\Entity\PowerDNSDomain;

class DnsManager extends EntityManagerDecorator
{
    /**
     * @var ObjectRepository
     */
    private $domainRepository;

    public function __construct(PowerDNSEntityManager $wrapped)
    {
        parent::__construct($wrapped);

        $this->domainRepository = $this->getRepository(PowerDNSDomain::class);
    }

    public function addDomainIfNotExists($name, User $user, $ipV4 = null, $ipV6 = null, $checkDNS = true)
    {
        $nameCanonical = \idn_to_ascii($name);
        $domain = $this->domainRepository->findOneBy(['name' => $nameCanonical]);

        if (!$domain) {
            $domain = new PowerDNSDomain();
            $domain->setName($name);
            $domain->setOwnerId($user->getId());
            $this->saveDomain($domain, $ipV4, $ipV6, $checkDNS);
        }

        return $domain;
    }
}

Produces:

 ------ ---------------------------------------------------------------------- 
  Line   src/AppBundle/Manager/DnsManager.php                                  
 ------ ---------------------------------------------------------------------- 
  170    Method AppBundle\Manager\DnsManager::addDomainIfNotExists() should    
         return PowerDNSBundle\Entity\PowerDNSDomain but returns object.       

Replacing the line by $domain = $this->getRepository(PowerDNSDomain::class)->findOneBy(['name' => $nameCanonical]); directly "solve" the issue.

It seems the property assignation of repositories is not handled by the extension.

ondrejmirtes commented 6 years ago

This is currently not possible because there's no way how to write a @var above a property that would tell that there's EntityRepository<PowerDNSDomain>. That would require a lot of coordinated effort across phpstan/phpdoc-parser and phpstan/phpstan which is planned for the future but not on the immediate roadmap. cc @JanTvrdik thoughts?

JanTvrdik commented 6 years ago

Well, phpstan/phpstan uses DI, so in theory extensions can extend a LOT more stuff, than is officially documented, i.e. phpstan/phpstan-doctrine can decorate the TypeNodeResolver service to resolve EntityRepository<PowerDNSDomain> to EntityRepositoryType. No changes to phpstan/phpdoc-parser are required.

That being said, extending and decorating core phpstan services should probably not be encouraged.

ondrejmirtes commented 6 years ago

That's nice to know! I don't consider TypeNodeResolver as a public API with any BC incentives so I'd rather have an official extension point, but this can be used meanwhile...

lcobucci commented 6 years ago

@Soullivaneuh @ondrejmirtes it looks like this has already been fixed in newer versions of PHPStan (as proven by #37).

Edit: this comment is actually invalid, PHPStan is not validating the method calls as I expected.

ondrejmirtes commented 5 years ago

This is now supported, but you have to write @var ObjectRepository<PowerDNSDomain>.

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.