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.

Sign up for GitHub

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

Jump to bottom

Repository methods considered pure #550

Open
derrabus opened this issue Feb 27, 2024 · 4 comments · May be fixed by #648
Open

Repository methods considered pure #550

derrabus opened this issue Feb 27, 2024 · 4 comments · May be fixed by #648

Comments

@derrabus
Copy link

PHPStan 1.10.59, phpstan-doctrine 1.3.62, phpstan-phpunit 1.3.16, bleeding edge + strict rules

I have a piece of code in my tests that roughly looks like this:

self::assertNull($userRepository->findOneBy(['email' => 'foo@example.com']));

// Calls to the class I'm testing that should lead to new database records.

$user = $userRepository->findOneBy(['email' => 'foo@example.com']);
self::assertInstanceOf(User::class, $staff2);

PHPStan emits an error:

Call to static method PHPUnit\Framework\Assert::assertInstanceOf() with 'App\Entity\User' and null will always evaluate to false.

If I remove the first assertNull() call, the error is gone. Apparently, PHPStan treats the findOneBy() call as pure which is a mistake from my POV.
@nCrazed
Copy link

nCrazed commented Mar 13, 2025

Based on this issue for phpstan it self: phpstan/phpstan#5727

Could the fix be as simple as adding @phpstan-impure annotations to EntityRepository.stub?

@nCrazed
Copy link

nCrazed commented Mar 13, 2025 •
edited
Loading

Can confirm that copying over and enabling the stub in my project then adding the annotation fixes the problem for me.

But it does introduce a new error 😢:

  Line   stubs/EntityRepository.stub
 ------ --------------------------------------------------------------
  12     Class Doctrine\ORM\EntityRepository declared multiple times:

Happy to submit a PR if there no objections to the approach

nCrazed added a commit to nCrazed/phpstan-doctrine that referenced this issue Mar 13, 2025
@nCrazed
Mark nullable EntityRepository methods as impure
a1d55bb 
The result of these functions depends on the underlying database state
which can change between multiple invocations. For example:

Calling find() or findOneBy() and asserting the result to be null as a
precondition of a test will generate false errors later in the test when the
underlying record is added and the method is called again

See phpstan#550
@nCrazed nCrazed linked a pull request Mar 13, 2025 that will close this issue
Open
@ruudk
Copy link
Contributor

ruudk commented Mar 24, 2025

I have the same issue, leading me to believe that all methods on the repository should be considered impure.

@nCrazed
Copy link

nCrazed commented Mar 24, 2025

@ruudk which methods in particular are you seeing this issue with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Mark nullable EntityRepository methods as impure nCrazed/phpstan-doctrine
3 participants
@ruudk @nCrazed @derrabus