[VarDumper] Fixed dumping of terminated generator #21542

Merged
merged 1 commit into from Feb 6, 2017

Projects

None yet

4 participants

@lyrixx
Member
lyrixx commented Feb 6, 2017
Q A
Branch? 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -
+ try {
+ $reflectionGenerator = new \ReflectionGenerator($c);
+ } catch (\Exception $e) {
+ return $a;
@nicolas-grekas
nicolas-grekas Feb 6, 2017 Member

let's add a virtual prop that says closed: true?

@lyrixx
lyrixx Feb 6, 2017 Member

Good idea. Added.

@@ -99,28 +110,28 @@ public static function castReflectionGenerator(\ReflectionGenerator $c, array $a
if ($c->getThis()) {
$a[$prefix.'this'] = new CutStub($c->getThis());
}
- $x = $c->getFunction();
+ $function = $c->getFunction();
@nicolas-grekas
nicolas-grekas Feb 6, 2017 Member

diff irrelevant for review

@@ -76,7 +76,18 @@ public static function castClosure(\Closure $c, array $a, Stub $stub, $isNested)
public static function castGenerator(\Generator $c, array $a, Stub $stub, $isNested)
{
- return class_exists('ReflectionGenerator', false) ? self::castReflectionGenerator(new \ReflectionGenerator($c), $a, $stub, $isNested) : $a;
+ if (!class_exists('ReflectionGenerator', false)) {
+ return $a;
@nicolas-grekas
nicolas-grekas Feb 6, 2017 Member

closed: true/false virtual prop?

@lyrixx
lyrixx Feb 6, 2017 Member

How can I know?

@nicolas-grekas
nicolas-grekas Feb 6, 2017 Member

call ->valid()?

@lyrixx
lyrixx Feb 6, 2017 Member

I can not because it alters the generator. That's why I used the try catch method and not ->valid().

@lyrixx lyrixx [VarDumper] Fixed dumping of terminated generator
c5094a0
@nicolas-grekas

👍

+ // Cannot create ReflectionGenerator based on a terminated Generator
+ try {
+ $reflectionGenerator = new \ReflectionGenerator($c);
+ } catch (\Exception $e) {
@nicolas-grekas
nicolas-grekas Feb 6, 2017 Member

I confirm that an Exception is thrown here, even-though I'd personally have expected a ReflectionException
ping @jpauli.

@jpauli
jpauli Feb 7, 2017 Contributor

This has been fixed as part of PHP 7.2 in php/php-src@ed4216c

@nicolas-grekas
Member

Thank you @lyrixx.

@nicolas-grekas nicolas-grekas merged commit c5094a0 into symfony:2.8 Feb 6, 2017

2 of 3 checks passed

fabbot.io [braces] Is not configurable.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nicolas-grekas nicolas-grekas added a commit that referenced this pull request Feb 6, 2017
@nicolas-grekas nicolas-grekas bug #21542 [VarDumper] Fixed dumping of terminated generator (lyrixx)
This PR was merged into the 2.8 branch.

Discussion
----------

[VarDumper] Fixed dumping of terminated generator

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

Commits
-------

c5094a0 [VarDumper] Fixed dumping of terminated generator
c346f2a
@lyrixx lyrixx deleted the lyrixx:dump-generator branch Feb 6, 2017
@fabpot fabpot referenced this pull request Feb 17, 2017
Merged

Release v3.2.4 #21640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment