minor #29172 [Fwb][EventDispatcher][HttpKernel] Fix getClosureScopeClass usage to describe callables (ogizanagi)

nicolas-grekas · nicolas-grekas · commit 49ddced9657c · 2018-11-11T12:13:24.000+01:00
This PR was merged into the 3.4 branch. Discussion ---------- [Fwb][EventDispatcher][HttpKernel] Fix getClosureScopeClass usage to describe callables | Q | A | ------------- | --- | Branch? | 3.4  | Bug fix? | yes | New feature? | no  | BC breaks? | no  | Deprecations? | no  | Tests pass? | yes  | Fixed tickets | #29054  | License | MIT | Doc PR | N/A `\ReflectionFunctionAbstract::getClosureScopeClass` returns a `\ReflectionClass` instance, not the class name. Before this patch: ```diff --- Expected +++ Actual @@ @@ -'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen' +'Class [ <user> class Symfony\Component\EventDispatcher\Tests\Debug\FooListener ] { + @@ [...]/src/Symfony/Component/EventDispatcher/Tests/Debug/WrappedListenerTest.php 28-33 + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [0] { + } + + - Methods [1] { + Method [ <user> public method listen ] { + @@ [...]/src/Symfony/Component/EventDispatcher/Tests/Debug/WrappedListenerTest.php 30 - 32 + } + } +} +::listen' ``` Commits ------- 61e4592 [Fwb][EventDispatcher][HttpKernel] Fix getClosureScopeClass usage to describe callables
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php
@@ -374,10 +374,9 @@ private function getCallableData($callable, array $options = array())
             }
             $data['name'] = $r->name;
 
-            $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;
-            if ($scopeClass = $r->getClosureScopeClass() ?: $class) {
-                $data['class'] = $scopeClass;
-                if (!$class) {
+            if ($class = $r->getClosureScopeClass()) {
+                $data['class'] = $class->name;
+                if (!$r->getClosureThis()) {
                     $data['static'] = true;
                 }
             }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/MarkdownDescriptor.php
@@ -360,10 +360,9 @@ protected function describeCallable($callable, array $options = array())
             }
             $string .= "\n".sprintf('- Name: `%s`', $r->name);
 
-            $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;
-            if ($scopeClass = $r->getClosureScopeClass() ?: $class) {
-                $string .= "\n".sprintf('- Class: `%s`', $class);
-                if (!$class) {
+            if ($class = $r->getClosureScopeClass()) {
+                $string .= "\n".sprintf('- Class: `%s`', $class->name);
+                if (!$r->getClosureThis()) {
                     $string .= "\n- Static: yes";
                 }
             }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/TextDescriptor.php
@@ -474,10 +474,7 @@ private function formatCallable($callable)
                 return 'Closure()';
             }
             if ($class = $r->getClosureScopeClass()) {
-                return sprintf('%s::%s()', $class, $r->name);
-            }
-            if ($class = $r->getClosureThis()) {
-                return sprintf('%s::%s()', \get_class($class), $r->name);
+                return sprintf('%s::%s()', $class->name, $r->name);
             }
 
             return $r->name.'()';
diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php b/src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
@@ -586,10 +586,9 @@ private function getCallableDocument($callable)
             }
             $callableXML->setAttribute('name', $r->name);
 
-            $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;
-            if ($scopeClass = $r->getClosureScopeClass() ?: $class) {
-                $callableXML->setAttribute('class', $class);
-                if (!$class) {
+            if ($class = $r->getClosureScopeClass()) {
+                $callableXML->setAttribute('class', $class->name);
+                if (!$r->getClosureThis()) {
                     $callableXML->setAttribute('static', 'true');
                 }
             }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Console/Descriptor/ObjectsProvider.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Console/Descriptor/ObjectsProvider.php
@@ -155,7 +155,7 @@ public static function getEventDispatchers()
 
     public static function getCallables()
     {
-        return array(
+        $callables = array(
             'callable_1' => 'array_key_exists',
             'callable_2' => array('Symfony\\Bundle\\FrameworkBundle\\Tests\\Console\\Descriptor\\CallableClass', 'staticMethod'),
             'callable_3' => array(new CallableClass(), 'method'),
@@ -164,6 +164,12 @@ public static function getCallables()
             'callable_6' => function () { return 'Closure'; },
             'callable_7' => new CallableClass(),
         );
+
+        if (\PHP_VERSION_ID >= 70100) {
+            $callables['callable_from_callable'] = \Closure::fromCallable(new CallableClass());
+        }
+
+        return $callables;
     }
 }
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.json b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.json
@@ -0,0 +1,5 @@
+{
+    "type": "closure",
+    "name": "__invoke",
+    "class": "Symfony\\Bundle\\FrameworkBundle\\Tests\\Console\\Descriptor\\CallableClass"
+}
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.md b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.md
@@ -0,0 +1,4 @@
+
+- Type: `closure`
+- Name: `__invoke`
+- Class: `Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\CallableClass`
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.txt b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.txt
@@ -0,0 +1 @@
+Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\CallableClass::__invoke()
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Descriptor/callable_from_callable.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<callable type="closure" name="__invoke" class="Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\CallableClass"/>
diff --git a/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php b/src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php
@@ -46,10 +46,8 @@ public function __construct($listener, $name, Stopwatch $stopwatch, EventDispatc
             $r = new \ReflectionFunction($listener);
             if (false !== strpos($r->name, '{closure}')) {
                 $this->pretty = $this->name = 'closure';
-            } elseif ($this->name = $r->getClosureScopeClass()) {
-                $this->pretty = $this->name.'::'.$r->name;
-            } elseif ($class = $r->getClosureThis()) {
-                $this->name = \get_class($class);
+            } elseif ($class = $r->getClosureScopeClass()) {
+                $this->name = $class->name;
                 $this->pretty = $this->name.'::'.$r->name;
             } else {
                 $this->pretty = $this->name = $r->name;
diff --git a/src/Symfony/Component/EventDispatcher/Tests/Debug/WrappedListenerTest.php b/src/Symfony/Component/EventDispatcher/Tests/Debug/WrappedListenerTest.php
@@ -0,0 +1,64 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\EventDispatcher\Tests\Debug;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\EventDispatcher\Debug\WrappedListener;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Component\Stopwatch\Stopwatch;
+
+class WrappedListenerTest extends TestCase
+{
+    /**
+     * @dataProvider provideListenersToDescribe
+     */
+    public function testListenerDescription(callable $listener, $expected)
+    {
+        $wrappedListener = new WrappedListener($listener, null, $this->getMockBuilder(Stopwatch::class)->getMock(), $this->getMockBuilder(EventDispatcherInterface::class)->getMock());
+
+        $this->assertStringMatchesFormat($expected, $wrappedListener->getPretty());
+    }
+
+    public function provideListenersToDescribe()
+    {
+        $listeners = array(
+            array(new FooListener(), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::__invoke'),
+            array(array(new FooListener(), 'listen'), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen'),
+            array(array('Symfony\Component\EventDispatcher\Tests\Debug\FooListener', 'listenStatic'), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listenStatic'),
+            array('var_dump', 'var_dump'),
+            array(function () {}, 'closure'),
+        );
+
+        if (\PHP_VERSION_ID >= 70100) {
+            $listeners[] = array(\Closure::fromCallable(array(new FooListener(), 'listen')), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen');
+            $listeners[] = array(\Closure::fromCallable(array('Symfony\Component\EventDispatcher\Tests\Debug\FooListener', 'listenStatic')), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listenStatic');
+            $listeners[] = array(\Closure::fromCallable(function () {}), 'closure');
+        }
+
+        return $listeners;
+    }
+}
+
+class FooListener
+{
+    public function listen()
+    {
+    }
+
+    public function __invoke()
+    {
+    }
+
+    public static function listenStatic()
+    {
+    }
+}
diff --git a/src/Symfony/Component/HttpKernel/DataCollector/RequestDataCollector.php b/src/Symfony/Component/HttpKernel/DataCollector/RequestDataCollector.php
@@ -393,9 +393,7 @@ protected function parseController($controller)
             $controller['method'] = $r->name;
 
             if ($class = $r->getClosureScopeClass()) {
-                $controller['class'] = $class;
-            } elseif ($class = $r->getClosureThis()) {
-                $controller['class'] = \get_class($class);
+                $controller['class'] = $class->name;
             } else {
                 return $r->name;
             }
diff --git a/src/Symfony/Component/VarDumper/Caster/ReflectionCaster.php b/src/Symfony/Component/VarDumper/Caster/ReflectionCaster.php
@@ -40,13 +40,7 @@ public static function castClosure(\Closure $c, array $a, Stub $stub, $isNested,
         $a = static::castFunctionAbstract($c, $a, $stub, $isNested, $filter);
 
         if (false === strpos($c->name, '{closure}')) {
-            if (isset($a[$prefix.'class'])) {
-                $stub->class = $a[$prefix.'class']->value.'::'.$c->name;
-            } elseif (isset($a[$prefix.'this'])) {
-                $stub->class = $a[$prefix.'this']->class.'::'.$c->name;
-            } else {
-                $stub->class = $c->name;
-            }
+            $stub->class = isset($a[$prefix.'class']) ? $a[$prefix.'class']->value.'::'.$c->name : $c->name;
             unset($a[$prefix.'class']);
         }
 

Original file line number	Diff line number	Diff line change
`@@ -374,10 +374,9 @@ private function getCallableData($callable, array $options = array())`
`374`	`374`	`}`
`375`	`375`	`$data['name'] = $r->name;`
`376`	`376`
`377`		`- $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;`
`378`		`- if ($scopeClass = $r->getClosureScopeClass() ?: $class) {`
`379`		`- $data['class'] = $scopeClass;`
`380`		`- if (!$class) {`
	`377`	`+ if ($class = $r->getClosureScopeClass()) {`
	`378`	`+ $data['class'] = $class->name;`
	`379`	`+ if (!$r->getClosureThis()) {`
`381`	`380`	`$data['static'] = true;`
`382`	`381`	`}`
`383`	`382`	`}`
Original file line number	Diff line number	Diff line change
`@@ -360,10 +360,9 @@ protected function describeCallable($callable, array $options = array())`
`360`	`360`	`}`
`361`	`361`	$string .= "\n".sprintf('- Name: `%s`', $r->name);
`362`	`362`
`363`		`- $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;`
`364`		`- if ($scopeClass = $r->getClosureScopeClass() ?: $class) {`
`365`		- $string .= "\n".sprintf('- Class: `%s`', $class);
`366`		`- if (!$class) {`
	`363`	`+ if ($class = $r->getClosureScopeClass()) {`
	`364`	+ $string .= "\n".sprintf('- Class: `%s`', $class->name);
	`365`	`+ if (!$r->getClosureThis()) {`
`367`	`366`	`$string .= "\n- Static: yes";`
`368`	`367`	`}`
`369`	`368`	`}`
Original file line number	Diff line number	Diff line change
`@@ -474,10 +474,7 @@ private function formatCallable($callable)`
`474`	`474`	`return 'Closure()';`
`475`	`475`	`}`
`476`	`476`	`if ($class = $r->getClosureScopeClass()) {`
`477`		`- return sprintf('%s::%s()', $class, $r->name);`
`478`		`- }`
`479`		`- if ($class = $r->getClosureThis()) {`
`480`		`- return sprintf('%s::%s()', \get_class($class), $r->name);`
	`477`	`+ return sprintf('%s::%s()', $class->name, $r->name);`
`481`	`478`	`}`
`482`	`479`
`483`	`480`	`return $r->name.'()';`
Original file line number	Diff line number	Diff line change
`@@ -586,10 +586,9 @@ private function getCallableDocument($callable)`
`586`	`586`	`}`
`587`	`587`	`$callableXML->setAttribute('name', $r->name);`
`588`	`588`
`589`		`- $class = ($class = $r->getClosureThis()) ? \get_class($class) : null;`
`590`		`- if ($scopeClass = $r->getClosureScopeClass() ?: $class) {`
`591`		`- $callableXML->setAttribute('class', $class);`
`592`		`- if (!$class) {`
	`589`	`+ if ($class = $r->getClosureScopeClass()) {`
	`590`	`+ $callableXML->setAttribute('class', $class->name);`
	`591`	`+ if (!$r->getClosureThis()) {`
`593`	`592`	`$callableXML->setAttribute('static', 'true');`
`594`	`593`	`}`
`595`	`594`	`}`
Original file line number	Diff line number	Diff line change
`@@ -155,7 +155,7 @@ public static function getEventDispatchers()`
`155`	`155`
`156`	`156`	`public static function getCallables()`
`157`	`157`	`{`
`158`		`- return array(`
	`158`	`+ $callables = array(`
`159`	`159`	`'callable_1' => 'array_key_exists',`
`160`	`160`	`'callable_2' => array('Symfony\\Bundle\\FrameworkBundle\\Tests\\Console\\Descriptor\\CallableClass', 'staticMethod'),`
`161`	`161`	`'callable_3' => array(new CallableClass(), 'method'),`
`@@ -164,6 +164,12 @@ public static function getCallables()`
`164`	`164`	`'callable_6' => function () { return 'Closure'; },`
`165`	`165`	`'callable_7' => new CallableClass(),`
`166`	`166`	`);`
	`167`	`+`
	`168`	`+ if (\PHP_VERSION_ID >= 70100) {`
	`169`	`+ $callables['callable_from_callable'] = \Closure::fromCallable(new CallableClass());`
	`170`	`+ }`
	`171`	`+`
	`172`	`+ return $callables;`
`167`	`173`	`}`
`168`	`174`	`}`
`169`	`175`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\CallableClass::__invoke()`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+<?xml version="1.0" encoding="UTF-8"?>`
	`2`	`+<callable type="closure" name="__invoke" class="Symfony\Bundle\FrameworkBundle\Tests\Console\Descriptor\CallableClass"/>`