Skip to content

Commit

Permalink
feature #4318 Allow Twig callable arguments to use camel or snake nam…
Browse files Browse the repository at this point in the history 
…es (fabpot)

This PR was squashed before being merged into the 3.x branch.

Discussion
----------

Allow Twig callable arguments to use camel or snake names

Closes #3475

Twig callables (functions/filters/tests) now accept snake and camel argument names independently of the name of the underlying PHP callable parameter names.

Commits
-------

52ded20 Allow Twig callable arguments to use camel or snake names
  • Loading branch information
@fabpot
fabpot committed Sep 18, 2024
2 parents b8b0313 + 52ded20 commit 70886ec
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 3 additions & 0 deletions doc/deprecated.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
39 changes: 27 additions & 12 deletions src/Util/CallableArgumentsExtractor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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';
Expand All @@ -86,22 +89,23 @@ 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());
}

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);
Expand All @@ -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());
}
}

Expand All @@ -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]);
}
Expand All @@ -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()
Expand All @@ -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
Expand Down
66 changes: 62 additions & 4 deletions tests/Util/CallableArgumentsExtractorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
Expand All @@ -29,15 +34,15 @@ 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']);
}

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']);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -119,21 +172,26 @@ public function customFunction($arg1, $arg2 = 'default', $arg3 = [])
{
}

public function customFunctionSnakeCamel($someName, $some_city)
{
}

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;
Expand Down

0 comments on commit 70886ec

Please sign in to comment.