Throw a SyntaxError exception at compile time when a Twig callable has not the minimum number of required arguments

This commit is contained in:
Fabien Potencier
2024-08-15 22:44:06 +02:00
parent 55733a2da1
commit 0823d23488
12 changed files with 87 additions and 74 deletions
+1
View File
@@ -1,5 +1,6 @@
# 3.12.0 (2024-XX-XX)
* Throw a SyntaxError exception at compile time when a Twig callable has not the minimum number of required arguments
* Add a `CallableArgumentsExtractor` class
* Deprecate passing a name to `FunctionExpression`, `FilterExpression`, and `TestExpression`;
pass a `TwigFunction`, `TwigFilter`, or `TestFilter` instead
+5
View File
@@ -128,4 +128,9 @@ abstract class AbstractTwigCallable implements TwigCallableInterface
{
return $this->options['alternative'];
}
public function getMinimalNumberOfRequiredArguments(): int
{
return ($this->options['needs_charset'] ? 1 : 0) + ($this->options['needs_environment'] ? 1 : 0) + ($this->options['needs_context'] ? 1 : 0) + \count($this->arguments);
}
}
+2
View File
@@ -46,4 +46,6 @@ interface TwigCallableInterface extends \Stringable
public function getDeprecatedVersion(): string;
public function getAlternative(): ?string;
public function getMinimalNumberOfRequiredArguments(): int;
}
+5
View File
@@ -61,4 +61,9 @@ final class TwigFilter extends AbstractTwigCallable
{
return $this->options['pre_escape'];
}
public function getMinimalNumberOfRequiredArguments(): int
{
return parent::getMinimalNumberOfRequiredArguments() + 1;
}
}
+34 -32
View File
@@ -49,7 +49,8 @@ final class CallableArgumentsExtractor
*/
public function extractArguments(Node $arguments): array
{
$parameters = [];
$rc = new ReflectionCallable($this->twigCallable->getCallable(), $this->type, $this->name);
$extractedArguments = [];
$named = false;
foreach ($arguments as $name => $node) {
if (!\is_int($name)) {
@@ -59,24 +60,27 @@ final class CallableArgumentsExtractor
throw new SyntaxError(\sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
}
$parameters[$name] = $node;
$extractedArguments[$name] = $node;
}
if (!$named && !$this->twigCallable->isVariadic()) {
return $parameters;
$min = $this->twigCallable->getMinimalNumberOfRequiredArguments();
if (count($extractedArguments) < $rc->getReflector()->getNumberOfRequiredParameters() - $min) {
throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $rc->getReflector()->getParameters()[$min + count($extractedArguments)]->getName(), $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
}
return $extractedArguments;
}
if (!$callable = $this->twigCallable->getCallable()) {
if ($named) {
$message = \sprintf('Named arguments are not supported for %s "%s".', $this->type, $this->name);
} else {
$message = \sprintf('Arbitrary positional arguments are not supported for %s "%s".', $this->type, $this->name);
throw new SyntaxError(\sprintf('Named arguments are not supported for %s "%s".', $this->type, $this->name));
}
throw new \LogicException($message);
throw new SyntaxError(\sprintf('Arbitrary positional arguments are not supported for %s "%s".', $this->type, $this->name));
}
[$callableParameters, $isPhpVariadic] = $this->getCallableParameters();
[$callableParameters, $isPhpVariadic] = $this->getCallableParameters($rc);
$arguments = [];
$names = [];
$missingArguments = [];
@@ -94,8 +98,8 @@ final class CallableArgumentsExtractor
$names[] = $name;
if (\array_key_exists($name, $parameters)) {
if (\array_key_exists($pos, $parameters)) {
if (\array_key_exists($name, $extractedArguments)) {
if (\array_key_exists($pos, $extractedArguments)) {
throw new SyntaxError(\sprintf('Argument "%s" is defined twice for %s "%s".', $name, $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
}
@@ -107,23 +111,23 @@ final class CallableArgumentsExtractor
}
$arguments = array_merge($arguments, $optionalArguments);
$arguments[] = $parameters[$name];
unset($parameters[$name]);
$arguments[] = $extractedArguments[$name];
unset($extractedArguments[$name]);
$optionalArguments = [];
} elseif (\array_key_exists($pos, $parameters)) {
} elseif (\array_key_exists($pos, $extractedArguments)) {
$arguments = array_merge($arguments, $optionalArguments);
$arguments[] = $parameters[$pos];
unset($parameters[$pos]);
$arguments[] = $extractedArguments[$pos];
unset($extractedArguments[$pos]);
$optionalArguments = [];
++$pos;
} elseif ($callableParameter->isDefaultValueAvailable()) {
$optionalArguments[] = new ConstantExpression($callableParameter->getDefaultValue(), -1);
} elseif ($callableParameter->isOptional()) {
if (empty($parameters)) {
if (!$extractedArguments) {
break;
} else {
$missingArguments[] = $name;
}
$missingArguments[] = $name;
} else {
throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $name, $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
}
@@ -131,13 +135,13 @@ final class CallableArgumentsExtractor
if ($this->twigCallable->isVariadic()) {
$arbitraryArguments = $isPhpVariadic ? new VariadicExpression([], -1) : new ArrayExpression([], -1);
foreach ($parameters as $key => $value) {
foreach ($extractedArguments as $key => $value) {
if (\is_int($key)) {
$arbitraryArguments->addElement($value);
} else {
$arbitraryArguments->addElement($value, new ConstantExpression($key, -1));
}
unset($parameters[$key]);
unset($extractedArguments[$key]);
}
if ($arbitraryArguments->count()) {
@@ -146,11 +150,11 @@ final class CallableArgumentsExtractor
}
}
if (!empty($parameters)) {
$unknownParameter = null;
foreach ($parameters as $parameter) {
if ($parameter instanceof Node) {
$unknownParameter = $parameter;
if ($extractedArguments) {
$unknownArgument = null;
foreach ($extractedArguments as $extractedArgument) {
if ($extractedArgument instanceof Node) {
$unknownArgument = $extractedArgument;
break;
}
}
@@ -158,10 +162,10 @@ final class CallableArgumentsExtractor
throw new SyntaxError(
\sprintf(
'Unknown argument%s "%s" for %s "%s(%s)".',
\count($parameters) > 1 ? 's' : '', implode('", "', array_keys($parameters)), $this->type, $this->name, implode(', ', $names)
\count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->type, $this->name, implode(', ', $names)
),
$unknownParameter ? $unknownParameter->getTemplateLine() : $this->node->getTemplateLine(),
$unknownParameter ? $unknownParameter->getSourceContext() : $this->node->getSourceContext()
$unknownArgument ? $unknownArgument->getTemplateLine() : $this->node->getTemplateLine(),
$unknownArgument ? $unknownArgument->getSourceContext() : $this->node->getSourceContext()
);
}
@@ -173,11 +177,9 @@ final class CallableArgumentsExtractor
return strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'], ['\\1_\\2', '\\1_\\2'], $name));
}
private function getCallableParameters(): array
private function getCallableParameters(ReflectionCallable $rc): array
{
$rc = new ReflectionCallable($this->twigCallable->getCallable(), $this->type, $this->name);
$r = $rc->getReflector();
$callableName = $rc->getName();
$parameters = $r->getParameters();
if ($this->node->hasNode('node')) {
@@ -206,7 +208,7 @@ final class CallableArgumentsExtractor
array_pop($parameters);
$isPhpVariadic = true;
} else {
throw new \LogicException(\sprintf('The last parameter of "%s" for %s "%s" must be an array with default value, eg. "array $arg = []".', $callableName, $this->type, $this->name));
throw new SyntaxError(\sprintf('The last parameter of "%s" for %s "%s" must be an array with default value, eg. "array $arg = []".', $rc->getName(), $this->type, $this->name));
}
}
-22
View File
@@ -234,28 +234,6 @@ EOHTML
}
}
public function testTwigArgumentCountErrorThrowsRuntimeExceptions()
{
$loader = new ArrayLoader([
'argument-error.html' => <<<EOHTML
{# max requires at least one argument #}
{{ max() }}
EOHTML
]);
$twig = new Environment($loader, ['debug' => true, 'cache' => false]);
$template = $twig->load('argument-error.html');
try {
$template->render();
$this->fail();
} catch (RuntimeError $e) {
$this->assertEquals(2, $e->getTemplateLine());
$this->assertEquals('argument-error.html', $e->getSourceContext()->getName());
}
}
public function getErroredTemplates()
{
return [
@@ -0,0 +1,8 @@
--TEST--
"cycle" function without enough args and a named argument
--TEMPLATE--
{{ cycle(position=2) }}
--DATA--
return []
--EXCEPTION--
Twig\Error\SyntaxError: Value for argument "values" is required for function "cycle" in "index.twig" at line 2.
@@ -0,0 +1,8 @@
--TEST--
"max" function without an argument throws a compile time exception
--TEMPLATE--
{{ max() }}
--DATA--
return []
--EXCEPTION--
Twig\Error\SyntaxError: Value for argument "value" is required for function "max" in "index.twig" at line 2.
+6 -2
View File
@@ -67,7 +67,7 @@ EOF
];
$environment = new Environment(new ArrayLoader());
$environment->addFunction($function = new TwigFunction('foo', 'foo', []));
$environment->addFunction($function = new TwigFunction('foo', 'Twig\Tests\Node\foo', []));
$expr = new FunctionExpression($function, new Node(), 1);
$node = new DeprecatedNode($expr, 1, 'deprecated');
@@ -80,7 +80,7 @@ EOF
$tests[] = [$node, <<<EOF
// line 1
\$$varName = foo();
\$$varName = Twig\Tests\Node\\foo();
trigger_deprecation("twig/twig", "1.1", \$$varName." in \"foo.twig\" at line 1.");
EOF
, $environment];
@@ -88,3 +88,7 @@ EOF
return $tests;
}
}
function foo()
{
}
+3 -3
View File
@@ -86,13 +86,13 @@ class FilterTest extends NodeTestCase
// needs environment
$node = $this->createFilter($environment, $string, 'bar');
$tests[] = [$node, 'twig_tests_filter_dummy($this->env, "abc")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_filter_dummy($this->env, "abc")', $environment];
$node = $this->createFilter($environment, $string, 'bar_closure');
$tests[] = [$node, twig_tests_filter_dummy::class.'($this->env, "abc")', $environment];
$node = $this->createFilter($environment, $string, 'bar', [new ConstantExpression('bar', 1)]);
$tests[] = [$node, 'twig_tests_filter_dummy($this->env, "abc", "bar")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_filter_dummy($this->env, "abc", "bar")', $environment];
// arbitrary named arguments
$node = $this->createFilter($environment, $string, 'barbar');
@@ -170,7 +170,7 @@ class FilterTest extends NodeTestCase
{
$env = new Environment(new ArrayLoader());
$env->addFilter(new TwigFilter('anonymous', function () {}));
$env->addFilter(new TwigFilter('bar', 'twig_tests_filter_dummy', ['needs_environment' => true]));
$env->addFilter(new TwigFilter('bar', 'Twig\Tests\Node\Expression\twig_tests_filter_dummy', ['needs_environment' => true]));
$env->addFilter(new TwigFilter('bar_closure', \Closure::fromCallable(twig_tests_filter_dummy::class), ['needs_environment' => true]));
$env->addFilter(new TwigFilter('barbar', 'Twig\Tests\Node\Expression\twig_tests_filter_barbar', ['needs_context' => true, 'is_variadic' => true]));
$env->addFilter(new TwigFilter('magic_static', __NAMESPACE__.'\ChildMagicCallStub::magicStaticCall'));
+12 -12
View File
@@ -38,31 +38,31 @@ class FunctionTest extends NodeTestCase
$tests = [];
$node = $this->createFunction($environment, 'foo');
$tests[] = [$node, 'twig_tests_function_dummy()', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy()', $environment];
$node = $this->createFunction($environment, 'foo_closure');
$tests[] = [$node, twig_tests_function_dummy::class.'()', $environment];
$node = $this->createFunction($environment, 'foo', [new ConstantExpression('bar', 1), new ConstantExpression('foobar', 1)]);
$tests[] = [$node, 'twig_tests_function_dummy("bar", "foobar")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy("bar", "foobar")', $environment];
$node = $this->createFunction($environment, 'bar');
$tests[] = [$node, 'twig_tests_function_dummy($this->env)', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($this->env)', $environment];
$node = $this->createFunction($environment, 'bar', [new ConstantExpression('bar', 1)]);
$tests[] = [$node, 'twig_tests_function_dummy($this->env, "bar")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($this->env, "bar")', $environment];
$node = $this->createFunction($environment, 'foofoo');
$tests[] = [$node, 'twig_tests_function_dummy($context)', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($context)', $environment];
$node = $this->createFunction($environment, 'foofoo', [new ConstantExpression('bar', 1)]);
$tests[] = [$node, 'twig_tests_function_dummy($context, "bar")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($context, "bar")', $environment];
$node = $this->createFunction($environment, 'foobar');
$tests[] = [$node, 'twig_tests_function_dummy($this->env, $context)', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($this->env, $context)', $environment];
$node = $this->createFunction($environment, 'foobar', [new ConstantExpression('bar', 1)]);
$tests[] = [$node, 'twig_tests_function_dummy($this->env, $context, "bar")', $environment];
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_function_dummy($this->env, $context, "bar")', $environment];
// named arguments
$node = $this->createFunction($environment, 'date', [
@@ -105,11 +105,11 @@ class FunctionTest extends NodeTestCase
{
$env = new Environment(new ArrayLoader());
$env->addFunction(new TwigFunction('anonymous', function () {}));
$env->addFunction(new TwigFunction('foo', 'twig_tests_function_dummy', []));
$env->addFunction(new TwigFunction('foo', 'Twig\Tests\Node\Expression\twig_tests_function_dummy', []));
$env->addFunction(new TwigFunction('foo_closure', \Closure::fromCallable(twig_tests_function_dummy::class), []));
$env->addFunction(new TwigFunction('bar', 'twig_tests_function_dummy', ['needs_environment' => true]));
$env->addFunction(new TwigFunction('foofoo', 'twig_tests_function_dummy', ['needs_context' => true]));
$env->addFunction(new TwigFunction('foobar', 'twig_tests_function_dummy', ['needs_environment' => true, 'needs_context' => true]));
$env->addFunction(new TwigFunction('bar', 'Twig\Tests\Node\Expression\twig_tests_function_dummy', ['needs_environment' => true]));
$env->addFunction(new TwigFunction('foofoo', 'Twig\Tests\Node\Expression\twig_tests_function_dummy', ['needs_context' => true]));
$env->addFunction(new TwigFunction('foobar', 'Twig\Tests\Node\Expression\twig_tests_function_dummy', ['needs_environment' => true, 'needs_context' => true]));
$env->addFunction(new TwigFunction('barbar', 'Twig\Tests\Node\Expression\twig_tests_function_barbar', ['is_variadic' => true]));
return $env;
@@ -82,7 +82,7 @@ class CallableArgumentsExtractorTest extends TestCase
public function testResolveArgumentsWithMissingParameterForArbitraryArguments()
{
$this->expectException(\LogicException::class);
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('The last parameter of "Twig\\Tests\\Util\\CallableArgumentsExtractorTest::customFunctionWithArbitraryArguments" for function "foo" must be an array with default value, eg. "array $arg = []".');
$this->getArguments('foo', [$this, 'customFunctionWithArbitraryArguments'], [], true);
@@ -97,7 +97,7 @@ class CallableArgumentsExtractorTest extends TestCase
public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnFunction()
{
$this->expectException(\LogicException::class);
$this->expectException(SyntaxError::class);
$this->expectExceptionMessageMatches('#^The last parameter of "Twig\\\\Tests\\\\Util\\\\custom_call_test_function" for function "foo" must be an array with default value, eg\\. "array \\$arg \\= \\[\\]"\\.$#');
$this->getArguments('foo', 'Twig\Tests\Util\custom_call_test_function', [], true);
@@ -105,7 +105,7 @@ class CallableArgumentsExtractorTest extends TestCase
public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnObject()
{
$this->expectException(\LogicException::class);
$this->expectException(SyntaxError::class);
$this->expectExceptionMessageMatches('#^The last parameter of "Twig\\\\Tests\\\\Util\\\\CallableTestClass\\:\\:__invoke" for function "foo" must be an array with default value, eg\\. "array \\$arg \\= \\[\\]"\\.$#');
$this->getArguments('foo', new CallableTestClass(), [], true);