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

Allow Twig callable arguments to use camel or snake names #4318

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Allow Twig callable arguments to use camel or snake names #4318

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: 2 additions & 0 deletions CHANGELOG
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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