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

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!

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 16
  • Comments: 26 (15 by maintainers)

Commits related to this issue

Most upvoted comments

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

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.

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

Yep, see:

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.

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

To be closed, since #2887 have been merged

@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.

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 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.

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

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

@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.