From 3c8128a0acf549940c811efab53884a6368f324a Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Wed, 7 Jun 2023 14:45:55 +0100
Subject: [PATCH] Issue #3270148 by jonathanshaw, murilohp, catch, smustgrave,
longwave, larowlan, andypost, alexpott, quietone: Provide a mechanism for
deprecation of variables used in twig templates
---
core/lib/Drupal/Core/Render/theme.api.php | 17 ++
.../Drupal/Core/Template/TwigExtension.php | 23 +++
.../Template/TwigNodeCheckDeprecations.php | 48 +++++
.../TwigNodeVisitorCheckDeprecations.php | 77 ++++++++
.../theme-test-deprecations-child.html.twig | 3 +
...eme-test-deprecations-hook-theme.html.twig | 2 +
...eme-test-deprecations-preprocess.html.twig | 8 +
.../modules/theme_test/theme_test.module | 39 ++++
.../Core/Theme/TwigDeprecationsTest.php | 168 ++++++++++++++++++
9 files changed, 385 insertions(+)
create mode 100644 core/lib/Drupal/Core/Template/TwigNodeCheckDeprecations.php
create mode 100644 core/lib/Drupal/Core/Template/TwigNodeVisitorCheckDeprecations.php
create mode 100644 core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-child.html.twig
create mode 100644 core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-hook-theme.html.twig
create mode 100644 core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-preprocess.html.twig
create mode 100644 core/tests/Drupal/KernelTests/Core/Theme/TwigDeprecationsTest.php
diff --git a/core/lib/Drupal/Core/Render/theme.api.php b/core/lib/Drupal/Core/Render/theme.api.php
index cd33aecbc29b..a05eba2c778b 100644
--- a/core/lib/Drupal/Core/Render/theme.api.php
+++ b/core/lib/Drupal/Core/Render/theme.api.php
@@ -68,6 +68,23 @@
* rendered by the Twig template; the processed variables that the Twig template
* receives are documented in the header of the default Twig template file.
*
+ * Theme hooks can declare a variable deprecated using the reserved
+ * 'deprecations' variable. For example:
+ * @code
+ * search_result' => [
+ * 'variables' => [
+ * 'result' => NULL,
+ * 'new_result' => NULL,
+ * 'plugin_id' => NULL,
+ * 'deprecations' => [
+ * 'result' => "'result' is deprecated in drupal:X.0.0 and is removed from drupal:Y.0.0. Use 'new_result' instead. See https://www.example.com."
+ * ]
+ * ],
+ * ],
+ * @endcode
+ * Template engines should trigger a deprecation error if a deprecated
+ * variable is used in a template.
+ *
* @section sec_overriding_theme_hooks Overriding Theme Hooks
* Themes may register new theme hooks within a hook_theme() implementation, but
* it is more common for themes to override default implementations provided by
diff --git a/core/lib/Drupal/Core/Template/TwigExtension.php b/core/lib/Drupal/Core/Template/TwigExtension.php
index 68bdee440774..60c0ce0ddf4c 100644
--- a/core/lib/Drupal/Core/Template/TwigExtension.php
+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -159,6 +159,7 @@ public function getNodeVisitors() {
// render_var -> TwigExtension->renderVar() function.
return [
new TwigNodeVisitor(),
+ new TwigNodeVisitorCheckDeprecations(),
];
}
@@ -709,6 +710,28 @@ public function suggestThemeHook(?array $element, string|\Stringable $suggestion
return $element;
}
+ /**
+ * Triggers a deprecation error if a variable is deprecated.
+ *
+ * @param array $context
+ * A Twig context array.
+ * @param array $used_variables
+ * The names of the variables used in a template.
+ *
+ * @see \Drupal\Core\Template\TwigNodeCheckDeprecations
+ */
+ public function checkDeprecations(array $context, array $used_variables): void {
+ if (!isset($context['deprecations'])) {
+ return;
+ }
+
+ foreach ($used_variables as $name) {
+ if (isset($context['deprecations'][$name]) && \array_key_exists($name, $context)) {
+ @trigger_error($context['deprecations'][$name], E_USER_DEPRECATED);
+ }
+ }
+ }
+
/**
* Adds a value into the class attributes of a given element.
*
diff --git a/core/lib/Drupal/Core/Template/TwigNodeCheckDeprecations.php b/core/lib/Drupal/Core/Template/TwigNodeCheckDeprecations.php
new file mode 100644
index 000000000000..5f458df56ff7
--- /dev/null
+++ b/core/lib/Drupal/Core/Template/TwigNodeCheckDeprecations.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace Drupal\Core\Template;
+
+use Twig\Compiler;
+use Twig\Node\Expression\ArrayExpression;
+use Twig\Node\Expression\ConstantExpression;
+use Twig\Node\Node;
+
+/**
+ * A node that checks deprecated variable usage.
+ *
+ * @see \Drupal\Core\Template\TwigNodeVisitorCheckDeprecations
+ * @see \Drupal\Core\Template\TwigExtension::checkDeprecations()
+ */
+class TwigNodeCheckDeprecations extends Node {
+
+ /**
+ * The named variables used in the template.
+ */
+ protected array $usedNames = [];
+
+ /**
+ * {@inheritdoc}
+ */
+ public function __construct(array $usedNames) {
+ $this->usedNames = $usedNames;
+ parent::__construct();
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function compile(Compiler $compiler) {
+ $usedNamesNode = new ArrayExpression([], $this->getTemplateLine());
+ foreach ($this->usedNames as $name) {
+ $usedNamesNode->addElement(new ConstantExpression($name, $this->getTemplateLine()));
+ }
+
+ $compiler->write("\$this->env->getExtension('\Drupal\Core\Template\TwigExtension')\n");
+ $compiler->indent();
+ $compiler->write("->checkDeprecations(\$context, ");
+ $compiler->subcompile($usedNamesNode);
+ $compiler->raw(");");
+ $compiler->outdent();
+ }
+
+}
diff --git a/core/lib/Drupal/Core/Template/TwigNodeVisitorCheckDeprecations.php b/core/lib/Drupal/Core/Template/TwigNodeVisitorCheckDeprecations.php
new file mode 100644
index 000000000000..c54ebb689be1
--- /dev/null
+++ b/core/lib/Drupal/Core/Template/TwigNodeVisitorCheckDeprecations.php
@@ -0,0 +1,77 @@
+<?php
+
+namespace Drupal\Core\Template;
+
+use Twig\Environment;
+use Twig\Node\Expression\AssignNameExpression;
+use Twig\Node\Expression\NameExpression;
+use Twig\Node\ModuleNode;
+use Twig\Node\Node;
+use Twig\NodeVisitor\AbstractNodeVisitor;
+
+/**
+ * Provides a Node Visitor to trigger errors if deprecated variables are used.
+ *
+ * Every use of a named variable is tracked, and the used variable names are
+ * passed to TwigExtension::checkDeprecations at runtime for comparison against
+ * those in the 'deprecated' array in the template context.
+ *
+ * @see \Drupal\Core\Template\TwigNodeCheckDeprecations
+ */
+class TwigNodeVisitorCheckDeprecations extends AbstractNodeVisitor {
+
+ /**
+ * The named variables used in the template from the context.
+ */
+ protected array $usedNames = [];
+
+ /**
+ * The named variables set within the template.
+ */
+ protected array $assignedNames = [];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function doEnterNode(Node $node, Environment $env) {
+ if ($node instanceof ModuleNode) {
+ $this->usedNames = [];
+ $this->assignedNames = [];
+ }
+ elseif ($node instanceof AssignNameExpression) {
+ // Setting a variable makes subsequent usage is safe.
+ $this->assignedNames[$node->getAttribute('name')] = $node->getAttribute('name');
+ }
+ elseif ($node instanceof NameExpression) {
+ // Track each usage of a variable, unless set within the template.
+ $name = $node->getAttribute('name');
+ if (!in_array($name, $this->assignedNames)) {
+ $this->usedNames[$name] = $name;
+ }
+ }
+ return $node;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function doLeaveNode(Node $node, Environment $env) {
+ // At the end of the template, check the used variables are not deprecated.
+ if ($node instanceof ModuleNode) {
+ if (!empty($this->usedNames)) {
+ $checkNode = new Node([new TwigNodeCheckDeprecations($this->usedNames), $node->getNode('display_end')]);
+ $node->setNode('display_end', $checkNode);
+ }
+ }
+ return $node;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getPriority(): int {
+ // Just above the Optimizer, which is the normal last one.
+ return 256;
+ }
+
+}
diff --git a/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-child.html.twig b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-child.html.twig
new file mode 100644
index 000000000000..752dbfd5d427
--- /dev/null
+++ b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-child.html.twig
@@ -0,0 +1,3 @@
+{# Test use of deprecated variables in a child template. #}
+{{- foo }}|
+{{- gaz }}
\ No newline at end of file
diff --git a/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-hook-theme.html.twig b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-hook-theme.html.twig
new file mode 100644
index 000000000000..d319b84fc618
--- /dev/null
+++ b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-hook-theme.html.twig
@@ -0,0 +1,2 @@
+{# Test variables deprecated in hook_theme(). #}
+{{ foo }}
\ No newline at end of file
diff --git a/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-preprocess.html.twig b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-preprocess.html.twig
new file mode 100644
index 000000000000..00737cef3ed8
--- /dev/null
+++ b/core/modules/system/tests/modules/theme_test/templates/theme-test-deprecations-preprocess.html.twig
@@ -0,0 +1,8 @@
+{# Test use of variables deprecated in a preprocess hook. #}
+{% set set_var = 'set_var' %}
+{{- foo }}|
+{{- set_var }}|
+{%- for for_var in contents %}
+ {{- for_var }}|
+{%- endfor %}
+{{- bar }}
\ No newline at end of file
diff --git a/core/modules/system/tests/modules/theme_test/theme_test.module b/core/modules/system/tests/modules/theme_test/theme_test.module
index 1dafdaba0582..31ec4dc80653 100644
--- a/core/modules/system/tests/modules/theme_test/theme_test.module
+++ b/core/modules/system/tests/modules/theme_test/theme_test.module
@@ -63,6 +63,33 @@ function theme_test_theme($existing, $type, $theme, $path) {
'message' => '',
],
];
+ $items['theme_test_deprecations_preprocess'] = [
+ 'variables' => [
+ 'foo' => '',
+ 'bar' => '',
+ 'gaz' => '',
+ 'set_var' => '',
+ 'for_var' => '',
+ 'contents' => [],
+ ],
+ ];
+ $items['theme_test_deprecations_child'] = [
+ 'variables' => [
+ 'foo' => '',
+ 'bar' => '',
+ 'gaz' => '',
+ ],
+ ];
+ $items['theme_test_deprecations_hook_theme'] = [
+ 'variables' => [
+ 'foo' => '',
+ 'bar' => '',
+ 'deprecations' => [
+ 'foo' => "'foo' is deprecated in drupal:X.0.0 and is removed from drupal:Y.0.0. Use 'new_foo' instead. See https://www.example.com.",
+ 'bar' => "'bar' is deprecated in drupal:X.0.0 and is removed from drupal:Y.0.0. Use 'new_bar' instead. See https://www.example.com.",
+ ],
+ ],
+ ];
return $items;
}
@@ -174,6 +201,18 @@ function theme_test_theme_suggestions_node(array $variables) {
function template_preprocess_theme_test_registered_by_module() {
}
+/**
+ * Implements template_preprocess_HOOK() for theme_test_deprecations_preprocess.
+ *
+ * Default template: theme-test-deprecations-preprocess.html.twig.
+ *
+ * @param array $variables
+ * An associative array of variables.
+ */
+function template_preprocess_theme_test_deprecations_preprocess(array &$variables) {
+ $variables = array_merge($variables, \Drupal::state()->get('theme_test.theme_test_deprecations_preprocess'));
+}
+
/**
* Implements hook_library_info_alter().
*/
diff --git a/core/tests/Drupal/KernelTests/Core/Theme/TwigDeprecationsTest.php b/core/tests/Drupal/KernelTests/Core/Theme/TwigDeprecationsTest.php
new file mode 100644
index 000000000000..422315a79776
--- /dev/null
+++ b/core/tests/Drupal/KernelTests/Core/Theme/TwigDeprecationsTest.php
@@ -0,0 +1,168 @@
+<?php
+
+namespace Drupal\KernelTests\Core\Theme;
+
+use Drupal\KernelTests\KernelTestBase;
+
+/**
+ * Tests deprecating variables passed to twig templates.
+ *
+ * @see \Drupal\Core\Template\TwigExtension::checkDeprecations()
+ * @see \Drupal\Core\Template\TwigNodeVisitorCheckDeprecations
+ * @see \Drupal\Core\Template\TwigNodeCheckDeprecations
+ * @group Twig
+ * @group legacy
+ */
+class TwigDeprecationsTest extends KernelTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ protected static $modules = ['system', 'theme_test'];
+
+ /**
+ * Test deprecating variables at definition in hook_theme().
+ */
+ public function testHookThemeDeprecations(): void {
+ $element = [
+ '#theme' => 'theme_test_deprecations_hook_theme',
+ '#foo' => 'foo',
+ '#bar' => 'bar',
+ ];
+ // Both 'foo' and 'bar' are deprecated in theme_test_hook_theme(),
+ // but 'bar' is not used in theme-test-deprecations-hook-theme.html.twig.
+ $this->expectDeprecation($this->getDeprecationMessage('foo'));
+ $this->assertEquals('foo', $this->container->get('renderer')->renderRoot($element));
+ }
+
+ /**
+ * Test theme_test_deprecations_preprocess renders without deprecations.
+ */
+ public function testThemeTestDeprecations(): void {
+ $this->assertRendered('foo|set_var|bar', []);
+ }
+
+ /**
+ * Test deprecation of undefined variable triggers no error.
+ */
+ public function testUndefinedDeprecation(): void {
+ $preprocess = [
+ 'deprecations' => [
+ 'undefined' => $this->getDeprecationMessage('undefined'),
+ ],
+ ];
+ $this->assertRendered('foo|set_var|bar', $preprocess);
+ }
+
+ /**
+ * Test deprecation of single variable triggers error.
+ */
+ public function testSingleDeprecation(): void {
+ $preprocess = [
+ 'deprecations' => [
+ 'foo' => $this->getDeprecationMessage('foo'),
+ ],
+ ];
+ $this->expectDeprecation($this->getDeprecationMessage('foo'));
+ $this->assertRendered('foo|set_var|bar', $preprocess);
+ }
+
+ /**
+ * Test deprecation of empty variable triggers error.
+ */
+ public function testEmptyDeprecation(): void {
+ $preprocess = [
+ 'foo' => '',
+ 'deprecations' => [
+ 'foo' => $this->getDeprecationMessage('foo'),
+ ],
+ ];
+ $this->expectDeprecation($this->getDeprecationMessage('foo'));
+ $this->assertRendered('|set_var|bar', $preprocess);
+ }
+
+ /**
+ * Test deprecation of multiple variables triggers errors.
+ */
+ public function testMultipleDeprecations(): void {
+ $preprocess = [
+ 'deprecations' => [
+ 'foo' => $this->getDeprecationMessage('foo'),
+ 'bar' => $this->getDeprecationMessage('bar'),
+ ],
+ ];
+ $this->expectDeprecation($this->getDeprecationMessage('foo'));
+ $this->expectDeprecation($this->getDeprecationMessage('bar'));
+ $this->assertRendered('foo|set_var|bar', $preprocess);
+ }
+
+ /**
+ * Test deprecation of variables assigned inside template triggers no error.
+ */
+ public function testAssignedVariableDeprecation(): void {
+ $preprocess = [
+ 'contents' => ['content'],
+ 'deprecations' => [
+ 'set_var' => $this->getDeprecationMessage('set_var'),
+ 'for_var' => $this->getDeprecationMessage('for_var'),
+ ],
+ ];
+ $this->assertRendered('foo|set_var|content|bar', $preprocess);
+ }
+
+ /**
+ * Test deprecation of variables in parent does not leak to child.
+ */
+ public function testParentVariableDeprecation(): void {
+ // 'foo' is used before the child template is processed, so this test
+ // shows that processing the child does not lead to parent usage being
+ // forgotten.
+ // 'gaz' is used in the child template but only deprecated in the parent
+ // template, so no deprecation error is triggered for it.
+ $preprocess = [
+ 'contents' => [
+ 'child' => [
+ '#theme' => 'theme_test_deprecations_child',
+ '#foo' => 'foo-child',
+ '#bar' => 'bar-child',
+ '#gaz' => 'gaz-child',
+ ],
+ ],
+ 'deprecations' => [
+ 'foo' => $this->getDeprecationMessage('foo'),
+ 'gaz' => $this->getDeprecationMessage('gaz'),
+ ],
+ ];
+ $this->assertRendered('foo|set_var|foo-child|gaz-child|bar', $preprocess);
+ }
+
+ /**
+ * Assert that 'theme_test_deprecations_preprocess' renders expected text.
+ *
+ * @param string $expected
+ * The expected text.
+ * @param array $preprocess
+ * An array to merge in theme_test_deprecations_preprocess_preprocess().
+ */
+ protected function assertRendered($expected, array $preprocess): void {
+ \Drupal::state()->set('theme_test.theme_test_deprecations_preprocess', $preprocess);
+ $element = [
+ '#theme' => 'theme_test_deprecations_preprocess',
+ '#foo' => 'foo',
+ '#bar' => 'bar',
+ '#gaz' => 'gaz',
+ '#set_var' => 'overridden',
+ ];
+ $this->assertEquals($expected, $this->container->get('renderer')->renderRoot($element));
+ }
+
+ /**
+ * Get an example deprecation message for a named variable.
+ */
+ protected function getDeprecationMessage($variable): string {
+ return "'{$variable}' is deprecated in drupal:X.0.0 and is removed from drupal:Y.0.0. Use 'new_{$variable}' instead. See https://www.example.com.";
+ }
+
+}
--
GitLab