Skip to content

[VarDumper] fix dump of closures created from callables #29054

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.

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

Merged
merged 1 commit into from
Nov 6, 2018

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

We are missing displaying full information about closures created using ReflectionMethod::getClosure() or Closure::fromCallable().

This PR fixes it. For VarDumper but also other places where we have logic to display them.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Nov 1, 2018
@nicolas-grekas nicolas-grekas force-pushed the vd-from-callables branch 2 times, most recently from eff20df to 1034a66 Compare November 1, 2018 15:37
@nicolas-grekas nicolas-grekas merged commit 1c1818b into symfony:3.4 Nov 6, 2018
nicolas-grekas added a commit that referenced this pull request Nov 6, 2018
…icolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[VarDumper] fix dump of closures created from callables

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

We are missing displaying full information about closures created using `ReflectionMethod::getClosure()` or `Closure::fromCallable()`.

This PR fixes it. For VarDumper but also other places where we have logic to display them.

Commits
-------

1c1818b [VarDumper] fix dump of closures created from callables
@nicolas-grekas nicolas-grekas deleted the vd-from-callables branch November 6, 2018 17:20
@stof
Copy link
Member

stof commented Nov 6, 2018

the new behavior is tested in VarDumper, but not in other places

nicolas-grekas added a commit that referenced this pull request Nov 11, 2018
…ass usage to describe callables (ogizanagi)

This PR was merged into the 3.4 branch.

Discussion
----------

[Fwb][EventDispatcher][HttpKernel] Fix getClosureScopeClass usage to describe callables

| Q             | A
| ------------- | ---
| Branch?       | 3.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #29054   <!-- #-prefixed issue number(s), if any -->
| 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
@fabpot fabpot mentioned this pull request Nov 16, 2018
This was referenced Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants