Wrong return type for UnitOfWork::getOriginalEntityData #6683

Closed
opened 2026-01-22 15:36:59 +01:00 by admin · 6 comments
Owner

Originally created by @NicoHaase on GitHub (Apr 7, 2021).

In #8521, a new return type for getOriginalEntityData has been introduced (see https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L2992). array<string, array<string, mixed>> does not look correct to me: when calling that method, it returns array<string, mixed>

Originally created by @NicoHaase on GitHub (Apr 7, 2021). In #8521, a new return type for `getOriginalEntityData` has been introduced (see https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L2992). `array<string, array<string, mixed>>` does not look correct to me: when calling that method, it returns `array<string, mixed>`
admin closed this issue 2026-01-22 15:36:59 +01:00
Author
Owner

@greg0ire commented on GitHub (Apr 7, 2021):

I think you are correct. Can you please send a PR against 2.8.x?

It looks like I have blindly copied the annotation for the property: 5ac036de02/lib/Doctrine/ORM/UnitOfWork.php (L149-L151)

@greg0ire commented on GitHub (Apr 7, 2021): I think you are correct. Can you please send a PR against 2.8.x? It looks like I have blindly copied the annotation for the property: https://github.com/doctrine/orm/blob/5ac036de0273c6fc819d1e1835ab4f548dc24727/lib/Doctrine/ORM/UnitOfWork.php#L149-L151
Author
Owner

@NicoHaase commented on GitHub (Apr 7, 2021):

@greg0ire I've linked a PR. What surprises me: I've adjusted the annotation, but nothing else. Is this such a special case that no code in the testsuite calls this method? I had expected that I also had to change other things anywhere else in the code

@NicoHaase commented on GitHub (Apr 7, 2021): @greg0ire I've linked a PR. What surprises me: I've adjusted the annotation, but nothing else. Is this such a special case that no code in the testsuite calls this method? I had expected that I also had to change other things anywhere else in the code
Author
Owner

@tjveldhuizen commented on GitHub (Apr 7, 2021):

The tests directory is not analyzed by PHPStan, and PHPUnit does not do anything with the annotations. So it doesn't surprise me that it was not discovered before.

@tjveldhuizen commented on GitHub (Apr 7, 2021): The tests directory is not analyzed by PHPStan, and PHPUnit does not do anything with the annotations. So it doesn't surprise me that it was not discovered before.
Author
Owner

@NicoHaase commented on GitHub (Apr 7, 2021):

The annotation I've changed is not part of any test directory

@NicoHaase commented on GitHub (Apr 7, 2021): The annotation I've changed is not part of any test directory
Author
Owner

@tjveldhuizen commented on GitHub (Apr 7, 2021):

But if the method where the annotation relates to is used in a test (which it doe, I assume), PHPStan would analyze it if it would analyse the tests.

@tjveldhuizen commented on GitHub (Apr 7, 2021): But if the method where the annotation relates to is used in a test (which it doe, I assume), PHPStan would analyze it if it would analyse the tests.
Author
Owner

@greg0ire commented on GitHub (Apr 7, 2021):

I think the actual reason is that we run Psalm and PHPstan at a too lenient level for them to detect this ATM:

2a87821b28/phpstan.neon (L2)

2a87821b28/psalm.xml (L3)

If you want to work on this, be my guest 🙂

@greg0ire commented on GitHub (Apr 7, 2021): I think the actual reason is that we run Psalm and PHPstan at a too lenient level for them to detect this ATM: https://github.com/doctrine/orm/blob/2a87821b28d017b1b718c481f39d95b32894aa47/phpstan.neon#L2 https://github.com/doctrine/orm/blob/2a87821b28d017b1b718c481f39d95b32894aa47/psalm.xml#L3 If you want to work on this, be my guest :slightly_smiling_face:
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6683