-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PropertyAccess] fix adder/setter priority #29350
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
Conversation
karser
commented
Nov 27, 2018
Q | A |
---|---|
Branch? | 3.4 |
Bug fix? | yes |
New feature? | no |
BC breaks? | it fixes BC break introduced by #28966 |
Deprecations? | no |
Tests pass? | no, so far it is a reproducer |
Fixed tickets | #29340 |
License | MIT |
I reverted (locally) changes from #28966 and I'm trying to fix it differently. @xabbuh indeed, the problem is in I added a check if the property name plural. So it uses adder/remover only if the property name is actual plural. That is - @xabbuh @nicolas-grekas makes sense? |
my idea to fix the issue would be #29355 |
@xabbuh I see - it shouldn't cache the What do you think of this approach?
|
waiting for symfony/symfony#29350
waiting for symfony/symfony#29350
waiting for symfony/symfony#29350
9db236e
to
ea447cb
Compare
@@ -707,6 +708,16 @@ private function getWriteAccessInfo($class, $property, $value) | |||
$camelized = $this->camelize($property); | |||
$singulars = (array) Inflector::singularize($camelized); | |||
|
|||
if ($useAdderAndRemover && !\in_array($camelized, $singulars)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, this does not work for words where singular and plural are the same (aircraft for example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the added test in #29355
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there is a lot of exceptions That are Both Plural and Singular. Although I'm still confused why an empty array is passed to getWriteAccessInfo()
. Just to implicitly allow adder/remover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably something like that, though I don't know for sure. Maybe the git history reveals some more background information.
Closing in favor of #29355, thanks for your help! |