[PropertyInfo][ReflectionExtractor] Check the array mutator prefixes last when the property is singular #36305
Conversation
…last when the property is singular
| @@ -61,6 +61,9 @@ class ReflectionExtractor implements PropertyListExtractorInterface, PropertyTyp | |||
| */ | |||
| private $arrayMutatorPrefixes; | |||
|
|
|||
| private $arrayMutatorPrefixesFirst; | |||
There was a problem hiding this comment.
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.
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.
the refacto should target master
| @@ -330,7 +336,9 @@ private function getMutatorMethod($class, $property) | |||
| $ucProperty = ucfirst($property); | |||
| $ucSingulars = (array) Inflector::singularize($ucProperty); | |||
|
|
|||
| foreach ($this->mutatorPrefixes as $prefix) { | |||
| $mutatorPrefixes = \in_array($ucProperty, $ucSingulars, true) ? $this->arrayMutatorPrefixesLast : $this->arrayMutatorPrefixesFirst; | |||
There was a problem hiding this comment.
If the the property singulars contains the property itself, we can assume the property is singular isn'it?
|
Thank you @fancyweb. |
|
@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.