Throw an exception when passing an entity alias#295
Conversation
| $this->initialize(); | ||
| } | ||
|
|
||
| if (class_exists($className, false) && (new ReflectionClass($className))->isAnonymous()) { |
There was a problem hiding this comment.
could this be on the hot path? strpos for @ should be enough to check for anonymous classes if yes
There was a problem hiding this comment.
That's the same check than done in getMetadataFor already.
Many users have missed the deprecation about using an entity alias when they migrated to doctrine/persistence 3. Passing an entity alias was triggering a weird error due to generating an invalid PSR-6 cache key due to `:` being a reserved keyword. So this adds a dedicated check reporting that it is an invalid class name instead.
|
Since this is an improvement, should it target 3.1.x? |
It's not a feature though :) It's literally an improvement so breaking change introduced in 3.x is visible to endusers |
|
To me, improvements and features are both minor changes; I don't think an improvement qualifies as a patch. Semver mentions "improvements" in the "minor" section also. |
|
Well, I'm fine targetting 3.1.x if 3.1.0 is released soon. But I'd like for this error message to reach the users ASAP so that they stop reporting issues on Symfony saying that cache keys are invalid. |
|
I don't see an issue with releasing it right after merging this. IMO we should try to stick with semver since we don't have constraints on our release schedule. Every time I break something as part of a patch release when it should have gone in a minor mine, there's always some user who makes sure to point that out. |
|
We also have a bugfix to release and releasing both PRs as 3.0.2 sounds like less work that 3.0.2 and later 3.1.0 with just a friendly exception :) But that's just how I'd proceed (feel free to "blame me" if we do it this way), do as you feel is right :) |
|
In the end, I agree that it's not a huge deal, and I take good note that you will be the scapegoat for this time 😜 Let's not make this more complicated that it needs to be. |
|
Thanks @stof! |
5 participants
Many users have missed the deprecation about using an entity alias when they migrated to doctrine/persistence 3.
Passing an entity alias was triggering a weird error due to generating an invalid PSR-6 cache key due to
:being a reserved keyword. So this adds a dedicated check reporting that it is an invalid class name instead.