Skip to content

Commit b9b8d52

Browse files
fabpotfabpot
committed
minor twigphp#4217 Refactor code (fabpot)
This PR was merged into the 3.x branch. Discussion ---------- Refactor code Commits ------- 31037d0 Refactor code
2 parents 63e35b9 + 31037d0 commit b9b8d52

9 files changed

+58
-50
lines changed

‎src/Node/Expression/CallExpression.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ protected function compileCallable(Compiler $compiler)
3434
if (\is_string($callable) && !str_contains($callable, '::')) {
3535
$compiler->raw($callable);
3636
} else {
37-
$rc = $this->reflectCallable($callable);
37+
$rc = $this->reflectCallable($twigCallable);
3838
$r = $rc->getReflector();
3939
$callable = $rc->getCallable();
4040

@@ -271,7 +271,7 @@ protected function normalizeName(string $name): string
271271
private function getCallableParameters($callable, bool $isVariadic): array
272272
{
273273
$twigCallable = $this->getAttribute('twig_callable');
274-
$rc = $this->reflectCallable($callable);
274+
$rc = $this->reflectCallable($twigCallable);
275275
$r = $rc->getReflector();
276276
$callableName = $rc->getName();
277277

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

312-
private function reflectCallable($callable): ReflectionCallable
312+
private function reflectCallable(TwigCallableInterface $callable): ReflectionCallable
313313
{
314314
if (!$this->reflector) {
315-
$this->reflector = new ReflectionCallable($callable, $this->getAttribute('type'), $this->getAttribute('name'));
315+
$this->reflector = new ReflectionCallable($callable);
316316
}
317317

318318
return $this->reflector;

‎src/Parser.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public function subparse($test, bool $dropNeedle = false): Node
157157
if (null !== $test) {
158158
$e = new SyntaxError(\sprintf('Unexpected "%s" tag', $token->getValue()), $token->getLine(), $this->stream->getSourceContext());
159159

160-
$callable = (new ReflectionCallable($test))->getCallable();
160+
$callable = (new ReflectionCallable(new TwigTest('decision', $test)))->getCallable();
161161
if (\is_array($callable) && $callable[0] instanceof TokenParserInterface) {
162162
$e->appendMessage(\sprintf(' (expecting closing tag for the "%s" tag defined near line %s).', $callable[0]->getTag(), $lineno));
163163
}

‎src/TwigCallableInterface.php

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ interface TwigCallableInterface extends \Stringable
1818
{
1919
public function getName(): string;
2020

21+
public function getType(): string;
22+
2123
public function getDynamicName(): string;
2224

2325
/**

‎src/TwigFilter.php

+5
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ public function __construct(string $name, $callable = null, array $options = [])
3939
], $this->options);
4040
}
4141

42+
public function getType(): string
43+
{
44+
return 'filter';
45+
}
46+
4247
public function getSafe(Node $filterArgs): ?array
4348
{
4449
if (null !== $this->options['is_safe']) {

‎src/TwigFunction.php

+5
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public function __construct(string $name, $callable = null, array $options = [])
3838
], $this->options);
3939
}
4040

41+
public function getType(): string
42+
{
43+
return 'function';
44+
}
45+
4146
public function getParserCallable(): ?callable
4247
{
4348
return $this->options['parser_callable'];

‎src/TwigTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ public function __construct(string $name, $callable = null, array $options = [])
3535
], $this->options);
3636
}
3737

38+
public function getType(): string
39+
{
40+
return 'test';
41+
}
42+
3843
public function needsCharset(): bool
3944
{
4045
return false;

‎src/Util/CallableArgumentsExtractor.php

+15-28
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
use Twig\Node\Expression\VariadicExpression;
1818
use Twig\Node\Node;
1919
use Twig\TwigCallableInterface;
20-
use Twig\TwigFilter;
21-
use Twig\TwigFunction;
22-
use Twig\TwigTest;
2320

2421
/**
2522
* @author Fabien Potencier <[email protected]>
@@ -28,59 +25,51 @@
2825
*/
2926
final class CallableArgumentsExtractor
3027
{
31-
private string $type;
32-
private string $name;
28+
private ReflectionCallable $rc;
3329

3430
public function __construct(
3531
private Node $node,
3632
private TwigCallableInterface $twigCallable,
3733
) {
38-
$this->type = match (true) {
39-
$twigCallable instanceof TwigFunction => 'function',
40-
$twigCallable instanceof TwigFilter => 'filter',
41-
$twigCallable instanceof TwigTest => 'test',
42-
default => throw new \LogicException('Unknown callable type.'),
43-
};
44-
$this->name = $twigCallable->getName();
34+
$this->rc = new ReflectionCallable($twigCallable);
4535
}
4636

4737
/**
4838
* @return array<Node>
4939
*/
5040
public function extractArguments(Node $arguments): array
5141
{
52-
$rc = new ReflectionCallable($this->twigCallable->getCallable(), $this->type, $this->name);
5342
$extractedArguments = [];
5443
$named = false;
5544
foreach ($arguments as $name => $node) {
5645
if (!\is_int($name)) {
5746
$named = true;
5847
$name = $this->normalizeName($name);
5948
} elseif ($named) {
60-
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());
49+
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());
6150
}
6251

6352
$extractedArguments[$name] = $node;
6453
}
6554

6655
if (!$named && !$this->twigCallable->isVariadic()) {
6756
$min = $this->twigCallable->getMinimalNumberOfRequiredArguments();
68-
if (\count($extractedArguments) < $rc->getReflector()->getNumberOfRequiredParameters() - $min) {
69-
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());
57+
if (\count($extractedArguments) < $this->rc->getReflector()->getNumberOfRequiredParameters() - $min) {
58+
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());
7059
}
7160

7261
return $extractedArguments;
7362
}
7463

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

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

83-
[$callableParameters, $isPhpVariadic] = $this->getCallableParameters($rc);
72+
[$callableParameters, $isPhpVariadic] = $this->getCallableParameters();
8473
$arguments = [];
8574
$names = [];
8675
$missingArguments = [];
@@ -100,13 +89,13 @@ public function extractArguments(Node $arguments): array
10089

10190
if (\array_key_exists($name, $extractedArguments)) {
10291
if (\array_key_exists($pos, $extractedArguments)) {
103-
throw new SyntaxError(\sprintf('Argument "%s" is defined twice for %s "%s".', $name, $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
92+
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());
10493
}
10594

10695
if (\count($missingArguments)) {
10796
throw new SyntaxError(\sprintf(
10897
'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".',
109-
$name, $this->type, $this->name, implode(', ', $names), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments)
98+
$name, $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $names), \count($missingArguments) > 1 ? 's' : '', implode('", "', $missingArguments)
11099
), $this->node->getTemplateLine(), $this->node->getSourceContext());
111100
}
112101

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

130119
$missingArguments[] = $name;
131120
} else {
132-
throw new SyntaxError(\sprintf('Value for argument "%s" is required for %s "%s".', $name, $this->type, $this->name), $this->node->getTemplateLine(), $this->node->getSourceContext());
121+
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());
133122
}
134123
}
135124

@@ -162,7 +151,7 @@ public function extractArguments(Node $arguments): array
162151
throw new SyntaxError(
163152
\sprintf(
164153
'Unknown argument%s "%s" for %s "%s(%s)".',
165-
\count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->type, $this->name, implode(', ', $names)
154+
\count($extractedArguments) > 1 ? 's' : '', implode('", "', array_keys($extractedArguments)), $this->twigCallable->getType(), $this->twigCallable->getName(), implode(', ', $names)
166155
),
167156
$unknownArgument ? $unknownArgument->getTemplateLine() : $this->node->getTemplateLine(),
168157
$unknownArgument ? $unknownArgument->getSourceContext() : $this->node->getSourceContext()
@@ -177,11 +166,9 @@ private function normalizeName(string $name): string
177166
return strtolower(preg_replace(['/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'], ['\\1_\\2', '\\1_\\2'], $name));
178167
}
179168

180-
private function getCallableParameters(ReflectionCallable $rc): array
169+
private function getCallableParameters(): array
181170
{
182-
$r = $rc->getReflector();
183-
184-
$parameters = $r->getParameters();
171+
$parameters = $this->rc->getReflector()->getParameters();
185172
if ($this->node->hasNode('node')) {
186173
array_shift($parameters);
187174
}
@@ -208,7 +195,7 @@ private function getCallableParameters(ReflectionCallable $rc): array
208195
array_pop($parameters);
209196
$isPhpVariadic = true;
210197
} else {
211-
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));
198+
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()));
212199
}
213200
}
214201

‎src/Util/ReflectionCallable.php

+7-3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Twig\Util;
1313

14+
use Twig\TwigCallableInterface;
15+
1416
/**
1517
* @author Fabien Potencier <[email protected]>
1618
*
@@ -22,8 +24,10 @@ final class ReflectionCallable
2224
private $callable = null;
2325
private $name;
2426

25-
public function __construct($callable, string $debugType = 'unknown', string $debugName = 'unknown')
26-
{
27+
public function __construct(
28+
private TwigCallableInterface $twigCallable,
29+
) {
30+
$callable = $twigCallable->getCallable();
2731
if (\is_string($callable) && false !== $pos = strpos($callable, '::')) {
2832
$callable = [substr($callable, 0, $pos), substr($callable, 2 + $pos)];
2933
}
@@ -40,7 +44,7 @@ public function __construct($callable, string $debugType = 'unknown', string $de
4044
try {
4145
$closure = \Closure::fromCallable($callable);
4246
} catch (\TypeError $e) {
43-
throw new \LogicException(\sprintf('Callback for %s "%s" is not callable in the current scope.', $debugType, $debugName), 0, $e);
47+
throw new \LogicException(\sprintf('Callback for %s "%s" is not callable in the current scope.', $twigCallable->getType(), $twigCallable->getName()), 0, $e);
4448
}
4549
$this->reflector = $r = new \ReflectionFunction($closure);
4650

‎tests/Node/Expression/CallTest.php

+14-14
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class CallTest extends TestCase
2424
{
2525
public function testGetArguments()
2626
{
27-
$node = $this->createFunctionExpression('date');
27+
$node = $this->createFunctionExpression('date', 'date');
2828
$this->assertEquals(['U', null], $this->getArguments($node, ['date', ['format' => 'U', 'timestamp' => null]]));
2929
}
3030

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

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

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

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

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

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

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

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

@@ -73,19 +73,19 @@ public function testResolveArgumentsWithMissingValueForOptionalArgument()
7373
$this->expectException(SyntaxError::class);
7474
$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".');
7575

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

8080
public function testResolveArgumentsOnlyNecessaryArgumentsForCustomFunction()
8181
{
82-
$node = $this->createFunctionExpression('custom_function');
82+
$node = $this->createFunctionExpression('custom_function', [$this, 'customFunction']);
8383
$this->assertEquals(['arg1'], $this->getArguments($node, [[$this, 'customFunction'], ['arg1' => 'arg1']]));
8484
}
8585

8686
public function testGetArgumentsForStaticMethod()
8787
{
88-
$node = $this->createFunctionExpression('custom_static_function');
88+
$node = $this->createFunctionExpression('custom_static_function', __CLASS__.'::customStaticFunction');
8989
$this->assertEquals(['arg1'], $this->getArguments($node, [__CLASS__.'::customStaticFunction', ['arg1' => 'arg1']]));
9090
}
9191

@@ -94,15 +94,15 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArguments()
9494
$this->expectException(\LogicException::class);
9595
$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 = []".');
9696

97-
$node = $this->createFunctionExpression('foo', true);
97+
$node = $this->createFunctionExpression('foo', [$this, 'customFunctionWithArbitraryArguments'], true);
9898
$this->getArguments($node, [[$this, 'customFunctionWithArbitraryArguments'], []]);
9999
}
100100

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

@@ -111,7 +111,7 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnF
111111
$this->expectException(\LogicException::class);
112112
$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 \\= \\[\\]"\\.$#');
113113

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

@@ -120,7 +120,7 @@ public function testResolveArgumentsWithMissingParameterForArbitraryArgumentsOnO
120120
$this->expectException(\LogicException::class);
121121
$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 \\= \\[\\]"\\.$#');
122122

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

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

147-
private function createFunctionExpression($name, $isVariadic = false): Node_Expression_Call
147+
private function createFunctionExpression($name, $callable, $isVariadic = false): Node_Expression_Call
148148
{
149-
return new Node_Expression_Call(new TwigFunction($name, null, ['is_variadic' => $isVariadic]), new Node([]), 0);
149+
return new Node_Expression_Call(new TwigFunction($name, $callable, ['is_variadic' => $isVariadic]), new Node([]), 0);
150150
}
151151
}
152152

0 commit comments

Comments
 (0)