Twig functions becoming internal

[cs278]

Since 3.9 it appears all of Twigs filter/functions underlying PHP implementations are marked as internal, could this be reconsidered in some cases? I'm assuming internal means the signature might change in non backwards compatible ways, if it doesn't I guess this isn't really a problem.

The two cases that affect myself are twig_escape_filter and twig_date_converter.

For example I have a filter which generates some HTML:

new TwigFilter('phone_number_link', function (Environment $env, $number) {
    $anchor = $this->libphonenumber->format($number, PhoneNumberFormat::RFC3966);

    return sprintf(
        '<a href="%s">%s</a>',
        \twig_escape_filter($env, $anchor, 'html_attr'),
        \twig_escape_filter($env, $this->formatDisplay($number), 'html'),
    );
}, ['needs_environment' => true, 'is_safe' => ['html']]),

I've consulted the documentation but it doesn't seem to offer any examples on how to deal with more complicated situations such as this, so perhaps this is the wrong way of doing it but it's worked since Twig 1.x.

Likewise with dates I have some custom filters to limit developers to a limited set of predefined formats, the filters look a bit like this:

new TwigFilter('date_formal_format', function (Environment $env, $date, ?string $locale = null) {
    if (null === $date) {
        return null;
    }

    return DateFormat::formalDate(twig_date_converter($env, $date), $locale ?? $this->locale);
}, ['needs_environment' => true]),

Being able to use twig_date_converter means the Twig timezone setting works as expected.

I did some searching on GitHub and found numerous other cases.

https://github.com/theoboldt/juvem/blob/98af699c95c46e919dc1690ab1730754ebd65141/app/src/AppBundle/Twig/Extension/CustomFieldValue.php#L244
https://github.com/njh/twig-html-helpers/blob/d3dc45f6a9ac67512126c02a8d11c7a09889d942/lib/Twig/Extension/HTMLHelpers.php#L53
https://github.com/symfony/ux/blob/d4df61465571381ffa8692268fca6acd09b32feb/src/StimulusBundle/src/Dto/StimulusAttributes.php#L223
https://github.com/symfony/ux/blob/d4df61465571381ffa8692268fca6acd09b32feb/src/LiveComponent/src/Util/LiveAttributesCollection.php#L121
https://github.com/studiometa/twig-toolkit/blob/9e73108d84c11171ef69fd34dd8946afee8e9a5c/src/Helpers/Html.php#L209
https://github.com/mautic/mautic/blob/175ceac9098f0b9515f34820a0eafd924877864b/plugins/MauticFocusBundle/Model/FocusModel.php#L183
https://github.com/symfony/symfony/blob/a2d03c548c2e897be78504fd4a389bb7eb41e756/src/Symfony/Bundle/WebProfilerBundle/Twig/WebProfilerExtension.php#L116
https://github.com/drupal/drupal/blob/449f6fbf7c058988ba6f18a8e70cba4bf6639941/core/lib/Drupal/Core/Template/TwigExtension.php#L464
https://github.com/phpbb/phpbb/blob/4c721de243967ed4cf372caef4e54142ee6bcf6c/phpBB/phpbb/template/twig/extension.php#L215
https://github.com/IISHF/web2019/blob/0ca849abf7f247abfcf01521b6cdeee22e1d637b/src/Infrastructure/Twig/DateExtension.php#L108-L115
https://github.com/imarc/cms/blob/ef349e03202628638247a71158ff04c49da2068a/src/web/twig/Extension.php#L482
https://github.com/twigphp/intl-extra/blob/00cd46d7860cf7e814f73b290edf32801a47cafc/IntlExtension.php#L282

[ericmorand]

Just my two cents here: that those functions became internal is perfectly legit; that they became internal in the context of a minor version is very controversial and requires some explanation.

[fabpot]

They are deprecated, but still here, that's the way we work for Symfony and Twig. We first deprecate so that you've time to upgrade to the new way, and we then remove it in the next major. twig_date_converter was missing but it was fixed, so the latest version of 3.9 should work fine now.

[ericmorand]

Thanks for the clarification. I stand corrected. 🙏

[cs278]

Perhaps I made this unclear by using the old names, I understand they have been deprecated and there are replacements, my concern is those replacements are marked with @internal annotations. As an example twig_escape_filter() is replaced with:

Twig/src/Extension/EscaperExtension.php

Lines 170 to 182 in ad93431

	/**
	* Escapes a string.
	*
	* @param mixed $string The value to be escaped
	* @param string $strategy The escaping strategy
	* @param string $charset The charset
	* @param bool $autoescape Whether the function is called by the auto-escaping feature (true) or by the developer (false)
	*
	* @return string|Markup
	*
	* @internal
	*/
	public static function escape(Environment $env, $string, $strategy = 'html', $charset = null, $autoescape = false)

It seems crazy to me that I'm not supposed to use Twig to escape content I generate in a custom Twig extension.

[ericmorand]

@cs278 I totally agree. A method that was not marked as internal - and thus that was implicitly public - disappeared and it's replacement was marked internal, all of this during a minor version change.

[stof]

The non-internal functions did not disappear. They are marked as deprecated, which means they still work until Twig 4.0.

And the new internal methods are not meant to be replacements for public usages of those functions. The replacement is to stop using Twig internal APIs directly.

[cs278]

And the new internal methods are not meant to be replacements for public usages of those functions. The replacement is to stop using Twig internal APIs directly.

So what is the migration path then? Your reply seems to suggest there is a different way of calling these from an extension but I cannot find anything in the documentation or changelog, clearly I'm not alone in missing this knowledge:

https://github.com/symfony/ux/blob/d4df61465571381ffa8692268fca6acd09b32feb/src/LiveComponent/src/Util/LiveAttributesCollection.php#L115-L123

[diffy0712]

Hello,

Regarding the 3.9 changes I got a problem where a publicly available function twig_get_attribute was removed from the public api breaking the yiisoft/yii2-twig package when upgrading above 'twig:3.9'. There is an open issue in the yiisoft/yii2-twig repo for this. This function is called at this line. I found that the function was removed in this pr.

I think this breaking change should at least be mentioned in the changelog and being in a mayor release.
This might just be some unfortunate mistake, but should be addressed so it might not break others relying on it.

Am I missing something on this?

Thank you

[xabbuh]

I think this breaking change should at least be mentioned in the changelog and being in a mayor release.

twig_get_attribute() was an internal function which was never meant to be called from third-party packages. That's why it could be removed in a minor release and why the change was also not listed in the changelog.

[diffy0712]

I see, that is completely reasonable. Unfortunately the yiisoft/yii2-twig used it and by removing it, it broke that package, which is an issue, but it should be handled there and nothing to do with twig.
Thank you for your reply :)

[fabpot]

Thank you all for the feedback.

One option would be to review all the new internal methods and determine which ones should be part of the public API because they are useful when creating Twig extensions. For instance, the EscaperExtension::escape() method should probably be part of the public API.

[fabpot]

#4044 is a first step that would allow extracting a public API for some methods.

[cs278]

Here's a rough idea of the methods I think should at the very least become part of the public API.

Method	API?
Twig\Extension\CoreExtension::cycle()	No
Twig\Extension\CoreExtension::random()	No
Twig\Extension\CoreExtension::dateFormatFilter()	Yes
Twig\Extension\CoreExtension::dateModifyFilter()	?
Twig\Extension\CoreExtension::sprintf()	No
Twig\Extension\CoreExtension::dateConverter()	Yes
Twig\Extension\CoreExtension::replaceFilter()	No
Twig\Extension\CoreExtension::round()	No
Twig\Extension\CoreExtension::numberFormatFilter()	Yes
Twig\Extension\CoreExtension::urlencodeFilter()	No
Twig\Extension\CoreExtension::arrayMerge()	No
Twig\Extension\CoreExtension::slice()	No
Twig\Extension\CoreExtension::first()	No
Twig\Extension\CoreExtension::last()	No
Twig\Extension\CoreExtension::joinFilter()	No
Twig\Extension\CoreExtension::splitFilter()	No
Twig\Extension\CoreExtension::defaultFilter()	No
Twig\Extension\CoreExtension::getArrayKeysFilter()	No
Twig\Extension\CoreExtension::reverseFilter()	No
Twig\Extension\CoreExtension::sortFilter()	No
Twig\Extension\CoreExtension::inFilter()	No
Twig\Extension\CoreExtension::compare()	No
Twig\Extension\CoreExtension::matches()	No
Twig\Extension\CoreExtension::trimFilter()	No
Twig\Extension\CoreExtension::nl2br()	No
Twig\Extension\CoreExtension::spaceless()	?
Twig\Extension\CoreExtension::convertEncoding()	?
Twig\Extension\CoreExtension::lengthFilter()	No
Twig\Extension\CoreExtension::upperFilter()	No
Twig\Extension\CoreExtension::lowerFilter()	No
Twig\Extension\CoreExtension::striptags()	No
Twig\Extension\CoreExtension::titleStringFilter()	No
Twig\Extension\CoreExtension::capitalizeStringFilter()	No
Twig\Extension\CoreExtension::callMacro()	No
Twig\Extension\CoreExtension::ensureTraversable()	No
Twig\Extension\CoreExtension::toArray()	No
Twig\Extension\CoreExtension::testEmpty()	No
Twig\Extension\CoreExtension::testIterable()	No
Twig\Extension\CoreExtension::include()	No
Twig\Extension\CoreExtension::source()	No
Twig\Extension\CoreExtension::constant()	No
Twig\Extension\CoreExtension::constantIsDefined()	No
Twig\Extension\CoreExtension::arrayBatch()	No
Twig\Extension\CoreExtension::getAttribute()	No
Twig\Extension\CoreExtension::arrayColumn()	No
Twig\Extension\CoreExtension::arrayFilter()	No
Twig\Extension\CoreExtension::arrayMap()	No
Twig\Extension\CoreExtension::arrayReduce()	No
Twig\Extension\CoreExtension::arraySome()	No
Twig\Extension\CoreExtension::arrayEvery()	No
Twig\Extension\CoreExtension::checkArrowInSandbox()	No
Twig\Extension\CoreExtension::captureOutput()	No
Twig\Extension\DebugExtension::dump()	No
Twig\Extension\EscaperExtension::raw()	No
Twig\Extension\EscaperExtension::escapeFilterIsSafe()	?
Twig\Extension\EscaperExtension::escape()	Yes
Twig\Extension\StringLoaderExtension::templateFromString()	No

I've mainly gone for the complicated ones like escape() that offer a lot of utility to extension authors or those that change their behaviour according to Twig configuration like numberFormatFilter().

I generated the list with:

collect.php

<?php

use Twig\Extension as Ext;

require 'vendor/autoload.php';

$ref = trim(shell_exec('git rev-parse HEAD'));
$exts = [Ext\CoreExtension::class, Ext\DebugExtension::class, Ext\EscaperExtension::class];
$exts = [...$exts, Ext\StagingExtension::class, Ext\StringLoaderExtension::class, Ext\YieldNotReadyExtension::class];

function isInternal(\ReflectionMethod $m): bool
{
    return \str_contains($m->getDocComment(), '@internal');
}

echo "| Method | API? |\n";
echo "| ------ | ---- |\n";

foreach ($exts as $ext) {
    $class = new \ReflectionClass($ext);
    foreach ($class->getMethods() as $method) {
        if ($method->isPublic() && $method->isStatic() && isInternal($method)) {
            echo sprintf("| [`%s::%s()`](https://github.com/twigphp/Twig/blob/%s/src/Extension/%s.php#L%u-L%u) |     |\n", $class->getName(), $method->getName(), $ref, $class->getShortName(), $method->getStartLine(), $method->getEndLine());
        }
    }
}