Deprecate using parentheses when testing a macro with the defined test

CHANGELOG

@@ -14,7 +14,8 @@
  * Deprecate defining a macro more than once in the same template
  * Deprecate `TemplateVariable` and `AssignTemplateVariable`; use `MacroVariable` and `AssignMacroVariable` instead
  * Deprecate calling or testing a macro with a name whose case differs from its definition; macro names will be case-sensitive in 4.0
- * Deprecate omitting parentheses when calling or testing a macro; it will throw a `SyntaxError` in 4.0
+ * Deprecate omitting parentheses when calling a macro; it will throw a `SyntaxError` in 4.0
+ * Deprecate using parentheses when testing a macro with the `defined` test; it will throw a `SyntaxError` in 4.0
  * Deprecate calling a macro without a value for an argument that has no default value; the argument will be required in 4.0
  * Deprecate passing extra or unknown arguments to a macro that does not declare a variadic argument; it will throw in 4.0
  * Add support for declaring an explicit variadic macro argument (`{% macro foo(a, ...rest) %}`)

doc/deprecated.rst

@@ -332,10 +332,16 @@ Macros
 * Passing an unknown named argument to a macro is deprecated as of Twig 3.29 and
   will throw in Twig 4.0. Declare an explicit variadic argument to accept it.
 
-* Omitting parentheses when calling or testing a macro (e.g.
-  ``macros.input`` or ``macros.(name)``) is deprecated as of Twig 3.29 and
-  will throw a ``SyntaxError`` in Twig 4.0. Add parentheses after the macro
-  name (e.g. ``macros.input()`` or ``macros.(name)()``).
+* Omitting parentheses when calling a macro (e.g. ``macros.input`` or
+  ``macros.(name)``) is deprecated as of Twig 3.29 and will throw a
+  ``SyntaxError`` in Twig 4.0. Add parentheses after the macro name (e.g.
+  ``macros.input()`` or ``macros.(name)()``).
+
+* Using parentheses when testing a macro with the ``defined`` test (e.g.
+  ``macros.input() is defined``) is deprecated as of Twig 3.29 and will throw a
+  ``SyntaxError`` in Twig 4.0. The test checks the macro itself, not a call, so
+  remove the parentheses after the macro name (e.g.
+  ``macros.input is defined``).
 
 * Calling a macro without a value for an argument that has no default value is
   deprecated as of Twig 3.29; such an argument will be required in Twig 4.0

doc/tags/macro.rst

@@ -192,6 +192,15 @@ You can check if a macro is defined via the ``defined`` test:
         OK
     {% endif %}
 
+Note that the test applies to the macro itself, not to a call: don't use
+parentheses after the macro name when testing it.
+
+.. deprecated:: 3.29
+
+    Using parentheses when testing a macro with the ``defined`` test (e.g.
+    ``macros.hello() is defined``) is deprecated as of Twig 3.29; it will
+    throw a ``SyntaxError`` in Twig 4.0.
+
 Named Macro End-Tags
 --------------------
 

src/ExpressionParser/Infix/DotExpressionParser.php

@@ -72,13 +72,11 @@ final class DotExpressionParser extends AbstractExpressionParser implements Infi
         }
 
         if ($isMacroTarget) {
-            if (Template::METHOD_CALL !== $type) {
-                trigger_deprecation('twig/twig', '3.29', 'Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "%s" at line %d.', $stream->getSourceContext()->getName(), $expr->getTemplateLine());
-            }
-
             $name = $attribute instanceof ConstantExpression ? (string) $attribute->getAttribute('value') : $attribute;
+            $node = new MacroReferenceExpression(new MacroVariable($expr->getAttribute('name'), $expr->getTemplateLine()), $name, $arguments, $expr->getTemplateLine());
+            $node->setHasCallParentheses(Template::METHOD_CALL === $type);
 
-            return new MacroReferenceExpression(new MacroVariable($expr->getAttribute('name'), $expr->getTemplateLine()), $name, $arguments, $expr->getTemplateLine());
+            return $node;
         }
 
         return new GetAttrExpression($expr, $attribute, $arguments, $type, $lineno, $nullSafe);

src/ExpressionParser/Infix/FunctionExpressionParser.php

@@ -47,7 +47,11 @@ final class FunctionExpressionParser extends AbstractExpressionParser implements
         // FromTokenParser, which maps the local alias to the macro name and the
         // template it comes from.
         if (null !== $alias = $parser->getImportedSymbol('function', $name)) {
-            return new MacroReferenceExpression($alias['node']->getNode('var'), $alias['name'], $this->parseCallableArguments($parser, $line, false, true), $line);
+            $arguments = $this->parseCallableArguments($parser, $line, false, true);
+            $node = new MacroReferenceExpression($alias['node']->getNode('var'), $alias['name'], $arguments, $line);
+            $node->setHasCallParentheses(true);
+
+            return $node;
         }
 
         $args = $this->parseNamedArguments($parser, false);

src/ExpressionParser/Infix/IsExpressionParser.php

@@ -46,8 +46,15 @@ class IsExpressionParser extends AbstractExpressionParser implements InfixExpres
             $arguments = new Nodes([0 => $parser->parseExpression($this->getPrecedence())]);
         }
 
-        if ('defined' === $test->getName() && $expr instanceof NameExpression && null !== $alias = $parser->getImportedSymbol('function', $expr->getAttribute('name'))) {
-            $expr = new MacroReferenceExpression($alias['node']->getNode('var'), $alias['name'], new ArrayExpression([], $expr->getTemplateLine()), $expr->getTemplateLine());
+        if ('defined' === $test->getName()) {
+            if ($expr instanceof NameExpression && null !== $alias = $parser->getImportedSymbol('function', $expr->getAttribute('name'))) {
+                $expr = new MacroReferenceExpression($alias['node']->getNode('var'), $alias['name'], new ArrayExpression([], $expr->getTemplateLine()), $expr->getTemplateLine());
+                $expr->setHasCallParentheses(false);
+            }
+
+            if ($expr instanceof MacroReferenceExpression && $expr->hasCallParentheses()) {
+                trigger_deprecation('twig/twig', '3.29', 'Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "%s" at line %d.', $stream->getSourceContext()->getName(), $expr->getTemplateLine());
+            }
         }
 
         $ready = $test instanceof TwigTest;

src/Node/Expression/MacroReferenceExpression.php

@@ -25,6 +25,8 @@ class MacroReferenceExpression extends AbstractExpression implements SupportDefi
     use SupportDefinedTestDeprecationTrait;
     use SupportDefinedTestTrait;
 
+    private bool $hasCallParentheses = true;
+
     /**
      * @param string|AbstractExpression $name The bare macro name (a static identifier) or, for a dynamic
      *                                        call, an expression resolving to the macro name
@@ -43,6 +45,22 @@ class MacroReferenceExpression extends AbstractExpression implements SupportDefi
         parent::__construct($nodes, $attributes, $lineno);
     }
 
+    /**
+     * @internal
+     */
+    public function setHasCallParentheses(bool $hasCallParentheses): void
+    {
+        $this->hasCallParentheses = $hasCallParentheses;
+    }
+
+    /**
+     * @internal
+     */
+    public function hasCallParentheses(): bool
+    {
+        return $this->hasCallParentheses;
+    }
+
     public function __clone()
     {
         // The template node must not be deep-cloned because its name is

src/NodeVisitor/CorrectnessNodeVisitor.php

@@ -16,6 +16,7 @@ use Twig\Error\SyntaxError;
 use Twig\Node\BlockNode;
 use Twig\Node\BlockReferenceNode;
 use Twig\Node\ConfigNode;
+use Twig\Node\Expression\MacroReferenceExpression;
 use Twig\Node\MacroNode;
 use Twig\Node\ModuleNode;
 use Twig\Node\Node;
@@ -32,6 +33,7 @@ use Twig\Node\TextNode;
 final class CorrectnessNodeVisitor implements NodeVisitorInterface
 {
     private ?\WeakMap $rootNodes = null;
+    private ?\WeakMap $checkedMacroReferences = null;
     /**
      * Stack of the output-wrapping tags ("if", "for", "set", ...) currently open;
      * the top one is the nearest tag a "block" definition would be nested under.
@@ -59,6 +61,10 @@ final class CorrectnessNodeVisitor implements NodeVisitorInterface
             $this->checkConfigTag($node);
         }
 
+        if ($node instanceof MacroReferenceExpression) {
+            $this->checkMacroCallParentheses($node);
+        }
+
         if ($node instanceof BlockReferenceNode) {
             $this->checkBlockDefinitionNesting($node);
         }
@@ -88,6 +94,7 @@ final class CorrectnessNodeVisitor implements NodeVisitorInterface
     {
         $this->resetState();
         $this->rootNodes = new \WeakMap();
+        $this->checkedMacroReferences = new \WeakMap();
         $this->hasParent = $node->hasNode('parent');
 
         foreach ($this->getRootNodes($node) as $n) {
@@ -101,6 +108,7 @@ final class CorrectnessNodeVisitor implements NodeVisitorInterface
     private function resetState(): void
     {
         $this->rootNodes = null;
+        $this->checkedMacroReferences = null;
         $this->tagStack = [];
         $this->hasParent = false;
         $this->blockDepth = 0;
@@ -152,6 +160,18 @@ final class CorrectnessNodeVisitor implements NodeVisitorInterface
         }
     }
 
+    private function checkMacroCallParentheses(MacroReferenceExpression $node): void
+    {
+        if (isset($this->checkedMacroReferences[$node])) {
+            return;
+        }
+        $this->checkedMacroReferences[$node] = true;
+
+        if (false === $node->hasCallParentheses() && !$node->isDefinedTestEnabled()) {
+            trigger_deprecation('twig/twig', '3.29', 'Omitting parentheses when calling a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "%s" at line %d.', $node->getSourceContext()->getName(), $node->getTemplateLine());
+        }
+    }
+
     private function checkConfigTag(ConfigNode $node): void
     {
         if ('extends' === $node->getNodeTag()) {

tests/Fixtures/macros/call_without_parentheses.legacy.test

@@ -1,16 +1,19 @@
 --TEST--
 Omitting parentheses when calling macros is deprecated
 --DEPRECATION--
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 4.
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 5.
+Since twig/twig 3.29: Omitting parentheses when calling a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 4.
+Since twig/twig 3.29: Omitting parentheses when calling a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 5.
+Since twig/twig 3.29: Omitting parentheses when calling a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 6.
 --TEMPLATE--
 {% import _self as macros %}
 {% set name = 'hello' %}
 {{ macros.hello }}
 {{ _self.(name) }}
+{{ (macros.hello) ?? 'fallback' }}
 {% macro hello() %}Hello{% endmacro %}
 --DATA--
 return []
 --EXPECT--
 Hello
 Hello
+Hello

tests/Fixtures/tests/defined_for_macros.test

@@ -9,28 +9,25 @@
 {% from _self import hello, bar %}
 
 {% block content %}
-    {{~ macros.hello() is defined ? 'OK' : 'KO' }}
+    {{~ macros.hello is defined ? 'OK' : 'KO' }}
 
-    {{~ macros_ext.lol() is defined ? 'OK' : 'KO' }}
+    {{~ macros_ext.lol is defined ? 'OK' : 'KO' }}
 
-    {{~ macros.foo() is not defined ? 'OK' : 'KO' }}
+    {{~ macros.foo is not defined ? 'OK' : 'KO' }}
 
-    {{~ macros_ext.hello() is not defined ? 'OK' : 'KO' }}
+    {{~ macros_ext.hello is not defined ? 'OK' : 'KO' }}
 
     {{~ hello is defined ? 'OK' : 'KO' }}
-    {{~ hello() is defined ? 'OK' : 'KO' }}
 
     {{~ lol is defined ? 'OK' : 'KO' }}
-    {{~ lol() is defined ? 'OK' : 'KO' }}
 
     {{~ baz is not defined ? 'OK' : 'KO' }}
-    {{~ baz() is not defined ? 'OK' : 'KO' }}
 
-    {{~ _self.hello() is defined ? 'OK' : 'KO' }}
+    {{~ _self.hello is defined ? 'OK' : 'KO' }}
 
-    {{~ _self.bar() is not defined ? 'OK' : 'KO' }}
+    {{~ _self.bar is not defined ? 'OK' : 'KO' }}
 
-    {{~ _self.lol() is defined ? 'OK' : 'KO' }}
+    {{~ _self.lol is defined ? 'OK' : 'KO' }}
 {% endblock %}
 
 {% macro hello(name) %}{% endmacro %}
@@ -50,13 +47,10 @@ OK
 
 OK
 
-OK
 OK
 
-OK
-OK
-
-OK
+OK
+
 OK
 
 OK

tests/Fixtures/tests/defined_for_macros_dynamic_name.test

@@ -5,8 +5,8 @@
 
 {% set known = 'hello' %}
 {% set unknown = 'missing' %}
-{{~ macros.(known)() is defined ? 'OK' : 'KO' }}
-{{~ macros.(unknown)() is not defined ? 'OK' : 'KO' }}
+{{~ macros.(known) is defined ? 'OK' : 'KO' }}
+{{~ macros.(unknown) is not defined ? 'OK' : 'KO' }}
 
 {% macro hello(name) %}{% endmacro %}
 --DATA--

tests/Fixtures/tests/defined_for_macros_dynamic_name_with_parentheses.legacy.test

@@ -0,0 +1,17 @@
+--TEST--
+Using parentheses when testing dynamic macro names with "defined" is deprecated
+--DEPRECATION--
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 5.
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 6.
+--TEMPLATE--
+{% import _self as macros %}
+{% set known = 'hello' %}
+{% set unknown = 'missing' %}
+{{ macros.(known)() is defined ? 'OK' : 'KO' }}
+{{ macros.(unknown)() is not defined ? 'OK' : 'KO' }}
+{% macro hello() %}{% endmacro %}
+--DATA--
+return []
+--EXPECT--
+OK
+OK

tests/Fixtures/tests/defined_for_macros_dynamic_name_without_parentheses.legacy.test

@@ -1,17 +0,0 @@
---TEST--
-Omitting parentheses when testing dynamic macro names is deprecated
---DEPRECATION--
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 5.
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 6.
---TEMPLATE--
-{% import _self as macros %}
-{% set known = 'hello' %}
-{% set unknown = 'missing' %}
-{{ macros.(known) is defined ? 'OK' : 'KO' }}
-{{ macros.(unknown) is not defined ? 'OK' : 'KO' }}
-{% macro hello() %}{% endmacro %}
---DATA--
-return []
---EXPECT--
-OK
-OK

tests/Fixtures/tests/defined_for_macros_with_parentheses.legacy.test

@@ -0,0 +1,22 @@
+--TEST--
+Using parentheses when testing static macro names with "defined" is deprecated
+--DEPRECATION--
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 4.
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 5.
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 6.
+Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index.twig" at line 7.
+--TEMPLATE--
+{% import _self as macros %}
+{% from _self import hello %}
+{{ macros.hello() is defined ? 'OK' : 'KO' }}
+{{ _self.missing() is not defined ? 'OK' : 'KO' }}
+{{ hello() is defined ? 'OK' : 'KO' }}
+{{ (hello()) is defined ? 'OK' : 'KO' }}
+{% macro hello() %}{% endmacro %}
+--DATA--
+return []
+--EXPECT--
+OK
+OK
+OK
+OK

tests/Fixtures/tests/defined_for_macros_without_parentheses.legacy.test

@@ -1,15 +0,0 @@
---TEST--
-Omitting parentheses when testing static macro names is deprecated
---DEPRECATION--
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 3.
-Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index.twig" at line 4.
---TEMPLATE--
-{% import _self as macros %}
-{{ macros.hello is defined ? 'OK' : 'KO' }}
-{{ _self.missing is not defined ? 'OK' : 'KO' }}
-{% macro hello() %}{% endmacro %}
---DATA--
-return []
---EXPECT--
-OK
-OK

tests/ParserTest.php

@@ -142,7 +142,9 @@ EOF, 'index')));
     {
         foreach (['_self', 'macros'] as $target) {
             yield $target.' static with parentheses' => [$target.'.foo()'];
+            yield $target.' grouped static with parentheses' => ['('.$target.'.foo())'];
             yield $target.' dynamic with parentheses' => [$target.'.(name)()'];
+            yield $target.' grouped dynamic with parentheses' => ['('.$target.'.(name)())'];
         }
     }
 
@@ -157,7 +159,7 @@ EOF, 'index')));
     {
         $twig = new Environment(new ArrayLoader());
 
-        $this->expectDeprecation('Since twig/twig 3.29: Omitting parentheses when calling or testing a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index" at line 1.');
+        $this->expectDeprecation('Since twig/twig 3.29: Omitting parentheses when calling a macro is deprecated and will throw a SyntaxError in Twig 4.0; add parentheses after the macro name in "index" at line 1.');
 
         $module = $twig->parse($twig->tokenize(new Source("{% import _self as macros %}{{ $expression }}", 'index')));
         $macroReferences = [];
@@ -173,10 +175,53 @@ EOF, 'index')));
     {
         foreach (['_self', 'macros'] as $target) {
             yield $target.' static without parentheses' => [$target.'.foo'];
+            yield $target.' grouped static without parentheses' => ['('.$target.'.foo)'];
             yield $target.' dynamic without parentheses' => [$target.'.(name)'];
+            yield $target.' grouped dynamic without parentheses' => ['('.$target.'.(name))'];
         }
     }
 
+    /**
+     * @dataProvider provideMacroTargetExpressionsWithoutParentheses
+     */
+    #[DataProvider('provideMacroTargetExpressionsWithoutParentheses')]
+    public function testMacroTargetsWithoutParenthesesAreAllowedInDefinedTest(string $expression): void
+    {
+        $twig = new Environment(new ArrayLoader());
+
+        $module = $twig->parse($twig->tokenize(new Source("{% import _self as macros %}{{ $expression is defined }}{{ $expression is not defined }}", 'index')));
+        $macroReferences = [];
+        $attributeExpressions = [];
+
+        $this->collectExpressions($module, $macroReferences, $attributeExpressions);
+
+        $this->assertCount(2, $macroReferences);
+        $this->assertSame([], $attributeExpressions);
+    }
+
+    /**
+     * @dataProvider provideMacroTargetExpressions
+     *
+     * @group legacy
+     */
+    #[DataProvider('provideMacroTargetExpressions')]
+    #[Group('legacy')]
+    public function testMacroTargetsWithParenthesesAreDeprecatedInDefinedTest(string $expression): void
+    {
+        $twig = new Environment(new ArrayLoader());
+
+        $this->expectDeprecation('Since twig/twig 3.29: Using parentheses when testing a macro with the "defined" test is deprecated and will throw a SyntaxError in Twig 4.0; remove the parentheses after the macro name in "index" at line 1.');
+
+        $module = $twig->parse($twig->tokenize(new Source("{% import _self as macros %}{{ $expression is defined }}", 'index')));
+        $macroReferences = [];
+        $attributeExpressions = [];
+
+        $this->collectExpressions($module, $macroReferences, $attributeExpressions);
+
+        $this->assertCount(1, $macroReferences);
+        $this->assertSame([], $attributeExpressions);
+    }
+
     public function testImplicitMacroArgumentDefaultValues(): void
     {
         $template = '{% macro marco (po, lo = true) %}{% endmacro %}';