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

[olinox14]

Feature Request

replacing

orm/src/Mapping/AssociationMapping.php

Line 138 in 4baa7bd

assert($mapping instanceof ManyToManyOwningSideMapping);

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.

[greg0ire]

Can you send a PR? If you do, read #11208 to select the right branch.

[greg0ire]

Also, maybe it makes sense to do something similar for

orm/src/Mapping/AssociationMapping.php

Line 132 in 4baa7bd

assert($mapping instanceof ManyToManyAssociationMapping);

[olinox14]

Done, here is the PR : #11896

[beberlei]

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

[olinox14]

@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.

[greg0ire]

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

[olinox14]

@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]

@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.

[greg0ire]

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

[olinox14]

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