If you have the error reporting level of php correctly you can se how it throws the following:

The Twig_Node::getLine method is deprecated since version 1.27 and will be removed in 2.0. Use getTemplateLine() instead.

At vendor/twig/twig/lib/Twig/Node.php

Remove the uses of that method.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rodrigoaguilera created an issue. See original summary.

rodrigoaguilera’s picture

Status: Needs review » Needs work

The last submitted patch, 2: 2884675-replace-twig-getline-2.patch, failed testing. View results

joelpittet’s picture

Well need to support both likely, not exactly sure how, open to suggestions.

Maybe an override method with the new method calling its parent and statically calling the old method if the new one doesn't exist(spitballed)

joelpittet’s picture

By static I mean statically storing the method name to avoid multiple method_exists calls

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

TR’s picture

Version: 8.4.x-dev » 8.5.x-dev
Priority: Normal » Major
Status: Needs work » Needs review

Why do we "need to support both"? This is an internal Drupal core usage of a deprecated function in the Twig API. Drupal core does not provide the getLine()/getTemplateLine() functions. Drupal core controls which versions of Twig get distributed with which version of Drupal, so we can fix this deprecated usage for 8.5.x and leave the old usage in 8.4.x.

Fixing this is rather more important now because of how testing is handling deprecation for contrib - turning on test failures for use of deprecated functions immediately broke all the contrib tests in 8.5.x, in part because Core is full of deprecated usages like this.

This use of a deprecated function causes deprecation errors in all my modules and all my functional tests. I don't call this directly, I just use core. And since the testing gives no hint of where the usage occurs, finding all these is real pain. And avoiding things like this is impossible because it's not in my code or even in functions I call from my code - it's way down the stack somewhere in the rendering layer. Core needs to do the right thing here and fix these usages.

TR’s picture

Status: Needs review » Reviewed & tested by the community

Oh, and yes, this patch applies cleanly and removes the (unhelpful) deprecated errors that I get when I run my tests:
The Twig_Node::getLine method is deprecated since version 1.27 and will be removed in 2.0. Use getTemplateLine() instead: 146x
(That 146x mean 146 usages of this deprecated function, and all it tells me is that they happened when running my test - I don't get a line number or a call stack or anything.)

joelpittet’s picture

Version: 8.5.x-dev » 8.4.x-dev
Related issues: +#2864031: Update twig/twig from v1.25.0 to v1.32.0

@TR that's a good point we upgraded past 1.27 already so this can be fixed in 8.5.x and we won't backport it to 8.4.x

This is where we committed an upgrade past 1.27
#2864031: Update twig/twig from v1.25.0 to v1.32.0

Going to run the tests again to ensure they pass.

joelpittet’s picture

Version: 8.4.x-dev » 8.5.x-dev

Didn't mean to move the version branch.

joelpittet’s picture

RTBC++

Thanks @TR

cburschka’s picture

@TR that's a good point we upgraded past 1.27 already so this can be fixed in 8.5.x and we won't backport it to 8.4.x

Since the update was in 8.4.x and this only changes internal usage, wouldn't it be okay to backport?

catch’s picture

Status: Reviewed & tested by the community » Fixed

8.4.x only requires 1.23.1 in composer.json so this isn't backportable, but looks good for 8.5.x

Committed 962122e and pushed to 8.5.x. Thanks!

Status: Fixed » Reviewed & tested by the community
  • catch committed 962122e on 8.5.x
    Issue #2884675 by rodrigoaguilera, joelpittet, TR: Remove twig uses of...
cburschka’s picture

Oh - I only looked at the .lock, not the composer.json requirements. Yeah, that makes sense.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Just moving back to fixed due to the commit bot crosspost.

Status: Fixed » Closed (fixed)

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