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

Doctrine\ORM\EntityRepositoy::count invalid type hint #7523

Closed
crtl opened this issue Dec 14, 2018 · 5 comments
Closed

Doctrine\ORM\EntityRepositoy::count invalid type hint #7523

crtl opened this issue Dec 14, 2018 · 5 comments

Comments

@crtl
Copy link

crtl commented Dec 14, 2018

Bug Report

Q A
BC Break no
Version 2.6.3

Summary

Doctrine\ORM\EntityRepository::count(array $criteria) should be Doctrine\ORM\EntityRepository::count($criteria), because it calls Doctrine\ORM\Persister\Entity\EntityPersister::count which accepts array|\Doctrine\Common\Collections\Criteria as argument.

@Ocramius
Copy link
Member

Changing the signature is not really feasible due to inheritance anymore. If you want to fix it, it should target 3.0.

@bednic
Copy link

bednic commented Apr 16, 2019

Due to what inheritance?! The method ::count is on EntityRepository and only there. So fix type signature is not hard thing, just remove array. I did it myself, but it's really annoying doing it on every project. EntityPersister::count() still isn't typed and is annotated accepting array or Criteria. Remove array type doesn't break anything.

@Tomsgu
Copy link

Tomsgu commented Apr 16, 2019

@bednic If anyone has extended EntityRepository, then it's actually a problem.

@bednic
Copy link

bednic commented Apr 16, 2019

@Tomsgu It shouldn't be, cause if u have right setup on environment, then warnings are suppresed. And non-matching method type signatures are warning, not error. But OK, this is "privilege" of PHP. So there is another solution, like add new method, ::countByCriteria which will accept Criteria object. Then everybody will be happy. But someone screw this up, so would be nice, if it get fixed. IMHO even in big project this fix is like findAll > replace ... Who actually has multiple Entity-Repositories in project where is overwriting base methods like count.

@Ocramius
Copy link
Member

Who actually has multiple Entity-Repositories in project where is overwriting base methods like count.

A lot of folks, which is why this is labeled as such.

Closing as per my initial labeling in #7523 (comment)

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

No branches or pull requests

4 participants