Problem/Motivation

Following #3467293: twig/twig 3.11.0 introduces (for Drupal) breaking changes and #3473195: twig/twig has a possible sandbox bypass <v3.14.0 there are several Twig deprecations that are skipped for now, we need to resolve these so when Twig 4 is released we can upgrade cleanly:

%Since twig/twig 3.11: Changing the value of a "filter" node in a NodeVisitor class is not supported anymore.%

%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "attach_library" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "add_component_context" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%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.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "validate_component_props" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%

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

%Since twig/twig 3.12: The "tag" constructor argument of the "Drupal\\Core\\Template\\TwigNodeTrans" class is deprecated and ignored%

%Since twig/twig 3.12: Twig Filter "spaceless" is deprecated%

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3473440

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:

Comments

longwave created an issue. See original summary.

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

finnsky changed the visibility of the branch 3473440-fix-twig-3 to hidden.

andypost’s picture

Status: Active » Needs work
finnsky’s picture

Status: Needs work » Needs review

Spaceless. Seems easier one from this bundle rewritten

We can remove it without any problems and get better perfomance
https://github.com/twigphp/Twig/pull/4236
https://github.com/twigphp/Twig/pull/4308

Probably every deprecation thing requires own subticket. Not sure. I gonna continue here in new branch for every deprecation to have full picture.
Sending for review to understand if approach is correct.

finnsky’s picture

Issue summary: View changes
finnsky’s picture

Issue summary: View changes
finnsky’s picture

Issue summary: View changes
finnsky’s picture

Issue summary: View changes
finnsky’s picture

Issue summary: View changes

finnsky’s picture

Issue summary: View changes
finnsky’s picture

So we have 4(5) types of deprecations

1. "Not passing an instance of "TwigFunction" when creating a "XXX" function"

Fixed here in the test-passing-function branch. Can be moved to a separate task.

2. "The "tag" constructor argument"

Also fixed here.

3. "spaceless is deprecated"

Fixed for tests here. Also fixed tests.

4 and 5 are related to modifications of the default escape filter using drupal_escape

https://twig.symfony.com/doc/3.x/filters/escape.html#custom-escapers
it says here that
"Built-in escapers cannot be overridden mainly because they should be considered as the final implementation and also for better performance."

The last point definitely deserves a separate task. All the rest can be merged here or in their subtasks

bbrala’s picture

Just so you know.

This complely broke upgrade status and the update bot. Since we now ship deprecations with core. The d11 readiness dashboard showed like +200k errors suddenly and we eventually tracked it back here.

finnsky’s picture

Gonna check that last deprecations type here.

gábor hojtsy’s picture

finnsky’s picture

@gábor hojtsy

It's hard to say now. In the current task (which should later become meta) I consider all deprecations at once and globally. Several are already ready to receive their subtask and be merged.

As for the task that you linked, I think it has the wrong name. We can't just rewrite the functions there. But consider how drupal_escape and trans work in general. And add it as required by the modern version of Twig. This is currently in progress.

longwave’s picture

Title: Fix Twig 3 deprecations » [meta] Fix Twig 3 deprecations
Status: Needs review » Needs work

There are four child issues now, one with its own MR, the three MRs from here need moving to their individual issues - but I think that covers all the newly added deprecations.

finnsky’s picture

I would say 5 issues but not sure yet ;) As i said better to review also how `trans` and `drupal_escape` works

finnsky’s picture

Imo we shouldn't just `fix deprecation message` here. But fix deprecations deeper, with research of new twig extending style. This issue is RTBC of course. But - can we do it better? Idk yet.

finnsky changed the visibility of the branch 3473440-twig-deprecations to hidden.

finnsky changed the visibility of the branch test-pasing-function to hidden.

finnsky changed the visibility of the branch test-tag-constructor-argument to hidden.

bbrala’s picture

Issue tags: +Needs change record

While reviewing the child issues i concluded we might need a change record here (or on the twig update issue?).

Contrib will want to know how to fix these errors, having a guideline on where to find the core fixes will help a lot.

gábor hojtsy’s picture

Imo we shouldn't just `fix deprecation message` here. But fix deprecations deeper, with research of new twig extending style. This issue is RTBC of course. But - can we do it better? Idk yet.

I think it would be great to have a "good enough" solution ASAP, and then spend any time to fix it even better later. Because these deprecated API uses produce errors with most runs of Upgrade Status on all contrib projects and people can't do anything about it. When we ran Project Status on all contrib modules, these produced 190 thousand twig deprecacted API calls detected across all of Drupal 10 contrib :) This also means that none of those are getting project update bot patches/MRs because they have these unsolvable issues. So a quick solution would be important to unblock the rest of the ecosystem / tooling, then a better solution is not out of question :)

gábor hojtsy’s picture

These were the top errors found in contrib by Project Analysis (data in https://git.drupalcode.org/project/deprecation_status/-/blob/d3d2a182959... including lists of projects for each error if someone wants to validate that Upgrade Status is not reporting the errors anymore):

  • 177 765 times (in 1324 projects): Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\FilterExpression" class is deprecated.
  • 10734 times (in 641 projects): 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.
  • 1080 times (in 209 projects): Since twig/twig 3.12: The "tag" constructor argument of the "Drupal\Core\Template\TwigNodeTrans" class is deprecated and ignored (check which TokenParser class set it to "trans"), the tag is now automatically set by the Parser when needed.
  • 1100 times (in 274 projects): Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\Filter\RawFilter" class is deprecated.
  • 864 times (in 121 projects): Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\Filter\DefaultFilter" class is deprecated.
  • 512 times (in 147 projects): Since twig/twig 3.12: Twig Filter "spaceless" is deprecated. See https://drupal.org/node/3071078.

Are these all being resolved in the children? :)

Also these two were found in one project each, not a priority IMHO:

  • Since twig/twig 3.12: Character """ at position 2 should not be escaped; the "\ character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character." in formatage_models 4.x-dev
  • Since twig/twig 3.12: Character "T" at position 7 should not be escaped; the "\ character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character." in festeva 1.0.x-dev
bbrala’s picture

Yeah all being handled in the children.

finnsky’s picture

yep. now all deprecations covered
not sure is this

"good enough" solution ASAP

but at least

solution ASAP

:)

bbrala’s picture

Only the spaceless deprecation fix needs work, 2 are merged, 1 is RTBC. Almost there ^^

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

longwave’s picture

Status: Needs work » Fixed
Issue tags: -Needs change record
Related issues: +#3486170: Remove use of deprecated "spaceless" filter in core templates

The last issue was solved in #3486170: Remove use of deprecated "spaceless" filter in core templates, closing this one out.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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