Named Arguments of Filters and Functions are not passed as-is and forced to snake_case

[filecage]

Problem Description

I stumbled upon this super odd behaviour when implementing a custom function in Twig and realised that given the template

{{ foo(barValue='bar', booValue='boo') }}

the two named arguments, barValue and booValue will be passed to the foo function as bar_value and boo_value. This is due to the "name normalization" done in the CallExpression on line 242 in the current 3.x branch. This has been introduced in a59dcde, around 8 years ago and was part of the initial named arguments implementation.

As this behaviour is undocumented, technically unnecessary (named arguments are passed as array keys and PHP allows all strings to be array keys) and defnitely unexpected, I'm suggesting to classifiy this as a bug.

Proposed Fix

To fix it without introducing backwards incompatible changes, I'm proposing a new option for functions and filters that is called legacy_named_arguments and that defaults to true in 3.x and can be set to false in upcoming major releases.

• Setting legacy_named_arguments to true means: keep the current behaviour. In my opinion it makes sense to trigger a E_USER_DEPRECATED when a variable key has been "normalized" (i. e. input key !== output key). However, this might have a very big impact on a lot of existing Symfony installations.

• Setting legacy_named_arguments to false means: leave the input argument as-is. The lexer decides whether a variable name is valid or not (which it already does implicitly here).

I am happy to contribute in a PR once you've confirmed the issue.

[weaverryan]

This looks like a legit issue to me. We were trying to create a variadic Twig function, but apparently, when we pass in the named args, they are snake-cased, as you mentioned here. Your resolution sounds very reasonable, except that I'd probably skip the deprecation. The reason is just because (as you realized also) requiring everyone to set legacy_named_arguments => true to skip the deprecation would be a huge pain. But if simply have a way to opt-intoo" normal, non-snaked-cased args, then that would be enough.

Cheers!

[iquito]

Why not keep the old syntax as it is ({{ foo(barValue='bar', booValue='boo') }} will continue to be snakecased) and change the behavior for a syntax that is nearer to PHP and how named arguments look like there ({{ foo(barValue: 'bar', booValue: 'boo') }}).

A colon within an argument list is currently a syntax error in Twig, so as far as I can tell this should not lead to any problems or syntax ambiguities. It would also be in line with the array syntax in Twig, which already looks like the PHP named argument syntax (example: {key: 'value', otherkey: 'othervalue'}).

[kbond]

Why not keep the old syntax as it is ({{ foo(barValue='bar', booValue='boo') }} will continue to be snakecased) and change the behavior for a syntax that is nearer to PHP and how named arguments look like there ({{ foo(barValue: 'bar', booValue: 'boo') }}).

This seems like the best solution - we can then completely deprecate the old syntax.

[smnandre]

Handling : instead of = would be done in the ExpressionParser i guess, and i'm not sure how to "link" that with the snake_case transformation.

But i 100% vote for having a way to not snake_case every time.

[filecage]

Personally, I would also advise against mixing up the introduction of a (possibly) better syntax and implicitly providing a behaviour fix for this issue. It's also a misleading idea that this is more convenient than deprecating the current behaviour with a deprecation warning just because the former is less transparent about that change. Transparency should be a target, not a burden.