WIP: Twig AST extractor for Timber

[drzraf]

Current Twig parser operates by

1. Transforming Twig as PHP code = tokenize() + parse() + render()
2. Using a PhpCode extractor (and its specific configuration about function name)

The disadvantages are:

• Twig rendered PHP code can be transformed/wrapped in call_user_func() making that gettext functions undetectable by PhpCode extractor.
  (See for example Don't overwrap WordPress i18n functions timber/timber#1753).
• Can't handle templates making use of custom Twig extensions (very common)

This patchset implements an extractor that:

• Parses Twig generated AST tree (= tokenize()+parse())
• Recursively iterates over node three to find function gettext calls.

Advantages:

• Operating sooner, at the AST level, Twig expressions like {{ __("foo", "domain") }} aren't yet wrapped.
• More robust because it directly iterates over the AST from Twig parser instead of looking at PHP source-code.
• Supports initialized $twig environment, thus supporting user-defined extensions?
• Possibly more efficient.

ToDos:

1. $options['twig'] being a static variable, we can NOT initialize it in a row with different parameters.
   Would get a LogicException: Unable to add function "action" as extensions have already been initialized.
   Solution: don't use static variables for this.

2. Translation/Notes comments are not yet supported.

3. Timber needs a tweak to avoid a fatal error:
   Add, to lib/Timber.php

  function add_action() { }
  function apply_filters() {}

The issue is that when loaded through composer Timber constructor forcefully calls init()
which make use of WP functions.

[rubas]

@oscarotero
Is this something you would consider merging?

Extracting over AST without parsing and rendering is imo a huge advantage and they way to go.

Especially all the custom Twig extensions are a pain to deal with ...

Can't handle templates making use of custom Twig extensions (very common)

This would allow us in Wordpress with Twig land to finally automate the extraction process for all our projects and integrate it in our CI pipelines.

[oscarotero]

@rubas Yes, I like the idea of extracting gettext entries from Twig templates using its AST, instead the generated php code. I just have some questions:

I would rather not add more features to gettext 4, so would you consider migrating to gettext v5? The new version is splited in different packages so we could create a gettext/twig-scanner package (just as we have php-scanner and js-scanner).
Note that gettext 5 is supported by php >= 7.2.

[drzraf]

Good to know.
How to do advise to deal with loader's particularities. In this MR, we depends upon timber presence in order to load its extensions. Would you accept that this twig-scanner package be kind of Timber-centric when it comes to extensions to begin with and only later evolve to support other Twig-extensions bridge?
composer.json : require or suggest Timber?
I hope I can give a try by next week.

[oscarotero]

I'm not fully familiarized with Twig and Timber, but the advantages of having multiple packages is that we can create scanners for specific needs.

The ideal solution would be having a generic twig-scanner package that accept any twig instance with any extension (Timber is an extension of Twig, right?).
If it's too hard, we can create a timber-scanner, or maybe a generic twig-scanner and then a timber-scanner extending twig-scanner.

[gchtr]

The ideal solution would be having a generic twig-scanner package that accept any twig instance with any extension (Timber is an extension of Twig, right?).

You’re right, and I agree. I’d first make a twig-scanner, then we can check how we make that work in Timber.

Timber brings together WordPress and Twig. And it is not the only Twig player in WordPress land. There are quite a few plugins that use Twig templates, mainly for admin views. I guess they would also benefit from a twig-scanner. And maybe there should be a wordpress-scanner before we try a timber-scanner?

[oscarotero]

@gchtr Great. You can take the others scanners as a guide, for example PHP-Scanner:

• PhpFunctionsScanner is the responsible for scanning and returning an array of ParsedFunction
• PhpScanner is responsible for instatiating the PhpFunctionsScanner and use it.

I think the key here is creating a TwigFunctionsScanner that can parse and return all gettext entries of a twig template. Then, we can create other classes using the same TwigFunctionsScanner but with different configuration.

It's really easy to create a wordpress-scanner, because it's just a PHPFunctionScanner with specific configuration for WordPress (it uses different fuction names _(), _e(), esc_html__(), etc)

[drzraf]

One thing to keep in mind is that Twig comments (for prefixed translations "notes") are unlikely to be supported because Twig tokenizer does not (currently) create a dedicated token for them that would be available in the AST.

[oscarotero]

Ok, I think we can live without comments for now, until Twig add tokens for that.

[drzraf]

Ok. Initial implementation there : https://github.com/drzraf/Twig-Scanner/ but it first needs a tweak wrt #255.
Let me know if we can put that under php-gettext namespace for a more rational packagist naming convention.

[oscarotero]

@drzraf Great!
I think the scanner can be improved because you're mapping the twig functions to php functions, keeping the same arguments in the same order in order to reuse the same handlers. I think this scanner should be more independent, but it's a good start.

What do you think about transfering that repository into php-gettext organization so we can continue the work here? (And change the vendor package drzraf/twig-scanner to gettext/twig-scanner).

[drzraf]

I guess you meant php-gettext ?
=> You don’t have the permission to create public repositories on php-gettext

[oscarotero]

@drzraf yes, sorry, I mean php-gettext.
You don't need to create a repository but transfer the current drzraf/Twig-Scanner to php-gettext organization, so you will continue being a maintainer. (Go to settings -> Danger zone -> Transfer ownership)

[drzraf]

Yes, that's what I tried. And indeed it says:

Transfer ownership
Transfer this repository to another user or to an organization where you have the ability to create repositories. 

and trying results in You don’t have the permission to create public repositories on php-gettext

[oscarotero]

Ok, I just send you an invitation to join to the organization

[oscarotero]

Closed this and keep working at php-gettext/Twig-Scanner#1