[Inflector][String] Move Inflector in String

[fancyweb]

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	yes
Tickets	https://github.com/orgs/symfony/projects/1#card-30499514
License	MIT
Doc PR	-

Needs #35091.

Should we have a standalone inflector (like the Slugger) or 2 new methods (pluralize and singularize) on the AbstractString class? I implemented both but since we only handle English I finally preferred the first one.

TODO (after the "move" is OK):

• Deprecate the Inflector component
• Use the String inflector in Symfony's code

[fabpot]

I like the idea

[nicolas-grekas]

Me too :)

standalone inflector (like the Slugger)

I think this is the way to go personally.

[stof]

should we really remove any API dealing with string direclty like in the existing component ?

Also, we cannot deprecate a stable component in favor of an experimental one IMO.

[stof]

• Deprecate the Inflector component
• Use the String inflector in Symfony's code

Note that these 2 TODOs look impossible to me as long as String is experimental (we should not make PropertyAccess and PropertyInfo stable components depend on an experimental one.

Also, given the performance sensitivity of these 2 components, I would like to see benchmarks of using your new Inflector vs using the existing one relying on PHP strings.
As all the English inflection rules are pure ASCII, I don't see any benefit for using the String abstraction (which probably adds overhead compared to native strings).

Also, what is the migration plan ? The current Inflector has a non-configurable static API. Your new inflector would require injecting an inflector in PropertyAccess and PropertyInfo. But changing the inflection rules used to access properties can become quite unexpected (if your project changes them, it might break all forms shipped by bundles where they expect PropertyAccess to work based on the core rules).

[nicolas-grekas]

Note that these 2 TODOs look impossible to me as long as String is experimental

true, and there is no reason to make the component experimental anymore to me.

Also, given the performance sensitivity of these 2 components, I would like to see benchmarks of using your new Inflector vs using the existing one relying on PHP strings.

the implementation should use native string functionsn, that would fix most of the perf considerations.

Also, what is the migration plan ? The current Inflector has a non-configurable static API. Your new inflector would require injecting an inflector in PropertyAccess and PropertyInfo. But changing the inflection rules used to access properties can become quite unexpected (if your project changes them, it might break all forms shipped by bundles where they expect PropertyAccess to work based on the core rules).

This happens with all services, so we don't have to care much here. Also the current consumers would just hardcode instantiating the new inflector if we really don't want that to ever happen.

[fancyweb]

Also, given the performance sensitivity of these 2 components, I would like to see benchmarks of using your new Inflector vs using the existing one relying on PHP strings.

From my benchmark, using the String component makes singularize between 5 to 7 times slower.

As all the English inflection rules are pure ASCII, I don't see any benefit for using the String abstraction (which probably adds overhead compared to native strings).

the implementation should use native string functionsn, that would fix most of the perf considerations.

Since what we have is an English inflector, do you mean we should directly return the passed string suffixed with s (for pluralize as example) when it does not match [a-zA-Z]+ and keep the current code (using native string functions) for the rest?

[fancyweb]

Basically I copy pasted the whole code and I only changed return $string to return [$string] to always return an array.

[fancyweb]

I reworked on this with a simple approach. We just copy paste the existing code in the String component. We add an interface. The only code change we make is that singularize and pluralize always return an array of strings.

I deprecated the Inflector component. The deprecated Inflector component uses the String component.

The PropertyInfo ReflectionExtractor class can now receive an InflectorInterface in its constructor. By default, it instantiates an EnglishInflector so there is no BC break.

I guess we don't need to make this more complicated.

Thanks to @nicolas-grekas for his suggestions btw.

[nicolas-grekas]

LGTM, just minor details.

[fabpot]

Thank you @fancyweb.