nette / php-generator

๐Ÿ˜ Generates neat PHP code for you. Supports new PHP 8.3 features.
https://doc.nette.org/php-generator
Other
2.11k stars 138 forks source link

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

Closed dakur closed 3 years ago

dakur commented 3 years ago

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 commented 3 years ago

use setInitialized()

dakur commented 3 years ago

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

dg commented 3 years ago

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 commented 3 years ago

But it is true that an explicit setValue(null) could call setInitialized() ๐Ÿค”

dakur commented 3 years ago

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. ๐Ÿ‘