From 52ded207d33d5bc721d19d5f1db8e8c39a8e1b76 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 14 Sep 2024 10:30:39 +0200 Subject: [PATCH] Allow Twig callable arguments to use camel or snake names --- CHANGELOG | 2 + doc/deprecated.rst | 3 + src/Util/CallableArgumentsExtractor.php | 39 +++++++---- tests/Util/CallableArgumentsExtractorTest.php | 66 +++++++++++++++++-- 4 files changed, 94 insertions(+), 16 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ad3841a5de9..ea986469c38 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ # 3.15.0 (2024-XX-XX) + * Allow Twig callable argument names to be free-form (snake-case or camelCase) independently of the PHP callable signature + They were automatically converted to snake-cased before * Deprecate the `attribute` function; use the `.` notation and wrap the name with parenthesis instead * Add support for argument unpackaging * Add JSON support for the file extension escaping strategy diff --git a/doc/deprecated.rst b/doc/deprecated.rst index 7222b4f664b..3a80f632249 100644 --- a/doc/deprecated.rst +++ b/doc/deprecated.rst @@ -286,3 +286,6 @@ Functions/Filters/Tests $twig->addFunction(new TwigFunction('upper', 'upper', [ 'deprecation_info' => new DeprecatedCallableInfo('twig/twig', '3.12'), ])); + +* For variadic arguments, use snake-case for the argument name to ease the + transition to 4.0. diff --git a/src/Util/CallableArgumentsExtractor.php b/src/Util/CallableArgumentsExtractor.php index fedee621581..d8625169d70 100644 --- a/src/Util/CallableArgumentsExtractor.php +++ b/src/Util/CallableArgumentsExtractor.php @@ -40,22 +40,25 @@ public function __construct( public function extractArguments(Node $arguments): array { $extractedArguments = []; + $extractedArgumentNameMap = []; $named = false; foreach ($arguments as $name => $node) { if (!\is_int($name)) { $named = true; - $name = $this->normalizeName($name); } elseif ($named) { throw new SyntaxError(\sprintf('Positional arguments cannot be used after named arguments for %s "%s".', $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); } - $extractedArguments[$name] = $node; + $extractedArguments[$normalizedName = $this->normalizeName($name)] = $node; + $extractedArgumentNameMap[$normalizedName] = $name; } if (!$named && !$this->twigCallable->isVariadic()) { $min = $this->twigCallable->getMinimalNumberOfRequiredArguments(); if (\count($extractedArguments) < $this->rc->getReflector()->getNumberOfRequiredParameters() - $min) { - throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $this->rc->getReflector()->getParameters()[$min + \count($extractedArguments)]->getName(), $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); + $argName = $this->toSnakeCase($this->rc->getReflector()->getParameters()[$min + \count($extractedArguments)]->getName()); + + throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $argName, $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); } return $extractedArguments; @@ -76,7 +79,7 @@ public function extractArguments(Node $arguments): array $optionalArguments = []; $pos = 0; foreach ($callableParameters as $callableParameter) { - $callableParameterName = $this->normalizeName($callableParameter->name); + $callableParameterName = $callableParameter->name; if (\PHP_VERSION_ID >= 80000 && 'range' === $callable) { if ('start' === $callableParameterName) { $callableParameterName = 'low'; @@ -86,8 +89,9 @@ public function extractArguments(Node $arguments): array } $callableParameterNames[] = $callableParameterName; + $normalizedCallableParameterName = $this->normalizeName($callableParameterName); - if (\array_key_exists($callableParameterName, $extractedArguments)) { + if (\array_key_exists($normalizedCallableParameterName, $extractedArguments)) { if (\array_key_exists($pos, $extractedArguments)) { throw new SyntaxError(\sprintf('Argument "%s" is defined twice for %s "%s".', $callableParameterName, $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); } @@ -95,13 +99,13 @@ public function extractArguments(Node $arguments): array if (\count($missingArguments)) { throw new SyntaxError(\sprintf( 'Argument "%s" could not be assigned for %s "%s(%s)" because it is mapped to an internal PHP function which cannot determine default value for optional argument%s "%s".', - $callableParameterName, $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $callableParameterNames), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments) + $callableParameterName, $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', array_map([$this, 'toSnakeCase'], $callableParameterNames)), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments) ), $this->node->getTemplateLine(), $this->node->getSourceContext()); } $arguments = array_merge($arguments, $optionalArguments); - $arguments[] = $extractedArguments[$callableParameterName]; - unset($extractedArguments[$callableParameterName]); + $arguments[] = $extractedArguments[$normalizedCallableParameterName]; + unset($extractedArguments[$normalizedCallableParameterName]); $optionalArguments = []; } elseif (\array_key_exists($pos, $extractedArguments)) { $arguments = array_merge($arguments, $optionalArguments); @@ -118,7 +122,7 @@ public function extractArguments(Node $arguments): array $missingArguments[] = $callableParameterName; } else { - throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $callableParameterName, $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); + throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $this->toSnakeCase($callableParameterName), $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext()); } } @@ -128,7 +132,13 @@ public function extractArguments(Node $arguments): array if (\is_int($key)) { $arbitraryArguments->addElement($value); } else { - $arbitraryArguments->addElement($value, new ConstantExpression($key, $this->node->getTemplateLine())); + $originalKey = $extractedArgumentNameMap[$key]; + if ($originalKey !== $this->toSnakeCase($originalKey)) { + trigger_deprecation('twig/twig', '3.15', \sprintf('Using "snake_case" for variadic arguments is required for a smooth upgrade with Twig 4.0; rename "%s" to "%s" in "%s" at line %d.', $originalKey, $this->toSnakeCase($originalKey), $this->node->getSourceContext()->getName(), $this->node->getTemplateLine())); + } + $arbitraryArguments->addElement($value, new ConstantExpression($this->toSnakeCase($originalKey), $this->node->getTemplateLine())); + // I Twig 4.0, don't convert the key: + // $arbitraryArguments->addElement($value, new ConstantExpression($originalKey, $this->node->getTemplateLine())); } unset($extractedArguments[$key]); } @@ -151,7 +161,7 @@ public function extractArguments(Node $arguments): array throw new SyntaxError( \sprintf( 'Unknown argument%s "%s" for %s "%s(%s)".', - \count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $callableParameterNames) + \count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', array_map([$this, 'toSnakeCase'], $callableParameterNames)) ), $unknownArgument ? $unknownArgument->getTemplateLine() : $this->node->getTemplateLine(), $unknownArgument ? $unknownArgument->getSourceContext() : $this->node->getSourceContext() @@ -163,7 +173,12 @@ public function extractArguments(Node $arguments): array private function normalizeName(string $name): string { - return strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'], ['\\1_\\2', '\\1_\\2'], $name)); + return strtolower(str_replace('_', '', $name)); + } + + private function toSnakeCase(string $name): string + { + return strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z0-9])([A-Z])/'], '\1_\2', $name)); } private function getCallableParameters(): array diff --git a/tests/Util/CallableArgumentsExtractorTest.php b/tests/Util/CallableArgumentsExtractorTest.php index cfea0f8a941..182283183ad 100644 --- a/tests/Util/CallableArgumentsExtractorTest.php +++ b/tests/Util/CallableArgumentsExtractorTest.php @@ -12,15 +12,20 @@ */ use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Twig\Error\SyntaxError; use Twig\Node\Expression\ConstantExpression; use Twig\Node\Expression\FunctionExpression; +use Twig\Node\Expression\VariadicExpression; use Twig\Node\Node; +use Twig\Source; use Twig\TwigFunction; use Twig\Util\CallableArgumentsExtractor; class CallableArgumentsExtractorTest extends TestCase { + use ExpectDeprecationTrait; + public function testGetArguments() { $this->assertEquals(['U', null], $this->getArguments('date', 'date', ['format' => 'U', 'timestamp' => null])); @@ -29,7 +34,7 @@ public function testGetArguments() public function testGetArgumentsWhenPositionalArgumentsAfterNamedArguments() { $this->expectException(SyntaxError::class); - $this->expectExceptionMessage('Positional arguments cannot be used after named arguments for function "date".'); + $this->expectExceptionMessage('Positional arguments cannot be used after named arguments for function "date" in "test.twig" at line 2.'); $this->getArguments('date', 'date', ['timestamp' => 123456, 'Y-m-d']); } @@ -37,7 +42,7 @@ public function testGetArgumentsWhenPositionalArgumentsAfterNamedArguments() public function testGetArgumentsWhenArgumentIsDefinedTwice() { $this->expectException(SyntaxError::class); - $this->expectExceptionMessage('Argument "format" is defined twice for function "date".'); + $this->expectExceptionMessage('Argument "format" is defined twice for function "date" in "test.twig" at line 2.'); $this->getArguments('date', 'date', ['Y-m-d', 'format' => 'U']); } @@ -80,6 +85,54 @@ public function testGetArgumentsForStaticMethod() $this->assertEquals(['arg1'], $this->getArguments('custom_static_function', __CLASS__.'::customStaticFunction', ['arg1' => 'arg1'])); } + /** + * @dataProvider getGetArgumentsConversionData + */ + public function testGetArgumentsConversion($arg1, $arg2) + { + $this->assertEquals([null], $this->getArguments('custom', eval("return fn (\$$arg1) => '';"), [$arg1 => null])); + $this->assertEquals([null], $this->getArguments('custom', eval("return fn (\$$arg2) => '';"), [$arg2 => null])); + $this->assertEquals([null], $this->getArguments('custom', eval("return fn (\$$arg1) => '';"), [$arg2 => null])); + $this->assertEquals([null], $this->getArguments('custom', eval("return fn (\$$arg2) => '';"), [$arg1 => null])); + } + + public static function getGetArgumentsConversionData() + { + yield ['some_name', 'some_name']; + yield ['someName', 'some_name']; + yield ['no_svg', 'noSVG']; + yield ['error_404', 'error404']; + yield ['errCode_404', 'err_code_404']; + yield ['errCode404', 'err_code_404']; + yield ['aBc', 'a_b_c']; + yield ['aBC', 'a_b_c']; + } + + /** + * @group legacy + */ + public function testGetArgumentsConversionForVariadics() + { + $this->expectDeprecation('Since twig/twig 3.15: Using "snake_case" for variadic arguments is required for a smooth upgrade with Twig 4.0; rename "someNumberVariadic" to "some_number_variadic" in "test.twig" at line 2.'); + + $this->assertEquals([ + new ConstantExpression('a', 0), + new ConstantExpression(12, 0), + new VariadicExpression([ + new ConstantExpression('some_text_variadic', 2), new ConstantExpression('a', 0), + new ConstantExpression('some_number_variadic', 2), new ConstantExpression(12, 0), + ], 2), + ], $this->getArguments('custom', eval("return fn (string \$someText, int \$some_number, ...\$args) => '';"), ['some_text' => 'a', 'someNumber' => 12, 'some_text_variadic' => 'a', 'someNumberVariadic' => 12], true)); + } + + public function testGetArgumentsError() + { + $this->expectException(SyntaxError::class); + $this->expectExceptionMessage('Value for argument "some_name" is required for function "custom_static_function" in "test.twig" at line 2.'); + + $this->getArguments('custom_static_function', [$this, 'customFunctionSnakeCamel'], ['someCity' => 'Paris']); + } + public function testResolveArgumentsWithMissingParameterForArbitraryArguments() { $this->expectException(SyntaxError::class); @@ -119,6 +172,10 @@ public function customFunction($arg1, $arg2 = 'default', $arg3 = []) { } + public function customFunctionSnakeCamel($someName, $some_city) + { + } + public function customFunctionWithArbitraryArguments() { } @@ -126,14 +183,15 @@ public function customFunctionWithArbitraryArguments() private function getArguments(string $name, $callable, array $args, bool $isVariadic = false): array { $function = new TwigFunction($name, $callable, ['is_variadic' => $isVariadic]); - $node = new ExpressionCall($function, new Node([]), 0); + $node = new ExpressionCall($function, new Node([]), 2); + $node->setSourceContext(new Source('', 'test.twig')); foreach ($args as $name => $arg) { $args[$name] = new ConstantExpression($arg, 0); } $arguments = (new CallableArgumentsExtractor($node, $function))->extractArguments(new Node($args)); foreach ($arguments as $name => $argument) { - $arguments[$name] = $argument->getAttribute('value'); + $arguments[$name] = $isVariadic ? $argument : $argument->getAttribute('value'); } return $arguments;