Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PropertyInfo][ReflectionExtractor] Check the array mutator prefixes last when the property is singular#36305
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…last when the property is singular
| */ | ||
| private$arrayMutatorPrefixes; | ||
| private$arrayMutatorPrefixesFirst; |
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.
We don't need these new class properties if we do some refacto on the existing $mutatorPrefixes and $arrayMutatorPrefixes ones. Basically, currently, $mutatorPrefixes includes the values of $arrayMutatorPrefixes. We could separate the values and join them when we need them.
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.
As these properties are marked@internal, doing the refactoring is ok. I'm in favor for the refactor if it simplifies the code.
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.
the refacto should target master
| $ucSingulars = (array) Inflector::singularize($ucProperty); | ||
| foreach ($this->mutatorPrefixesas$prefix) { | ||
| $mutatorPrefixes =\in_array($ucProperty,$ucSingulars,true) ?$this->arrayMutatorPrefixesLast :$this->arrayMutatorPrefixesFirst; |
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.
If the the property singulars contains the property itself, we can assume the property is singular isn'it?
nicolas-grekas commentedApr 6, 2020
Thank you@fancyweb. |
nicolas-grekas commentedApr 6, 2020
@fancyweb now merged up to master, refacto unlocked :) |
Check the related tickets that have a very descriptive example.
If the property is singular, we should prioritize non array mutator prefixes and do the opposite for plural property. It relies on some guessing but it actually fixes real world scenarios.