Twig: Declaration of Twig_Extensions_TokenParser_Trans::parse() must be compatible with Twig\TokenParser\TokenParserInterface::parse(Twig\Token $token)

[bruceheller]

Hello,

It seems that there is an error inside the latest release of Twig 1.x (1.38.0), after I updated I have this error upon each cache clear on Symfony.

Declaration of Twig_Extensions_TokenParser_Trans::parse() must be compatible with Twig\TokenParser\TokenParserInterface::parse(Twig\Token $token) in [...]/vendor/twig/extensions/lib/Twig/Extensions/TokenParser/Trans.php on line 86

Is there any breaking change? I haven't seen any in the changelog...

Thanks!

[lserwatka]

It breaks our software https://github.com/ezsystems/ezplatform after Twig upgrade to v2.7.0 as well.

[Kocal]

Yep, see:

• https://symfony.com/blog/new-in-twig-namespaced-classes
• https://github.com/twigphp/Twig/pull/2863
• https://github.com/twigphp/Twig/pull/2869
• changelog

In your case, the package https://github.com/twigphp/Twig-extensions should be updated to use the class namespaces (e.g.: \Twig\Environment) and not class aliases (e.g.: \Twig_Environment) anymore.

[kunicmarko20]

Breaks https://github.com/sonata-project/SonataMediaBundle/issues/1574

[stof]

@fabpot this is an effect of changing the non-aliased classes in the Twig codebase, in case the other class is not loaded yet.
A solution might be to add some class_exists calls in Twig files defining a typehinted interface signature, to be sure that the typehinted class is loaded (so that its alias is loaded).
Previously, we had to ensure that when using the typehinted class name in the codebase (which is why Symfony contains such class_exists). Now, we would need it when using the non-namespaced classes. But we cannot rely on child classes doing that (or they could switch to namespaced names instead). But the parent class could do it.

[PhilETaylor]

Breaks kriswallsmith/assetic v1.4.0 https://github.com/kriswallsmith/assetic/issues/888

As this is probably a wontfix, because its progress, for the time being you can downgrade your twig in your composer.json and run another composer update

"twig/twig": "2.6.*",

[Kocal]

Also, is there something that users can do? Like, patching libraries that use \Twig_*?

Or Twig will be patched?

Thanks

[Soviann]

@PhilETaylor for certain reasons I'm still using twig 1.* so I can only hope it will be fixed for this branch.

[PhilETaylor]

@NVasse if you are using twig 1.* then you would not have this issue, as the change to Namespaces was only introduced in Twig 2.7.0

• https://symfony.com/blog/new-in-twig-namespaced-classes
• #2863
• #2869
• changelog

[Kocal]

It has been added in 1.* too

Refs:

• https://github.com/twigphp/Twig/pull/2862
• https://github.com/twigphp/Twig/blob/1.x/CHANGELOG#L17

[stof]

@PhilETaylor 1.38 was also changed to use the namespaced classes in the core.

[kiler129]

Now the problem is even bigger, since latest security patch got applied to the newest branch with new namespaces. Libraries are still using old global namespaces and thus updating twig/twig is not possible.

Example: https://github.com/nelmio/NelmioSecurityBundle/issues/197

[Nattfarinn]

@PhilETaylor This is a BC break, but it doesn't seem like intentional BC. Intentional flow seems to be:
1) \Twig_Token class is requested,
2) Autoloader loads lib/Twig/Token.php,
3) File contains class_exists('Twig\Token'),
4) Autoloader loads src/Token.php,
5) File contains both, class Token (namespace Twig) and class_alias('Twig\Token', 'Twig_Token')

It looks like it should work, everything is in place... but it doesn't ~for some reason~.

Edit:

typehints don't trigger autoloading (https://github.com/twigphp/Twig/issues/2886#issuecomment-472023909)

... and there is the reason.

[goba]

Drupal folks having this same problem with 1.38, see https://www.drupal.org/project/drupal/issues/3039408

I am wondering how is it normal for Twig to change class namespaces under people in a supposedly stable version.

[stof]

Well, what changed is which of the name (namespaced or non-namespaced) is an alias to the other one. And we forgot one detail there: typehints don't trigger autoloading, so the signature will be considered non-matching if the child class uses the alias and it was not loaded yet.
I opened #2887 to fix it.

[stof]

Note that code using namespaced names are not affected by the change (but they had to use themselves the loading trick to avoid being affected before, as it was not included in the core).

[goba]

@stof amazing, thanks!

[kiler129]

So effectively when https://github.com/twigphp/Twig/pull/2887 is merged the code using old NS should still work? It seemed weird that minor release caused a BC break ;)

[PhilETaylor]

@stof Awesome. Thanks

[goba]

@kiler129 mistakes happen, and it is clear now that it was not intentional. :) @stof++

[stof]

@kiler129 note that adding the same class_exists in your file implementing the interface also fixes it

And that was not an intended BC break. That was a bug.

[OskarStark]

If I understand correctly it’s the same problem we had at Sonata a few weeks ago and @greg0ire wrote a blog post about it

https://dev.to/greg0ire/how-to-deprecate-a-type-in-php-48cf

[Kocal]

To be closed, since #2887 have been merged

[fabpot]

fixed in 1.38.1 and 2.7.1 (which I've just released).

[Kocal]

@fabpot it correctly fixed https://github.com/php-translation/symfony-bundle/issues/288, thanks @stof and you! :smile:

[mdeboer]

Thank you for fixing this so quickly!

[bruceheller]

Thanks @stof and @fabpot for the quick fix!