8000 Fix support for named closures · twigphp/Twig@163f074 · GitHub
[go: up one dir, main page]
Skip to content

Commit 163f074

Browse files
nicolas-grekasnicolas-grekas
committed
Fix support for named closures
1 parent ab36653 commit 163f074

File tree

9 files changed

+76
-89
lines changed

9 files changed

+76
-89
lines changed

‎.github/workflows/ci.yml

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,23 @@ jobs:
2626
- '7.4'
2727
- '8.0'
2828
- '8.1'
29-
composer-options: ['']
3029
experimental: [false]
3130

3231
steps:
3332
- name: "Checkout code"
34-
uses: actions/checkout@v2.3.3
33+
uses 8000 : actions/checkout@v2
3534

3635
- name: "Install PHP with extensions"
37-
uses: shivammathur/setup-php@2.7.0
36+
uses: shivammathur/setup-php@v2
3837
with:
3938
coverage: "none"
4039
php-version: ${{ matrix.php-version }}
4140
ini-values: memory_limit=-1
42-
tools: composer:v2
4341

4442
- name: "Add PHPUnit matcher"
4543
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
4644

47-
- name: "Set composer cache directory"
48-
id: composer-cache
49-
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
50-
51-
- name: "Cache composer"
52-
uses: actions/cache@v2.1.2
53-
with:
54-
path: ${{ steps.composer-cache.outputs.dir }}
55-
key: ${{ runner.os }}-${{ matrix.php-version }}-composer-${{ hashFiles('composer.json') }}
56-
restore-keys: ${{ runner.os }} 3262 -${{ matrix.php-version }}-composer-
57-
58-
- run: composer install ${{ matrix.composer-options }}
45+
- run: composer install
5946

6047
- name: "Install PHPUnit"
6148
run: vendor/bin/simple-phpunit install
@@ -92,35 +79,22 @@ jobs:
9279
- 'extra/markdown-extra'
9380
- 'extra/string-extra'
9481
- 'extra/twig-extra-bundle'
95-
composer-options: ['']
9682
experimental: [false]
9783

9884
steps:
9985
- name: "Checkout code"
100-
uses: actions/checkout@v2.3.3
86+
uses: actions/checkout@v2
10187

10288
- name: "Install PHP with extensions"
103-
uses: shivammathur/setup-php@2.7.0
89+
uses: shivammathur/setup-php@v2
10490
with:
10591
coverage: "none"
10692
php-version: ${{ matrix.php-version }}
10793
ini-values: memory_limit=-1
108-
tools: composer:v2
10994

11095
- name: "Add PHPUnit matcher"
11196
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
11297

113-
- name: "Set composer cache directory"
114-
id: composer-cache
115-
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
116-
117-
- name: "Cache composer"
118-
uses: actions/cache@v2.1.2
119-
with:
120-
path: ${{ steps.composer-cache.outputs.dir }}
121-
key: ${{ runner.os }}-${{ matrix.php-version }}-${{ matrix.extension }}-${{ hashFiles('composer.json') }}
122-
restore-keys: ${{ runner.os }}-${{ matrix.php-version }}-${{ matrix.extension }}-
123-
12498
- run: composer install
12599

126100
- name: "Install PHPUnit"
@@ -129,10 +103,6 @@ jobs:
129103
- name: "PHPUnit version"
130104
run: vendor/bin/simple-phpunit --version
131105

132-
- if: matrix.extension == 'extra/markdown-extra' && matrix.php-version == '8.0'
133-
working-directory: ${{ matrix.extension}}
134-
run: composer config platform.php 7.4.99
135-
136106
- name: "Composer install"
137107
working-directory: ${{ matrix.extension}}
138108
run: composer install
@@ -158,10 +128,10 @@ jobs:
158128

159129
steps:
160130
- name: "Checkout code"
161-
uses: actions/checkout@v2.3.3
131+
uses: actions/checkout@v2
162132

163133
- name: "Install PHP with extensions"
164-
uses: shivammathur/setup-php@2.7.0
134+
uses: shivammathur/setup-php@v2
165135
with:
166136
coverage: "none"
167137
extensions: "gd, pdo_sqlite"

‎doc/filters/format_datetime.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ You can tweak the output for the date part and the time part:
3030
3131
Supported values are: ``none``, ``short``, ``medium``, ``long``, and ``full``.
3232

33-
For greater flexibility, you can even define your own pattern (see the `ICU
34-
user guide`_ for supported patterns).
33+
For greater flexibility, you can even define your own pattern
34+
(see the `ICU user guide`_ for supported patterns).
3535

3636
.. code-block:: twig
3737

‎src/ExtensionSet.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,6 @@ public function addExtension(ExtensionInterface $extension)
149149
throw new \LogicException(sprintf('Unable to register extension "%s" as it is already registered.', $class));
150150
}
151151

152-
// For BC/FC with namespaced aliases
153-
$class = (new \ReflectionClass($class))->name;
154152
$this->extensions[$class] = $extension;
155153
}
156154

‎src/Node/Expression/CallExpression.php

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,21 @@ protected function compileCallable(Compiler $compiler)
2424
{
2525
$callable = $this->getAttribute('callable');
2626

27-
$closingParenthesis = false;
28-
$isArray = false;
2927
if (\is_string($callable) && false === strpos($callable, '::')) {
3028
$compiler->raw($callable);
3129
} else {
32-
list($r, $callable) = $this->reflectCallable($callable);
33-
if ($r instanceof \ReflectionMethod && \is_string($callable[0])) {
34-
if ($r->isStatic()) {
30+
[$r, $callable] = $this->reflectCallable($callable);
31+
32+
if (\is_string($callable)) {
33+
$compiler->raw($callable);
34+
} elseif (\is_array($callable) && \is_string($callable[0])) {
35+
if (!$r instanceof \ReflectionMethod || $r->isStatic()) {
3536
$compiler->raw(sprintf('%s::%s', $callable[0], $callable[1]));
3637
} else {
3738
$compiler->raw(sprintf('$this->env->getRuntime(\'%s\')->%s', $callable[0], $callable[1]));
3839
}
39-
} elseif ($r instanceof \ReflectionMethod && $callable[0] instanceof ExtensionInterface) {
40-
// For BC/FC with namespaced aliases
41-
$class = (new \ReflectionClass(\get_class($callable[0])))->name;
40+
} elseif (\is_array($callable) && $callable[0] instanceof ExtensionInterface) {
41+
$class = \get_class($callable[0]);
4242
if (!$compiler->getEnvironment()->hasExtension($class)) {
4343
// Compile a non-optimized call to trigger a \Twig\Error\RuntimeError, which cannot be a compile-time error
4444
$compiler->raw(sprintf('$this->env->getExtension(\'%s\')', $class));
@@ -48,17 +48,11 @@ protected function compileCallable(Compiler $compiler)
4848

4949
$compiler->raw(sprintf('->%s', $callable[1]));
5050
} else {
51-
$closingParenthesis = true;
52-
$isArray = true;
53-
$compiler->raw(sprintf('call_user_func_array($this->env->get%s(\'%s\')->getCallable(), ', ucfirst($this->getAttribute('type')), $this->getAttribute('name')));
51+
$compiler->raw(sprintf('$this->env->get%s(\'%s\')->getCallable()', ucfirst($this->getAttribute('type')), $this->getAttribute('name')));
5452
}
5553
}
5654

57-
$this->compileArguments($compiler, $isArray);
58-
59-
if ($closingParenthesis) {
60-
$compiler->raw(')');
61-
}
55+
$this->compileArguments($compiler);
6256
}
6357

6458
protected function compileArguments(Compiler $compiler, $isArray = false)
@@ -245,10 +239,7 @@ protected function normalizeName($name)
245239

246240
private function getCallableParameters($callable, bool $isVariadic): array
247241
{
248-
list($r) = $this->reflectCallable($callable);
249-
if (null === $r) {
250-
return [[], false];
251-
}
242+
[$r, , $callableName] = $this->reflectCallable($callable);
252243

253244
$parameters = $r->getParameters();
254245
if ($this->hasNode('node')) {
@@ -275,11 +266,6 @@ private function getCallableParameters($callable, bool $isVariadic): array
275266
array_pop($parameters);
276267
$isPhpVariadic = true;
277268
} else {
278-
$callableName = $r->name;
279-
if ($r instanceof \ReflectionMethod) {
280-
$callableName = $r->getDeclaringClass()->name.'::'.$callableName;
281-
}
282-
283269
throw new \LogicException(sprintf('The last parameter of "%s" for %s "%s" must be an array with default value, eg. "array $arg = []".', $callableName, $this->getAttribute('type'), $this->getAttribute('name')));
284270
}
285271
}
@@ -293,30 +279,33 @@ private function reflectCallable($callable)
293279
return $this->reflector;
294280
}
295281

296-
if (\is_array($callable)) {
297-
if (!method_exists($callable[0], $callable[1])) {
298-
// __call()
299-
return [null, []];
300-
}
282+
if (\is_string($callable) && false !== $pos = strpos($callable, '::')) {
283+
$callable = [substr($callable, 0, $pos), substr($callable, 2 + $pos)];
284+
}
285+
286+
if (\is_array($callable) && method_exists($callable[0], $callable[1])) {
301287
$r = new \ReflectionMethod($callable[0], $callable[1]);
302-
} elseif (\is_object($callable) && !$callable instanceof \Closure) {
303-
$r = new \ReflectionObject($callable);
304-
$r = $r->getMethod('__invoke');
305-
$callable = [$callable, '__invoke'];
306-
} elseif (\is_string($callable) && false !== $pos = strpos($callable, '::')) {
307-
$class = substr($callable, 0, $pos);
308-
$method = substr($callable, $pos + 2);
309-
if (!method_exists($class, $method)) {
310-
// __staticCall()
311-
return [null, []];
312-
}
313-
$r = new \ReflectionMethod($callable);
314-
$callable = [$class, $method];
288+
289+
return $this->reflector = [$r, $callable, $r->class.'::'.$r->name];
290+
}
291+
292+
$r = new \ReflectionFunction(\Closure::fromCallable($callable));
293+
294+
if (false !== strpos($r->name, '{closure}')) {
295+
return $this->reflector = [$r, $callable, 'Closure'];
296+
}
297+
298+
if ($object = $r->getClosureThis()) {
299+
$callable = [$object, $r->name];
300+
$callableName = (\function_exists('get_debug_type') ? get_debug_type($object) : \get_class($object)).'::'.$r->name;
301+
} elseif ($class = $r->getClosureScopeClass()) {
302+
$callable = [$class, $r->name];
303+
$callableName = $class.'::'.$r->name;
315304
} else {
316-
$r = new \ReflectionFunction($callable);
305+
$callable = $callableName = $r->name;
317306
}
318307

319-
return $this->reflector = [$r, $callable];
308+
return $this->reflector = [$r, $callable, $callableName];
320309
}
321310
}
322311

‎tests/Fixtures/functions/magic_call.test

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
__call calls
33
--TEMPLATE--
44
{{ 'foo'|magic_call }}
5+
{{ 'foo'|magic_call_closure }}
56
--DATA--
67
return []
78
--EXPECT--
89
magic_foo
10+
magic_foo

‎tests/IntegrationTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ public function getFilters()
168168
new TwigFilter('static_call_string', 'Twig\Tests\TwigTestExtension::staticCall'),
169169
new TwigFilter('static_call_array', ['Twig\Tests\TwigTestExtension', 'staticCall']),
170170
new TwigFilter('magic_call', [$this, 'magicCall']),
171+
new TwigFilter('magic_call_closure', \Closure::fromCallable([$this, 'magicCall'])),
171172
new TwigFilter('magic_call_string', 'Twig\Tests\TwigTestExtension::magicStaticCall'),
172173
new TwigFilter('magic_call_array', ['Twig\Tests\TwigTestExtension', 'magicStaticCall']),
173174
new TwigFilter('*_path', [$this, 'dynamic_path']),

‎tests/Node/Expression/FilterTest.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Twig\Environment;
1515
use Twig\Error\SyntaxError;
16+
use Twig\Extension\AbstractExtension;
1617
use Twig\Loader\ArrayLoader;
1718
use Twig\Loader\LoaderInterface;
1819
use Twig\Node\Expression\ConstantExpression;
@@ -39,8 +40,23 @@ public function getTests()
3940
{
4041
$environment = new Environment($this->createMock(LoaderInterface::class));
4142
$environment->addFilter(new TwigFilter('bar', 'twig_tests_filter_dummy', ['needs_environment' => true]));
43+
$environment->addFilter(new TwigFilter('bar_closure', \Closure::fromCallable(twig_tests_filter_dummy::class), ['needs_environment' => true]));
4244
$environment->addFilter(new TwigFilter('barbar', 'Twig\Tests\Node\Expression\twig_tests_filter_barbar', ['needs_context' => true, 'is_variadic' => true]));
4345

46+
$extension = new class() extends AbstractExtension {
47+
public function getFilters(): array
48+
{
49+
return [
50+
new TwigFilter('foo', \Closure::fromCallable([$this, 'foo'])),
51+
];
52+
}
53+
54+
public function foo()
55+
{
56+
}
57+
};
58+
$environment->addExtension($extension);
59+
4460
$tests = [];
4561

4662
$expr = new ConstantExpression('foo', 1);
@@ -77,12 +93,15 @@ public function getTests()
7793

7894
// filter as an anonymous function
7995
$node = $this->createFilter(new ConstantExpression('foo', 1), 'anonymous');
80-
$tests[] = [$node, 'call_user_func_array($this->env->getFilter(\'anonymous\')->getCallable(), ["foo"])'];
96+
$tests[] = [$node, '$this->env->getFilter(\'anonymous\')->getCallable()("foo")'];
8197

8298
// needs environment
8399
$node = $this->createFilter($string, 'bar');
84100
$tests[] = [$node, 'twig_tests_filter_dummy($this->env, "abc")', $environment];
85101

102+
$node = $this->createFilter($string, 'bar_closure');
103+
$tests[] = [$node, twig_tests_filter_dummy::class.'($this->env, "abc")', $environment];
104+
86105
$node = $this->createFilter($string, 'bar', [new ConstantExpression('bar', 1)]);
87106
$tests[] = [$node, 'twig_tests_filter_dummy($this->env, "abc", "bar")', $environment];
88107

@@ -104,6 +123,10 @@ public function getTests()
104123
]);
105124
$tests[] = [$node, 'Twig\Tests\Node\Expression\twig_tests_filter_barbar($context, "abc", "1", "2", [0 => "3", "foo" => "bar"])', $environment];
106125

126+
// from extension
127+
$node = $this->createFilter($string, 'foo');
128+
$tests[] = [$node, sprintf('$this->extensions[\'%s\']->foo("abc")', \get_class($extension)), $environment];
129+
107130
return $tests;
108131
}
109132

‎tests/Node/Expression/FunctionTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function getTests()
3636
{
3737
$environment = new Environment($this->createMock(LoaderInterface::class));
3838
$environment->addFunction(new TwigFunction('foo', 'twig_tests_function_dummy', []));
39+
$environment->addFunction(new TwigFunction('foo_closure', \Closure::fromCallable(twig_tests_function_dummy::class), []));
3940
$environment->addFunction(new TwigFunction('bar', 'twig_tests_function_dummy', ['needs_environment' => true]));
4041
$environment->addFunction(new TwigFunction('foofoo', 'twig_tests_function_dummy', ['needs_context' => true]));
4142
$environment->addFunction(new TwigFunction('foobar', 'twig_tests_function_dummy', ['needs_environment' => true, 'needs_context' => true]));
@@ -46,6 +47,9 @@ public function getTests()
4647
$node = $this->createFunction('foo');
4748
$tests[] = [$node, 'twig_tests_function_dummy()', $environment];
4849

50+
$node = $this->createFunction('foo_closure');
51+
$tests[] = [$node, twig_tests_function_dummy::class.'()', $environment];
52+
4953
$node = $this->createFunction('foo', [new ConstantExpression('bar', 1), new ConstantExpression('foobar', 1)]);
5054
$tests[] = [$node, 'twig_tests_function_dummy("bar", "foobar")', $environment];
5155

@@ -94,7 +98,7 @@ public function getTests()
9498

9599
// function as an anonymous function
96100
$node = $this->createFunction('anonymous', [new ConstantExpression('foo', 1)]);
97-
$tests[] = [$node, 'call_user_func_array($this->env->getFunction(\'anonymous\')->getCallable(), ["foo"])'];
101+
$tests[] = [$node, '$this->env->getFunction(\'anonymous\')->getCallable()("foo")'];
98102

99103
return $tests;
100104
}

‎tests/Node/Expression/TestTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function getTests()
4848

4949
// test as an anonymous function
5050
$node = $this->createTest(new ConstantExpression('foo', 1), 'anonymous', [new ConstantExpression('foo', 1)]);
51-
$tests[] = [$node, 'call_user_func_array($this->env->getTest(\'anonymous\')->getCallable(), ["foo", "foo"])'];
51+
$tests[] = [$node, '$this->env->getTest(\'anonymous\')->getCallable()("foo", "foo")'];
5252

5353
// arbitrary named arguments
5454
$string = new ConstantExpression('abc', 1);

0 commit comments

Comments
 (0)
0