Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[VarDumper] show uninitialized properties#50076
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[VarDumper] show uninitialized properties#50076
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedApr 20, 2023
Hey! Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.3 for features". Cheers! Carsonbot |
lyrixx left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
wahoo, super good start!
| $stubClass =$stub->value::class; | ||
| $stub->attr['uninitialized'] = []; | ||
| foreach ($uninitializedPropsas$key =>$value) { | ||
| $rp =new \ReflectionProperty($stubClass,$key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you add a local cache for this?
| $stub->attr['uninitialized'] = []; | ||
| foreach ($uninitializedPropsas$key =>$value) { | ||
| $rp =new \ReflectionProperty($stubClass,$key); | ||
| $stub->attr['uninitialized'][] = [$key =>$rp->getType()->getName()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this is broken for any union or intersection types. Not all ReflectionType are ReflectionNamedType.
| $stub->attr['uninitialized'] = []; | ||
| foreach ($uninitializedPropsas$key =>$value) { | ||
| $rp =new \ReflectionProperty($stubClass,$key); | ||
| $stub->attr['uninitialized'][] = [$key =>$rp->getType()->getName()]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why using a list of single-item arrays with an unknown key for this$stub->attr['uninitialized'] ? why not using$stub->attr['uninitialized'][$key] = $type here ?
| } | ||
| $cursor->skipChildren =false; | ||
| // Add uninitialized properties | ||
| foreach ($item->attr["uninitialized"]as$uninitializedProperty) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
shouldn't this be done insidedumpChildren instead ? Otherwise, things will be weird when the cursor is meant to dump without children.
| $stub->value =$v; | ||
| $stub->handle =$h; | ||
| $a =$this->castObject($stub,0 <$i); | ||
| if ($stub->value !=='') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Does this work for nested objects or should we implement that logic incastObject ?
And I'm wondering whether this should be implemented inCaster::castObject by adding those properties as a specialUninitializedStub, so that this is donebefore any custom caster runs. This way, if a caster decides to cut a property, this would also cut it if it is uninitialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree with this. I would add this to AbstractCloner::castObject, where there is already$this->classInfo that we can use to store thoseUninitializedStub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok I'll try implementing this solution. Thanks
7d978a0 toa4684c3Comparenicolas-grekas commentedJul 27, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I went with another approach in#51130 |
…kas)This PR was merged into the 6.4 branch.Discussion----------[VarDumper] Dump uninitialized properties| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |Fix#50054| License | MIT| Doc PR | -Replaces#50076Given this:```phpclass Foo { private int|float $foo; public $baz;}$f = new Foo;unset($f->baz);dump($f);```This displays:Commits-------54468db [VarDumper] Dump uninitialized properties
Uh oh!
There was an error while loading.Please reload this page.
First commit for the feature. Please review this pull request and help me doing this in a good way 🙏
I am not sure if it's good to store uninitialized properties to the attr of the stub. Any better ideas ?
Missing: