Contrib.social

Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.12.0

Open on Drupal.org →
Created on 16 September 2024, 2 months ago
Updated 19 September 2024, about 2 months ago

Problem/Motivation

twig/twig Released version 3.12.0 (See https://github.com/twigphp/Twig/releases/tag/v3.12.0) which introduces new deprecations.

Amongst those is this one that affects core, and is currently suppressed in core/.deprecation-ignore.txt:

%Since twig/twig 3.12: Getting node "filter" on a "Twig\\Node\\Expression\\FilterExpression" class is deprecated.%

I encountered this when running vendor/bin/drush upgrade_status:analyze on a custom module, and was seeing a lot of the following messages:

Check manually 25 | Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\FilterExpression" class is deprecated.

Seems like any variable printed in a twig template with {{ varname }} would trigger this error presumably because autoescaping is enabled when printing variables.

To find out more about this and how to resolve, see the Twig deprecation notice: https://github.com/twigphp/Twig/blob/3.x/doc/deprecated.rst and search for `Twig\Node\Expression\FilterExpression`, and this commit.

Steps to reproduce

Remove the suppression from core/.deprecation-ignore.txt and "shiver at the wall of tests complaining about the deprecation warning."

Proposed resolution

To solve this particular warning, I believe that the way we change the 'escape' filter to use Drupal's own 'drupal_escape' filter needs refactoring.

I think something like changing:

    // Change the 'escape' filter to our own 'drupal_escape' filter.
    elseif ($node instanceof FilterExpression) {
      $name = $node->getNode('filter')->getAttribute('value');
      if ('escape' == $name || 'e' == $name) {
        // Use our own escape filter that is MarkupInterface aware.
        $node->getNode('filter')->setAttribute('value', 'drupal_escape');

        // Store that we have a filter active already that knows
        // how to deal with render arrays.
        $this->skipRenderVarFunction = TRUE;
      }
    }

to 

    // Change the 'escape' filter to our own 'drupal_escape' filter.
    elseif ($node instanceof FilterExpression) {
      $name = $node->getAttribute('twig_callable')->getName();
      if ('escape' == $name || 'e' == $name) {
        $node->setAttribute('twig_callable', 'drupal_escape');

        // Store that we have a filter active already that knows how to deal
        // with render arrays.
        $this->skipRenderVarFunction = TRUE;
      }
    }

There is another twig escape filter related issue:

- https://www.drupal.org/project/drupal/issues/3457168 🐛 Since twig/twig 3.9: error with "twig_escape_filter" function usage in /core/lib/Drupal/Core/Template/TwigExtension.php Needs work

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component
Base  →

Last updated about 10 hours ago

Created by

🇿🇦South Africa foxtrotcharlie

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @foxtrotcharlie
  • Comment 2 months ago →
    🇿🇦South Africa foxtrotcharlie
  • Assigned to foxtrotcharlie
  • Comment 2 months ago →
    🇿🇦South Africa foxtrotcharlie
  • Comment 2 months ago →
    🇿🇦South Africa foxtrotcharlie
  • Comment 2 months ago →
    🇿🇦South Africa foxtrotcharlie
  • Merge request !9509Use twig_callable attribute instad of getNode('filter') → (Closed) created by foxtrotcharlie
  • Pipeline finished with Failed
    2 months ago
    Total: 784s
    #284553
  • Comment about 2 months ago →
    🇺🇸United States bobburns

    patch 3474692 throws

    Error: Call to a member function getCallable() on string in Twig\Node\Expression\CallExpression->getTwigCallable() (line 350 of /var/www/public_html/vendor/twig/twig/src/Node/Expression/CallExpression.php).

    Backtrace
    #0 /var/www/public_html/vendor/twig/twig/src/Node/Expression/CallExpression.php(31): Twig\Node\Expression\CallExpression->getTwigCallable()
    #1 /var/www/public_html/vendor/twig/twig/src/Node/Expression/FilterExpression.php(71): Twig\Node\Expression\CallExpression->compileCallable(Object(Twig\Compiler))
    #2 /var/www/public_html/vendor/twig/twig/src/Compiler.php(99): Twig\Node\Expression\FilterExpression->compile(Object(Twig\Compiler))
    #3 /var/www/public_html/vendor/twig/twig/src/Node/PrintNode.php(40): Twig\Compiler->subcompile(Object(Twig\Node\Expression\FilterExpression))
    #4 /var/www/public_html/vendor/twig/twig/src/Compiler.php(99): Twig\Node\PrintNode->compile(Object(Twig\Compiler))
    #5 /var/www/public_html/vendor/twig/twig/src/Node/Node.php(108): Twig\Compiler->subcompile(Object(Twig\Node\PrintNode))
    #6 /var/www/public_html/vendor/twig/twig/src/Compiler.php(99): Twig\Node\Node->compile(Object(Twig\Compiler))

    . . . more

  • First commit to issue fork.
  • Pipeline finished with Failed
    about 2 months ago
    Total: 731s
    #294305
  • First commit to issue fork.
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 107s
    #294542
  • Pipeline finished with Failed
    about 2 months ago
    Total: 498s
    #294550
  • Pipeline finished with Failed
    about 2 months ago
    Total: 298s
    #294608
  • Pipeline finished with Success
    about 2 months ago
    Total: 319s
    #294632
  • Comment about 2 months ago →
    🇬🇧United Kingdom longwave UK

    Tests are green!

  • Comment about 2 months ago →
    🇳🇱Netherlands bbrala Netherlands

    Reviewed this mr.

    1. Checked the MR and the fixes are great.
    2. Checked core for any other places where we try to get filter, but haven't found any but there.
    3. Tests are all green.
    4. Undoing the comment change from the earliest commit is also great to remove the out-of-scope change.

    All is looking good! Awesome <3

  • Comment about 2 months ago →
    System Message
    • catch → committed 6ed1d153 on 10.3.x 
      Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...
  • Comment about 2 months ago →
    System Message
    • catch → committed ec293e3f on 10.4.x 
      Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...
  • Comment about 2 months ago →
    System Message
    • catch → committed 08c899e0 on 11.0.x 
      Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...
  • Comment about 2 months ago →
    System Message
    • catch → committed 336ba833 on 11.x 
      Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...
  • Comment about 2 months ago →
    🇬🇧United Kingdom catch

    OK this looks good - nice to get rid of all the deprecaton ignore lines.

    Committed/pushed to 11.x and cherry-picked back through to 10.3.x, thanks!

  • Comment about 2 months ago →
    System Message
  • Comment about 1 month ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Comment 26 days ago →
    🇮🇳India hiramanpatil

    I am using Drupal 10.3.5 with PHP 8.3.5 and seeing the following warnings in the build logs when running unit tests using ExistingSite base class. Is there any direct patch available to resolve these?

    365x: Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\FilterExpression" class is deprecated.
    365x in RisesmartRoutingTest::testAllRoutesAccess from Drupal\Tests\custom_module\ExistingSite

    163x: Since twig/twig 3.11: Changing the value of a "filter" node in a NodeVisitor class is not supported anymore.
    163x in RisesmartRoutingTest::testAllRoutesAccess from Drupal\Tests\custom_module\ExistingSite

    15x: Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "render_var" function of type "Twig\Node\Expression\FunctionExpression" is deprecated.
    15x in RisesmartRoutingTest::testAllRoutesAccess from Drupal\Tests\custom_module\ExistingSite

    4x: Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\Filter\DefaultFilter" class is deprecated.
    4x in RisesmartRoutingTest::testAllRoutesAccess from Drupal\Tests\custom_module\ExistingSite

    Thanks,

  • Comment 23 days ago →
    🇺🇸United States maddentim

    @hiramanpatil I believe this is fixed in 10.3.6.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024