-
Notifications
You must be signed in to change notification settings - Fork 96
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
New rule: forbid replacing a persistent Doctrine collection field instead of mutating the collection #505
Comments
Hi @stof, just found this issue and I had some questions. If I understand correctly
shouldn't be implemented this way in an entity with a ToMany relation. How would you write it then ? Does it mean we have to write:
Or I misunderstood ? |
@VincentLanglet your suggested code would indeed work (modulo the wrong variable name used in the snippet, which would be caught by phpstan), but would still not be optimal if some items are both in the existing collection and the new one as you would still delete them from the join table for clearing and then adding them again. |
When a field is a toMany relation, Doctrine relies on a special Collection implementation to track changes.
A common mistake is a write a setter that replaces the collection entirely instead of writing into the existing collection. This will make things that your collection field went from an empty list to its new content when checking for changes (as it lost all tracking for the existing content), which will do very bad things for the database data (in the best case, it triggers an error due to duplicate data in a unique index. In the worse case, it silently corrupts your data).
It would be great if phpstan-doctrine could prevent such mistakes for people using it.
The text was updated successfully, but these errors were encountered: