False positive in twig scanning, with reproducer

Created on 18 August 2024, 4 months ago
Updated 3 September 2024, 3 months ago

Problem/Motivation

Upgrade_status incorrectly reports a syntax errors when using a custom filter in a template.

Steps to reproduce

  • Fresh Drupal 10.3 install
  • Install and enable attached bug reproduction module and upgrade_status
  • Scan bugdemo module, observe reported errror

  • Visit /bugdemo, observe that filter actually works

Template is as follows:

<ul>
  <li>Natural: {{ price }}</li>
  <li>Raw: {{ price|raw }}</li>
  <li>Desired: {{ price|priceFormat }}</li>
</ul>

Twig plugin is:


namespace Drupal\bugdemo\Twig;

use Drupal\Core\Url;
use Drupal\Core\Utility\LinkGeneratorInterface;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;

class BugdemoExtension extends AbstractExtension {

  /**
   * @var \Drupal\Core\Utility\LinkGeneratorInterface
   */
  protected $generator;

  protected $defaultLink = NULL;

  public function __construct(LinkGeneratorInterface $generator) {
    $this->generator = $generator;
  }

  /**
   *
   */
  public function getFilters() {
    $filters = parent::getFilters();
    $filters[] = new TwigFilter('priceFormat', [$this, 'priceFormat']);

    return $filters;
  }

  /**
   *
   */
  public function getGlobals() {
    $globals = parent::getGlobals();
    $globals += [
      'copyright' => '&copy; 2015 OSInet',
    ];
    return $globals;
  }

  /**
   *
   */
  public function getName() {
    return 'wof4';

  }

  /**
   * @param float $price
   *
   * @return string
   */
  public function priceFormat($price) {
    if (!isset($price) || $price < 0.01) {
      $price = $this->getMissingPriceLink();
    }
    else {
      $price = number_format($price, 2, ',', '');
    }

    return $price;
  }

  /**
   *
   */
  public function getMissingPriceLink() {
    if (!isset($this->defaultLink)) {
      $link_text = 'n.c.';
      $link_url = 'https://osinet.fr/contact';

      $url = Url::fromUri($link_url);
      $this->defaultLink = $this->generator->generate($link_text, $url);
    }

    return $this->defaultLink;
  }

}

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

None.

API changes

None.

Data model changes

None.

πŸ› Bug report
Status

Fixed

Version

4.3

Component

Code

Created by

πŸ‡«πŸ‡·France fgm Paris, France

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

Comments & Activities

  • Issue created by @fgm
  • πŸ‡«πŸ‡·France fgm Paris, France
  • πŸ‡«πŸ‡·France fgm Paris, France
  • πŸ‡«πŸ‡·France fgm Paris, France
  • πŸ‡«πŸ‡·France fgm Paris, France
  • πŸ‡­πŸ‡ΊHungary GΓ‘bor Hojtsy Hungary

    Updating issue summary with key code excerpts for easier understanding. The Twig logic depends entirely on Twig itself reporting the errors, so not sure why it would not find your plugin. Especially given that you say the module would be installed/enabled.

  • πŸ‡­πŸ‡ΊHungary GΓ‘bor Hojtsy Hungary

    This is the practical part of our Twig deprecation checking. Looks like the error you are getting is from a Twig SyntaxError exception:

        $deprecations = [];
    
        set_error_handler(function ($type, $msg, $file, $line) use (&$deprecations) {
          if (\E_USER_DEPRECATED === $type) {
            if (preg_match('!([a-zA-Z0-9\_\-\/]+.html\.twig)!', $msg, $file_matches)) {
              // Caught a Twig syntax based deprecation, record file name and line
              // number from the message we caught.
              preg_match('/(\d+).?$/', $msg, $line_matches);
              $msg = preg_replace('! in (.+)\.twig at line \d+\.!', '.', $msg);
              $msg .= ' See https://drupal.org/node/3071078.';
              $deprecations[] = new DeprecationMessage(
                $msg,
                $file_matches[1],
                $line_matches[1] ?? 0,
                'TwigDeprecationAnalyzer'
              );
            }
            else {
              // Otherwise record the deprecation from the original caught error.
              $deprecations[] = new DeprecationMessage(
                $msg,
                $file,
                $line,
                'TwigDeprecationAnalyzer'
              );
            }
          }
        });
    
        $iterator = new TemplateDirIterator(
          new TwigRecursiveIterator($extension->getPath())
        );
        foreach ($iterator as $name => $contents) {
          try {
            $this->twigEnvironment->parse($this->twigEnvironment->tokenize(new Source($contents, $name)));
          } catch (SyntaxError $e) {
            // Report twig syntax error which stops us from parsing it.
            $deprecations[] = new DeprecationMessage(
              'Twig template ' . $name . ' contains a syntax error and cannot be parsed.',
              $name,
              $e->getTemplateLine(),
             'TwigDeprecationAnalyzer'
            );
          }
        }
        restore_error_handler();
    
    
  • πŸ‡«πŸ‡·France fgm Paris, France

    Since you appear to suppose that the module might not be installed, here is a zoomed out copy showing it is indeed installed.

  • πŸ‡«πŸ‡·France fgm Paris, France

    Also, the second screen cap shows that there is indeed no syntax error being reported : the filter is invoked and returns the expected values.

  • Status changed to Fixed 4 months ago
  • πŸ‡«πŸ‡·France fgm Paris, France

    With help from @GΓ‘bor Hojtsy, I found the issue:

    • Extensions do not require a tags: [{ name: twig.extension }] property on the service defining them to work with core
    • They do require it to be autoregistered during scanning, and possibly other situations
    • The original module and this demo module did not contain the tag on the extension service

    So there is this discrepancy between the way things actually work in core, and the way they are scanned, which may or may not be a bug in core or in scanning. But the fix is simple: just tag extensions and they will work in both situations.

  • πŸ‡«πŸ‡·France fgm Paris, France

    Could have been avoided by some documentation, see #2264047: [meta] Document service definition tags β†’

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

Production build 0.71.5 2024