VincentLanglet / Twig-CS-Fixer

A tool to automatically fix Twig Coding Standards issues
MIT License
223 stars 22 forks source link

Allow variables starting with dollar sign ($) #313

Closed nussjustin-hmmh closed 1 week ago

nussjustin-hmmh commented 1 week ago

Context

We'd like to use Twig-CS-Fixer for our Shopware projects (custom plugins / Symfony bundles).

Shopware uses Twig both directly from PHP, but also from JavaScript via twigjs/twig.js.

Since twig.js implements the same syntax as the reference Twig implementation, Twig-CS-Fixer mostly works fine, but we ran into a little problem:

Shopware uses a Variable / Function called $tc for translations (e.g. $tc('hello')). Note the dollar sign at the start.

While this is supported by twig.js, the reference Twig implementation and Twig-CS-Fixer do not support variables starting with a dollar sign.

For compatibility it would be nice for Twig-CS-Fixer to also allow the dollar sign at the start of variables.

Expected behavior

Variables starting with a dollar sign can be parsed.

Write here.

Actual behavior

Tokenizing fails.

 KO .../example.html.twig
 ------- --------------------------------------------------------------------- 
  FATAL   >>   | Unable to tokenize file: Unexpected character "$" at line 3.  
 ------- --------------------------------------------------------------------- 
VincentLanglet commented 1 week ago

Hi @nussjustin-hmmh, thanks for the contribution.

I'm not sure about the https://github.com/VincentLanglet/Twig-CS-Fixer/pull/314 change since I copy pasted the regex from Twig/Twig https://github.com/twigphp/Twig/blob/6fa404ffdf1bbce56c51c708b2cd036205011e24/src/Lexer.php#L46

If you look at https://fiddle.nette.org/twig/#df4ba04b4a it's reported as an error, so I'm not sure why I should support dollar sign for everyone and not add a way to customize some part of the Lexer.

IMHO we may split twig.js support and twig support.

If twig.js is based on twig, do you know where come from the extra support for $ in there code base (extra logic/different regex ?)

I might consider introducing (here https://github.com/VincentLanglet/Twig-CS-Fixer/blob/a0e552fbd6fe264f09f6389a950347a7bf70cfb9/src/Console/Command/TwigCsFixerCommand.php#L135) something like

$tokenizer = new Tokenizer($twig, $config->getTokenizerConfig());

but first I'd like to understand the way twigjs works.

nussjustin-hmmh commented 1 week ago

Ok, I just realised that I didn't think this through.

Shopware does use twig.js but only for Blocks and not for the actual output. That is done via Vue.js, which means any {{ ... }} is actually handled by Vue and Shopware even overrides the twig.js config to ignore most tokens.

See https://github.com/shopware/shopware/blob/a28ca230ab22ec83c04feb4c4a6c23e61f591410/src/Administration/Resources/app/administration/src/core/factory/template.factory.js#L45-L50

That also means that running Twig-CS-Fixer on the templates is probably not even worth it trying.

Sorry for the trouble.

I'd say we close this issue and the PR. WDYT?

VincentLanglet commented 1 week ago

Indeed