Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add types for referenced extensions inside Template #4429

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

Add types for referenced extensions inside Template #4429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Node/CheckSecurityCallNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class CheckSecurityCallNode extends Node
public function compile(Compiler $compiler)
{
$compiler
->write("\$this->sandbox = \$this->extensions[SandboxExtension::class];\n")
->write("\$this->sandbox = \$this->getExtension(SandboxExtension::class);\n")
->write("\$this->checkSecurity();\n")
;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Node/Expression/CallExpression.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected function compileCallable(Compiler $compiler)
// Compile a non-optimized call to trigger a \Twig\Error\RuntimeError, which cannot be a compile-time error
$compiler->raw(\sprintf('$this->env->getExtension(\'%s\')', $class));
} else {
$compiler->raw(\sprintf('$this->extensions[\'%s\']', ltrim($class, '\\')));
$compiler->raw(\sprintf('$this->getExtension(\'%s\')', ltrim($class, '\\')));
Comment on lines 53 to +55
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, why aren't we always using $this->env->getExtension? That one already has proper typing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this was a performance optimization to avoid extra method calls (based on the comment a file lines above).

This could actually be supported for the existing code if phpstan/phpstan#9521 was implemented in phpstan, as the type of Template::$extensions is class-string-map<T of ExtensionInterface, T>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unfortunately we don't have it yet.

}

$compiler->raw(\sprintf('->%s', $callable[1]));
Expand Down
4 changes: 2 additions & 2 deletions src/Profiler/Node/EnterProfileNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public function __construct(string $extensionName, string $type, string $name, s
public function compile(Compiler $compiler): void
{
$compiler
->write(\sprintf('$%s = $this->extensions[', $this->getAttribute('var_name')))
->write(\sprintf('$%s = $this->getExtension(', $this->getAttribute('var_name')))
->repr($this->getAttribute('extension_name'))
->raw("];\n")
->raw(");\n")
->write(\sprintf('$%s->enter($%s = new \Twig\Profiler\Profile($this->getTemplateName(), ', $this->getAttribute('var_name'), $this->getAttribute('var_name').'_prof'))
->repr($this->getAttribute('type'))
->raw(', ')
Expand Down
13 changes: 13 additions & 0 deletions src/Template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use Twig\Error\Error;
use Twig\Error\RuntimeError;
use Twig\Extension\ExtensionInterface;

/**
* Default base class for compiled templates.
Expand Down Expand Up @@ -508,6 +509,18 @@ protected function getTemplateForMacro(string $name, array $context, int $line,
throw new RuntimeError(\sprintf('Macro "%s" is not defined in template "%s".', substr($name, \strlen('macro_')), $this->getTemplateName()), $line, $source);
}

/**
* @template TExtension of ExtensionInterface
*
* @param class-string<TExtension> $class
*
* @return TExtension
*/
protected function getExtension(string $class): ExtensionInterface
{
return $this->extensions[$class];
}

/**
* Auto-generated method to display the template with the given context.
*
Expand Down
10 changes: 5 additions & 5 deletions tests/Node/Expression/FilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,22 @@ public static function provideTests(): iterable
$node = self::createFilter($environment, $expr, 'upper');
$node = self::createFilter($environment, $node, 'number_format', [new ConstantExpression(2, 1), new ConstantExpression('.', 1), new ConstantExpression(',', 1)]);

$tests[] = [$node, '$this->extensions[\'Twig\Extension\CoreExtension\']->formatNumber(Twig\Extension\CoreExtension::upper($this->env->getCharset(), "foo"), 2, ".", ",")'];
$tests[] = [$node, '$this->getExtension(\'Twig\Extension\CoreExtension\')->formatNumber(Twig\Extension\CoreExtension::upper($this->env->getCharset(), "foo"), 2, ".", ",")'];

// named arguments
$date = new ConstantExpression(0, 1);
$node = self::createFilter($environment, $date, 'date', [
'timezone' => new ConstantExpression('America/Chicago', 1),
'format' => new ConstantExpression('d/m/Y H:i:s P', 1),
]);
$tests[] = [$node, '$this->extensions[\'Twig\Extension\CoreExtension\']->formatDate(0, "d/m/Y H:i:s P", "America/Chicago")'];
$tests[] = [$node, '$this->getExtension(\'Twig\Extension\CoreExtension\')->formatDate(0, "d/m/Y H:i:s P", "America/Chicago")'];

// skip an optional argument
$date = new ConstantExpression(0, 1);
$node = self::createFilter($environment, $date, 'date', [
'timezone' => new ConstantExpression('America/Chicago', 1),
]);
$tests[] = [$node, '$this->extensions[\'Twig\Extension\CoreExtension\']->formatDate(0, null, "America/Chicago")'];
$tests[] = [$node, '$this->getExtension(\'Twig\Extension\CoreExtension\')->formatDate(0, null, "America/Chicago")'];

// underscores vs camelCase for named arguments
$string = new ConstantExpression('abc', 1);
Expand Down Expand Up @@ -103,7 +103,7 @@ public static function provideTests(): iterable
$tests[] = [$node, 'Twig\Tests\Node\Expression\FilterTestExtension::staticMethod("abc")', $environment];

$node = self::createFilter($environment, $string, 'first_class_callable_object');
$tests[] = [$node, '$this->extensions[\'Twig\Tests\Node\Expression\FilterTestExtension\']->objectMethod("abc")', $environment];
$tests[] = [$node, '$this->getExtension(\'Twig\Tests\Node\Expression\FilterTestExtension\')->objectMethod("abc")', $environment];
}

$node = self::createFilter($environment, $string, 'barbar', [
Expand All @@ -116,7 +116,7 @@ public static function provideTests(): iterable

// from extension
$node = self::createFilter($environment, $string, 'foo');
$tests[] = [$node, \sprintf('$this->extensions[\'%s\']->foo("abc")', \get_class(self::createExtension())), $environment];
$tests[] = [$node, \sprintf('$this->getExtension(\'%s\')->foo("abc")', \get_class(self::createExtension())), $environment];

$node = self::createFilter($environment, $string, 'foobar');
$tests[] = [$node, '$this->env->getFilter(\'foobar\')->getCallable()("abc")', $environment];
Expand Down
2 changes: 1 addition & 1 deletion tests/Node/Expression/FunctionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public static function provideTests(): iterable
'timezone' => new ConstantExpression('America/Chicago', 1),
'date' => new ConstantExpression(0, 1),
]);
$tests[] = [$node, '$this->extensions[\'Twig\Extension\CoreExtension\']->convertDate(0, "America/Chicago")'];
$tests[] = [$node, '$this->getExtension(\'Twig\Extension\CoreExtension\')->convertDate(0, "America/Chicago")'];

// arbitrary named arguments
$node = self::createFunction($environment, 'barbar');
Expand Down