Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disallow method call with inheritance #269

Open
verfriemelt-dot-org opened this issue Oct 11, 2024 · 3 comments
Open

disallow method call with inheritance #269

verfriemelt-dot-org opened this issue Oct 11, 2024 · 3 comments

Comments

@verfriemelt-dot-org
Copy link

verfriemelt-dot-org commented Oct 11, 2024

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.

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

@spaze
Copy link
Owner

spaze commented Oct 11, 2024

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
Copy link
Author

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
Copy link
Owner

spaze commented Oct 12, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants