Skip to content
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

Printer handles null value of Property as no value incorrectly #71

Closed
dakur opened this issue Nov 5, 2020 · 5 comments
Closed

Printer handles null value of Property as no value incorrectly #71

dakur opened this issue Nov 5, 2020 · 5 comments

Comments

@dakur
Copy link

dakur commented Nov 5, 2020

Version: 3.4.1

Bug Description

When printing class, there is this line:

($property->getValue() === null && !$property->isInitialized() ? '' : ' = ' . $this->dump($property->getValue(), strlen($def) + 3)) // 3 = ' = '

which basically means that if I set value of Property to null intentionally, this value is not printed.

Steps To Reproduce

E.g. $property->setPrivate()->setValue(null) generates private $property;
But $property->setPrivate()->setValue('test') generates private $property = 'test'; properly

Expected Behavior

It should generate private $property = null;

Possible Solution

Mark null state with some constant instead of null. So it would look something like this:

class Property
{
    public const NO_DEFAULT_VALUE = '$73aad02e-7f5a-4ef2-8cdd-0fd09e6d9863§'; // hopefully no one is going to pass this string as default value

    // ...
}

// in printer:
($property->getValue() === Property::NO_DEFAULT_VALUE && !$property->isInitialized() ? '' : ' = ' . $this->dump($property->getValue(), strlen($def) + 3)) // 3 = ' = '
@dg
Copy link
Member

dg commented Nov 5, 2020

use setInitialized()

@dg dg closed this as completed Nov 5, 2020
@dakur
Copy link
Author

dakur commented Nov 5, 2020

@dg So you mean that this setInitialized is here to bypass this nullability problem?

@dg
Copy link
Member

dg commented Nov 6, 2020

Yes, because PHP calls them initialized https://www.php.net/manual/en/reflectionproperty.isinitialized.php.

They only make sense if the property has a type, otherwise = null is unnecessary.

@dg
Copy link
Member

dg commented Nov 6, 2020

But it is true that an explicit setValue(null) could call setInitialized() 🤔

@dakur
Copy link
Author

dakur commented Nov 9, 2020

Yes, that was actually the point of the issue – API did not work in the way developer expected and did not throw any error on usage either.

The fix solves this, thanks. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants