geckon01/Twig
1
0
Fork 0
mirror of https://github.com/twigphp/Twig.git synced 2026-09-01 21:17:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove lazy macro import resolution

Browse Source
This commit is contained in:
Fabien Potencier
2026-08-27 09:24:44 +02:00
parent d850901a18
commit cf971e1a59
17 changed files with 34 additions and 387 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG
-1
View File
@@ -8,7 +8,6 @@
* Fix object and mapping destructuring evaluating the right-hand expression more than once
* Fix sequence destructuring of iterators throwing a `TypeError`
* Add `TempestMarkdown` to use `tempest/markdown` as the `markdown_to_html` converter
* Fix imported macros not resolving their own template-level macro imports
* Add the `include_only` function to render a template without giving it access to the current context
* Add the `Twig\Sandbox\SandboxInterface` interface and `Twig\Sandbox\Sandbox` class to render untrusted templates through a dedicated, always-sandboxed environment crafted for it
* Add the `Twig\Extension\SandboxBridgeExtension` to render sandboxed templates from trusted templates with an explicit output escaping strategy
doc/advanced.rst
-2
View File
@@ -101,8 +101,6 @@ What? Implementation difficulty? How often? When?
*operator* simple rare Values transformation
========== ========================== ========== =========================
.. _environment-globals:
Globals
-------
doc/tags/macro.rst
+10 -22
View File
@@ -166,32 +166,20 @@ macros are available in all blocks and other macros defined in the current
template, but they are not available in included templates or child templates;
you need to explicitly re-import macros in each template.
A macro can use the imports declared at the top level of its own template::
.. caution::
{# forms.twig #}
{% import "fields.twig" as fields %}
When a macro is called from another template, the body of the template where
it is defined is not executed. Do not rely on imports declared at the top
level of that template; import the dependencies inside the macro instead:
{% macro input(name) %}
{{ fields.text(name) }}
{% endmacro %}
.. code-block:: twig
This also works when the macro is called from another template. For such an
import to be available inside macros, it must follow two rules:
{# forms.twig #}
{% macro input(name) %}
{% from "fields.twig" import text %}
* It must be declared at the top level of the template, not nested in another
tag like ``if`` or ``for``;
* The imported template name must be a literal string or an expression that
only depends on :ref:`global variables <environment-globals>`; it cannot
use other variables.
To use a dynamic template name, pass it as a macro argument and import it
inside the macro body::
{% macro input(name, theme) %}
{% import theme as fields %}
{{ fields.text(name) }}
{% endmacro %}
{{ text(name) }}
{% endmacro %}
Imported macros are not available in the body of ``embed`` tags, you need
to explicitly re-import macros inside the tag.
src/MacroNamespace.php
+1 -21
View File
@@ -24,8 +24,7 @@ final class MacroNamespace
*/
public function __construct(
private Template $template,
private array $macros = [],
private ?\Closure $importsLoader = null,
private array $macros,
) {
}
@@ -85,7 +84,6 @@ final class MacroNamespace
{
if (isset($this->macros[$name])) {
$this->template->ensureSecurityChecked();
$this->loadImports();
return $this->macros[$name];
}
@@ -95,7 +93,6 @@ final class MacroNamespace
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();
$this->loadImports();
return $macro;
}
@@ -118,23 +115,6 @@ final class MacroNamespace
}
}
private function loadImports(): void
{
if (null === $loader = $this->importsLoader) {
return;
}
// clear before loading so that circular imports don't recurse infinitely
$this->importsLoader = null;
try {
$loader();
} catch (\Throwable $e) {
$this->importsLoader = $loader;
throw $e;
}
}
private function getParent(array $context): ?self
{
if (!$parent = $this->template->getParent($context)) {
src/Node/MacroImportsNode.php
-80
View File
@@ -1,80 +0,0 @@
<?php
/*
* This file is part of Twig.
*
* (c) Fabien Potencier
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Twig\Node;
use Twig\Attribute\YieldReady;
use Twig\Compiler;
use Twig\Node\Expression\Variable\AssignMacroVariable;
/**
* Compiles the lazy loader for the top-level imports used by the macros
* declared in a template.
*
* @internal
*/
#[YieldReady]
final class MacroImportsNode extends Node
{
public function __construct(Node $body)
{
$imports = [];
$this->collectTopLevelImports($body, $imports);
parent::__construct($imports);
}
public function compile(Compiler $compiler): void
{
$compiler
->raw("function (): void {\n")
->indent()
->write("if (\$this->skipLazyMacroImports) {\n")
->indent()
->write("return;\n")
->outdent()
->write("}\n\n")
->write("\$this->ensureSecurityChecked();\n")
->write("\$context = \$this->env->getGlobals();\n")
->write("\$macros = \$this->macros;\n")
;
foreach ($this as $import) {
$compiler->subcompile($import);
}
$compiler
->outdent()
->write('}')
;
}
/**
* @param list<ImportNode> $imports
*/
private function collectTopLevelImports(Node $node, array &$imports): void
{
if ($node instanceof ImportNode) {
$var = $node->getNode('var');
if ($var instanceof AssignMacroVariable && $var->getAttribute('global')) {
$imports[] = $node;
}
return;
}
if (!$node instanceof BodyNode && !$node instanceof Nodes) {
return;
}
foreach ($node as $child) {
$this->collectTopLevelImports($child, $imports);
}
}
}
src/Node/MacrosNode.php
+7 -48
View File
@@ -17,7 +17,7 @@ use Twig\Compiler;
/**
* Represents the macros declared in a template.
*
* It compiles the macro namespace of the template.
* It compiles to the method returning the macro registry of the template.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
@@ -27,7 +27,7 @@ final class MacrosNode extends Node
/**
* @param array<string, MacroNode> $macros
*/
public function __construct(array $macros = [], private ?MacroImportsNode $imports = null)
public function __construct(array $macros = [])
{
foreach ($macros as $name => $macro) {
if (!$macro instanceof MacroNode) {
@@ -38,14 +38,6 @@ final class MacrosNode extends Node
parent::__construct($macros);
}
public function __clone()
{
parent::__clone();
if (null !== $this->imports) {
$this->imports = clone $this->imports;
}
}
public function setNode(string $name, Node $node): void
{
if (!$node instanceof MacroNode) {
@@ -55,51 +47,16 @@ final class MacrosNode extends Node
parent::setNode($name, $node);
}
/**
* @internal
*/
public function hasImports(): bool
{
return \count($this) && null !== $this->imports && \count($this->imports);
}
public function compile(Compiler $compiler): void
{
if (!\count($this)) {
return;
}
$compiler->write("private ?MacroNamespace \$macroNamespace = null;\n");
if ($this->hasImports()) {
$compiler->write("private bool \$skipLazyMacroImports = false;\n");
}
$compiler
->raw("\n")
->write("public function getMacroNamespace(): MacroNamespace\n", "{\n")
->write("protected function loadDeclaredMacros(): array\n", "{\n")
->indent()
->write('return $this->macroNamespace ??= new MacroNamespace($this, ')
;
$this->compileMacros($compiler);
if ($this->hasImports()) {
$compiler
->raw(', ')
->subcompile($this->imports)
;
}
$compiler
->raw(");\n")
->outdent()
->write("}\n\n")
;
}
private function compileMacros(Compiler $compiler): void
{
$compiler
->raw("[\n")
->write("return [\n")
->indent()
;
@@ -116,7 +73,9 @@ final class MacrosNode extends Node
$compiler
->outdent()
->write(']')
->write("];\n")
->outdent()
->write("}\n\n")
;
}
}
src/Node/SkipLazyMacroImportsNode.php
-27
View File
@@ -1,27 +0,0 @@
<?php
/*
* This file is part of Twig.
*
* (c) Fabien Potencier
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Twig\Node;
use Twig\Attribute\YieldReady;
use Twig\Compiler;
/**
* @internal
*/
#[YieldReady]
final class SkipLazyMacroImportsNode extends Node
{
public function compile(Compiler $compiler): void
{
$compiler->write("\$this->skipLazyMacroImports = true;\n");
}
}
src/Parser.php
+2 -10
View File
@@ -26,7 +26,6 @@ use Twig\Node\EmptyNode;
use Twig\Node\Expression\AbstractExpression;
use Twig\Node\Expression\Variable\AssignMacroVariable;
use Twig\Node\Expression\Variable\MacroVariable;
use Twig\Node\MacroImportsNode;
use Twig\Node\MacroNode;
use Twig\Node\MacrosNode;
use Twig\Node\ModuleNode;
@@ -34,7 +33,6 @@ use Twig\Node\Node;
use Twig\Node\NodeDocumentation;
use Twig\Node\Nodes;
use Twig\Node\PrintNode;
use Twig\Node\SkipLazyMacroImportsNode;
use Twig\Node\TextNode;
use Twig\TokenParser\TokenParserInterface;
use Twig\Util\ReflectionCallable;
@@ -128,12 +126,11 @@ class Parser
$body = $this->cleanupBodyForChildTemplates($body);
}
$body = new BodyNode([$body]);
$node = new ModuleNode(
$body,
new BodyNode([$body]),
$this->parent,
$this->blocks ? new Nodes($this->blocks) : new EmptyNode(),
new MacrosNode($this->macros, new MacroImportsNode($body)),
new MacrosNode($this->macros),
$this->traits ? new Nodes($this->traits) : new EmptyNode(),
$this->embeddedTemplates ? new Nodes($this->embeddedTemplates) : new EmptyNode(),
$stream->getSourceContext(),
@@ -146,11 +143,6 @@ class Parser
*/
$node = $traverser->traverse($node);
$macros = $node->getNode('macros');
if ($macros instanceof MacrosNode && $macros->hasImports()) {
$node->setNode('display_start', new Nodes([new SkipLazyMacroImportsNode(), $node->getNode('display_start')]));
}
// restore previous stack so previous parse() call can resume working
foreach (array_pop($this->stack) as $key => $val) {
$this->$key = $val;
src/Template.php
+9 -1
View File
@@ -482,7 +482,15 @@ abstract class Template
*/
public function getMacroNamespace(): MacroNamespace
{
return $this->macroNamespace ??= new MacroNamespace($this);
return $this->macroNamespace ??= new MacroNamespace($this, $this->loadDeclaredMacros());
}
/**
* @return array<string, TwigMacro>
*/
protected function loadDeclaredMacros(): array
{
return [];
}
/**
tests/CallMacroTest.php
-74
View File
@@ -13,7 +13,6 @@ namespace Twig\Tests;
use PHPUnit\Framework\TestCase;
use Twig\Environment;
use Twig\Error\LoaderError;
use Twig\Error\RuntimeError;
use Twig\Extension\CoreExtension;
use Twig\Loader\ArrayLoader;
@@ -22,7 +21,6 @@ use Twig\Sandbox\SecurityNotAllowedMethodError;
use Twig\Sandbox\SecurityPolicy;
use Twig\Source;
use Twig\Template;
use Twig\TwigFunction;
class CallMacroTest extends TestCase
{
@@ -36,78 +34,6 @@ class CallMacroTest extends TestCase
$this->assertSame('Hi World', (string) $this->callMacro($template, 'greet', ['name' => 'World', 'greeting' => 'Hi']));
}
public function testNestedMacroImportsResolveAgainstGlobals(): void
{
$twig = new Environment(new ArrayLoader([
'index' => '{% import "outer" as outer %}{{ outer.render() }}',
'outer' => '{% import macro_template as macros %}{% macro render() %}{{ macros.render() }}{% endmacro %}',
'first' => '{% macro render() %}first{% endmacro %}',
]));
$twig->addGlobal('macro_template', 'first');
$this->assertSame('first', $twig->render('index', []));
}
public function testNestedMacroImportsCannotUseTheImportingContext(): void
{
$twig = new Environment(new ArrayLoader([
'index' => '{% import "outer" as outer %}{{ outer.render() }}',
'outer' => '{% import macro_template as macros %}{% macro render() %}{{ macros.render() }}{% endmacro %}',
'first' => '{% macro render() %}first{% endmacro %}',
]), ['strict_variables' => true]);
$this->expectException(RuntimeError::class);
$this->expectExceptionMessage('Variable "macro_template" does not exist');
$twig->render('index', ['macro_template' => 'first']);
}
public function testNestedMacroImportsAreInitializedOncePerTemplate(): void
{
$twig = new Environment(new ArrayLoader([
'index' => '{% import "outer" as outer %}{{ outer.render() }}{{ outer.render() }}',
'outer' => '{% import pick() as macros %}{% macro render() %}{{ macros.render() }}{% endmacro %}',
'first' => '{% macro render() %}first{% endmacro %}',
]));
$calls = 0;
$twig->addFunction(new TwigFunction('pick', static function () use (&$calls): string {
++$calls;
return 'first';
}));
$this->assertSame('firstfirst', $twig->render('index', []));
$this->assertSame(1, $calls);
}
public function testFailedNestedMacroImportsFailTheSameWayOnRetry(): void
{
$twig = new Environment(new ArrayLoader([
'index' => '{% import "outer" as outer %}{{ outer.render() }}',
'outer' => '{% import "missing" as macros %}{% macro render() %}{{ macros.render() }}{% endmacro %}',
]));
foreach ([1, 2] as $attempt) {
try {
$twig->render('index', []);
$this->fail('Expected LoaderError');
} catch (LoaderError $e) {
$this->assertStringContainsString('Template "missing" is not defined', $e->getMessage(), "Attempt $attempt");
}
}
}
public function testRenderTimeImportsKeepUsingTheRenderContext(): void
{
$twig = new Environment(new ArrayLoader([
'index' => '{% include "outer" %}{% import "outer" as outer %}{{ outer.render() }}',
'outer' => '{% import macro_template as macros %}{% macro render() %}{{ macros.render() }}{% endmacro %}',
'first' => '{% macro render() %}first{% endmacro %}',
]), ['strict_variables' => true]);
$this->assertSame('first', $twig->render('index', ['macro_template' => 'first']));
}
public function testMacroNamespaceOnlyExposesMacroOperations(): void
{
$template = $this->load(['index' => '{% macro greet(name) %}Hi {{ name }}{% endmacro %}']);
tests/Extension/SandboxTest.php
-23
View File
@@ -1255,29 +1255,6 @@ EOF
$this->assertEquals('<p>username</p>', $twig->load('index')->render([]));
}
public function testMacroImportExpressionIsCheckedBeforeExecution(): void
{
$evilCalls = 0;
$twig = $this->getEnvironment(true, [], [
'caller.twig' => '{% import "macros.twig" as macros %}{{ macros.render() }}',
'macros.twig' => '{% from evil() import render as dependency %}{% macro render() %}{{ dependency() }}{% endmacro %}',
'dependency.twig' => '{% macro render() %}ok{% endmacro %}',
], ['from', 'import', 'macro']);
$twig->addFunction(new TwigFunction('evil', static function () use (&$evilCalls): string {
++$evilCalls;
return 'dependency.twig';
}));
try {
$twig->render('caller.twig');
$this->fail('Expected SecurityNotAllowedFunctionError');
} catch (SecurityNotAllowedFunctionError $e) {
$this->assertSame('evil', $e->getFunctionName());
}
$this->assertSame(0, $evilCalls, 'The forbidden function must not be invoked before the security check runs.');
}
public function testSelfMacroReferenceWithStringLiteralDoesNotInjectPhp(): void
{
$twig = $this->getEnvironment(true, [], ['index' => '{{ _self.(\'foo + 1; trigger_error("BAD-MACRO-REF") //\')() }}']);
tests/Fixtures/tags/macro/circular_nested_macro_imports.test
-15
View File
@@ -1,15 +0,0 @@
--TEST--
Circular nested macro imports
--TEMPLATE--
{% import "macros1.twig" as macros %}
{{ macros.macro1() }}
--TEMPLATE(macros1.twig)--
{% import "macros2.twig" as macros %}
{% macro macro1() %}[{{ macros.macro2() }}]{% endmacro %}
--TEMPLATE(macros2.twig)--
{% import "macros1.twig" as macros %}
{% macro macro2() %}ok{% endmacro %}
--DATA--
return []
--EXPECT--
[ok]
tests/Fixtures/tags/macro/conditional_nested_macro_import.test
-14
View File
@@ -1,14 +0,0 @@
--TEST--
A conditional macro import is not initialized outside its control flow
--TEMPLATE--
{% import "macros.twig" as macros %}
{{ macros.macro() }}
--TEMPLATE(macros.twig)--
{% if false %}
{% import "missing.twig" as unused %}
{% endif %}
{% macro macro() %}ok{% endmacro %}
--DATA--
return []
--EXPECT--
ok
tests/Fixtures/tags/macro/from_nested_macro_imports.test
-14
View File
@@ -1,14 +0,0 @@
--TEST--
"from" tag with a macro that calls a macro imported in another template
--TEMPLATE--
{% from "macros2.twig" import macro2 %}
{{ macro2() }}
--TEMPLATE(macros2.twig)--
{% from "macros1.twig" import macro1 %}
{% macro macro2() %}[{{ macro1() }}]{% endmacro %}
--TEMPLATE(macros1.twig)--
{% macro macro1() %}ok{% endmacro %}
--DATA--
return []
--EXPECT--
[ok]
tests/Fixtures/tags/macro/import_nested_macro_imports.test
-14
View File
@@ -1,14 +0,0 @@
--TEST--
"import" tag with a macro that calls a macro imported in another template
--TEMPLATE--
{% import "macros2.twig" as macros2 %}
{{ macros2.macro2() }}
--TEMPLATE(macros2.twig)--
{% import "macros1.twig" as macros1 %}
{% macro macro2() %}[{{ macros1.macro1() }}]{% endmacro %}
--TEMPLATE(macros1.twig)--
{% macro macro1() %}ok{% endmacro %}
--DATA--
return []
--EXPECT--
[ok]
tests/Fixtures/tags/macro/nested_macro_imports_context.test
-14
View File
@@ -1,14 +0,0 @@
--TEST--
Nested macro imports cannot use the importing template's context
--TEMPLATE--
{% from "macros2.twig" import macro2 %}
{{ macro2() }}
--TEMPLATE(macros2.twig)--
{% from macros_template import macro1 %}
{% macro macro2() %}[{{ macro1() }}]{% endmacro %}
--TEMPLATE(macros1.twig)--
{% macro macro1() %}ok{% endmacro %}
--DATA--
return ['macros_template' => 'macros1.twig']
--EXCEPTION--
Twig\Error\RuntimeError: Variable "macros_template" does not exist in "macros2.twig" at line 2.
tests/Node/MacrosTest.php
+5 -7
View File
@@ -54,16 +54,14 @@ class MacrosTest extends NodeTestCase
public static function provideTests(): iterable
{
yield 'without macros, nothing is compiled' => [new MacrosNode(), ''];
yield 'without macros, no method is compiled' => [new MacrosNode(), ''];
$macro = self::createMacro();
yield 'with macros, the namespace is compiled' => [new MacrosNode(['foo' => $macro]), <<<EOF
private ?MacroNamespace \$macroNamespace = null;
public function getMacroNamespace(): MacroNamespace
yield 'with macros, the registry method is compiled' => [new MacrosNode(['foo' => $macro]), <<<EOF
protected function loadDeclaredMacros(): array
{
return \$this->macroNamespace ??= new MacroNamespace(\$this, [
return [
"foo" => new \\Twig\\TwigMacro("foo", function (\$foo = null, ...\$varargs): string|Markup {
// line 1
\$macros = \$this->macros;
@@ -79,7 +77,7 @@ public function getMacroNamespace(): MacroNamespace
yield from [];
})(), false))) ? '' : new Markup(\$tmp, \$this->env->getCharset());
}, ["foo" => true], false),
]);
];
}
EOF, new Environment(new ArrayLoader(), ['use_yield' => true]),
];