Repository: twigphp/Twig (php)

Issue:
attribute() not looking for camelCase method
Hi,
I think it's a sort of bug, but it may be classified as a whish.

I'm using Symfony. I've added the non public attribute $event_login to an entity class. 
I meet a twig error when using {{ attribute(item, field) }} : Neither the property "event_login" nor one of the methods "event_login()", "getevent_login()"/"isevent_login()"/"hasevent_login()" or "__call()" exist and have public access in class "App\Entity\Event".
So I add the getevent_login() getter ...
Then I meet Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException : Can't get a way to read the property "event_login" in class "App\Entity\Event".
So I add the getEventLogin() getter ...

Why cannot they use the same getter ?

After a few exchanges, man states that a method should always been camelCase() (see https://www.php-fig.org/psr/psr-1/ ).
Note that using underscores in attributes is allowed.
That's why PropertyAccess use getEventLogin() ... 

So, why does the twig attribute() function use getevent_login() instead of getEventLogin() ?

Should not it at least also check for getEventLogin() existence ?

Regards

Pull request:
Allow to use a dynamic attribute on the . operator via ()
Closes #3598

As one can think that the `attribute` function has some special power compared to the `.` operator (like using Symfony PropertyAccess), I think it's better to use the `.` operator for dynamic attributes as well.

Using `()` makes it clear that the behavior is the same as the `.` operator (using `()` is similar to the way one can use dynamic mapping names (like in `{ (name): value }`).


Patch:
diff --git a/CHANGELOG b/CHANGELOG
index ec47be48..ad3841a5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,6 @@
 # 3.15.0 (2024-XX-XX)
 
+ * Deprecate the `attribute` function; use the `.` notation and wrap the name with parenthesis instead
  * Add support for argument unpackaging
  * Add JSON support for the file extension escaping strategy
  * Support Markup instances (and any other \Stringable) as dynamic mapping keys
diff --git a/doc/deprecated.rst b/doc/deprecated.rst
index 599f8d8f..a648b054 100644
--- a/doc/deprecated.rst
+++ b/doc/deprecated.rst
@@ -8,8 +8,25 @@ feature that was deprecated in Twig 3.x is removed in Twig 4.0).
 Functions
 ---------
 
- * The ``twig_test_iterable`` function is deprecated; use the native PHP
-   ``is_iterable`` function instead.
+* The ``twig_test_iterable`` function is deprecated; use the native PHP
+  ``is_iterable`` function instead.
+
+* The ``attribute`` function is deprecated as of Twig 3.15. Use the ``.``
+  operator instead and wrap the name with parenthesis:
+
+  .. code-block:: twig
+
+    {# before #}
+    {{ attribute(object, method) }}
+    {{ attribute(object, method, arguments) }}
+    {{ attribute(array, item) }}
+
+    {# after #}
+    {{ object.(method) }}
+    {{ object.(method)(arguments) }}
+    {{ array[item] }}
+
+  Note that it won't be removed in 4.0 to allow a smoother upgrade path.
 
 Extensions
 ----------
diff --git a/doc/functions/attribute.rst b/doc/functions/attribute.rst
index 96ec6ea1..7757fa08 100644
--- a/doc/functions/attribute.rst
+++ b/doc/functions/attribute.rst
@@ -1,6 +1,15 @@
 ``attribute``
 =============
 
+.. warning::
+
+    The ``attribute`` filter is deprecated as of Twig 3.15. Use the
+    :ref:`dot operator <dot_operator>` that now accepts any expression
+    when wrapped with parenthesis.
+
+    Note that this filter will still be available in Twig 4.0 to allow a
+    smoother upgrade path.
+
 The ``attribute`` function can be used to access a "dynamic" attribute of a
 variable:
 
diff --git a/doc/templates.rst b/doc/templates.rst
index e1c637ae..f84c4163 100644
--- a/doc/templates.rst
+++ b/doc/templates.rst
@@ -774,6 +774,27 @@ The following operators don't fit into any of the other categories:
 
       {{ user.name }}
 
+  After the ``.``, you can use any expression by wrapping it with parenthesis
+  ``()``.
+
+  One use case is when the attribute contains special characters (like ``-``
+  that would be interpreted as the minus operator):
+
+  .. code-block:: twig
+
+      {# equivalent to the non-working user.first-name #}
+      {{ user.('first-name') }}
+
+  Another use case is when the attribute is "dynamic" (defined via a variable):
+
+  .. code-block:: twig
+
+      {{ user.(name) }}
+      {{ user.('get' ~ name) }}
+
+  Before Twig 3.15, use the :doc:`attribute <functions/attribute>` function
+  instead for the two previous use cases.
+
   .. sidebar:: PHP Implementation
 
       To resolve ``user.name`` to a PHP call, Twig uses the following algorithm
@@ -803,20 +824,6 @@ The following operators don't fit into any of the other categories:
       * if not, and if ``strict_variables`` is ``false``, return ``null``;
       * if not, throw an exception.
 
-  .. note::
-
-      If you want to access a dynamic attribute of a variable, use the
-      :doc:`attribute<functions/attribute>` function instead.
-
-      The ``attribute`` function is also useful when the attribute contains
-      special characters (like ``-`` that would be interpreted as the minus
-      operator):
-
-      .. code-block:: twig
-
-          {# equivalent to the non-working user.first-name #}
-          {{ attribute(user, 'first-name') }}
-
 * ``?:``: The ternary operator:
 
   .. code-block:: twig
diff --git a/src/ExpressionParser.php b/src/ExpressionParser.php
index 37e94c6b..07786eea 100644
--- a/src/ExpressionParser.php
+++ b/src/ExpressionParser.php
@@ -492,6 +492,18 @@ class ExpressionParser
         $arguments = new ArrayExpression([], $lineno);
         $type = Template::ANY_CALL;
         if ('.' == $token->getValue()) {
+            if ($stream->nextIf(Token::PUNCTUATION_TYPE, '(')) {
+                $arg = $this->parseExpression();
+                $stream->expect(Token::PUNCTUATION_TYPE, ')');
+                if ($stream->test(Token::PUNCTUATION_TYPE, '(')) {
+                    $type = Template::METHOD_CALL;
+                    foreach ($this->parseArguments() as $n) {
+                        $arguments->addElement($n);
+                    }
+                }
+
+                return new GetAttrExpression($node, $arg, $arguments, $type, $lineno);
+            }
             $token = $stream->next();
             if (
                 Token::NAME_TYPE == $token->getType()
diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php
index b40d41ff..e0e6ed5d 100644
--- a/src/Extension/CoreExtension.php
+++ b/src/Extension/CoreExtension.php
@@ -1966,6 +1966,15 @@ final class CoreExtension extends AbstractExtension
         $fakeFunction = new TwigFunction('attribute', fn ($variable, $attribute, $arguments = null) => null);
         $args = (new CallableArgumentsExtractor($fakeNode, $fakeFunction))->extractArguments($args);
 
+        /*
+        Deprecation to uncomment sometimes during the lifetime of the 4.x branch
+        $src = $parser->getStream()->getSourceContext();
+        $dep = new DeprecatedCallableInfo('twig/twig', '3.15', 'The "attribute" function is deprecated, use the "." notation instead.');
+        $dep->setName('attribute');
+        $dep->setType('function');
+        $dep->triggerDeprecation($src->getPath() ?: $src->getName(), $line);
+        */
+
         return new GetAttrExpression($args[0], $args[1], $args[2] ?? null, Template::ANY_CALL, $line);
     }
 }


Test patch:
diff --git a/tests/Fixtures/expressions/dynamic_attribute.test b/tests/Fixtures/expressions/dynamic_attribute.test
new file mode 100644
index 00000000..930c6f17
--- /dev/null
+++ b/tests/Fixtures/expressions/dynamic_attribute.test
@@ -0,0 +1,18 @@
+--TEST--
+"." notation with dynamic attributes
+--TEMPLATE--
+{{ obj.(method) }}
+{{ array.(item) }}
+{{ obj.("bar")("a", "b") }}
+{{ obj.("bar")(...arguments) }}
+{{ obj.(method) is defined ? 'ok' : 'ko' }}
+{{ obj.(nonmethod) is defined ? 'ok' : 'ko' }}
+--DATA--
+return ['obj' => new Twig\Tests\TwigTestFoo(), 'method' => 'foo', 'array' => ['foo' => 'bar'], 'item' => 'foo', 'nonmethod' => 'xxx', 'arguments' => ['a', 'b']]
+--EXPECT--
+foo
+bar
+bar_a-b
+bar_a-b
+ok
+ko
diff --git a/tests/Fixtures/functions/attribute.test b/tests/Fixtures/functions/attribute.legacy.test
similarity index 100%
rename from tests/Fixtures/functions/attribute.test
rename to tests/Fixtures/functions/attribute.legacy.test
diff --git a/tests/Fixtures/functions/attribute_with_wrong_args.test b/tests/Fixtures/functions/attribute_with_wrong_args.legacy.test
similarity index 100%
rename from tests/Fixtures/functions/attribute_with_wrong_args.test
rename to tests/Fixtures/functions/attribute_with_wrong_args.legacy.test
diff --git a/tests/Fixtures/tests/defined_for_attribute.legacy.test b/tests/Fixtures/tests/defined_for_attribute.legacy.test
new file mode 100644
index 00000000..5fd2fe3f
--- /dev/null
+++ b/tests/Fixtures/tests/defined_for_attribute.legacy.test
@@ -0,0 +1,35 @@
+--TEST--
+"defined" support for attribute
+--TEMPLATE--
+{{ attribute(nested, "definedVar")     is     defined ? 'ok' : 'ko' }}
+{{ attribute(nested, "undefinedVar")   is not defined ? 'ok' : 'ko' }}
+{{ attribute(nested, definedVarName)   is     defined ? 'ok' : 'ko' }}
+{{ attribute(nested, undefinedVarName) is not defined ? 'ok' : 'ko' }}
+--DATA--
+return [
+    'nested' => [
+        'definedVar' => 'defined',
+    ],
+    'definedVarName' => 'definedVar',
+    'undefinedVarName' => 'undefinedVar',
+]
+--EXPECT--
+ok
+ok
+ok
+ok
+--DATA--
+return [
+    'nested' => [
+        'definedVar' => 'defined',
+    ],
+    'definedVarName' => 'definedVar',
+    'undefinedVarName' => 'undefinedVar',
+]
+--CONFIG--
+return ['strict_variables' => false]
+--EXPECT--
+ok
+ok
+ok
+ok
diff --git a/tests/Fixtures/tests/defined_for_attribute.test b/tests/Fixtures/tests/defined_for_attribute.test
index 5fd2fe3f..6fc6eb68 100644
--- a/tests/Fixtures/tests/defined_for_attribute.test
+++ b/tests/Fixtures/tests/defined_for_attribute.test
@@ -1,10 +1,10 @@
 --TEST--
-"defined" support for attribute
+"defined" support for dynamic attribute
 --TEMPLATE--
-{{ attribute(nested, "definedVar")     is     defined ? 'ok' : 'ko' }}
-{{ attribute(nested, "undefinedVar")   is not defined ? 'ok' : 'ko' }}
-{{ attribute(nested, definedVarName)   is     defined ? 'ok' : 'ko' }}
-{{ attribute(nested, undefinedVarName) is not defined ? 'ok' : 'ko' }}
+{{ nested.("definedVar")     is     defined ? 'ok' : 'ko' }}
+{{ nested.("undefinedVar")   is not defined ? 'ok' : 'ko' }}
+{{ nested.(definedVarName)   is     defined ? 'ok' : 'ko' }}
+{{ nested.(undefinedVarName) is not defined ? 'ok' : 'ko' }}
 --DATA--
 return [
     'nested' => [
diff --git a/tests/TemplateTest.php b/tests/TemplateTest.php
index 7018f4af..402c4f35 100644
--- a/tests/TemplateTest.php
+++ b/tests/TemplateTest.php
@@ -76,7 +76,7 @@ class TemplateTest extends TestCase
             ['{{ array.a() }}', 'Impossible to invoke a method ("a") on a sequence/mapping in "%s" at line 1.'],
             ['{{ empty_array.a }}', 'Key "a" does not exist as the sequence/mapping is empty in "%s" at line 1.'],
             ['{{ array.a }}', 'Key "a" for sequence/mapping with keys "foo" does not exist in "%s" at line 1.'],
-            ['{{ attribute(array, -10) }}', 'Key "-10" for sequence/mapping with keys "foo" does not exist in "%s" at line 1.'],
+            ['{{ array.(-10) }}', 'Key "-10" for sequence/mapping with keys "foo" does not exist in "%s" at line 1.'],
             ['{{ array_access.a }}', 'Neither the property "a" nor one of the methods "a()", "geta()"/"isa()"/"hasa()" or "__call()" exist and have public access in class "Twig\Tests\TemplateArrayAccessObject" in "%s" at line 1.'],
             ['{% from _self import foo %}{% macro foo(obj) %}{{ obj.missing_method() }}{% endmacro %}{{ foo(array_access) }}', 'Neither the property "missing_method" nor one of the methods "missing_method()", "getmissing_method()"/"ismissing_method()"/"hasmissing_method()" or "__call()" exist and have public access in class "Twig\Tests\TemplateArrayAccessObject" in "%s" at line 1.'],
             ['{{ magic_exception.test }}', 'An exception has been thrown during the rendering of a template ("Hey! Don\'t try to isset me!") in "%s" at line 1.'],