doctrine/archived-orm
1
0
Fork 0
mirror of https://github.com/doctrine/orm.git synced 2026-03-24 06:52:09 +01:00
Code Issues 5.3k Packages Projects Releases 100 Wiki Activity

To make assert($mapping instanceof ManyToManyOwningSideMapping) error more explicit #7493

New Issue
Open
opened 2026-01-22 15:52:18 +01:00 by admin · 10 comments
admin commented 2026-01-22 15:52:18 +01:00
Owner
Copy Link

Originally created by @olinox14 on GitHub (Mar 27, 2025).

Feature Request

replacing
4baa7bd252/src/Mapping/AssociationMapping.php (L138)

by something like :

if (!$mapping instanceof ManyToManyOwningSideMapping) {
    throw new \Exception(
        "Mapping error on field '" .
        $mapping->fieldName . '[' . $mapping->sourceEntity . ']' .
        "': joinTable can only be set on many-to-many owning side.");
}

Would ease the debugging of this error, as today, the assertion error does not provide any information about the cause of the error.

Originally created by @olinox14 on GitHub (Mar 27, 2025). ### Feature Request replacing https://github.com/doctrine/orm/blob/4baa7bd25218f363dc18286a9e1d32e2148b7fee/src/Mapping/AssociationMapping.php#L138 by something like : if (!$mapping instanceof ManyToManyOwningSideMapping) { throw new \Exception( "Mapping error on field '" . $mapping->fieldName . '[' . $mapping->sourceEntity . ']' . "': joinTable can only be set on many-to-many owning side."); } Would ease the debugging of this error, as today, the assertion error does not provide any information about the cause of the error.
admin commented 2026-01-22 15:52:19 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Mar 27, 2025):

Can you send a PR? If you do, read https://github.com/doctrine/orm/issues/11208 to select the right branch.

@greg0ire commented on GitHub (Mar 27, 2025): Can you send a PR? If you do, read https://github.com/doctrine/orm/issues/11208 to select the right branch.
admin commented 2026-01-22 15:52:20 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Mar 27, 2025):

Also, maybe it makes sense to do something similar for 4baa7bd252/src/Mapping/AssociationMapping.php (L132)

@greg0ire commented on GitHub (Mar 27, 2025): Also, maybe it makes sense to do something similar for https://github.com/doctrine/orm/blob/4baa7bd25218f363dc18286a9e1d32e2148b7fee/src/Mapping/AssociationMapping.php#L132
admin commented 2026-01-22 15:52:20 +01:00
Author
Owner
Copy Link

@olinox14 commented on GitHub (Mar 29, 2025):

Done, here is the PR : https://github.com/doctrine/orm/pull/11896

@olinox14 commented on GitHub (Mar 29, 2025): Done, here is the PR : https://github.com/doctrine/orm/pull/11896
admin commented 2026-01-22 15:52:20 +01:00
Author
Owner
Copy Link

@beberlei commented on GitHub (Mar 29, 2025):

Does this even happen? The assert seems to be to make ststic analysis happy

@beberlei commented on GitHub (Mar 29, 2025): Does this even happen? The assert seems to be to make ststic analysis happy
admin commented 2026-01-22 15:52:21 +01:00
Author
Owner
Copy Link

@olinox14 commented on GitHub (Mar 29, 2025):

@beberlei I'm not sure about the static analysis, but it would certainly be more dev-friendly. In the current state, the dev only receives an AssertionError stating :

assert($mapping instanceof ManyToManyOwningSideMapping)

with no information about which relation is at fault or why. An explicit exception describing the origin of the problem and suggesting a solution would save time and trouble for the user.

Before :

assert($mapping instanceof ManyToManyOwningSideMapping)

After :

Mapping error on field 'myField [App\Entity\MyEntity]': joinTable can only be set on many-to-many owning side.
@olinox14 commented on GitHub (Mar 29, 2025): @beberlei I'm not sure about the static analysis, but it would certainly be more dev-friendly. In the current state, the dev only receives an AssertionError stating : assert($mapping instanceof ManyToManyOwningSideMapping) with no information about which relation is at fault or why. An explicit exception describing the origin of the problem and suggesting a solution would save time and trouble for the user. Before : assert($mapping instanceof ManyToManyOwningSideMapping) After : Mapping error on field 'myField [App\Entity\MyEntity]': joinTable can only be set on many-to-many owning side.
admin commented 2026-01-22 15:52:21 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Mar 29, 2025):

@olinox14 given your phrasing, I assumed this happened to you in real life. Is that really the case?

@greg0ire commented on GitHub (Mar 29, 2025): @olinox14 given your phrasing, I assumed this happened to you in real life. Is that really the case?
admin commented 2026-01-22 15:52:21 +01:00
Author
Owner
Copy Link

@olinox14 commented on GitHub (Mar 29, 2025):

@greg0ire Yeah, you're right :)

I was upgrading a legacy project, with hundreds of relations, and encountered this assertion error. I had to resort to using xdebug to find the culprit, hence the issue.

@olinox14 commented on GitHub (Mar 29, 2025): @greg0ire Yeah, you're right :) I was upgrading a legacy project, with hundreds of relations, and encountered this assertion error. I had to resort to using xdebug to find the culprit, hence the issue.
admin commented 2026-01-22 15:52:21 +01:00
Author
Owner
Copy Link

@olinox14 commented on GitHub (Mar 29, 2025):

@beberlei Oh right, sorry, I just realized what you meant to say, that these asserts were to quiet phpstan or whatever to complaining. Sorry for the hasty reply. Well, like I was saying previously, you can run into this assertion error if you misplace the JoinTable attribute.

@olinox14 commented on GitHub (Mar 29, 2025): @beberlei Oh right, sorry, I just realized what you meant to say, that these asserts were to quiet phpstan or whatever to complaining. Sorry for the hasty reply. Well, like I was saying previously, you can run into this assertion error if you misplace the JoinTable attribute.
admin commented 2026-01-22 15:52:23 +01:00
Author
Owner
Copy Link

@greg0ire commented on GitHub (Mar 30, 2025):

Maybe you could add a test showing exactly that situation in your PR

@greg0ire commented on GitHub (Mar 30, 2025): Maybe you could add a test showing exactly that situation in your PR
admin commented 2026-01-22 15:52:24 +01:00
Author
Owner
Copy Link

@olinox14 commented on GitHub (Mar 31, 2025):

Done, I've added two unit tests for each of the new exceptions, just waiting for the CI to run

@olinox14 commented on GitHub (Mar 31, 2025): Done, I've added two unit tests for each of the new exceptions, just waiting for the CI to run
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
BC Break
Bug
Can't Fix
Deprecation
Documentation
DQL
Duplicate
Failing Test
Hacktoberfest
Improvement
Improvement
Incomplete
Invalid
Missing Tests
New Feature
PR Created
pull-request
Mirrored from GitHub Pull Request
 Question
Regression
Status: Waiting feedback
Waiting feedback
WIP
Won't Fix
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
admin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7493
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?