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
- Fix Twig namespace bug | https://github.com/twigphp/Twig/issues/2886 — committed to zolhorvath/clarodist by deleted user 5 years ago
- bug #2887 Force loading class aliases used in typehints (stof) This PR was merged into the 1.x branch. Discussion ---------- Force loading class aliases used in typehints Fixes #2886 Commits ----... — committed to twigphp/Twig by fabpot 5 years ago
- Fix #197: Prevent installing broken 2.7.0 & 1.38.0 Twig releases See https://github.com/twigphp/Twig/issues/2886 for details — committed to kiler129/NelmioSecurityBundle by deleted user 5 years ago
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
Twigupgrade to v2.7.0 as well.To be closed, since #2887 have been merged
@kiler129 note that adding the same
class_existsin your file implementing the interface also fixes itAnd 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:
\Twig_Tokenclass is requested,lib/Twig/Token.php,class_exists('Twig\Token'),src/Token.php,class Token(namespace Twig) andclass_alias('Twig\Token', 'Twig_Token')It looks like it should work, everything is in place… but it doesn’t ~for some reason~.
Edit:
… 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/twigis not possible.Example: https://github.com/nelmio/NelmioSecurityBundle/issues/197
@PhilETaylor 1.38 was also changed to use the namespaced classes in the core.
It has been added in 1.* too
Refs:
@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_existscalls 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 suchclass_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.