Skip to content

Commit

Permalink
minor #4217 Refactor code (fabpot)
Browse files Browse the repository at this point in the history 
This PR was merged into the 3.x branch.

Discussion
----------

Refactor code

Commits
-------

31037d0 Refactor code
  • Loading branch information
@fabpot
fabpot committed Aug 20, 2024
2 parents 63e35b9 + 31037d0 commit b9b8d52
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 50 deletions.
8 changes: 4 additions & 4 deletions src/Node/Expression/CallExpression.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ protected function compileCallable(Compiler $compiler)
if (\is_string($callable) && !str_contains($callable, '::')) {
$compiler->raw($callable);
} else {
$rc = $this->reflectCallable($callable);
$rc = $this->reflectCallable($twigCallable);
$r = $rc->getReflector();
$callable = $rc->getCallable();

Expand Down Expand Up @@ -271,7 +271,7 @@ protected function normalizeName(string $name): string
private function getCallableParameters($callable, bool $isVariadic): array
{
$twigCallable = $this->getAttribute('twig_callable');
$rc = $this->reflectCallable($callable);
$rc = $this->reflectCallable($twigCallable);
$r = $rc->getReflector();
$callableName = $rc->getName();

Expand Down Expand Up @@ -309,10 +309,10 @@ private function getCallableParameters($callable, bool $isVariadic): array
return [$parameters, $isPhpVariadic];
}

private function reflectCallable($callable): ReflectionCallable
private function reflectCallable(TwigCallableInterface $callable): ReflectionCallable
{
if (!$this->reflector) {
$this->reflector = new ReflectionCallable($callable, $this->getAttribute('type'), $this->getAttribute('name'));
$this->reflector = new ReflectionCallable($callable);
}

return $this->reflector;
Expand Down
2 changes: 1 addition & 1 deletion src/Parser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public function subparse($test, bool $dropNeedle = false): Node
if (null !== $test) {
$e = new SyntaxError(\sprintf('Unexpected "%s" tag', $token->getValue()), $token->getLine(), $this->stream->getSourceContext());

$callable = (new ReflectionCallable($test))->getCallable();
$callable = (new ReflectionCallable(new TwigTest('decision', $test)))->getCallable();
if (\is_array($callable) && $callable[0] instanceof TokenParserInterface) {
$e->appendMessage(\sprintf(' (expecting closing tag for the "%s" tag defined near line %s).', $callable[0]->getTag(), $lineno));
}
Expand Down
2 changes: 2 additions & 0 deletions src/TwigCallableInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ interface TwigCallableInterface extends \Stringable
{
public function getName(): string;

public function getType(): string;

public function getDynamicName(): string;

/**
Expand Down
5 changes: 5 additions & 0 deletions src/TwigFilter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ public function __construct(string $name, $callable = null, array $options = [])
], $this->options);
}

public function getType(): string
{
return 'filter';
}

public function getSafe(Node $filterArgs): ?array
{
if (null !== $this->options['is_safe']) {
Expand Down
5 changes: 5 additions & 0 deletions src/TwigFunction.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ public function __construct(string $name, $callable = null, array $options = [])
], $this->options);
}

public function getType(): string
{
return 'function';
}

public function getParserCallable(): ?callable
{
return $this->options['parser_callable'];
Expand Down
5 changes: 5 additions & 0 deletions src/TwigTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public function __construct(string $name, $callable = null, array $options = [])
], $this->options);
}

public function getType(): string
{
return 'test';
}

public function needsCharset(): bool
{
return false;
Expand Down
43 changes: 15 additions & 28 deletions src/Util/CallableArgumentsExtractor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
use Twig\Node\Expression\VariadicExpression;
use Twig\Node\Node;
use Twig\TwigCallableInterface;
use Twig\TwigFilter;
use Twig\TwigFunction;
use Twig\TwigTest;

/**
* @author Fabien Potencier <[email protected]>
Expand All @@ -28,59 +25,51 @@
*/
final class CallableArgumentsExtractor
{
private string $type;
private string $name;
private ReflectionCallable $rc;

public function __construct(
private Node $node,
private TwigCallableInterface $twigCallable,
) {
$this->type = match (true) {
$twigCallable instanceof TwigFunction => 'function',
$twigCallable instanceof TwigFilter => 'filter',
$twigCallable instanceof TwigTest => 'test',
default => throw new \LogicException('Unknown callable type.'),
};
$this->name = $twigCallable->getName();
$this->rc = new ReflectionCallable($twigCallable);
}

/**
* @return array<Node>
*/
public function extractArguments(Node $arguments): array
{
$rc = new ReflectionCallable($this->twigCallable->getCallable(), $this->type, $this->name);
$extractedArguments = [];
$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->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
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;
}

if (!$named && !$this->twigCallable->isVariadic()) {
$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());
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());
}

return $extractedArguments;
}

if (!$callable = $this->twigCallable->getCallable()) {
if ($named) {
throw new SyntaxError(\sprintf('Named arguments are not supported for %s "%s".', $this->type, $this->name));
throw new SyntaxError(\sprintf('Named arguments are not supported for %s "%s".', $this->twigCallable->getType(), $this->twigCallable->getName()));
}

throw new SyntaxError(\sprintf('Arbitrary positional arguments are not supported for %s "%s".', $this->type, $this->name));
throw new SyntaxError(\sprintf('Arbitrary positional arguments are not supported for %s "%s".', $this->twigCallable->getType(), $this->twigCallable->getName()));
}

[$callableParameters, $isPhpVariadic] = $this->getCallableParameters($rc);
[$callableParameters, $isPhpVariadic] = $this->getCallableParameters();
$arguments = [];
$names = [];
$missingArguments = [];
Expand All @@ -100,13 +89,13 @@ public function extractArguments(Node $arguments): array

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());
throw new SyntaxError(\sprintf('Argument "%s" is defined twice for %s "%s".', $name, $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".',
$name, $this->type, $this->name, implode(', ', $names), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments)
$name, $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $names), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments)
), $this->node->getTemplateLine(), $this->node->getSourceContext());
}

Expand All @@ -129,7 +118,7 @@ public function extractArguments(Node $arguments): array

$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());
throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $name, $this->twigCallable->getType(), $this->twigCallable->getName()), $this->node->getTemplateLine(), $this->node->getSourceContext());
}
}

Expand Down Expand Up @@ -162,7 +151,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->type, $this->name, implode(', ', $names)
\count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $names)
),
$unknownArgument ? $unknownArgument->getTemplateLine() : $this->node->getTemplateLine(),
$unknownArgument ? $unknownArgument->getSourceContext() : $this->node->getSourceContext()
Expand All @@ -177,11 +166,9 @@ 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));
}

private function getCallableParameters(ReflectionCallable $rc): array
private function getCallableParameters(): array
{
$r = $rc->getReflector();

$parameters = $r->getParameters();
$parameters = $this->rc->getReflector()->getParameters();
if ($this->node->hasNode('node')) {
array_shift($parameters);
}
Expand All @@ -208,7 +195,7 @@ private function getCallableParameters(ReflectionCallable $rc): array
array_pop($parameters);
$isPhpVariadic = true;
} else {
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));
throw new SyntaxError(\sprintf('The last parameter of "%s" for %s "%s" must be an array with default value, eg. "array $arg = []".', $this->rc->getName(), $this->twigCallable->getType(), $this->twigCallable->getName()));
}
}

Expand Down
10 changes: 7 additions & 3 deletions src/Util/ReflectionCallable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Twig\Util;

use Twig\TwigCallableInterface;

/**
* @author Fabien Potencier <[email protected]>
*
Expand All @@ -22,8 +24,10 @@ final class ReflectionCallable
private $callable = null;
private $name;

public function __construct($callable, string $debugType = 'unknown', string $debugName = 'unknown')
{
public function __construct(
private TwigCallableInterface $twigCallable,
) {
$callable = $twigCallable->getCallable();
if (\is_string($callable) && false !== $pos = strpos($callable, '::')) {
$callable = [substr($callable, 0, $pos), substr($callable, 2 + $pos)];
}
Expand All @@ -40,7 +44,7 @@ public function __construct($callable, string $debugType = 'unknown', string $de
try {
$closure = \Closure::fromCallable($callable);
} catch (\TypeError $e) {
throw new \LogicException(\sprintf('Callback for %s "%s" is not callable in the current scope.', $debugType, $debugName), 0, $e);
throw new \LogicException(\sprintf('Callback for %s "%s" is not callable in the current scope.', $twigCallable->getType(), $twigCallable->getName()), 0, $e);
}
$this->reflector = $r = new \ReflectionFunction($closure);

Expand Down
28 changes: 14 additions & 14 deletions tests/Node/Expression/CallTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class CallTest extends TestCase
{
public function testGetArguments()
{
$node = $this->createFunctionExpression('date');
$node = $this->createFunctionExpression('date', 'date');
$this->assertEquals(['U', null], $this->getArguments($node, ['date', ['format' => 'U', 'timestamp' => null]]));
}

Expand All @@ -33,7 +33,7 @@ public function testGetArgumentsWhenPositionalArgumentsAfterNamedArguments()
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('Positional arguments cannot be used after named arguments for function "date".');

$node = $this->createFunctionExpression('date');
$node = $this->createFunctionExpression('date', 'date');
$this->getArguments($node, ['date', ['timestamp' => 123456, 'Y-m-d']]);
}

Expand All @@ -42,7 +42,7 @@ public function testGetArgumentsWhenArgumentIsDefinedTwice()
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('Argument "format" is defined twice for function "date".');

$node = $this->createFunctionExpression('date');
$node = $this->createFunctionExpression('date', 'date');
$this->getArguments($node, ['date', ['Y-m-d', 'format' => 'U']]);
}

Expand All @@ -51,7 +51,7 @@ public function testGetArgumentsWithWrongNamedArgumentName()
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('Unknown argument "unknown" for function "date(format, timestamp)".');

$node = $this->createFunctionExpression('date');
$node = $this->createFunctionExpression('date', 'date');
$this->getArguments($node, ['date', ['Y-m-d', 'timestamp' => null, 'unknown' => '']]);
}

Expand All @@ -60,7 +60,7 @@ public function testGetArgumentsWithWrongNamedArgumentNames()
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('Unknown arguments "unknown1", "unknown2" for function "date(format, timestamp)".');

$node = $this->createFunctionExpression('date');
$node = $this->createFunctionExpression('date', 'date');
$this->getArguments($node, ['date', ['Y-m-d', 'timestamp' => null, 'unknown1' => '', 'unknown2' => '']]);
}

Expand All @@ -73,19 +73,19 @@ public function testResolveArgumentsWithMissingValueForOptionalArgument()
$this->expectException(SyntaxError::class);
$this->expectExceptionMessage('Argument "case_sensitivity" could not be assigned for function "substr_compare(main_str, str, offset, length, case_sensitivity)" because it is mapped to an internal PHP function which cannot determine default value for optional argument "length".');

$node = $this->createFunctionExpression('substr_compare');
$node = $this->createFunctionExpression('substr_compare', 'substr_compare');
$this->getArguments($node, ['substr_compare', ['abcd', 'bc', 'offset' => 1, 'case_sensitivity' => true]]);
}

public function testResolveArgumentsOnlyNecessaryArgumentsForCustomFunction()
{
$node = $this->createFunctionExpression('custom_function');
$node = $this->createFunctionExpression('custom_function', [$this, 'customFunction']);
$this->assertEquals(['arg1'], $this->getArguments($node, [[$this, 'customFunction'], ['arg1' => 'arg1']]));
}

public function testGetArgumentsForStaticMethod()
{
$node = $this->createFunctionExpression('custom_static_function');
$node = $this->createFunctionExpression('custom_static_function', __CLASS__.'::customStaticFunction');
$this->assertEquals(['arg1'], $this->getArguments($node, [__CLASS__.'::customStaticFunction', ['arg1' => 'arg1']]));
}

Expand All @@ -94,15 +94,15 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArguments()
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('The last parameter of "Twig\\Tests\\Node\\Expression\\CallTest::customFunctionWithArbitraryArguments" for function "foo" must be an array with default value, eg. "array $arg = []".');

$node = $this->createFunctionExpression('foo', true);
$node = $this->createFunctionExpression('foo', [$this, 'customFunctionWithArbitraryArguments'], true);
$this->getArguments($node, [[$this, 'customFunctionWithArbitraryArguments'], []]);
}

public function testGetArgumentsWithInvalidCallable()
{
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('Callback for function "foo" is not callable in the current scope.');
$node = $this->createFunctionExpression('foo', true);
$node = $this->createFunctionExpression('foo', '<not-a-callable>', true);
$this->getArguments($node, ['<not-a-callable>', []]);
}

Expand All @@ -111,7 +111,7 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnF
$this->expectException(\LogicException::class);
$this->expectExceptionMessageMatches('#^The last parameter of "Twig\\\\Tests\\\\Node\\\\Expression\\\\custom_call_test_function" for function "foo" must be an array with default value, eg\\. "array \\$arg \\= \\[\\]"\\.$#');

$node = $this->createFunctionExpression('foo', true);
$node = $this->createFunctionExpression('foo', 'Twig\Tests\Node\Expression\custom_call_test_function', true);
$this->getArguments($node, ['Twig\Tests\Node\Expression\custom_call_test_function', []]);
}

Expand All @@ -120,7 +120,7 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnO
$this->expectException(\LogicException::class);
$this->expectExceptionMessageMatches('#^The last parameter of "Twig\\\\Tests\\\\Node\\\\Expression\\\\CallableTestClass\\:\\:__invoke" for function "foo" must be an array with default value, eg\\. "array \\$arg \\= \\[\\]"\\.$#');

$node = $this->createFunctionExpression('foo', true);
$node = $this->createFunctionExpression('foo', new CallableTestClass(), true);
$this->getArguments($node, [new CallableTestClass(), []]);
}

Expand All @@ -144,9 +144,9 @@ private function getArguments($call, $args)
return $m->invokeArgs($call, $args);
}

private function createFunctionExpression($name, $isVariadic = false): Node_Expression_Call
private function createFunctionExpression($name, $callable, $isVariadic = false): Node_Expression_Call
{
return new Node_Expression_Call(new TwigFunction($name, null, ['is_variadic' => $isVariadic]), new Node([]), 0);
return new Node_Expression_Call(new TwigFunction($name, $callable, ['is_variadic' => $isVariadic]), new Node([]), 0);
}
}

Expand Down

0 comments on commit b9b8d52

Please sign in to comment.