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

@@ -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

core/lib/Drupal/Core/Template/TwigExtension.php

@@ -159,6 +159,7 @@ class TwigExtension extends AbstractExtension {
     // render_var -> TwigExtension->renderVar() function.
     return [
       new TwigNodeVisitor(),
+      new TwigNodeVisitorCheckDeprecations(),
     ];
   }
 
@@ -709,6 +710,28 @@ class TwigExtension extends AbstractExtension {
     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.
    *

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();
+  }
+
+}

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;
+  }
+
+}

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 }}

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 }}

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 }}

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().
  */

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.";
+  }
+
+}