From d7f8b4eb1c6e650a769a066c261d455f664fab0a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 27 Jul 2026 12:50:45 +0200 Subject: [PATCH] Redesign macro calls and argument handling --- CHANGELOG | 12 +- doc/deprecated.rst | 38 ++- doc/tags/macro.rst | 80 +++++- src/ExpressionParser/Infix/ArgumentsTrait.php | 5 +- .../Infix/DotExpressionParser.php | 29 +-- .../Infix/FunctionExpressionParser.php | 6 +- src/Extension/CoreExtension.php | 16 +- src/MacroNamespace.php | 126 ++++++++++ src/Node/ConfigNode.php | 2 +- .../Expression/MacroReferenceExpression.php | 91 ++----- src/Node/Expression/MethodCallExpression.php | 18 +- src/Node/Expression/TempNameExpression.php | 3 + .../Expression/Variable/MacroVariable.php | 2 +- src/Node/ImportNode.php | 4 +- src/Node/MacroDeclarationNode.php | 34 +++ src/Node/MacroNode.php | 75 ++++-- src/Node/MacrosNode.php | 81 ++++++ src/Node/ModuleNode.php | 11 +- src/Parser.php | 3 +- src/Template.php | 42 +--- src/Test/IntegrationTestCase.php | 17 ++ src/TokenParser/FromTokenParser.php | 7 +- src/TokenParser/MacroTokenParser.php | 33 ++- src/TwigMacro.php | 186 ++++++++++++++ tests/CallMacroTest.php | 234 ++++++++++++++++++ tests/Extension/SandboxStateChangeTest.php | 6 +- tests/Extension/SandboxTest.php | 4 +- .../call_without_parentheses.legacy.test | 4 +- .../macros/constant_name_non_identifier.test | 10 + .../macros/dynamic_name_non_string.test | 9 + .../implicit_optional_arguments.legacy.test | 27 ++ tests/Fixtures/macros/reserved_variables.test | 13 +- tests/Fixtures/macros/simple.test | 2 +- tests/Fixtures/macros/varargs.test | 4 +- .../macros/varargs_implicit.legacy.test | 24 ++ tests/Fixtures/macros/variadic.test | 22 ++ .../macros/variadic_default_value.test | 7 + tests/Fixtures/macros/variadic_duplicate.test | 7 + .../macros/variadic_invalid_name.test | 7 + tests/Fixtures/macros/variadic_named.test | 16 ++ tests/Fixtures/macros/variadic_not_last.test | 7 + .../macros/variadic_reserved_name.test | 14 ++ tests/Fixtures/tags/macro/basic.test | 2 +- tests/Fixtures/tags/macro/external.test | 2 +- tests/Fixtures/tags/macro/global.test | 2 +- .../Fixtures/tags/macro/named_arguments.test | 2 +- tests/Fixtures/tags/macro/self_import.test | 2 +- tests/Fixtures/tests/defined_for_macros.test | 14 -- .../defined_for_macros_dynamic_name.test | 4 - ...namic_name_without_parentheses.legacy.test | 17 ++ ...for_macros_without_parentheses.legacy.test | 15 ++ tests/Node/Expression/MacroReferenceTest.php | 40 +-- tests/Node/ImportTest.php | 2 +- tests/Node/MacroTest.php | 26 +- tests/Node/MacrosTest.php | 85 +++++++ tests/Node/ModuleTest.php | 41 ++- tests/NodeVisitor/CorrectnessTest.php | 3 +- tests/NodeVisitor/SandboxTest.php | 9 +- tests/ParserTest.php | 95 +++++++ tests/TwigMacroTest.php | 219 ++++++++++++++++ 60 files changed, 1625 insertions(+), 293 deletions(-) create mode 100644 src/MacroNamespace.php create mode 100644 src/Node/MacroDeclarationNode.php create mode 100644 src/Node/MacrosNode.php create mode 100644 src/TwigMacro.php create mode 100644 tests/CallMacroTest.php create mode 100644 tests/Fixtures/macros/constant_name_non_identifier.test create mode 100644 tests/Fixtures/macros/dynamic_name_non_string.test create mode 100644 tests/Fixtures/macros/implicit_optional_arguments.legacy.test create mode 100644 tests/Fixtures/macros/varargs_implicit.legacy.test create mode 100644 tests/Fixtures/macros/variadic.test create mode 100644 tests/Fixtures/macros/variadic_default_value.test create mode 100644 tests/Fixtures/macros/variadic_duplicate.test create mode 100644 tests/Fixtures/macros/variadic_invalid_name.test create mode 100644 tests/Fixtures/macros/variadic_named.test create mode 100644 tests/Fixtures/macros/variadic_not_last.test create mode 100644 tests/Fixtures/macros/variadic_reserved_name.test create mode 100644 tests/Fixtures/tests/defined_for_macros_dynamic_name_without_parentheses.legacy.test create mode 100644 tests/Fixtures/tests/defined_for_macros_without_parentheses.legacy.test create mode 100644 tests/Node/MacrosTest.php create mode 100644 tests/TwigMacroTest.php diff --git a/CHANGELOG b/CHANGELOG index a947bec2b9a..8929fd9b57c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -13,7 +13,17 @@ * Fix duplicated macro argument names triggering a PHP fatal error instead of a `SyntaxError` * Deprecate defining a macro more than once in the same template * Deprecate `TemplateVariable` and `AssignTemplateVariable`; use `MacroVariable` and `AssignMacroVariable` instead - * Deprecate omitting parentheses when calling a macro; it will throw a `SyntaxError` in 4.0 + * 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 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) %}`) + * Compile macros as closures stored in a per-template registry instead of `macro_`-prefixed PHP methods + * Represent imported macro namespaces with a narrow capability object instead of template instances + * Mark `Twig\Node\MacroNode` as `@final`; it will be final in Twig 4.0 + * Deprecate not passing a `MacrosNode` instance as the macros of a `ModuleNode` constructor + * Change `MacroReferenceExpression` to take the bare macro name instead of a `macro_`-prefixed method name + * Deprecate resolving a macro through a `macro_`-prefixed name; pass the bare macro name to `MacroReferenceExpression` # 3.28.0 (2026-07-03) diff --git a/doc/deprecated.rst b/doc/deprecated.rst index ca313ecddd0..1755129e2e0 100644 --- a/doc/deprecated.rst +++ b/doc/deprecated.rst @@ -11,6 +11,9 @@ Classes * The ``Twig\Markup`` class is considered final as of Twig 3.28 and will be final in Twig 4.0. Use ``Twig\Markup`` directly instead of extending it. +* The ``Twig\Node\MacroNode`` class is considered final as of Twig 3.29 and + will be final in Twig 4.0. + Functions --------- @@ -70,6 +73,9 @@ Nodes * Not passing a ``BodyNode`` instance as the body of a ``ModuleNode`` or ``MacroNode`` constructor is deprecated as of Twig 3.12. +* Not passing a ``MacrosNode`` instance as the macros of a ``ModuleNode`` + constructor is deprecated as of Twig 3.29. + * Not passing the ``$usedTests`` argument to ``Twig\Node\CheckSecurityNode::__construct()`` is deprecated as of Twig 3.28; the argument will be required in 4.0. @@ -318,11 +324,33 @@ Macros 3.29 and will throw a ``SyntaxError`` in Twig 4.0. Give each macro a unique 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)()``). Parentheses remain optional when - testing whether a macro is defined. +* Passing more arguments to a macro than it declares is deprecated as of Twig + 3.29 and will throw in Twig 4.0. Declare an explicit variadic argument + (``{% macro foo(a, ...rest) %}``) to accept extra positional and named + arguments instead of relying on the implicit ``varargs`` variable. + +* 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)()``). + +* 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 + (today it silently defaults to ``null``). To keep an argument optional, give + it an explicit default value (e.g. ``{% macro input(name, value = null) %}``). + +* Calling a macro (or testing it with the ``defined`` test) with a name whose + case differs from its definition (e.g. calling ``input`` as ``INPUT``) is + deprecated as of Twig 3.29; macro names will be case-sensitive in Twig 4.0. + Use the name exactly as defined. + +* Resolving a macro through a ``macro_``-prefixed name (e.g. via a + ``Twig\Node\Expression\MacroReferenceExpression`` node built with + ``macro_input``) is deprecated as of Twig 3.29 and will not resolve in Twig + 4.0; pass the bare macro name instead. Filters ------- diff --git a/doc/tags/macro.rst b/doc/tags/macro.rst index 3ebfdd86927..513dcfd4846 100644 --- a/doc/tags/macro.rst +++ b/doc/tags/macro.rst @@ -11,26 +11,62 @@ via macros (called ``forms.twig``): .. code-block:: html+twig - {% macro input(name, value, type = "text", size = 20) %} - + {% macro input(name, value = "", type = "text", size = 20) %} + {% endmacro %} - {% macro textarea(name, value, rows = 10, cols = 40) %} - + {% macro textarea(name, value = "", rows = 10, cols = 40) %} + {% endmacro %} -Each macro argument can have a default value (here ``text`` is the default value +A macro argument can have a default value (here ``text`` is the default value for ``type`` if not provided in the call). -Macros differ from native PHP functions in a few ways: +As with PHP function arguments, a macro argument is required unless it declares +a default value. Here, ``name`` is required while ``value``, ``type``, and +``size`` are optional. -* Arguments of a macro are always optional. +.. deprecated:: 3.29 -* If extra positional arguments are passed to a macro, they end up in the - special ``varargs`` variable as a list of values. + Calling a macro without a value for an argument that has no default value is + deprecated as of Twig 3.29; the argument will be required in Twig 4.0 (until + then, it defaults to ``null``). Give every optional argument an explicit + default value. -But as with PHP functions, macros don't have access to the current template -variables. +To accept an arbitrary number of extra arguments, declare an explicit variadic +argument as described below. + +Note that macros don't have access to the current template variables. + +A macro can declare an explicit variadic argument to collect any extra +positional and named arguments into a named variable, using the same ``...`` +notation as PHP: + +.. code-block:: html+twig + + {% macro tag(element, ...attributes) %} + <{{ element }} + {%- for key, value in attributes %} {{ key|e('html_attr') }}="{{ value }}"{% endfor -%} + > + {% endmacro %} + + {{ _self.tag("input", type: "text", name: "username") }} + +The variadic argument must be the last one and cannot have a default value. + +.. versionadded:: 3.29 + + Support for declaring an explicit variadic macro argument was added in Twig + 3.29. .. tip:: @@ -110,8 +146,13 @@ via the ``from`` tag:

{{ _self.input('password', '', 'password') }}

- {% macro input(name, value, type = "text", size = 20) %} - + {% macro input(name, value = "", type = "text", size = 20) %} + {% endmacro %} Macros Scoping @@ -162,3 +203,16 @@ readability (the name after the ``endmacro`` word must match the macro name): {% macro input() %} ... {% endmacro input %} + +Deprecating a Macro +------------------- + +Use the :doc:`deprecated ` tag at the top of a macro to deprecate +it; a deprecation notice is triggered whenever the macro is called: + +.. code-block:: html+twig + + {% macro input(name, value = "") %} + {% deprecated 'The "input" macro is deprecated, use "field" instead.' %} + + {% endmacro %} diff --git a/src/ExpressionParser/Infix/ArgumentsTrait.php b/src/ExpressionParser/Infix/ArgumentsTrait.php index 96bde555d3a..fe20138430a 100644 --- a/src/ExpressionParser/Infix/ArgumentsTrait.php +++ b/src/ExpressionParser/Infix/ArgumentsTrait.php @@ -14,6 +14,7 @@ use Twig\Error\SyntaxError; use Twig\Node\Expression\ArrayExpression; use Twig\Node\Expression\Binary\SetBinary; +use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\Unary\SpreadUnary; use Twig\Node\Expression\Variable\ContextVariable; use Twig\Node\Expression\Variable\LocalVariable; @@ -23,11 +24,11 @@ trait ArgumentsTrait { - private function parseCallableArguments(Parser $parser, int $line, bool $parseOpenParenthesis = true): ArrayExpression + private function parseCallableArguments(Parser $parser, int $line, bool $parseOpenParenthesis = true, bool $preserveNames = false): ArrayExpression { $arguments = new ArrayExpression([], $line); foreach ($this->parseNamedArguments($parser, $parseOpenParenthesis) as $k => $n) { - $arguments->addElement($n, new LocalVariable($k, $line)); + $arguments->addElement($n, \is_int($k) || !$preserveNames ? new LocalVariable($k, $line) : new ConstantExpression($k, $line)); } return $arguments; diff --git a/src/ExpressionParser/Infix/DotExpressionParser.php b/src/ExpressionParser/Infix/DotExpressionParser.php index e7642ab91a5..8ecde5c2e2e 100644 --- a/src/ExpressionParser/Infix/DotExpressionParser.php +++ b/src/ExpressionParser/Infix/DotExpressionParser.php @@ -60,34 +60,25 @@ public function parse(Parser $parser, AbstractExpression $expr, Token $token): A } } - if ($stream->test(Token::OPERATOR_TYPE, '(')) { - $type = Template::METHOD_CALL; - $arguments = $this->parseCallableArguments($parser, $token->getLine()); - } - $isMacroTarget = $expr instanceof NameExpression && ( null !== $parser->getImportedSymbol('template', $expr->getAttribute('name')) || '_self' === $expr->getAttribute('name') ); - if ( - $isMacroTarget - && $attribute instanceof ConstantExpression - && \is_string($name = $attribute->getAttribute('value')) - && preg_match('#^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$#D', $name) - ) { - $node = new MacroReferenceExpression(new MacroVariable($expr->getAttribute('name'), $expr->getTemplateLine()), 'macro_'.$name, $arguments, $expr->getTemplateLine()); - $node->setHasParentheses(Template::METHOD_CALL === $type); - - return $node; + if ($stream->test(Token::OPERATOR_TYPE, '(')) { + $type = Template::METHOD_CALL; + $arguments = $this->parseCallableArguments($parser, $token->getLine(), preserveNames: $isMacroTarget); } - if ($isMacroTarget && !$attribute instanceof ConstantExpression) { - $node = new MacroReferenceExpression(new MacroVariable($expr->getAttribute('name'), $expr->getTemplateLine()), $attribute, $arguments, $expr->getTemplateLine()); - $node->setHasParentheses(Template::METHOD_CALL === $type); + 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; - return $node; + return new MacroReferenceExpression(new MacroVariable($expr->getAttribute('name'), $expr->getTemplateLine()), $name, $arguments, $expr->getTemplateLine()); } return new GetAttrExpression($expr, $attribute, $arguments, $type, $lineno, $nullSafe); diff --git a/src/ExpressionParser/Infix/FunctionExpressionParser.php b/src/ExpressionParser/Infix/FunctionExpressionParser.php index c8851823d6f..ca2cce51f94 100644 --- a/src/ExpressionParser/Infix/FunctionExpressionParser.php +++ b/src/ExpressionParser/Infix/FunctionExpressionParser.php @@ -42,8 +42,12 @@ public function parse(Parser $parser, AbstractExpression $expr, Token $token): A $name = $expr->getAttribute('name'); + // A bare call to a macro imported via "from" is syntactically a function call; + // it is resolved through the "function" imported symbol registered by + // 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), $line); + return new MacroReferenceExpression($alias['node']->getNode('var'), $alias['name'], $this->parseCallableArguments($parser, $line, false, true), $line); } $args = $this->parseNamedArguments($parser, false); diff --git a/src/Extension/CoreExtension.php b/src/Extension/CoreExtension.php index b0c3de8e232..6ae95acf295 100644 --- a/src/Extension/CoreExtension.php +++ b/src/Extension/CoreExtension.php @@ -31,6 +31,7 @@ use Twig\ExpressionParser\Prefix\GroupingExpressionParser; use Twig\ExpressionParser\Prefix\LiteralExpressionParser; use Twig\ExpressionParser\Prefix\UnaryOperatorExpressionParser; +use Twig\MacroNamespace; use Twig\Markup; use Twig\Node\Expression\AbstractExpression; use Twig\Node\Expression\Binary\AddBinary; @@ -1358,20 +1359,9 @@ public static function capitalize(string $charset, $string): string * * to be removed in 4.0 */ - public static function callMacro(Template $template, string $method, array $args, int $lineno, array $context, Source $source) + public static function callMacro(MacroNamespace $namespace, string $method, array $args, int $lineno, array $context, Source $source) { - if (!method_exists($template, $method)) { - $parent = $template; - while ($parent = $parent->getParent($context)) { - if (method_exists($parent, $method)) { - return $parent->$method(...$args); - } - } - - throw new RuntimeError(\sprintf('Macro "%s" is not defined in template "%s".', substr($method, \strlen('macro_')), $template->getTemplateName()), $lineno, $source); - } - - return $template->$method(...$args); + return $namespace->call(substr($method, \strlen('macro_')), $args, $context, $lineno, $source); } /** diff --git a/src/MacroNamespace.php b/src/MacroNamespace.php new file mode 100644 index 00000000000..baab072193e --- /dev/null +++ b/src/MacroNamespace.php @@ -0,0 +1,126 @@ + $macros + */ + public function __construct( + private Template $template, + private array $macros, + ) { + } + + public function has(string $name, array $context): bool + { + if (null !== $declaration = $this->findDeclaredName($name, $context)) { + [$declaredName, $templateName] = $declaration; + if ($declaredName !== $name) { + trigger_deprecation('twig/twig', '3.29', 'Testing whether the macro "%s" (defined in template "%s") is defined as "%s" is deprecated; macro names will be case-sensitive in Twig 4.0 and this test will return false.', $declaredName, $templateName, $name); + } + + return true; + } + + return str_starts_with($name, 'macro_') && null !== $this->findDeclaredName(substr($name, \strlen('macro_')), $context); + } + + /** + * @param array $arguments + */ + public function call(string $name, array $arguments, array $context, int $line, Source $source): string|Markup + { + if (null === $macro = $this->resolve($name, $context)) { + if (!str_starts_with($name, 'macro_') || null === $macro = $this->resolve($bareName = substr($name, \strlen('macro_')), $context)) { + throw new RuntimeError(\sprintf('Macro "%s" is not defined in template "%s".', $name, $this->template->getTemplateName()), $line, $source); + } + + trigger_deprecation('twig/twig', '3.29', 'Calling the macro "%s" via the "macro_"-prefixed name "%s" is deprecated; pass the bare macro name to "%s" instead.', $bareName, $name, MacroReferenceExpression::class); + } + + return $macro->callLegacy($arguments, $source, $line); + } + + /** + * @return array{string, string}|null + */ + private function findDeclaredName(string $name, array $context): ?array + { + $namespace = $this; + while (true) { + if (isset($namespace->macros[$name])) { + return [$name, $namespace->template->getTemplateName()]; + } + foreach ($namespace->macros as $declaredName => $macro) { + if (0 === strcasecmp($declaredName, $name)) { + return [$declaredName, $namespace->template->getTemplateName()]; + } + } + + if (null === $namespace = $namespace->getParent($context)) { + return null; + } + } + } + + private function getDeclared(string $name): ?TwigMacro + { + if (isset($this->macros[$name])) { + $this->template->ensureSecurityChecked(); + + return $this->macros[$name]; + } + + foreach ($this->macros as $declaredName => $macro) { + if (0 === strcasecmp($declaredName, $name)) { + trigger_deprecation('twig/twig', '3.29', 'Calling the macro "%s" (defined in template "%s") as "%s" is deprecated; macro names will be case-sensitive in Twig 4.0.', $declaredName, $this->template->getTemplateName(), $name); + + $this->template->ensureSecurityChecked(); + + return $macro; + } + } + + return null; + } + + private function resolve(string $name, array $context): ?TwigMacro + { + $namespace = $this; + while (true) { + if (null !== $macro = $namespace->getDeclared($name)) { + return $macro; + } + + if (null === $namespace = $namespace->getParent($context)) { + return null; + } + } + } + + private function getParent(array $context): ?self + { + if (!$parent = $this->template->getParent($context)) { + return null; + } + + return $parent->unwrap()->getMacroNamespace(); + } +} diff --git a/src/Node/ConfigNode.php b/src/Node/ConfigNode.php index 758e057054f..da3a978fbc4 100644 --- a/src/Node/ConfigNode.php +++ b/src/Node/ConfigNode.php @@ -21,7 +21,7 @@ * @author Fabien Potencier */ #[YieldReady] -final class ConfigNode extends Node +class ConfigNode extends Node { public function __construct(int $lineno) { diff --git a/src/Node/Expression/MacroReferenceExpression.php b/src/Node/Expression/MacroReferenceExpression.php index 010a54b8fd2..69c0b2ecc97 100644 --- a/src/Node/Expression/MacroReferenceExpression.php +++ b/src/Node/Expression/MacroReferenceExpression.php @@ -25,12 +25,9 @@ class MacroReferenceExpression extends AbstractExpression implements SupportDefi use SupportDefinedTestDeprecationTrait; use SupportDefinedTestTrait; - private bool $hasParentheses = true; - /** - * @param string|AbstractExpression $name A static macro method name (e.g. "macro_foo") or, for a dynamic - * call, an expression resolving to the macro name (without the - * "macro_" prefix, which is added at runtime) + * @param string|AbstractExpression $name The bare macro name (a static identifier) or, for a dynamic + * call, an expression resolving to the macro name */ public function __construct(MacroVariable $template, string|AbstractExpression $name, AbstractExpression $arguments, int $lineno) { @@ -38,13 +35,6 @@ public function __construct(MacroVariable $template, string|AbstractExpression $ $attributes = ['name' => null]; if (\is_string($name)) { - // The name is emitted as raw PHP in compile() via "->{$name}(...)", - // so it must be a valid PHP method identifier. Reject anything else - // as a defense-in-depth against accidental PHP code injection from - // a caller that forgot to validate user-controlled input. - if (!preg_match('#^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$#D', $name)) { - throw new \LogicException(\sprintf('Macro name "%s" is not a valid PHP identifier.', $name)); - } $attributes['name'] = $name; } else { $nodes['name'] = $name; @@ -53,14 +43,6 @@ public function __construct(MacroVariable $template, string|AbstractExpression $ parent::__construct($nodes, $attributes, $lineno); } - /** - * @internal - */ - public function setHasParentheses(bool $hasParentheses): void - { - $this->hasParentheses = $hasParentheses; - } - public function __clone() { // The template node must not be deep-cloned because its name is @@ -73,78 +55,41 @@ public function __clone() public function compile(Compiler $compiler): void { - if (!$this->hasParentheses && !$this->definedTest) { - 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.', $this->getTemplateName(), $this->getTemplateLine()); - } - - if ($this->hasNode('name')) { - $this->compileDynamic($compiler); - - return; - } + $compiler->subcompile($this->getNode('template')); if ($this->definedTest) { - $compiler - ->subcompile($this->getNode('template')) - ->raw('->hasMacro(') - ->repr($this->getAttribute('name')) - ->raw(', $context') - ->raw(')') - ; + $compiler->raw('->has('); + $this->compileName($compiler); + $compiler->raw(', $context)'); return; } + $compiler->raw('->call('); + $this->compileName($compiler); $compiler - ->subcompile($this->getNode('template')) - ->raw('->getTemplateForMacro(') - ->repr($this->getAttribute('name')) + ->raw(', ') + ->subcompile($this->getNode('arguments')) ->raw(', $context, ') ->repr($this->getTemplateLine()) ->raw(', $this->getSourceContext())') - ->raw(\sprintf('->%s', $this->getAttribute('name'))) - ->raw('(...') - ->subcompile($this->getNode('arguments')) - ->raw(')') ; } public function getStringCoercedChildNames(): array { - // Dynamic macro names are prefixed via PHP string concatenation at runtime. + // Dynamic macro names are string-coerced at runtime. return $this->hasNode('name') ? ['name'] : []; } - private function compileDynamic(Compiler $compiler): void + private function compileName(Compiler $compiler): void { - // The macro method name is resolved at runtime from a context value; - // prefixing it with "macro_" constrains the dynamic method call to the - // template's macro methods only, and getTemplateForMacro()/hasMacro() - // validate that the method actually exists. - $var = $compiler->getVarName(); - - if ($this->definedTest) { - $compiler - ->subcompile($this->getNode('template')) - ->raw('->hasMacro(\'macro_\'.') - ->subcompile($this->getNode('name')) - ->raw(', $context)') - ; - - return; + // A dynamic macro name is resolved at runtime from a context value and + // string-coerced before the registry lookup. + if ($this->hasNode('name')) { + $compiler->raw('(string) ')->subcompile($this->getNode('name')); + } else { + $compiler->repr($this->getAttribute('name')); } - - $compiler - ->subcompile($this->getNode('template')) - ->raw(\sprintf('->getTemplateForMacro($%s = \'macro_\'.', $var)) - ->subcompile($this->getNode('name')) - ->raw(', $context, ') - ->repr($this->getTemplateLine()) - ->raw(', $this->getSourceContext())') - ->raw(\sprintf('->{$%s}', $var)) - ->raw('(...') - ->subcompile($this->getNode('arguments')) - ->raw(')') - ; } } diff --git a/src/Node/Expression/MethodCallExpression.php b/src/Node/Expression/MethodCallExpression.php index 4b180534df9..00a7287ef4f 100644 --- a/src/Node/Expression/MethodCallExpression.php +++ b/src/Node/Expression/MethodCallExpression.php @@ -34,25 +34,25 @@ public function compile(Compiler $compiler): void { if ($this->definedTest) { $compiler - ->raw('method_exists($macros[') + ->raw('$macros[') ->repr($this->getNode('node')->getAttribute('name')) - ->raw('], ') - ->repr($this->getAttribute('method')) - ->raw(')') + ->raw(']->has(') + ->repr(substr($this->getAttribute('method'), \strlen('macro_'))) + ->raw(', $context)') ; return; } $compiler - ->raw('CoreExtension::callMacro($macros[') + ->raw('$macros[') ->repr($this->getNode('node')->getAttribute('name')) - ->raw('], ') - ->repr($this->getAttribute('method')) + ->raw(']->call(') + ->repr(substr($this->getAttribute('method'), \strlen('macro_'))) ->raw(', ') ->subcompile($this->getNode('arguments')) - ->raw(', ') + ->raw(', $context, ') ->repr($this->getTemplateLine()) - ->raw(', $context, $this->getSourceContext())'); + ->raw(', $this->getSourceContext())'); } } diff --git a/src/Node/Expression/TempNameExpression.php b/src/Node/Expression/TempNameExpression.php index f6e5f55133f..d691ba51dc5 100644 --- a/src/Node/Expression/TempNameExpression.php +++ b/src/Node/Expression/TempNameExpression.php @@ -16,6 +16,9 @@ class TempNameExpression extends AbstractExpression { + // 4.0: re-evaluate "varargs" here once the implicit macro varargs bucket is removed + // (see MacroNode::VARARGS_NAME); the other names map to compiled variables ($context, + // $macros, $blocks, $this) and must stay. public const RESERVED_NAMES = ['varargs', 'context', 'macros', 'blocks', 'this']; // Prefix applied to reserved names so their compiled PHP variables cannot clash diff --git a/src/Node/Expression/Variable/MacroVariable.php b/src/Node/Expression/Variable/MacroVariable.php index 81da75409d3..032328120f5 100644 --- a/src/Node/Expression/Variable/MacroVariable.php +++ b/src/Node/Expression/Variable/MacroVariable.php @@ -30,7 +30,7 @@ public function compile(Compiler $compiler): void $name = $this->getName($compiler); if ('_self' === $name) { - $compiler->raw('$this'); + $compiler->raw('$this->getMacroNamespace()'); } else { $compiler ->raw('$macros[') diff --git a/src/Node/ImportNode.php b/src/Node/ImportNode.php index 154043b7f64..4d701f4b397 100644 --- a/src/Node/ImportNode.php +++ b/src/Node/ImportNode.php @@ -46,14 +46,14 @@ public function compile(Compiler $compiler): void $compiler->subcompile($this->getNode('var')); if ($this->getNode('expr') instanceof ContextVariable && '_self' === $this->getNode('expr')->getAttribute('name')) { - $compiler->raw('$this'); + $compiler->raw('$this->getMacroNamespace()'); } else { $compiler ->raw('$this->load(') ->subcompile($this->getNode('expr')) ->raw(', ') ->repr($this->getTemplateLine()) - ->raw(')->unwrap()') + ->raw(')->unwrap()->getMacroNamespace()') ; } diff --git a/src/Node/MacroDeclarationNode.php b/src/Node/MacroDeclarationNode.php new file mode 100644 index 00000000000..edc33623fb2 --- /dev/null +++ b/src/Node/MacroDeclarationNode.php @@ -0,0 +1,34 @@ + + * + * @internal + */ +#[YieldReady] +final class MacroDeclarationNode extends ConfigNode +{ + public function __construct(string $name, int $lineno) + { + parent::__construct($lineno); + + $this->setAttribute('name', $name); + } +} diff --git a/src/Node/MacroNode.php b/src/Node/MacroNode.php index 614a0e535d1..255ee851099 100644 --- a/src/Node/MacroNode.php +++ b/src/Node/MacroNode.php @@ -17,22 +17,32 @@ use Twig\Node\Expression\ArrayExpression; use Twig\Node\Expression\TempNameExpression; use Twig\Node\Expression\Variable\LocalVariable; +use Twig\TwigMacro; /** * Represents a macro node. * + * This class is considered final as of Twig 3.29 and will be final in Twig + * 4.0. + * + * @final since Twig 3.29 + * * @author Fabien Potencier */ #[YieldReady] class MacroNode extends Node { + // 4.0: "varargs" is only the implicit, deprecated extra-arguments bucket. Once the + // implicit bucket is removed, this constant and the reserved-name guard in the + // constructor go away (a regular argument may then be named "varargs"); only an + // explicit "...name" variadic remains. public const VARARGS_NAME = 'varargs'; /** * @param BodyNode $body * @param ArrayExpression $arguments */ - public function __construct(string $name, Node $body, Node $arguments, int $lineno) + public function __construct(string $name, Node $body, Node $arguments, int $lineno, ?string $variadicName = null) { if (!$body instanceof BodyNode) { trigger_deprecation('twig/twig', '3.12', \sprintf('Not passing a "%s" instance as the "body" argument of the "%s" constructor is deprecated ("%s" given).', BodyNode::class, static::class, $body::class)); @@ -54,53 +64,69 @@ public function __construct(string $name, Node $body, Node $arguments, int $line if (TempNameExpression::RESERVED_NAME_PREFIX.self::VARARGS_NAME === $argName) { throw new SyntaxError(\sprintf('The argument "%s" in macro "%s" cannot be defined because the variable "%s" is reserved for arbitrary arguments.', self::VARARGS_NAME, $name, self::VARARGS_NAME), $pair['value']->getTemplateLine(), $pair['value']->getSourceContext()); } + if (null !== $variadicName && $variadicName === $this->stripReservedPrefix($argName)) { + throw new SyntaxError(\sprintf('The variadic argument "%s" in macro "%s" cannot have the same name as another argument.', $variadicName, $name), $pair['value']->getTemplateLine(), $pair['value']->getSourceContext()); + } if (isset($seen[$argName])) { throw new SyntaxError(\sprintf('Argument "%s" is defined twice for macro "%s".', $this->stripReservedPrefix($argName), $name), $pair['value']->getTemplateLine(), $pair['value']->getSourceContext()); } $seen[$argName] = true; } - parent::__construct(['body' => $body, 'arguments' => $arguments], ['name' => $name], $lineno); + parent::__construct(['body' => $body, 'arguments' => $arguments], ['name' => $name, 'variadic_name' => $variadicName], $lineno); } public function compile(Compiler $compiler): void { - $compiler - ->addDebugInfo($this) - ->write(\sprintf('public function macro_%s(', $this->getAttribute('name'))) - ; + // 4.0 cleanup: only an explicitly declared variadic ("...name") gets a trailing + // "...$bucket" parameter and a context entry; a non-variadic macro must NOT emit + // "...$varargs" anymore (so extra arguments raise an error), and the implicit + // "varargs" context entry and VARARGS_NAME handling below go away. + $variadicName = $this->getAttribute('variadic_name'); + if (null === $variadicName) { + // Legacy implicit "varargs" bucket: to be removed in 4.0. + $bucketName = self::VARARGS_NAME; + $bucketVar = self::VARARGS_NAME; + } else { + $bucketName = $variadicName; + $bucketVar = \in_array($variadicName, TempNameExpression::RESERVED_NAMES, true) ? TempNameExpression::RESERVED_NAME_PREFIX.$variadicName : $variadicName; + } /** @var ArrayExpression $arguments */ $arguments = $this->getNode('arguments'); + + $compiler + ->raw('new \\'.TwigMacro::class.'(') + ->string($this->getAttribute('name')) + ->raw(', function (') + ; + foreach ($arguments->getKeyValuePairs() as $pair) { - $name = $pair['key']; - $default = $pair['value']; $compiler - ->subcompile($name) + ->subcompile($pair['key']) ->raw(' = ') - ->subcompile($default) + ->subcompile($pair['value']) ->raw(', ') ; } $compiler - ->raw('...$varargs') - ->raw("): string|Markup\n") - ->write("{\n") + ->raw('...$'.$bucketVar) + ->raw("): string|Markup {\n") ->indent() + ->addDebugInfo($this) ->write("\$macros = \$this->macros;\n") ->write("\$context = [\n") ->indent() ; foreach ($arguments->getKeyValuePairs() as $pair) { - $name = $pair['key']; - $var = $this->stripReservedPrefix($name->getAttribute('name')); + $var = $this->stripReservedPrefix($pair['key']->getAttribute('name')); $compiler ->write('') ->string($var) ->raw(' => ') - ->subcompile($name) + ->subcompile($pair['key']) ->raw(",\n") ; } @@ -109,9 +135,9 @@ public function compile(Compiler $compiler): void $compiler ->write('') - ->string(self::VARARGS_NAME) + ->string($bucketName) ->raw(' => ') - ->raw("\$varargs,\n") + ->raw('$'.$bucketVar.",\n") ->outdent() ->write("] + \$this->env->getGlobals();\n\n") ->write("\$blocks = [];\n\n") @@ -119,7 +145,18 @@ public function compile(Compiler $compiler): void ->subcompile($node) ->raw("\n") ->outdent() - ->write("}\n\n") + ->write('}, ') + ; + + $signature = []; + foreach ($arguments->getKeyValuePairs() as $pair) { + $default = $pair['value']; + $signature[$this->stripReservedPrefix($pair['key']->getAttribute('name'))] = !($default->hasAttribute('is_implicit') && $default->getAttribute('is_implicit')); + } + + $compiler + ->repr($signature) + ->raw(', '.(null !== $variadicName ? 'true' : 'false').')') ; } diff --git a/src/Node/MacrosNode.php b/src/Node/MacrosNode.php new file mode 100644 index 00000000000..d54c7f863ea --- /dev/null +++ b/src/Node/MacrosNode.php @@ -0,0 +1,81 @@ + + */ +#[YieldReady] +final class MacrosNode extends Node +{ + /** + * @param array $macros + */ + public function __construct(array $macros = []) + { + foreach ($macros as $name => $macro) { + if (!$macro instanceof MacroNode) { + throw new \InvalidArgumentException(\sprintf('Using "%s" for the macro "%s" of "%s" is not supported. You must pass a "%s" instance.', get_debug_type($macro), $name, static::class, MacroNode::class)); + } + } + + parent::__construct($macros); + } + + public function setNode(string $name, Node $node): void + { + if (!$node instanceof MacroNode) { + throw new \LogicException(\sprintf('A "%s" can only contain "%s" nodes; replacing the macro "%s" with a "%s" node is not supported.', static::class, MacroNode::class, $name, get_debug_type($node))); + } + + parent::setNode($name, $node); + } + + public function compile(Compiler $compiler): void + { + if (!\count($this)) { + return; + } + + $compiler + ->write("protected function loadDeclaredMacros(): array\n", "{\n") + ->indent() + ->write("return [\n") + ->indent() + ; + + /** @var MacroNode $macro */ + foreach ($this as $macro) { + $compiler + ->write('') + ->string($macro->getAttribute('name')) + ->raw(' => ') + ; + $compiler->subcompile($macro); + $compiler->raw(",\n"); + } + + $compiler + ->outdent() + ->write("];\n") + ->outdent() + ->write("}\n\n") + ; + } +} diff --git a/src/Node/ModuleNode.php b/src/Node/ModuleNode.php index b06ece8020e..ad67886a282 100644 --- a/src/Node/ModuleNode.php +++ b/src/Node/ModuleNode.php @@ -31,13 +31,19 @@ final class ModuleNode extends Node implements CoercesChildrenToStringInterface { /** - * @param BodyNode $body + * @param BodyNode $body + * @param MacrosNode $macros */ public function __construct(Node $body, ?AbstractExpression $parent, Node $blocks, Node $macros, Node $traits, $embeddedTemplates, Source $source) { if (!$body instanceof BodyNode) { trigger_deprecation('twig/twig', '3.12', \sprintf('Not passing a "%s" instance as the "body" argument of the "%s" constructor is deprecated.', BodyNode::class, static::class)); } + if (!$macros instanceof MacrosNode) { + trigger_deprecation('twig/twig', '3.29', \sprintf('Not passing a "%s" instance as the "macros" argument of the "%s" constructor is deprecated.', MacrosNode::class, static::class)); + + $macros = new MacrosNode(iterator_to_array($macros)); + } if (!$embeddedTemplates instanceof Node) { trigger_deprecation('twig/twig', '3.21', \sprintf('Not passing a "%s" instance as the "embedded_templates" argument of the "%s" constructor is deprecated.', Node::class, static::class)); @@ -167,6 +173,7 @@ protected function compileClassHeader(Compiler $compiler): void ->write("use Twig\Error\RuntimeError;\n") ->write("use Twig\Extension\CoreExtension;\n") ->write("use Twig\Extension\SandboxExtension;\n") + ->write("use Twig\MacroNamespace;\n") ->write("use Twig\Markup;\n") ->write("use Twig\Sandbox\SecurityError;\n") ->write("use Twig\Sandbox\SecurityNotAllowedTagError;\n") @@ -188,7 +195,7 @@ protected function compileClassHeader(Compiler $compiler): void ->indent() ->write("private Source \$source;\n") ->write("/**\n") - ->write(" * @var array\n") + ->write(" * @var array\n") ->write(" */\n") ->write("private array \$macros = [];\n\n") ; diff --git a/src/Parser.php b/src/Parser.php index 89b9c42b7ea..ca45e58e5ed 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -27,6 +27,7 @@ use Twig\Node\Expression\Variable\AssignMacroVariable; use Twig\Node\Expression\Variable\MacroVariable; use Twig\Node\MacroNode; +use Twig\Node\MacrosNode; use Twig\Node\ModuleNode; use Twig\Node\Node; use Twig\Node\Nodes; @@ -128,7 +129,7 @@ public function parse(TokenStream $stream, $test = null, bool $dropNeedle = fals new BodyNode([$body]), $this->parent, $this->blocks ? new Nodes($this->blocks) : new EmptyNode(), - $this->macros ? new Nodes($this->macros) : new EmptyNode(), + new MacrosNode($this->macros), $this->traits ? new Nodes($this->traits) : new EmptyNode(), $this->embeddedTemplates ? new Nodes($this->embeddedTemplates) : new EmptyNode(), $stream->getSourceContext(), diff --git a/src/Template.php b/src/Template.php index 3d74460b0e3..530532a5d59 100644 --- a/src/Template.php +++ b/src/Template.php @@ -41,6 +41,7 @@ abstract class Template protected $sandbox; private $useYield; + private ?MacroNamespace $macroNamespace = null; public function __construct( protected Environment $env, @@ -84,8 +85,8 @@ public function getParent(array $context): self|TemplateWrapper|false // functions, method calls) when the parent name is dynamic. Make sure // the sandbox security check runs first so those expressions cannot // bypass the allow-list when getParent() is reached before the first - // ensureSecurityChecked() call on this template (e.g. via - // getTemplateForMacro() or yieldBlock() into a pre-warmed instance). + // ensureSecurityChecked() call on this template (e.g. via a macro call + // resolved against a parent, or yieldBlock() into a pre-warmed instance). $this->ensureSecurityChecked(); if (!$parent = $this->doGetParent($context)) { @@ -506,37 +507,20 @@ public function yieldParentBlock($name, array $context, array $blocks = []): ite } } - protected function hasMacro(string $name, array $context): bool + /** + * @internal + */ + public function getMacroNamespace(): MacroNamespace { - if (method_exists($this, $name)) { - return true; - } - - if (!$parent = $this->getParent($context)) { - return false; - } - - return $parent->hasMacro($name, $context); + return $this->macroNamespace ??= new MacroNamespace($this, $this->loadDeclaredMacros()); } - protected function getTemplateForMacro(string $name, array $context, int $line, Source $source): self + /** + * @return array + */ + protected function loadDeclaredMacros(): array { - if (method_exists($this, $name)) { - $this->ensureSecurityChecked(); - - return $this; - } - - $parent = $this; - while ($parent = $parent->getParent($context)) { - if (method_exists($parent, $name)) { - $parent->ensureSecurityChecked(); - - return $parent; - } - } - - throw new RuntimeError(\sprintf('Macro "%s" is not defined in template "%s".', substr($name, \strlen('macro_')), $this->getTemplateName()), $line, $source); + return []; } /** diff --git a/src/Test/IntegrationTestCase.php b/src/Test/IntegrationTestCase.php index fe3184f5a48..d76a22cc80b 100644 --- a/src/Test/IntegrationTestCase.php +++ b/src/Test/IntegrationTestCase.php @@ -320,8 +320,21 @@ protected function doIntegrationTest($file, $message, $condition, $templateSourc } $template = $templates['index.twig']; + $captureRenderDeprecations = '' !== $deprecation; try { + if ($captureRenderDeprecations) { + $prevHandler = set_error_handler(static function ($type, $msg, $file, $line, $context = []) use (&$deprecations, &$prevHandler) { + if (\E_USER_DEPRECATED === $type) { + $deprecations[] = $msg; + + return true; + } + + return $prevHandler ? $prevHandler($type, $msg, $file, $line, $context) : false; + }); + } + $output = trim($template->render(eval($match[1].';')), "\n "); } catch (\Exception $e) { if (false !== $exception) { @@ -333,6 +346,10 @@ protected function doIntegrationTest($file, $message, $condition, $templateSourc $e = new Error(\sprintf('%s: %s', $e::class, $e->getMessage()), -1, null, $e); $output = trim(\sprintf('%s: %s', $e::class, $e->getMessage())); + } finally { + if ($captureRenderDeprecations) { + restore_error_handler(); + } } if (false !== $exception) { diff --git a/src/TokenParser/FromTokenParser.php b/src/TokenParser/FromTokenParser.php index af58e0b157f..791072ae76c 100644 --- a/src/TokenParser/FromTokenParser.php +++ b/src/TokenParser/FromTokenParser.php @@ -55,8 +55,13 @@ public function parse(Token $token): Node $internalRef = new AssignMacroVariable(new MacroVariable(null, $token->getLine()), $this->parser->isMainScope()); $node = new ImportNode($macro, $internalRef, $token->getLine()); + // Each alias is registered as a "function" imported symbol mapping the local name + // to the macro name and the internal macro variable: a bare call like "my_macro()" + // is syntactically a function call, so FunctionExpressionParser resolves it through + // this symbol into a MacroReferenceExpression. From there, "from" and "import" calls + // share the same runtime path (MacroNamespace::call()). foreach ($targets as $name => $alias) { - $this->parser->addImportedSymbol('function', $alias->getAttribute('name'), 'macro_'.$name, $internalRef); + $this->parser->addImportedSymbol('function', $alias->getAttribute('name'), $name, $internalRef); } return $node; diff --git a/src/TokenParser/MacroTokenParser.php b/src/TokenParser/MacroTokenParser.php index 7531fe02fe0..9168caa6d8c 100644 --- a/src/TokenParser/MacroTokenParser.php +++ b/src/TokenParser/MacroTokenParser.php @@ -13,12 +13,13 @@ use Twig\Error\SyntaxError; use Twig\Node\BodyNode; -use Twig\Node\ConfigNode; use Twig\Node\Expression\ArrayExpression; use Twig\Node\Expression\ConstantExpression; +use Twig\Node\Expression\TempNameExpression; use Twig\Node\Expression\Unary\NegUnary; use Twig\Node\Expression\Unary\PosUnary; use Twig\Node\Expression\Variable\LocalVariable; +use Twig\Node\MacroDeclarationNode; use Twig\Node\MacroNode; use Twig\Node\Node; use Twig\Token; @@ -39,7 +40,7 @@ public function parse(Token $token): Node $lineno = $token->getLine(); $stream = $this->parser->getStream(); $name = $stream->expect(Token::NAME_TYPE)->getValue(); - $arguments = $this->parseDefinition(); + [$arguments, $variadicName] = $this->parseDefinition($name); $stream->expect(Token::BLOCK_END_TYPE); $this->parser->pushLocalScope(); @@ -54,9 +55,9 @@ public function parse(Token $token): Node $this->parser->popLocalScope(); $stream->expect(Token::BLOCK_END_TYPE); - $this->parser->setMacro($name, new MacroNode($name, new BodyNode([$body]), $arguments, $lineno)); + $this->parser->setMacro($name, new MacroNode($name, new BodyNode([$body]), $arguments, $lineno, $variadicName)); - return new ConfigNode($lineno); + return new MacroDeclarationNode($name, $lineno); } public function decideBlockEnd(Token $token): bool @@ -69,9 +70,13 @@ public function getTag(): string return 'macro'; } - private function parseDefinition(): ArrayExpression + /** + * @return array{ArrayExpression, string|null} + */ + private function parseDefinition(string $macroName): array { $arguments = new ArrayExpression([], $this->parser->getCurrentToken()->getLine()); + $variadicName = null; $stream = $this->parser->getStream(); $stream->expect(Token::OPERATOR_TYPE, '(', 'A list of arguments must begin with an opening parenthesis'); while (!$stream->test(Token::PUNCTUATION_TYPE, ')')) { @@ -84,6 +89,22 @@ private function parseDefinition(): ArrayExpression } } + if ($stream->nextIf(Token::OPERATOR_TYPE, '...')) { + $token = $stream->expect(Token::NAME_TYPE, null, 'A variadic argument must be a name'); + $variadicName = (new LocalVariable($token->getValue(), $token->getLine()))->getAttribute('name'); + if (str_starts_with($variadicName, TempNameExpression::RESERVED_NAME_PREFIX)) { + $variadicName = substr($variadicName, \strlen(TempNameExpression::RESERVED_NAME_PREFIX)); + } + if ($stream->test(Token::OPERATOR_TYPE, '=')) { + throw new SyntaxError(\sprintf('The variadic argument "%s" in macro "%s" cannot have a default value.', $variadicName, $macroName), $token->getLine(), $stream->getSourceContext()); + } + if ($stream->nextIf(Token::PUNCTUATION_TYPE, ',') && !$stream->test(Token::PUNCTUATION_TYPE, ')')) { + throw new SyntaxError(\sprintf('The variadic argument "%s" in macro "%s" must be the last one.', $variadicName, $macroName), $token->getLine(), $stream->getSourceContext()); + } + + break; + } + $token = $stream->expect(Token::NAME_TYPE, null, 'An argument must be a name'); $name = new LocalVariable($token->getValue(), $this->parser->getCurrentToken()->getLine()); if ($token = $stream->nextIf(Token::OPERATOR_TYPE, '=')) { @@ -100,7 +121,7 @@ private function parseDefinition(): ArrayExpression } $stream->expect(Token::PUNCTUATION_TYPE, ')', 'A list of arguments must be closed by a parenthesis'); - return $arguments; + return [$arguments, $variadicName]; } // checks that the node only contains "constant" elements diff --git a/src/TwigMacro.php b/src/TwigMacro.php new file mode 100644 index 00000000000..70e11e7c174 --- /dev/null +++ b/src/TwigMacro.php @@ -0,0 +1,186 @@ + + * + * @internal This class is an implementation detail of the per-template macro + * registry: compiled templates instantiate it, but it is not part of + * the public API, neither on 3.x nor in 4.0. + */ +final class TwigMacro +{ + /** + * @var array + */ + private array $argumentIndexes = []; + + /** + * @var array + */ + private array $requiredNames = []; + + /** + * Twig argument names that do not match their compiled closure parameter name + * (reserved names get prefixed by the compiler). Only needed by callLegacy()'s + * native binding; to be removed in 4.0 together with it. + * + * @var array + */ + private array $renamedArguments = []; + + private int $requiredCount = 0; + + /** + * @param \Closure(mixed...): (string|Markup) $body The compiled macro body, invoked with the bound arguments in definition order + * @param array $arguments The declared argument names mapped to whether they have a default value, in definition order + */ + public function __construct( + private string $name, + private \Closure $body, + private array $arguments = [], + private bool $variadic = false, + ) { + $i = 0; + foreach ($arguments as $argName => $hasDefault) { + $this->argumentIndexes[$argName] = $i; + if (\in_array($argName, TempNameExpression::RESERVED_NAMES, true)) { + $this->renamedArguments[$argName] = TempNameExpression::RESERVED_NAME_PREFIX.$argName; + } + if (!$hasDefault) { + $this->requiredCount = $i + 1; + $this->requiredNames[$argName] = true; + } + ++$i; + } + } + + /** + * Invokes the macro the way Twig 3.x always has (lenient argument handling), + * while reporting the cases that will become errors in Twig 4.0. + * + * In 4.0, this becomes the strict call() method: the scan below already detects + * every deprecated case, so triggerLegacyDeprecations() turns into upfront + * throws and the rest stays as is. + * + * @param array $arguments Positional arguments keyed by their integer position + * and named arguments keyed by their name + * @param Source $source The source of the template making the call, used to enrich errors + * @param int $lineno The line of the call in that template, used to enrich errors + */ + public function callLegacy(array $arguments, Source $source, int $lineno): string|Markup + { + // Spreading the arguments as-is is equivalent to resolving them: PHP binds + // them natively, maps the named ones onto their parameter, fills the + // defaults, and collects the extra ones into the variadic bucket. Only the + // cases where PHP would report its own error (with closure-centric wording) + // are detected upfront and turned into Twig errors. + if (array_is_list($arguments)) { + $count = \count($arguments); + if ($count < $this->requiredCount || (!$this->variadic && $count > \count($this->arguments))) { + $this->triggerLegacyDeprecations($arguments, $count, $source, $lineno); + } + + return ($this->body)(...$arguments); + } + + $positionalCount = 0; + $namedRequired = 0; + $sawNamed = false; + $misordered = false; + $duplicate = null; + $hasUnknownNamed = false; + foreach ($arguments as $key => $value) { + if (\is_int($key)) { + $misordered = $misordered || $sawNamed; + ++$positionalCount; + } else { + $sawNamed = true; + if (null === $i = $this->argumentIndexes[$key] ?? null) { + $hasUnknownNamed = true; + } else { + if (null === $duplicate && $i < $positionalCount) { + $duplicate = $key; + } + if (isset($this->requiredNames[$key])) { + ++$namedRequired; + } + } + } + } + + if ($misordered) { + throw new RuntimeError(\sprintf('Positional arguments cannot be used after named arguments for macro "%s".', $this->name), $lineno, $source); + } + if (null !== $duplicate) { + throw new RuntimeError(\sprintf('Argument "%s" is defined twice for macro "%s".', $duplicate, $this->name), $lineno, $source); + } + + // For a fully named call, the coverage of the required arguments is exact; a + // mixed call falls back to the precise (and slower) per-argument check. + $mayMissRequired = 0 === $positionalCount + ? $namedRequired < \count($this->requiredNames) + : $positionalCount < $this->requiredCount; + + if ($mayMissRequired || (!$this->variadic && ($hasUnknownNamed || $positionalCount > \count($this->arguments)))) { + $this->triggerLegacyDeprecations($arguments, $positionalCount, $source, $lineno); + } + + foreach ($this->renamedArguments as $name => $parameterName) { + if (\array_key_exists($name, $arguments)) { + $arguments[$parameterName] = $arguments[$name]; + unset($arguments[$name]); + } + } + + return ($this->body)(...$arguments); + } + + /** + * To be removed in 4.0 (these deprecated cases become hard errors). + * + * @param array $arguments The full argument array; string keys are the named arguments + */ + private function triggerLegacyDeprecations(array $arguments, int $positionalCount, Source $source, int $lineno): void + { + if ($positionalCount < $this->requiredCount) { + foreach ($this->requiredNames as $argName => $required) { + if ($this->argumentIndexes[$argName] < $positionalCount || \array_key_exists($argName, $arguments)) { + continue; + } + + trigger_deprecation('twig/twig', '3.29', 'Not passing a value for the "%s" argument of macro "%s" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "%s" at line %d).', $argName, $this->name, $source->getName(), $lineno); + } + } + + if ($this->variadic) { + return; + } + + if ($positionalCount > \count($this->arguments)) { + trigger_deprecation('twig/twig', '3.29', 'Passing more arguments than the macro "%s" accepts is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept extra arguments (in "%s" at line %d).', $this->name, $source->getName(), $lineno); + } + + foreach ($arguments as $name => $value) { + if (\is_string($name) && !isset($this->argumentIndexes[$name])) { + trigger_deprecation('twig/twig', '3.29', 'Passing the unknown named argument "%s" to the macro "%s" is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept it (in "%s" at line %d).', $name, $this->name, $source->getName(), $lineno); + } + } + } +} diff --git a/tests/CallMacroTest.php b/tests/CallMacroTest.php new file mode 100644 index 00000000000..abd4553dc58 --- /dev/null +++ b/tests/CallMacroTest.php @@ -0,0 +1,234 @@ +load([ + 'index' => '{% macro greet(name, greeting = "Hello") %}{{ greeting }} {{ name }}{% endmacro %}', + ]); + + $this->assertSame('Hello World', (string) $this->callMacro($template, 'greet', ['World'])); + $this->assertSame('Hi World', (string) $this->callMacro($template, 'greet', ['name' => 'World', 'greeting' => 'Hi'])); + } + + public function testMacroNamespaceOnlyExposesMacroOperations(): void + { + $template = $this->load(['index' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}']); + $namespace = $template->getMacroNamespace(); + + $this->assertSame(['__construct', 'has', 'call'], array_map(static fn (\ReflectionMethod $method): string => $method->getName(), (new \ReflectionClass($namespace))->getMethods(\ReflectionMethod::IS_PUBLIC))); + $this->assertTrue($namespace->has('greet', [])); + $this->assertSame('Hi World', (string) $namespace->call('greet', ['World'], [], 1, new Source('', 'index'))); + $this->assertSame($namespace, $template->getMacroNamespace()); + $this->assertInstanceOf(MacroNamespace::class, $namespace); + } + + public function testMacroNamespaceDoesNotBenefitFromTheTemplateSandboxExemption(): void + { + $policy = new SecurityPolicy(); + + $this->expectException(SecurityNotAllowedMethodError::class); + $this->expectExceptionMessage('Calling "call" method on a "Twig\\MacroNamespace" object is not allowed.'); + + $policy->checkMethodAllowed($this->load(['index' => ''])->getMacroNamespace(), 'call'); + } + + public function testCallMacroLooksMacrosUpInParentTemplates(): void + { + $template = $this->load([ + 'index' => '{% extends "parent" %}', + 'parent' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}', + ]); + + $this->assertSame('Hi World', (string) $this->callMacro($template, 'greet', ['World'])); + } + + public function testLenientMacroCallReportsADeprecationAtTheCallSite(): void + { + $twig = new Environment(new ArrayLoader([ + 'index' => "{% from _self import greet %}\n{% macro greet(name) %}{% endmacro %}\n{{ greet('a', 'b') }}", + ])); + + $deprecations = $this->collectDeprecations(static fn () => $twig->render('index')); + + $this->assertSame([ + 'Since twig/twig 3.29: Passing more arguments than the macro "greet" accepts is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept extra arguments (in "index" at line 3).', + ], $deprecations); + } + + public function testCallingAMacroWithACaseMismatchedNameTriggersADeprecation(): void + { + $twig = new Environment(new ArrayLoader([ + 'index' => "{% import _self as m %}\n{% macro greet(name = '') %}Hi {{ name }}{% endmacro %}\n{{ m.GREET('World') }}", + ])); + + $output = null; + $deprecations = $this->collectDeprecations(static function () use ($twig, &$output) { + $output = $twig->render('index'); + }); + + $this->assertSame('Hi World', trim($output)); + $this->assertSame([ + 'Since twig/twig 3.29: Calling the macro "greet" (defined in template "index") as "GREET" is deprecated; macro names will be case-sensitive in Twig 4.0.', + ], $deprecations); + } + + public function testCallMacroLooksCaseMismatchedMacrosUpInParentTemplates(): void + { + $template = $this->load([ + 'index' => '{% extends "parent" %}', + 'parent' => '{% macro greet(name = "") %}Hi {{ name }}{% endmacro %}', + ]); + + $output = null; + $deprecations = $this->collectDeprecations(static function () use ($template, &$output) { + $output = (string) $template->getMacroNamespace()->call('Greet', ['World'], [], 1, new Source('', 'index')); + }); + + $this->assertSame('Hi World', $output); + $this->assertSame([ + 'Since twig/twig 3.29: Calling the macro "greet" (defined in template "parent") as "Greet" is deprecated; macro names will be case-sensitive in Twig 4.0.', + ], $deprecations); + } + + public function testHasMacroDeprecatesACaseMismatchedMatchOnly(): void + { + $template = $this->load(['index' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}']); + + $deprecations = $this->collectDeprecations(function () use ($template) { + $namespace = $template->getMacroNamespace(); + $this->assertTrue($namespace->has('greet', [])); + $this->assertTrue($namespace->has('GREET', [])); + $this->assertFalse($namespace->has('missing', [])); + }); + + $this->assertSame([ + 'Since twig/twig 3.29: Testing whether the macro "greet" (defined in template "index") is defined as "GREET" is deprecated; macro names will be case-sensitive in Twig 4.0 and this test will return false.', + ], $deprecations, 'The "is defined" test must deprecate only a case-mismatched macro name.'); + } + + public function testCallMacroResolvesALegacyPrefixedNameWithADeprecation(): void + { + $template = $this->load(['index' => '{% macro greet(name = "") %}Hi {{ name }}{% endmacro %}']); + + $output = null; + $deprecations = $this->collectDeprecations(static function () use ($template, &$output) { + $output = (string) $template->getMacroNamespace()->call('macro_greet', ['World'], [], 1, new Source('', 'index')); + }); + + $this->assertSame('Hi World', $output); + $this->assertSame([ + 'Since twig/twig 3.29: Calling the macro "greet" via the "macro_"-prefixed name "macro_greet" is deprecated; pass the bare macro name to "Twig\Node\Expression\MacroReferenceExpression" instead.', + ], $deprecations); + } + + public function testCallMacroPrefersAMacroActuallyNamedWithThePrefix(): void + { + $template = $this->load(['index' => '{% macro macro_greet(name = "") %}Prefixed {{ name }}{% endmacro %}{% macro greet(name = "") %}Bare {{ name }}{% endmacro %}']); + + $output = null; + $deprecations = $this->collectDeprecations(static function () use ($template, &$output) { + $output = (string) $template->getMacroNamespace()->call('macro_greet', ['World'], [], 1, new Source('', 'index')); + }); + + $this->assertSame('Prefixed World', $output); + $this->assertSame([], $deprecations); + } + + public function testCallMacroThrowsWithThePrefixedNameWhenNeitherNameExists(): void + { + $template = $this->load(['index' => 'no macro here']); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('Macro "macro_missing" is not defined in template "index"'); + + $template->getMacroNamespace()->call('macro_missing', [], [], 1, new Source('', 'index')); + } + + public function testHasMacroResolvesALegacyPrefixedNameSilently(): void + { + $template = $this->load(['index' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}']); + + $deprecations = $this->collectDeprecations(function () use ($template) { + $namespace = $template->getMacroNamespace(); + $this->assertTrue($namespace->has('macro_greet', [])); + $this->assertFalse($namespace->has('macro_missing', [])); + }); + + $this->assertSame([], $deprecations); + } + + public function testCallMacroThrowsForAnUnknownMacro(): void + { + $template = $this->load(['index' => 'no macro here']); + + $this->expectException(RuntimeError::class); + $this->expectExceptionMessage('Macro "missing" is not defined in template "index"'); + + $template->getMacroNamespace()->call('missing', [], [], 1, new Source('', 'index')); + } + + public function testDeprecatedCoreExtensionCallMacroAcceptsMacroNamespace(): void + { + $template = $this->load(['index' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}']); + + $this->assertSame('Hi Bob', (string) CoreExtension::callMacro($template->getMacroNamespace(), 'macro_greet', ['Bob'], 1, [], new Source('', 'index'))); + } + + private function callMacro(Template $template, string $name, array $arguments): mixed + { + return $template->getMacroNamespace()->call($name, $arguments, [], 1, new Source('', 'index')); + } + + private function collectDeprecations(callable $fn): array + { + $deprecations = []; + set_error_handler(static function ($type, $message) use (&$deprecations) { + if (\E_USER_DEPRECATED === $type) { + $deprecations[] = $message; + + return true; + } + + return false; + }); + + try { + $fn(); + } finally { + restore_error_handler(); + } + + return $deprecations; + } + + private function load(array $templates): Template + { + $twig = new Environment(new ArrayLoader($templates)); + + return $twig->load('index')->unwrap(); + } +} diff --git a/tests/Extension/SandboxStateChangeTest.php b/tests/Extension/SandboxStateChangeTest.php index 2b3558780c9..0b6af5c1958 100644 --- a/tests/Extension/SandboxStateChangeTest.php +++ b/tests/Extension/SandboxStateChangeTest.php @@ -271,9 +271,9 @@ public function testFunctionBypassThroughPreWarmedParent(): void public function testDynamicParentFilterRejectedWhenReachedViaMacroImport(): void { - // Regression: getTemplateForMacro() walks getParent() to find the - // macro on a parent template. When the imported template has a - // dynamic {% extends %}, doGetParent() evaluates the user expression. + // Regression: a macro call walks getParent() to find the macro on a + // parent template. When the imported template has a dynamic + // {% extends %}, doGetParent() evaluates the user expression. // The sandbox security check must run on the imported template // *before* doGetParent() executes, otherwise a forbidden filter on // the parent name escapes the allow-list. diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index c14c42d2e1f..12f6f6349b0 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -1257,7 +1257,7 @@ public function testMacrosInASandbox(): void public function testSelfMacroReferenceWithStringLiteralDoesNotInjectPhp(): void { - $twig = $this->getEnvironment(true, [], ['index' => '{{ _self.(\'foo + 1; trigger_error("BAD-MACRO-REF") //\') }}']); + $twig = $this->getEnvironment(true, [], ['index' => '{{ _self.(\'foo + 1; trigger_error("BAD-MACRO-REF") //\')() }}']); $compiled = $twig->compileSource($twig->getLoader()->getSourceContext('index')); $this->assertStringNotContainsString('trigger_error("BAD-MACRO-REF")', $compiled, 'Attacker-controlled string must not appear raw in compiled PHP source.'); @@ -1285,7 +1285,7 @@ public function testSelfMacroReferenceWithStringLiteralDoesNotInjectPhp(): void public function testImportedTemplateMacroReferenceWithBadIdentifierDoesNotInjectPhp(): void { - $payload = '{% import "m" as m %}{{ m.(\'foo + 1; trigger_error("BAD-IMPORT-REF") //\') }}'; + $payload = '{% import "m" as m %}{{ m.(\'foo + 1; trigger_error("BAD-IMPORT-REF") //\')() }}'; $twig = $this->getEnvironment(true, [], [ 'index' => $payload, 'm' => '{% macro greet() %}hi{% endmacro %}', diff --git a/tests/Fixtures/macros/call_without_parentheses.legacy.test b/tests/Fixtures/macros/call_without_parentheses.legacy.test index bdb93d6563d..b90f6d32408 100644 --- a/tests/Fixtures/macros/call_without_parentheses.legacy.test +++ b/tests/Fixtures/macros/call_without_parentheses.legacy.test @@ -1,8 +1,8 @@ --TEST-- Omitting parentheses when calling macros is deprecated --DEPRECATION-- -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 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. --TEMPLATE-- {% import _self as macros %} {% set name = 'hello' %} diff --git a/tests/Fixtures/macros/constant_name_non_identifier.test b/tests/Fixtures/macros/constant_name_non_identifier.test new file mode 100644 index 00000000000..cde672b5bc3 --- /dev/null +++ b/tests/Fixtures/macros/constant_name_non_identifier.test @@ -0,0 +1,10 @@ +--TEST-- +macro called with a constant name that is not a valid identifier +--TEMPLATE-- +{% import _self as macros %} + +{{ macros.('foo-bar')() }} +--DATA-- +return [] +--EXCEPTION-- +Twig\Error\RuntimeError: Macro "foo-bar" is not defined in template "index.twig" in "index.twig" at line 4. diff --git a/tests/Fixtures/macros/dynamic_name_non_string.test b/tests/Fixtures/macros/dynamic_name_non_string.test new file mode 100644 index 00000000000..3cdc002999d --- /dev/null +++ b/tests/Fixtures/macros/dynamic_name_non_string.test @@ -0,0 +1,9 @@ +--TEST-- +macro called with a dynamic name coerces the name before lookup +--TEMPLATE-- +{% import _self as macros %} +{{ macros.(name)() }} +--DATA-- +return ['name' => null] +--EXCEPTION-- +Twig\Error\RuntimeError: Macro "" is not defined in template "index.twig" in "index.twig" at line 3. diff --git a/tests/Fixtures/macros/implicit_optional_arguments.legacy.test b/tests/Fixtures/macros/implicit_optional_arguments.legacy.test new file mode 100644 index 00000000000..6f6c2f782f1 --- /dev/null +++ b/tests/Fixtures/macros/implicit_optional_arguments.legacy.test @@ -0,0 +1,27 @@ +--TEST-- +macro arguments without a default value are implicitly optional (deprecated) +--DEPRECATION-- +Since twig/twig 3.29: Not passing a value for the "a" argument of macro "test" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "index.twig" at line 10). +Since twig/twig 3.29: Not passing a value for the "b" argument of macro "test" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "index.twig" at line 10). +Since twig/twig 3.29: Not passing a value for the "a" argument of macro "test" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "index.twig" at line 11). +Since twig/twig 3.29: Not passing a value for the "b" argument of macro "test" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "index.twig" at line 11). +--TEMPLATE-- +{% import _self as test %} +{% from _self import test %} + +{% macro test(a, b) -%} + {{ a|default('a') }}
+ {{- b|default('b') }}
+{%- endmacro %} + +{{ test.test() }} +{{ test() }} +{{ test.test(1, "c") }} +{{ test(1, "c") }} +--DATA-- +return [] +--EXPECT-- +a
b
+a
b
+1
c
+1
c
diff --git a/tests/Fixtures/macros/reserved_variables.test b/tests/Fixtures/macros/reserved_variables.test index 05dd9213008..1587f1d2ede 100644 --- a/tests/Fixtures/macros/reserved_variables.test +++ b/tests/Fixtures/macros/reserved_variables.test @@ -2,13 +2,18 @@ macro --TEMPLATE-- {% from _self import test %} +{% import _self as macros %} -{% macro test(this) -%} - {{ this }} +{% macro test(blocks) -%} + {{ blocks }} {%- endmacro %} -{{ test(this) }} +{{ test(blocks) }} +{{ test(blocks: blocks) }} +{{ macros.test(blocks: blocks) }} --DATA-- -return ['this' => 'foo'] +return ['blocks' => 'foo'] --EXPECT-- foo +foo +foo diff --git a/tests/Fixtures/macros/simple.test b/tests/Fixtures/macros/simple.test index 8fc6b477fb2..f6bd688f288 100644 --- a/tests/Fixtures/macros/simple.test +++ b/tests/Fixtures/macros/simple.test @@ -4,7 +4,7 @@ macro {% import _self as test %} {% from _self import test %} -{% macro test(a, b) -%} +{% macro test(a = null, b = null) -%} {{ a|default('a') }}
{{- b|default('b') }}
{%- endmacro %} diff --git a/tests/Fixtures/macros/varargs.test b/tests/Fixtures/macros/varargs.test index dd4b5c9f471..73104dfd6c3 100644 --- a/tests/Fixtures/macros/varargs.test +++ b/tests/Fixtures/macros/varargs.test @@ -3,11 +3,11 @@ macro with arbitrary arguments --TEMPLATE-- {% from _self import test1, test2 %} -{% macro test1(var) %} +{% macro test1(var, ...varargs) %} {{- var }}: {{ varargs|join(", ") }} {% endmacro %} -{% macro test2() %} +{% macro test2(...varargs) %} {{- varargs|join(", ") }} {% endmacro %} diff --git a/tests/Fixtures/macros/varargs_implicit.legacy.test b/tests/Fixtures/macros/varargs_implicit.legacy.test new file mode 100644 index 00000000000..0dbfadb138b --- /dev/null +++ b/tests/Fixtures/macros/varargs_implicit.legacy.test @@ -0,0 +1,24 @@ +--TEST-- +macro with arbitrary arguments collected in the implicit "varargs" variable (deprecated) +--DEPRECATION-- +Since twig/twig 3.29: Passing more arguments than the macro "test1" accepts is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept extra arguments (in "index.twig" at line 12). +Since twig/twig 3.29: Passing more arguments than the macro "test2" accepts is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept extra arguments (in "index.twig" at line 13). +--TEMPLATE-- +{% from _self import test1, test2 %} + +{% macro test1(var) %} + {{- var }}: {{ varargs|join(", ") }} +{% endmacro %} + +{% macro test2() %} + {{- varargs|join(", ") }} +{% endmacro %} + +{{ test1("foo", "bar", "foobar") }} +{{ test2("foo", "bar", "foobar") }} +--DATA-- +return [] +--EXPECT-- +foo: bar, foobar + +foo, bar, foobar diff --git a/tests/Fixtures/macros/variadic.test b/tests/Fixtures/macros/variadic.test new file mode 100644 index 00000000000..060d4adbb89 --- /dev/null +++ b/tests/Fixtures/macros/variadic.test @@ -0,0 +1,22 @@ +--TEST-- +macro with an explicit variadic argument +--TEMPLATE-- +{% from _self import tag, list %} + +{% macro tag(name, ...attributes) -%} + <{{ name }}{% for attr in attributes %} {{ attr }}{% endfor %}> +{%- endmacro %} + +{% macro list(...items) -%} + {{ items|join(", ") }} +{%- endmacro %} + +{{ tag("div", "a", "b") }} +{{ tag("br") }} +{{ list(1, 2, 3) }} +--DATA-- +return [] +--EXPECT-- +
+
+1, 2, 3 diff --git a/tests/Fixtures/macros/variadic_default_value.test b/tests/Fixtures/macros/variadic_default_value.test new file mode 100644 index 00000000000..12fcff20222 --- /dev/null +++ b/tests/Fixtures/macros/variadic_default_value.test @@ -0,0 +1,7 @@ +--TEST-- +macro variadic argument cannot have a default value +--TEMPLATE-- +{% macro test(...rest = []) %} +{% endmacro %} +--EXCEPTION-- +Twig\Error\SyntaxError: The variadic argument "rest" in macro "test" cannot have a default value in "index.twig" at line 2. diff --git a/tests/Fixtures/macros/variadic_duplicate.test b/tests/Fixtures/macros/variadic_duplicate.test new file mode 100644 index 00000000000..e151824d0cf --- /dev/null +++ b/tests/Fixtures/macros/variadic_duplicate.test @@ -0,0 +1,7 @@ +--TEST-- +macro variadic argument cannot reuse another argument name +--TEMPLATE-- +{% macro test(a, ...a) %} +{% endmacro %} +--EXCEPTION-- +Twig\Error\SyntaxError: The variadic argument "a" in macro "test" cannot have the same name as another argument in "index.twig" at line 2. diff --git a/tests/Fixtures/macros/variadic_invalid_name.test b/tests/Fixtures/macros/variadic_invalid_name.test new file mode 100644 index 00000000000..565cd8d0559 --- /dev/null +++ b/tests/Fixtures/macros/variadic_invalid_name.test @@ -0,0 +1,7 @@ +--TEST-- +macro variadic argument cannot use a reserved literal name +--TEMPLATE-- +{% macro test(...true) %} +{% endmacro %} +--EXCEPTION-- +Twig\Error\SyntaxError: You cannot assign a value to "true" in "index.twig" at line 2. diff --git a/tests/Fixtures/macros/variadic_named.test b/tests/Fixtures/macros/variadic_named.test new file mode 100644 index 00000000000..5c640f65572 --- /dev/null +++ b/tests/Fixtures/macros/variadic_named.test @@ -0,0 +1,16 @@ +--TEST-- +macro explicit variadic argument also collects named arguments +--TEMPLATE-- +{% from _self import attributes %} + +{% macro attributes(...attrs) -%} + {% for name, value in attrs %}{{ name }}={{ value }}{{ not loop.last ? ' ' }}{% endfor %} +{%- endmacro %} + +{{ attributes(id: "main", class: "box") }} +{{ attributes(blocks: "list", varargs: "extra") }} +--DATA-- +return [] +--EXPECT-- +id=main class=box +blocks=list varargs=extra diff --git a/tests/Fixtures/macros/variadic_not_last.test b/tests/Fixtures/macros/variadic_not_last.test new file mode 100644 index 00000000000..e56353d11b6 --- /dev/null +++ b/tests/Fixtures/macros/variadic_not_last.test @@ -0,0 +1,7 @@ +--TEST-- +macro variadic argument must be the last one +--TEMPLATE-- +{% macro test(a, ...rest, b) %} +{% endmacro %} +--EXCEPTION-- +Twig\Error\SyntaxError: The variadic argument "rest" in macro "test" must be the last one in "index.twig" at line 2. diff --git a/tests/Fixtures/macros/variadic_reserved_name.test b/tests/Fixtures/macros/variadic_reserved_name.test new file mode 100644 index 00000000000..7ddc628ba88 --- /dev/null +++ b/tests/Fixtures/macros/variadic_reserved_name.test @@ -0,0 +1,14 @@ +--TEST-- +macro with an explicit variadic argument using a reserved name +--TEMPLATE-- +{% from _self import row %} + +{% macro row(label, ...context) -%} + {{ label }}:{% for key, value in context %} {{ key }}={{ value }}{% endfor %} +{%- endmacro %} + +{{ row("attrs", 1, id: "main") }} +--DATA-- +return [] +--EXPECT-- +attrs: 0=1 id=main diff --git a/tests/Fixtures/tags/macro/basic.test b/tests/Fixtures/tags/macro/basic.test index ae090f9a069..ddf234672fa 100644 --- a/tests/Fixtures/tags/macro/basic.test +++ b/tests/Fixtures/tags/macro/basic.test @@ -6,7 +6,7 @@ {{ macros.input('username') }} {{ macros.input('password', null, 'password', 1) }} -{% macro input(name, value, type, size) %} +{% macro input(name, value = null, type = null, size = null) %} {% endmacro %} --DATA-- diff --git a/tests/Fixtures/tags/macro/external.test b/tests/Fixtures/tags/macro/external.test index b28ca19f029..28e67b76cc2 100644 --- a/tests/Fixtures/tags/macro/external.test +++ b/tests/Fixtures/tags/macro/external.test @@ -6,7 +6,7 @@ {{ forms.input('username') }} {{ forms.input('password', null, 'password', 1) }} --TEMPLATE(forms.twig)-- -{% macro input(name, value, type, size) %} +{% macro input(name, value = null, type = null, size = null) %} {% endmacro %} --DATA-- diff --git a/tests/Fixtures/tags/macro/global.test b/tests/Fixtures/tags/macro/global.test index 832740eac56..11e0f99fb9c 100644 --- a/tests/Fixtures/tags/macro/global.test +++ b/tests/Fixtures/tags/macro/global.test @@ -6,7 +6,7 @@ {{ foo('foo') }} {{ foo() }} --TEMPLATE(forms.twig)-- -{% macro foo(name) %}{{ name|default('foo') }}{{ global }}{% endmacro %} +{% macro foo(name = null) %}{{ name|default('foo') }}{{ global }}{% endmacro %} --DATA-- return [] --EXPECT-- diff --git a/tests/Fixtures/tags/macro/named_arguments.test b/tests/Fixtures/tags/macro/named_arguments.test index 58bd15b2024..89e7c5643a6 100644 --- a/tests/Fixtures/tags/macro/named_arguments.test +++ b/tests/Fixtures/tags/macro/named_arguments.test @@ -5,7 +5,7 @@ {{ forms.input(size: 10, name: 'username') }} -{% macro input(name, value, type, size) %} +{% macro input(name, value = null, type = null, size = null) %} {% endmacro %} --DATA-- diff --git a/tests/Fixtures/tags/macro/self_import.test b/tests/Fixtures/tags/macro/self_import.test index ca3157dd884..eae97618bfc 100644 --- a/tests/Fixtures/tags/macro/self_import.test +++ b/tests/Fixtures/tags/macro/self_import.test @@ -6,7 +6,7 @@ {{ forms.input('username') }} {{ forms.input('password', null, 'password', 1) }} -{% macro input(name, value, type, size) %} +{% macro input(name, value = null, type = null, size = null) %} {% endmacro %} --DATA-- diff --git a/tests/Fixtures/tests/defined_for_macros.test b/tests/Fixtures/tests/defined_for_macros.test index 657b43e1788..3893f13ba73 100644 --- a/tests/Fixtures/tests/defined_for_macros.test +++ b/tests/Fixtures/tests/defined_for_macros.test @@ -9,16 +9,12 @@ {% 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' }} @@ -30,13 +26,10 @@ {{~ 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 %} @@ -50,15 +43,11 @@ return [] --EXPECT-- OK -OK -OK OK -OK OK -OK OK OK @@ -70,11 +59,8 @@ OK OK OK -OK OK -OK OK OK -OK diff --git a/tests/Fixtures/tests/defined_for_macros_dynamic_name.test b/tests/Fixtures/tests/defined_for_macros_dynamic_name.test index 76b1a381070..474f40a4add 100644 --- a/tests/Fixtures/tests/defined_for_macros_dynamic_name.test +++ b/tests/Fixtures/tests/defined_for_macros_dynamic_name.test @@ -5,9 +5,7 @@ {% set known = 'hello' %} {% set unknown = 'missing' %} -{{~ macros.(known) is defined ? 'OK' : 'KO' }} {{~ macros.(known)() is defined ? 'OK' : 'KO' }} -{{~ macros.(unknown) is not defined ? 'OK' : 'KO' }} {{~ macros.(unknown)() is not defined ? 'OK' : 'KO' }} {% macro hello(name) %}{% endmacro %} @@ -16,5 +14,3 @@ return [] --EXPECT-- OK OK -OK -OK diff --git a/tests/Fixtures/tests/defined_for_macros_dynamic_name_without_parentheses.legacy.test b/tests/Fixtures/tests/defined_for_macros_dynamic_name_without_parentheses.legacy.test new file mode 100644 index 00000000000..47616ace12a --- /dev/null +++ b/tests/Fixtures/tests/defined_for_macros_dynamic_name_without_parentheses.legacy.test @@ -0,0 +1,17 @@ +--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 diff --git a/tests/Fixtures/tests/defined_for_macros_without_parentheses.legacy.test b/tests/Fixtures/tests/defined_for_macros_without_parentheses.legacy.test new file mode 100644 index 00000000000..d1146f161fa --- /dev/null +++ b/tests/Fixtures/tests/defined_for_macros_without_parentheses.legacy.test @@ -0,0 +1,15 @@ +--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 diff --git a/tests/Node/Expression/MacroReferenceTest.php b/tests/Node/Expression/MacroReferenceTest.php index 485e9014227..ecf60a7741b 100644 --- a/tests/Node/Expression/MacroReferenceTest.php +++ b/tests/Node/Expression/MacroReferenceTest.php @@ -11,7 +11,6 @@ namespace Twig\Tests\Node\Expression; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; use Twig\Environment; @@ -24,26 +23,12 @@ class MacroReferenceTest extends TestCase { - /** - * @dataProvider provideInvalidMacroNames - */ - #[DataProvider('provideInvalidMacroNames')] - public function testConstructorRejectsNonIdentifierName(string $name): void + public function testConstructorAcceptsAnExpressionAsName(): void { - $this->expectException(\LogicException::class); - $this->expectExceptionMessage(\sprintf('Macro name "%s" is not a valid PHP identifier.', $name)); - - new MacroReferenceExpression(new MacroVariable('foo', 1), $name, new ArrayExpression([], 1), 1); - } + $node = new MacroReferenceExpression(new MacroVariable('foo', 1), new ContextVariable('name', 1), new ArrayExpression([], 1), 1); - public static function provideInvalidMacroNames(): iterable - { - yield 'empty' => ['']; - yield 'starts with digit' => ['1foo']; - yield 'contains space' => ['foo bar']; - yield 'contains semicolon' => ['foo;bar']; - yield 'PHP injection payload' => ['macro_foo + 1; trigger_error("BAD") //']; - yield 'contains NUL byte' => ["foo\x00bar"]; + $this->assertTrue($node->hasNode('name')); + $this->assertNull($node->getAttribute('name')); } #[Group('legacy')] @@ -60,7 +45,7 @@ public function testConstructorAcceptsDeprecatedTemplateVariable(): void return false; }); try { - $node = new MacroReferenceExpression(new TemplateVariable('foo', 1), 'macro_foo', new ArrayExpression([], 1), 1); + $node = new MacroReferenceExpression(new TemplateVariable('foo', 1), 'foo', new ArrayExpression([], 1), 1); } finally { restore_error_handler(); } @@ -69,15 +54,7 @@ public function testConstructorAcceptsDeprecatedTemplateVariable(): void $this->assertSame(['Since twig/twig 3.29: The "Twig\\Node\\Expression\\Variable\\TemplateVariable" class is deprecated, use "Twig\\Node\\Expression\\Variable\\MacroVariable" instead.'], $deprecations); } - public function testConstructorAcceptsAnExpressionAsName(): void - { - $node = new MacroReferenceExpression(new MacroVariable('foo', 1), new ContextVariable('name', 1), new ArrayExpression([], 1), 1); - - $this->assertTrue($node->hasNode('name')); - $this->assertNull($node->getAttribute('name')); - } - - public function testDynamicNamePrefixesMacroAtRuntime(): void + public function testDynamicNameResolvesMacroAtRuntime(): void { $env = new Environment(new ArrayLoader()); $compiler = new \Twig\Compiler($env); @@ -90,7 +67,8 @@ public function testDynamicNamePrefixesMacroAtRuntime(): void ); $compiler->compile($node); - $this->assertStringContainsString("getTemplateForMacro(\$_v0 = 'macro_'.", $compiler->getSource()); - $this->assertStringContainsString('->{$_v0}(...', $compiler->getSource()); + $this->assertStringContainsString('->call(', $compiler->getSource()); + $this->assertStringContainsString('($context["name"] ?? null), [], $context, 1, $this->getSourceContext())', $compiler->getSource()); + $this->assertStringNotContainsString("'macro_'.", $compiler->getSource()); } } diff --git a/tests/Node/ImportTest.php b/tests/Node/ImportTest.php index 23d969600c0..f6ecbfc1d41 100644 --- a/tests/Node/ImportTest.php +++ b/tests/Node/ImportTest.php @@ -75,7 +75,7 @@ public static function provideTests(): iterable $tests[] = [$node, <<macros["macro"] = \$this->load("foo.twig", 1)->unwrap(); +\$macros["macro"] = \$this->macros["macro"] = \$this->load("foo.twig", 1)->unwrap()->getMacroNamespace(); EOF ]; diff --git a/tests/Node/MacroTest.php b/tests/Node/MacroTest.php index 9af745bdda2..29a0b5cd860 100644 --- a/tests/Node/MacroTest.php +++ b/tests/Node/MacroTest.php @@ -44,7 +44,7 @@ public function testConstructor(): void $this->assertEquals('foo', $node->getAttribute('name')); } - public static function provideTests(): iterable + private static function createNode(): MacroNode { $arguments = new ArrayExpression([ new LocalVariable('foo', 1), @@ -56,12 +56,15 @@ public static function provideTests(): iterable ], 1); $body = new BodyNode([new TextNode('foo', 1)]); - $node = new MacroNode('foo', $body, $arguments, 1); - yield 'with use_yield = true' => [$node, << [self::createNode(), <<macros; \$context = [ "foo" => \$foo, @@ -76,14 +79,13 @@ public function macro_foo(\$foo = null, \$bar = "Foo", \$_underscore = null, ... yield "foo"; yield from []; })(), false))) ? '' : new Markup(\$tmp, \$this->env->getCharset()); -} +}, ["foo" => true, "bar" => true, "_underscore" => true], false) EOF, new Environment(new ArrayLoader(), ['use_yield' => true]), ]; - yield 'with use_yield = false' => [$node, << [self::createNode(), <<macros; \$context = [ "foo" => \$foo, @@ -98,7 +100,7 @@ public function macro_foo(\$foo = null, \$bar = "Foo", \$_underscore = null, ... yield "foo"; yield from []; })())) ? '' : new Markup(\$tmp, \$this->env->getCharset()); -} +}, ["foo" => true, "bar" => true, "_underscore" => true], false) EOF, new Environment(new ArrayLoader(), ['use_yield' => false]), ]; } diff --git a/tests/Node/MacrosTest.php b/tests/Node/MacrosTest.php new file mode 100644 index 00000000000..7a5687e6970 --- /dev/null +++ b/tests/Node/MacrosTest.php @@ -0,0 +1,85 @@ +expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Using "Twig\Node\TextNode" for the macro "foo" of "Twig\Node\MacrosNode" is not supported. You must pass a "Twig\Node\MacroNode" instance.'); + + new MacrosNode(['foo' => new TextNode('foo', 1)]); + } + + public function testItRejectsReplacingAMacroWithANonMacroNode(): void + { + $macros = new MacrosNode(['foo' => self::createMacro()]); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('A "Twig\Node\MacrosNode" can only contain "Twig\Node\MacroNode" nodes; replacing the macro "foo" with a "Twig\Node\TextNode" node is not supported.'); + + $macros->setNode('foo', new TextNode('foo', 1)); + } + + private static function createMacro(): MacroNode + { + $arguments = new ArrayExpression([ + new LocalVariable('foo', 1), + new ConstantExpression(null, 1), + ], 1); + + return new MacroNode('foo', new BodyNode([new TextNode('foo', 1)]), $arguments, 1); + } + + public static function provideTests(): iterable + { + yield 'without macros, no method is compiled' => [new MacrosNode(), '']; + + $macro = self::createMacro(); + + yield 'with macros, the registry method is compiled' => [new MacrosNode(['foo' => $macro]), << new \\Twig\\TwigMacro("foo", function (\$foo = null, ...\$varargs): string|Markup { + // line 1 + \$macros = \$this->macros; + \$context = [ + "foo" => \$foo, + "varargs" => \$varargs, + ] + \$this->env->getGlobals(); + + \$blocks = []; + + return ('' === \$tmp = implode('', iterator_to_array((function () use (&\$context, \$macros, \$blocks) { + yield "foo"; + yield from []; + })(), false))) ? '' : new Markup(\$tmp, \$this->env->getCharset()); + }, ["foo" => true], false), + ]; +} +EOF, new Environment(new ArrayLoader(), ['use_yield' => true]), + ]; + } +} diff --git a/tests/Node/ModuleTest.php b/tests/Node/ModuleTest.php index 90ea38883fc..c3cbad02ded 100644 --- a/tests/Node/ModuleTest.php +++ b/tests/Node/ModuleTest.php @@ -20,6 +20,8 @@ * file that was distributed with this source code. */ +use PHPUnit\Framework\Attributes\Group; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Twig\Environment; use Twig\Error\RuntimeError; use Twig\Loader\ArrayLoader; @@ -31,6 +33,7 @@ use Twig\Node\Expression\Variable\AssignMacroVariable; use Twig\Node\Expression\Variable\MacroVariable; use Twig\Node\ImportNode; +use Twig\Node\MacrosNode; use Twig\Node\ModuleNode; use Twig\Node\Nodes; use Twig\Node\SetNode; @@ -40,12 +43,14 @@ class ModuleTest extends NodeTestCase { + use ExpectDeprecationTrait; + public function testConstructor(): void { $body = new BodyNode([new TextNode('foo', 1)]); $parent = new ConstantExpression('layout.twig', 1); $blocks = new EmptyNode(); - $macros = new EmptyNode(); + $macros = new MacrosNode(); $traits = new EmptyNode(); $source = new Source('{{ foo }}', 'foo.twig'); $node = new ModuleNode($body, $parent, $blocks, $macros, $traits, new EmptyNode(), $source); @@ -57,6 +62,21 @@ public function testConstructor(): void $this->assertEquals($source->getName(), $node->getTemplateName()); } + /** + * @group legacy + */ + #[Group('legacy')] + public function testConstructorAcceptsDeprecatedMacrosNode(): void + { + $macros = new EmptyNode(); + + $this->expectDeprecation('Since twig/twig 3.29: Not passing a "Twig\\Node\\MacrosNode" instance as the "macros" argument of the "Twig\\Node\\ModuleNode" constructor is deprecated.'); + + $node = new ModuleNode(new BodyNode([]), null, new EmptyNode(), $macros, new EmptyNode(), new EmptyNode(), new Source('', 'index')); + + $this->assertInstanceOf(MacrosNode::class, $node->getNode('macros')); + } + public function testUseTagTemplateNameDoesNotInjectPhpInCompiledOutput(): void { $evilName = "evil' . print('BAD-EOL') . '.twig"; @@ -89,7 +109,7 @@ public static function provideTests(): iterable $body = new BodyNode([new TextNode('foo', 1)]); $extends = null; $blocks = new EmptyNode(); - $macros = new EmptyNode(); + $macros = new MacrosNode(); $traits = new EmptyNode(); $source = new Source('{{ foo }}', 'foo.twig'); @@ -102,6 +122,7 @@ public static function provideTests(): iterable use Twig\Error\RuntimeError; use Twig\Extension\CoreExtension; use Twig\Extension\SandboxExtension; +use Twig\MacroNamespace; use Twig\Markup; use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; @@ -117,7 +138,7 @@ class __TwigTemplate_%x extends Template { private Source \$source; /** - * @var array + * @var array */ private array \$macros = []; @@ -154,7 +175,7 @@ public function getTemplateName(): string */ public function getDebugInfo(): array { - return array ( 43 => 1,); + return array ( 44 => 1,); } public function getSourceContext(): Source @@ -178,6 +199,7 @@ public function getSourceContext(): Source use Twig\Error\RuntimeError; use Twig\Extension\CoreExtension; use Twig\Extension\SandboxExtension; +use Twig\MacroNamespace; use Twig\Markup; use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; @@ -193,7 +215,7 @@ class __TwigTemplate_%x extends Template { private Source \$source; /** - * @var array + * @var array */ private array \$macros = []; @@ -217,7 +239,7 @@ protected function doDisplay(array \$context, array \$blocks = []): iterable { \$macros = \$this->macros; // line 2 - \$macros["macro"] = \$this->macros["macro"] = \$this->load("foo.twig", 2)->unwrap(); + \$macros["macro"] = \$this->macros["macro"] = \$this->load("foo.twig", 2)->unwrap()->getMacroNamespace(); // line 1 \$this->parent = \$this->load("layout.twig", 1); yield from \$this->parent->unwrap()->yield(\$context, array_merge(\$this->blocks, \$blocks)); @@ -244,7 +266,7 @@ public function isTraitable(): bool */ public function getDebugInfo(): array { - return array ( 49 => 1, 47 => 2, 40 => 1,); + return array ( 50 => 1, 48 => 2, 41 => 1,); } public function getSourceContext(): Source @@ -273,6 +295,7 @@ public function getSourceContext(): Source use Twig\Error\RuntimeError; use Twig\Extension\CoreExtension; use Twig\Extension\SandboxExtension; +use Twig\MacroNamespace; use Twig\Markup; use Twig\Sandbox\SecurityError; use Twig\Sandbox\SecurityNotAllowedTagError; @@ -288,7 +311,7 @@ class __TwigTemplate_%x extends Template { private Source \$source; /** - * @var array + * @var array */ private array \$macros = []; @@ -338,7 +361,7 @@ public function isTraitable(): bool */ public function getDebugInfo(): array { - return array ( 49 => 2, 47 => 4, 40 => 2,); + return array ( 50 => 2, 48 => 4, 41 => 2,); } public function getSourceContext(): Source diff --git a/tests/NodeVisitor/CorrectnessTest.php b/tests/NodeVisitor/CorrectnessTest.php index ab2a9a41824..a9fdd80dc6d 100644 --- a/tests/NodeVisitor/CorrectnessTest.php +++ b/tests/NodeVisitor/CorrectnessTest.php @@ -20,6 +20,7 @@ use Twig\Node\EmptyNode; use Twig\Node\Expression\ConstantExpression; use Twig\Node\IncludeNode; +use Twig\Node\MacrosNode; use Twig\Node\ModuleNode; use Twig\Node\Node; use Twig\Node\Nodes; @@ -101,7 +102,7 @@ public static function getFilterBodyNodesWithBOMData() private function traverse(Node $input, Node $expected): Node { $source = new Source('', 'index'); - $input = new ModuleNode(new BodyNode([$input]), new ConstantExpression('parent', 1), new EmptyNode(), new EmptyNode(), new EmptyNode(), new EmptyNode(), $source); + $input = new ModuleNode(new BodyNode([$input]), new ConstantExpression('parent', 1), new EmptyNode(), new MacrosNode(), new EmptyNode(), new EmptyNode(), $source); $expected->setSourceContext($source); $env = new Environment(new ArrayLoader(['index' => ''])); diff --git a/tests/NodeVisitor/SandboxTest.php b/tests/NodeVisitor/SandboxTest.php index bda8fdb1865..78df86c719c 100644 --- a/tests/NodeVisitor/SandboxTest.php +++ b/tests/NodeVisitor/SandboxTest.php @@ -29,6 +29,7 @@ use Twig\Node\EmptyNode; use Twig\Node\Expression\AbstractExpression; use Twig\Node\Expression\Variable\ContextVariable; +use Twig\Node\MacrosNode; use Twig\Node\ModuleNode; use Twig\Node\Node; use Twig\Node\PrintNode; @@ -43,7 +44,7 @@ public function testGeneratorExpression(): void $env = new Environment(new ArrayLoader()); $expr = new ContextVariable('foo', 1); $expr->setAttribute('is_generator', true); - $node = new ModuleNode(new BodyNode([new PrintNode($expr, 1)]), null, new EmptyNode(), new EmptyNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); + $node = new ModuleNode(new BodyNode([new PrintNode($expr, 1)]), null, new EmptyNode(), new MacrosNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); $traverser = new NodeTraverser($env, [new SandboxNodeVisitor($env)]); $node = $traverser->traverse($node); @@ -57,7 +58,7 @@ public function testCustomNodeImplementingCoercesChildrenToStringInterfaceIsWrap $custom = new CustomCoercingExpression(new ContextVariable('foo', 1), new ContextVariable('bar', 1), 1); // wrap inside a PrintNode so it lives in a module; the wrapping must happen on the // custom node itself regardless of the print context - $node = new ModuleNode(new BodyNode([new PrintNode($custom, 1)]), null, new EmptyNode(), new EmptyNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); + $node = new ModuleNode(new BodyNode([new PrintNode($custom, 1)]), null, new EmptyNode(), new MacrosNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); $traverser = new NodeTraverser($env, [new SandboxNodeVisitor($env)]); $node = $traverser->traverse($node); @@ -70,7 +71,7 @@ public function testCustomNonExpressionNodeImplementingCoercesChildrenToStringIn { $env = new Environment(new ArrayLoader()); $custom = new CustomCoercingNode(['expr' => new ContextVariable('foo', 1)], [], 1); - $node = new ModuleNode(new BodyNode([$custom]), null, new EmptyNode(), new EmptyNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); + $node = new ModuleNode(new BodyNode([$custom]), null, new EmptyNode(), new MacrosNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); $traverser = new NodeTraverser($env, [new SandboxNodeVisitor($env)]); $node = $traverser->traverse($node); @@ -83,7 +84,7 @@ public function testSelfIsNeverWrapped(): void $env = new Environment(new ArrayLoader()); $self = new ContextVariable('_self', 1); $custom = new CustomCoercingNode(['expr' => $self], [], 1); - $node = new ModuleNode(new BodyNode([$custom]), null, new EmptyNode(), new EmptyNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); + $node = new ModuleNode(new BodyNode([$custom]), null, new EmptyNode(), new MacrosNode(), new EmptyNode(), new EmptyNode(), new Source('foo', 'foo')); $traverser = new NodeTraverser($env, [new SandboxNodeVisitor($env)]); $node = $traverser->traverse($node); diff --git a/tests/ParserTest.php b/tests/ParserTest.php index 6cc5ddaed72..3d11b4b0cef 100644 --- a/tests/ParserTest.php +++ b/tests/ParserTest.php @@ -20,13 +20,19 @@ * file that was distributed with this source code. */ +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Twig\Environment; use Twig\Error\SyntaxError; use Twig\Lexer; use Twig\Loader\ArrayLoader; use Twig\Node\EmptyNode; use Twig\Node\Expression\ConstantExpression; +use Twig\Node\Expression\GetAttrExpression; +use Twig\Node\Expression\MacroReferenceExpression; +use Twig\Node\MacroDeclarationNode; use Twig\Node\Node; use Twig\Node\TextNode; use Twig\Parser; @@ -37,6 +43,8 @@ class ParserTest extends TestCase { + use ExpectDeprecationTrait; + public function testUnknownTag(): void { $stream = new TokenStream([ @@ -113,6 +121,62 @@ public function testGetVarName(): void $this->addToAssertionCount(1); } + /** + * @dataProvider provideMacroTargetExpressions + */ + #[DataProvider('provideMacroTargetExpressions')] + public function testMacroTargetsOnlyCompileAsMacroReferences(string $expression): void + { + $twig = new Environment(new ArrayLoader()); + $module = $twig->parse($twig->tokenize(new Source("{% import _self as macros %}{{ $expression }}", 'index'))); + $macroReferences = []; + $attributeExpressions = []; + + $this->collectExpressions($module, $macroReferences, $attributeExpressions); + + $this->assertCount(1, $macroReferences); + $this->assertSame([], $attributeExpressions); + } + + public static function provideMacroTargetExpressions(): iterable + { + foreach (['_self', 'macros'] as $target) { + yield $target.' static with parentheses' => [$target.'.foo()']; + yield $target.' dynamic with parentheses' => [$target.'.(name)()']; + } + } + + /** + * @dataProvider provideMacroTargetExpressionsWithoutParentheses + * + * @group legacy + */ + #[DataProvider('provideMacroTargetExpressionsWithoutParentheses')] + #[Group('legacy')] + public function testMacroTargetsWithoutParenthesesAreDeprecated(string $expression): void + { + $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.'); + + $module = $twig->parse($twig->tokenize(new Source("{% import _self as macros %}{{ $expression }}", 'index'))); + $macroReferences = []; + $attributeExpressions = []; + + $this->collectExpressions($module, $macroReferences, $attributeExpressions); + + $this->assertCount(1, $macroReferences); + $this->assertSame([], $attributeExpressions); + } + + public static function provideMacroTargetExpressionsWithoutParentheses(): iterable + { + foreach (['_self', 'macros'] as $target) { + yield $target.' static without parentheses' => [$target.'.foo']; + yield $target.' dynamic without parentheses' => [$target.'.(name)']; + } + } + public function testImplicitMacroArgumentDefaultValues(): void { $template = '{% macro marco (po, lo = true) %}{% endmacro %}'; @@ -133,6 +197,19 @@ public function testImplicitMacroArgumentDefaultValues(): void $this->assertTrue($argumentNodes->getNode(3)->getAttribute('value')); } + public function testMacroDeclarationIsRepresentedInTheTemplateBody(): void + { + $twig = new Environment(new ArrayLoader()); + $module = $twig->parse($twig->tokenize(new Source('{% macro input() %}{% endmacro %}', 'index'))); + $declaration = $module->getNode('body')->getNode('0'); + + $this->assertInstanceOf(MacroDeclarationNode::class, $declaration); + $this->assertSame('input', $declaration->getAttribute('name')); + $this->assertSame('macro', $declaration->getNodeTag()); + $this->assertCount(1, $module->getNode('macros')); + $this->assertNotSame($declaration, $module->getNode('macros')->getNode('input')); + } + public function testEmbeddedTemplatesHaveSequentialIndices(): void { $template = new Source('{% embed "first" %}{% endembed %}{% embed "second" %}{% endembed %}', 'index'); @@ -207,6 +284,24 @@ public function testBodyForParentTemplates(): void $this->assertSame('block', $body->getNode('4')->getNodeTag()); } + /** + * @param list $macroReferences + * @param list $attributeExpressions + */ + private function collectExpressions(Node $node, array &$macroReferences, array &$attributeExpressions): void + { + if ($node instanceof MacroReferenceExpression) { + $macroReferences[] = $node; + } + if ($node instanceof GetAttrExpression) { + $attributeExpressions[] = $node; + } + + foreach ($node as $child) { + $this->collectExpressions($child, $macroReferences, $attributeExpressions); + } + } + protected function getParser() { $parser = new Parser(new Environment(new ArrayLoader())); diff --git a/tests/TwigMacroTest.php b/tests/TwigMacroTest.php new file mode 100644 index 00000000000..c1f7686a0db --- /dev/null +++ b/tests/TwigMacroTest.php @@ -0,0 +1,219 @@ + '', ['name' => false]); + + try { + $macro->callLegacy(['name' => 'a', 0 => 'b'], new Source('', 'index.twig'), 7); + + $this->fail('Expected a RuntimeError to be thrown.'); + } catch (RuntimeError $e) { + $this->assertSame('Positional arguments cannot be used after named arguments for macro "input" in "index.twig" at line 7.', $e->getMessage()); + } + } + + public function testLegacyCallDoesNotReportDeprecationsForACallThatThrows(): void + { + $macro = new TwigMacro('input', static fn () => '', ['name' => false]); + + $deprecations = $this->collectDeprecations(function () use ($macro) { + try { + $macro->callLegacy(['unknown' => 'a', 0 => 'b'], new Source('', 'index.twig'), 7); + + $this->fail('Expected a RuntimeError to be thrown.'); + } catch (RuntimeError $e) { + $this->assertSame('Positional arguments cannot be used after named arguments for macro "input" in "index.twig" at line 7.', $e->getMessage()); + } + }); + + $this->assertSame([], $deprecations); + } + + public function testLegacyCallRejectsAnArgumentDefinedTwice(): void + { + $macro = new TwigMacro('input', static fn () => '', ['name' => false]); + + try { + $macro->callLegacy([0 => 'a', 'name' => 'b'], new Source('', 'index.twig'), 7); + + $this->fail('Expected a RuntimeError to be thrown.'); + } catch (RuntimeError $e) { + $this->assertSame('Argument "name" is defined twice for macro "input" in "index.twig" at line 7.', $e->getMessage()); + } + } + + /** + * The body closures mirror what MacroNode compiles on 3.x: every declared + * argument becomes a defaulted parameter (reserved names prefixed) and a + * trailing variadic bucket collects the extra arguments. + * + * @dataProvider provideLegacyCalls + */ + #[DataProvider('provideLegacyCalls')] + public function testLegacyCallIsLenientButReportsFutureErrors(array $signature, bool $variadic, \Closure $bodyFactory, array $arguments, array $expectedArguments, array $expectedDeprecations): void + { + $captured = null; + $macro = new TwigMacro('test', $bodyFactory($captured), $signature, $variadic); + + $deprecations = $this->collectDeprecations(static function () use ($macro, $arguments) { + $macro->callLegacy($arguments, new Source('', 'index.twig'), 7); + }); + + $this->assertSame($expectedArguments, $captured); + $this->assertSame($expectedDeprecations, $deprecations); + } + + public static function provideLegacyCalls(): iterable + { + $nameOnly = static function (&$captured) { + return static function ($name = null, ...$varargs) use (&$captured) { + $captured = [$name, $varargs]; + + return ''; + }; + }; + + $nameValueType = static function (&$captured) { + return static function ($name = null, $value = 'v', $type = 't', ...$varargs) use (&$captured) { + $captured = [$name, $value, $type, $varargs]; + + return ''; + }; + }; + + yield 'missing argument without a default is lenient and deprecated' => [ + ['name' => false, 'value' => true], + false, + static function (&$captured) { + return static function ($name = null, $value = null, ...$varargs) use (&$captured) { + $captured = [$name, $value, $varargs]; + + return ''; + }; + }, + [], + [null, null, []], + ['Since twig/twig 3.29: Not passing a value for the "name" argument of macro "test" is deprecated and the argument will be required in Twig 4.0; give it a default value in the macro definition or pass a value when calling it (in "index.twig" at line 7).'], + ]; + + yield 'extra positional argument is lenient and deprecated' => [ + ['name' => false], + false, + $nameOnly, + [0 => 'a', 1 => 'b'], + ['a', ['b']], + ['Since twig/twig 3.29: Passing more arguments than the macro "test" accepts is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept extra arguments (in "index.twig" at line 7).'], + ]; + + yield 'unknown named argument is lenient and deprecated' => [ + ['name' => false], + false, + $nameOnly, + ['name' => 'a', 'extra' => 'b'], + ['a', ['extra' => 'b']], + ['Since twig/twig 3.29: Passing the unknown named argument "extra" to the macro "test" is deprecated and will throw in Twig 4.0; declare a variadic argument ("...name") in the macro definition to accept it (in "index.twig" at line 7).'], + ]; + + yield 'null named argument value satisfies a required argument' => [ + ['name' => false], + false, + $nameOnly, + ['name' => null], + [null, []], + [], + ]; + + yield 'a reserved argument name maps onto its prefixed parameter' => [ + ['name' => true, 'blocks' => true], + false, + static function (&$captured) { + return static function ($name = null, $͜blocks = 'default', ...$varargs) use (&$captured) { + $captured = [$name, $͜blocks, $varargs]; + + return ''; + }; + }, + ['blocks' => 'value'], + [null, 'value', []], + [], + ]; + + yield 'skipped optional arguments before a named argument get their defaults' => [ + ['name' => false, 'value' => true, 'type' => true], + false, + $nameValueType, + ['name' => 'a', 'type' => 'x'], + ['a', 'v', 'x', []], + [], + ]; + + yield 'positional and named arguments mix under the legacy call path' => [ + ['name' => false, 'value' => true, 'type' => true], + false, + $nameValueType, + [0 => 'a', 'type' => 'x'], + ['a', 'v', 'x', []], + [], + ]; + + yield 'known named arguments bind to their parameter and unknown ones fall into the variadic bucket' => [ + ['name' => false, 'value' => true, 'type' => true], + true, + $nameValueType, + [0 => 'a', 'type' => 'submit', 'extra' => 'value'], + ['a', 'v', 'submit', ['extra' => 'value']], + [], + ]; + + yield 'a variadic macro reports nothing' => [ + ['name' => false], + true, + $nameOnly, + [0 => 'a', 1 => 'b', 'extra' => 'c'], + ['a', ['b', 'extra' => 'c']], + [], + ]; + } + + private function collectDeprecations(callable $fn): array + { + $deprecations = []; + set_error_handler(static function ($type, $message) use (&$deprecations) { + if (\E_USER_DEPRECATED === $type) { + $deprecations[] = $message; + + return true; + } + + return false; + }); + + try { + $fn(); + } finally { + restore_error_handler(); + } + + return $deprecations; + } +}