Warning in TwigDeprecationAnalyzer when the error caught is not in a twig file

Created on 6 April 2023, almost 2 years ago
Updated 2 May 2024, 8 months ago

Problem/Motivation

When scanning any module that contains a twig file ,The scan fails with the following error.

[warning] Undefined array key 1 TwigDeprecationAnalyzer.php:41
 [error]  TypeError: Drupal\upgrade_status\DeprecationMessage::__construct(): Argument #2 ($file) must be of type string, null given, called in /Users/loze/Sites/d8dev/web/modules/contrib/upgrade_status/src/TwigDeprecationAnalyzer.php on line 42 in Drupal\upgrade_status\DeprecationMessage->__construct() (line 43 of /Users/loze/Sites/d8dev/web/modules/contrib/upgrade_status/src/DeprecationMessage.php) #0 /Users/loze/Sites/d8dev/web/modules/contrib/upgrade_status/src/TwigDeprecationAnalyzer.php(42): Drupal\upgrade_status\DeprecationMessage->__construct('\\Drupal\\node\\Pl...', NULL, '2983299')
#1 [internal function]: Drupal\upgrade_status\TwigDeprecationAnalyzer::Drupal\upgrade_status\{closure}('\\Drupal\\node\\Pl...')
#2 /Users/loze/Sites/d8dev/web/modules/contrib/upgrade_status/src/TwigDeprecationAnalyzer.php(44): array_map(Object(Closure), Array)

Scanning is successful on modules without any twig templates.

Happens to me both with drush and the UI.

🐛 Bug report
Status

Fixed

Version

4.0

Component

Code

Created by

🇺🇸United States loze Los Angeles

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

Merge Requests

Comments & Activities

  • Issue created by @loze
  • 🇺🇸United States loze Los Angeles

    Apparently this was coming from ctools 3.13, after upgrading to 3.x-dev it went away.

    What is happening is the deprecation message being reported was:

    \Drupal\node\Plugin\Condition\NodeType is deprecated in  drupal:9.3.0 and is removed from drupal:10.0.0. 
    Use  Drupal\Core\Entity\Plugin\Condition\EntityBundle instead.  
    See https://www.drupal.org/node/2983299 
    See https://drupal.org/node/3071078.  
    

    There is no filename or line number is this message

    analyze() in TwigDeprecationAnalyzer.php was breaking because $file_matches[1] was NULL when it was expecting a string.

    If i change

    return new DeprecationMessage(
            $message,
            $file_matches[1],
            $line_matches[1] ?? 0
          );
    

    to

    return new DeprecationMessage(
            $message,
            $file_matches[1] ?? '',
            $line_matches[1] ?? 0
          );
    

    the scan is able to complete, however in the report there is an entry that looks like this.

    FILE: web/
    
    STATUS         LINE                           MESSAGE                           
    --------------------------------------------------------------------------------
    Check manually 2983299\Drupal\node\Plugin\Condition\NodeType is deprecated in     
                        drupal:9.3.0 and is removed from drupal:10.0.0. Use         
                        Drupal\Core\Entity\Plugin\Condition\EntityBundle instead.   
                        See https://www.drupal.org/node/2983299 See                 
                        https://drupal.org/node/3071078.   
    

    Notice there is no filename, it just says "web/" and the line number is incorrect. its using the "2983299" from the last part of the d.o. url in the message.

    I had close to 10 of these in my report before upgrading ctools to the dev version.

  • 🇨🇦Canada joseph.olstad

    Thanks for this, helped a lot,
    yes it looks like ctools needs to cut a release 3.15
    3.14 doesn't do it, but 3.x-dev fixes the issue.

  • 🇭🇺Hungary Gábor Hojtsy Hungary

    So we are using https://github.com/twigphp/Twig/blob/3.x/src/Util/DeprecationCollector.php to collect the Twig deprecation errors. All that does is it sets its own error handler and collects all deprecation errors reported while compiling the Twig templates. It does not collect the file name and line number for the error messages though. I think we should subclass that and in case the error message does not come with a twig file name and line number, we should include the file name and line number from set_error_handler() that is currently not saved.

        public function collect(\Traversable $iterator): array
        {
            $deprecations = [];
            set_error_handler(function ($type, $msg) use (&$deprecations) {
                if (\E_USER_DEPRECATED === $type) {
                    $deprecations[] = $msg;
                }
            });
    
            foreach ($iterator as $name => $contents) {
                try {
                    $this->twig->parse($this->twig->tokenize(new Source($contents, $name)));
                } catch (SyntaxError $e) {
                    // ignore templates containing syntax errors
                }
            }
    
            restore_error_handler();
    
            return $deprecations;
        }
    
  • 🇭🇺Hungary Gábor Hojtsy Hungary

    This way we can keep reporting Twig level errors when they are twig level (like we do now) and we can also report the runtime errors found while parsing the twig file with their PHP file/line source information that are currently leading to the fails.

  • 🇳🇱Netherlands joshahubbers

    Created oldfashoned patch to include because .diff can change without notice.

  • 🇭🇺Hungary Gábor Hojtsy Hungary

    Fails were due to the newly introduced catching of twig syntax errors.

  • Status changed to RTBC 9 months ago
  • 🇭🇺Hungary Gábor Hojtsy Hungary

    Was looking into adding a test on 3rd party deprecations triggered from Twig compile but at least the trigger for this issue in 🐛 condition_info_alter deprecation triggers false possitive deprecation notices Fixed was resolved by removing the deprecation notice there and it was erroneously triggered widely :D Either way those could still show up with this, and the tests already prove that there is no regression. Also we gained a new feature of syntax errors reported from Twig compilation, which is good IMHO.

  • Pipeline finished with Skipped
    9 months ago
    #149798
  • Status changed to Fixed 9 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024