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

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3474692

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

foxtrotcharlie created an issue. See original summary.

foxtrotcharlie’s picture

foxtrotcharlie commented
Title: Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.11.0 » Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.12.0
foxtrotcharlie’s picture

foxtrotcharlie commented
Assigned: Unassigned » foxtrotcharlie
foxtrotcharlie’s picture

foxtrotcharlie commented
Parent issue: #3467979: Fix deprecation introduced in twig/twig 3.11.0 » #3473440: [meta] Fix Twig 3 deprecations
foxtrotcharlie’s picture

foxtrotcharlie commented
Issue summary: View changes

foxtrotcharlie opened merge request !9509

bobburns’s picture

bobburns commented

patch 3474692 throws on Drupal 10.3.5 php 8.3.11 twig 3.14

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

bbrala made their first commit to this issue’s fork.

longwave made their first commit to this issue’s fork.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
Issue tags: +Barcelona2024

Tests are green!

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community

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

  • catch committed 6ed1d153 on 10.3.x 
    Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...

  • catch committed ec293e3f on 10.4.x 
    Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...

  • catch committed 08c899e0 on 11.0.x 
    Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...

  • catch committed 336ba833 on 11.x 
    Issue #3474692 by longwave, foxtrotcharlie, bbrala: Fix "Twig\Node\...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

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!

catch closed merge request !9509

Status: Fixed » Closed (fixed)

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

hiramanpatil’s picture

hiramanpatil
Location Pune
commented

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,

maddentim’s picture

maddentim commented

@hiramanpatil I believe this is fixed in 10.3.6.