Create attributes `AsTwigFilter`, `AsTwigFunction` and `AsTwigTest` to ease extension development #3916

GromNaN · 2023-11-26T00:15:57Z

One drawback to writing extensions at present is that the declaration of functions/filters/tests is not directly adjacent to the methods. It's worse for runtime extensions because they need to be in 2 different classes. See SerializerExtension and SerializerRuntime as an example.

By using attributes for filters, functions and tests definition, we can make writing extensions more expressive, and use reflection to detect particular options (needs_environment, needs_context, is_variadic).

Example if we implemented the formatDate filter:

Twig/extra/intl-extra/IntlExtension.php

Lines 392 to 395 in aeeec9a

    
           public function formatDate(Environment $env, $date, ?string $dateFormat = 'medium', string $pattern = '', $timezone = null, string $calendar = 'gregorian', string $locale = null): string 
        
           { 
        
               return $this->formatDateTime($env, $date, $dateFormat, 'none', $pattern, $timezone, $calendar, $locale); 
        
           }

By using the AsTwigFilter attribute, it is not necessary to create the getFilters() method. The needs_environment option is detected from method signature. The name is still required as the method naming convention (camelCase) doesn't match with Twig naming convention (snake_case).

use Twig\Extension\Attribute\AsTwigFilter;
use Twig\Extension\Attribute\AsTwigExtension;

#[AsTwigExtension]
class IntlExtension
{
    #[AsTwigFilter(name: 'format_date')]
    public function formatDate(Environment $env, $date, ?string $dateFormat = 'medium', string $pattern = '', $timezone = null, string $calendar = 'gregorian', string $locale = null): string
    {
        return $this->formatDateTime($env, $date, $dateFormat, 'none', $pattern, $timezone, $calendar, $locale);
    }
}

This approach does not totally replace the current definition of extensions, which is still necessary for advanced needs. It does, however, make for more pleasant reading and writing.

This makes writing lazy-loaded runtime extension the easiest way to create Twig extension in Symfony: symfony/symfony#52748

Related to symfony/symfony#50016

Is there any need to cache the parsing of method attributes? They are only read at compile time, but that can have a performance impact during development or when using dynamic templates.

src/Extension/Extension.php

src/Extension/Attribute/AsTwigFilter.php

src/Extension/Attribute/AsTwigTest.php

src/Extension/Attribute/AsTwigFilter.php

src/Extension/Extension.php

GromNaN · 2023-11-26T09:36:45Z

I'll rework the implementation after reading discussions on symfony/symfony#50016

stof · 2023-11-27T10:17:41Z

src/Extension/AttributeExtension.php

+                    $parameters = $method->getParameters();
+                    $needsEnvironment = isset($parameters[0]) && Environment::class === $parameters[0]->getType()?->getName();
+                    $firstParam = $needsEnvironment ? 1 : 0;
+                    $needsContext = isset($parameters[$firstParam]) && 'context' === $parameters[$firstParam]->getName() && 'array' === $parameters[$firstParam]->getType()?->getName();


this special behavior of parameter named $context needs to be documented on the attributes.

I added the doc and example in the attribute class header.

The Symfony serialize filter accepts an array $context as 3rd argument. This will not conflict but this means developers could have an unexpected behavior if they don't know about this special argument handling.

GromNaN · 2023-11-30T08:51:41Z

src/Extension/WithLastModified.php

+
+namespace Twig\Extension;
+
+interface WithLastModified


This is also a new feature required to detect when the class that defines attributes have been updated.

Alternatively, we can make passing a class name to the ExtensionSet, that will be scanned by it. This has slightly more impact on existing classes.

@stof I can't replace the modification date by a hash, the Environment::isFresh relies on a date comparison between the ExtensionSet and the cache file mtime.

Twig/src/Environment.php

Lines 416 to 419 in aeeec9a

public function isTemplateFresh(string $name, int $time): bool

{

return $this->extensionSet->getLastModified() <= $time && $this->getLoader()->isFresh($name, $time);

}

LastModifiedAwareInterface? What's the existing convention in Twig?

Renamed to ModificationAwareInterface. The convention if to suffix with Interface.

With the last change, I removed this interface. ExtensionSet is now aware of the extension classes that use attributes, and check their modification date as well.

src/Extension/Attribute/AsTwigExtension.php

src/Extension/AttributeExtension.php

smnandre · 2023-12-02T04:45:25Z

src/Extension/Attribute/AsTwigExtension.php

+ * Identifies a class that uses PHP attributes to define filters, functions, or tests.
+ */
+#[\Attribute(\Attribute::TARGET_CLASS)]
+class AsTwigExtension


What is the purpose of this attribute ? I don't see (yet) how it is / will be used

It's similar to Twig\Extension\RuntimeExtensionInterface, it's a marker for classes that provide extension features, to be automatically tagged by Symfony dependency injection.

Given that extensions must implement the ExtensionInterface anyway, there is no need for a marker attribute. The Symfony dependency injection component is also able to perform auto-configuration based on implemented interfaces (and TwigBundle already does it for the Twig ExtensionInterface

It's more a question of DX. It's more meaningful to use an attribute than an interface as a marker as the developer will use attributes in the class. Implementing an interface without method is a deviation from the interface system.

I've made this attribute mandatory to prevent any object from being passed by mistake. In practice, it will be mandatory anyway for Symfony autoconfiguration.

tests/Extension/Fixtures/ExtensionWithAttributes.php

GromNaN

Ready for review @fabpot.

doc/advanced.rst

GromNaN

What is a bit strange is having one single instance of AttributeExtension be responsible for registering many runtimes. Maybe we should instead have one AttributeExtension per runtime? Then it might be easier to use this standalone : addExtension(new AttributeExtension(new AttributeBasedRuntime))) (and such runtimes could have a static factory to make this even easier to create). Note that this suggestion might be wrong as I didn't think of how runtimes should be made lazy-instantiated.

In the ExtensionSet, extensions are identified by their class name. The same extension class cannot be registered multiple times with distinct configuration.

I updated this PR so that extension can be registered directly using any class name or object that have the #[AsTwigExtension] attribute (the attribute becomes required).

    $twig = new \Twig\Environment($loader);
-    $twig->addExtension(new \Twig\Extension\AttributeExtension([
-        ProjectExtension::class,
-    ]));
+    $twig->addExtension(ProjectExtension::class);
    $twig->addRuntimeLoader(new \Twig\RuntimeLoader\FactoryLoader([
        ProjectExtension::class => function () use ($lipsumProvider) {
            return new ProjectExtension($lipsumProvider);
        },
    ]));

To use standalone (non-lazy-loaded):

$twig->addExtension(new ProjectExtension($lipsumProvider));

GromNaN · 2024-04-05T14:39:33Z

src/Environment.php

+    /**
+     * @param ExtensionInterface|object|class-string $extension
+     */
+    public function addExtension(object|string $extension)


This is a breaking change.

stof · 2024-04-05T14:41:27Z

src/Extension/AttributeExtension.php

+        }
+
+        // Assign all at the end to avoid inconsistent state in case of exception
+        $this->filters = $filters;


you should do array_values on those arrays instead of returning names as keys

stof · 2024-04-05T14:43:04Z

src/Attribute/AsTwigTest.php

+ *
+ * If the first argument of the method has Twig\Environment type-hint, the test will receive the current environment.
+ * If the next argument of the method is named $context and has array type-hint, the test will receive the current context.
+ * The last argument of the method is the value to be tested, if any.


this is wrong. It is not the last argument. It is the following argument. A test can have as many arguments as you want after the value being tested, which become arguments in the Twig signature.

stof · 2024-04-05T14:44:43Z

src/Attribute/AsTwigFilter.php

+        public ?array $preservesSafety = null,
+
+        /**
+         * Set to true if the filter is deprecated.


what is the string type about then ?

stof · 2024-04-05T14:45:14Z

src/Attribute/AsTwigFilter.php

+        public ?string $preEscape = null,
+
+        /**
+         * Preserves the safety of the value that the filter is applied to.


this misses the doc for the type of array values

stof · 2024-04-05T14:50:01Z

src/Extension/AttributeExtension.php

+                throw new \LogicException(sprintf('Extension class "%s" must have the attribute "%s" in order to use attributes', is_string($objectOrClass) ? $objectOrClass : get_debug_type($objectOrClass), AsTwigExtension::class));
+            }
+
+            foreach ($reflectionClass->getMethods() as $method) {


thinking about that, I'm wondering whether those asTwigFilter, AsTwigFunction and AsTwigTest attributes should be read by that extension at all.

An alternative solution would be to have an extension in which you inject this list of test, filter and function metadata, and letting TwigBundle perform all this attribute reading at build-time instead.
This would even removed the need for the AsTwigExtension attribute entirely (which would be quite confusing as the class having this attribute is not a twig extension but a twig runtime) because the autoconfiguration system is able to apply autoconfiguration based on method-level attributes already.

stof · 2024-04-05T14:52:06Z

src/ExtensionSet.php

+            if ($extension instanceof ExtensionInterface) {
+                $this->initExtension($extension);
+            } else {
+                $classes[] = $extension;


making $this->extensions contain a mix of extensions and of runtimes (but not all of them) looks weird to me

if this is a way to solve the case of getLastModified, I'd rather add a way to make ExtensionSet aware of a separate list of runtimes needing to be checked (which would allow solving the case where we don't invalidate the cache today if you change the argument names in a runtime while it actually impacts the template compilation if you use named parameters in Twig, by making TwigBundle inject the name of all runtimes in the list).

…as attribute properties

…extension to be registered

GromNaN force-pushed the attribute branch from 6bde02d to 20cf159 Compare November 26, 2023 00:21

GromNaN mentioned this pull request Nov 26, 2023

[RFC][TwigBundle] Register custom functions & filters using an attribute symfony/symfony#50016

Open

GromNaN changed the title ~~Allows registration of filters, functions and tests with an attribute~~ Create attributes AsTwigFilter, AsTwigFunction and AsTwigTest to improve extension development Nov 26, 2023

GromNaN commented Nov 26, 2023

View reviewed changes

GromNaN changed the title ~~Create attributes AsTwigFilter, AsTwigFunction and AsTwigTest to improve extension development~~ Create attributes AsTwigFilter, AsTwigFunction and AsTwigTest to ease extension development Nov 26, 2023

GromNaN force-pushed the attribute branch from 20cf159 to c432044 Compare November 26, 2023 00:51

derrabus reviewed Nov 26, 2023

View reviewed changes

src/Extension/Attribute/AsTwigFilter.php Outdated Show resolved Hide resolved

src/Extension/Extension.php Outdated Show resolved Hide resolved

GromNaN mentioned this pull request Nov 26, 2023

[TwigBundle] Enable #[AsTwigFilter], #[AsTwigFunction] and #[AsTwigTest] attributes to configure runtime extensions symfony/symfony#52748

Open

GromNaN force-pushed the attribute branch 3 times, most recently from d505321 to e64a54b Compare November 27, 2023 08:53

stof reviewed Nov 27, 2023

View reviewed changes

GromNaN force-pushed the attribute branch from 9d3dd5b to 99999e9 Compare November 30, 2023 08:49

GromNaN commented Nov 30, 2023

View reviewed changes

src/Extension/Attribute/AsTwigExtension.php Outdated Show resolved Hide resolved

smnandre reviewed Dec 2, 2023

View reviewed changes

src/Extension/AttributeExtension.php Outdated Show resolved Hide resolved

smnandre reviewed Dec 2, 2023

View reviewed changes

src/Extension/AttributeExtension.php Outdated Show resolved Hide resolved

smnandre reviewed Dec 2, 2023

View reviewed changes

tests/Extension/Fixtures/ExtensionWithAttributes.php Show resolved Hide resolved

GromNaN force-pushed the attribute branch 4 times, most recently from c627a38 to 4c1adc1 Compare December 10, 2023 21:38

GromNaN commented Dec 10, 2023

View reviewed changes

doc/advanced.rst Outdated Show resolved Hide resolved

GromNaN requested review from stof, derrabus and smnandre December 10, 2023 21:42

smnandre approved these changes Dec 12, 2023

View reviewed changes

GromNaN force-pushed the attribute branch from fb3ca3d to 5bfb738 Compare December 26, 2023 20:03

stof mentioned this pull request Jan 4, 2024

[Twig-Bridge] [RFC] Allow to declare twig extensions via config ? symfony/symfony#53403

Closed

stof mentioned this pull request Feb 13, 2024

Add #[YieldReady] to allow extensions to tell when they're ready for yielding #3999

Merged

GromNaN mentioned this pull request Feb 14, 2024

What would you like to see as major changes that would lead to a BC break for the next major release of Twig? #3951

Open

GromNaN force-pushed the attribute branch from 5bfb738 to d517ef1 Compare April 5, 2024 14:35

GromNaN commented Apr 5, 2024

View reviewed changes

GromNaN force-pushed the attribute branch from d517ef1 to 57cb0f4 Compare April 5, 2024 14:47

stof reviewed Apr 5, 2024

View reviewed changes

fabpot force-pushed the 3.x branch from 6df7e6d to 1876635 Compare October 4, 2024 18:59

GromNaN added 19 commits November 24, 2024 01:23

Allows registration of filter/function/test with an attribute

53aeae5

Undocument detected options

4e7ac90

Test repeated attributes

9ed0d44

Use attributes in standalone classes and add flatten promote options …

071ee08

…as attribute properties

Accept a class name to leverage runtime lazy-loading

a91c3d3

Fix CS

81cbbff

Track changes in runtime extensions

9685949

Fix types

270787e

Add AsTwigExtension attribute and make function name required

7b2cf4e

Update phpdoc

064760a

Move attributes to Twig\Attribute namespace

43a8950

Add pattern filter test case

8f095e0

Remove name unicity constraint, add phpdoc, refactor time computation

11b570d

Add docs

49c0f80

Mark attributes as final

1808c2c

Rename ModificationAwareInterface

2bf1fa1

Make AsTwigExtension extension first party instead of relying on the …

aa4f4cb

…extension to be registered

Use array_values for list of filters/functions/tests

f634240

Switch to the new DeprecatedCallableInfo object

1906c8f

GromNaN force-pushed the attribute branch from 57cb0f4 to 2edd448 Compare November 24, 2024 02:22

Improve doc and tests

d16812e

GromNaN force-pushed the attribute branch from 2edd448 to d16812e Compare November 24, 2024 02:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create attributes `AsTwigFilter`, `AsTwigFunction` and `AsTwigTest` to ease extension development #3916

Create attributes `AsTwigFilter`, `AsTwigFunction` and `AsTwigTest` to ease extension development #3916

GromNaN commented Nov 26, 2023 •

edited

Loading

GromNaN commented Nov 26, 2023

stof Nov 27, 2023

GromNaN Nov 30, 2023

GromNaN Nov 30, 2023

GromNaN Nov 30, 2023

GromNaN Dec 1, 2023

GromNaN Dec 8, 2023 •

edited

Loading

nicolas-grekas Dec 26, 2023

GromNaN Dec 26, 2023

GromNaN Apr 5, 2024

smnandre Dec 2, 2023

GromNaN Dec 2, 2023

stof Dec 15, 2023

GromNaN Dec 15, 2023 •

edited

Loading

GromNaN Apr 5, 2024

GromNaN left a comment

GromNaN left a comment •

edited

Loading

GromNaN Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

stof Apr 5, 2024

	public function formatDate(Environment $env, $date, ?string $dateFormat = 'medium', string $pattern = '', $timezone = null, string $calendar = 'gregorian', string $locale = null): string
	{
	return $this->formatDateTime($env, $date, $dateFormat, 'none', $pattern, $timezone, $calendar, $locale);
	}

	public function isTemplateFresh(string $name, int $time): bool
	{
	return $this->extensionSet->getLastModified() <= $time && $this->getLoader()->isFresh($name, $time);
	}

Create attributes AsTwigFilter, AsTwigFunction and AsTwigTest to ease extension development #3916

Are you sure you want to change the base?

Create attributes AsTwigFilter, AsTwigFunction and AsTwigTest to ease extension development #3916

Conversation

GromNaN commented Nov 26, 2023 • edited Loading

GromNaN commented Nov 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN left a comment

Choose a reason for hiding this comment

GromNaN left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Create attributes `AsTwigFilter`, `AsTwigFunction` and `AsTwigTest` to ease extension development #3916

Create attributes `AsTwigFilter`, `AsTwigFunction` and `AsTwigTest` to ease extension development #3916

GromNaN commented Nov 26, 2023 •

edited

Loading

GromNaN Dec 8, 2023 •

edited

Loading

GromNaN Dec 15, 2023 •

edited

Loading

GromNaN left a comment •

edited

Loading