spaze / phpstan-disallowed-calls

PHPStan rules to detect disallowed method & function calls, constant, namespace, attribute & superglobal usages
MIT License
263 stars 17 forks source link

disallow method call with inheritance #269

Open verfriemelt-dot-org opened 1 month ago

verfriemelt-dot-org commented 1 month ago

given a doctrine repository which looks like this:

/**
 * @extends ServiceEntityRepository<Version>
 *
 * @method Version|null find($id, $lockMode = null, $lockVersion = null)
 * @method Version|null findOneBy(array $criteria, array $orderBy = null)
 * @method Version[]    findAll()
 * @method Version[]    findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
class VersionRepository extends ServiceEntityRepository
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, Version::class);
    }
}

i want to disallow magic methods like $repo->findOneBy([ ... ]) which would work with:

  disallowedMethodCalls:
        -
            method: 'Doctrine\Persistence\ObjectRepository::find*()'
            message: 'to not use magic find*() methods'
            errorTip: 'add method to interface instead!'
            allowIn:
                - src/Repository/*
                - tests/*

but this will also disallow the following method:

class VersionRepository extends ServiceEntityRepository
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, Version::class);
    }

    public function findOneByNumber(string $versionNumber): ?Version
    {
      return $this->createQueryBuilder('v')
            ->andWhere('v.number = :val')
            ->setParameter('val', $versionNumber)
            ->getQuery()
            ->getOneOrNullResult();
    }
}
Calling Doctrine\Persistence\ObjectRepository::findOneByNumber() (as App\Repository\VersionRepository::findOneByNumber()) is forbidden, to not use magic find*() methods.

:thinking: is there an option or could an option be introduced, to limit the scope to only the parent class for that check?

spaze commented 1 month ago

There isn't an option to disable checking methods from child classes, and I'm not sure I'd like to add one as that may be easy to miss. Maybe you could list all the find methods instead of find*()? Or would directives like exclude or definedIn help you?

verfriemelt-dot-org commented 1 month ago

i had no luck with that so far, maybe i use it wrong?

here is a simple example illustrating the problem:

https://github.com/verfriemelt-dot-org/phpstan-disallowed-calls-example

spaze commented 1 month ago

Thanks for the example repo. I get this when analyzing it

 ------ ------------------------------------------------------------------------------------------------------------
  Line   Example.php
 ------ ------------------------------------------------------------------------------------------------------------
  21     Calling Doctrine\Persistence\ObjectRepository::findBy() (as App\Repository\VersionRepository::findBy()) is
         forbidden, to not use magic find*() methods.
         💡 add method to interface instead!
 ------ ------------------------------------------------------------------------------------------------------------

What do you expect PHPStan to report in the repo?