unset() on already initialised object property?

[jkuchar]

Let's open discussion, if this code, which breaks type hint of property should be allowed:

https://phpstan.org/r/f5b0a55c-b46e-45d3-9c38-e7c03f164dbf

<?php declare(strict_types = 1);

class A {
	public string $x = '';
}
$a = new A;
unset($a->x);

Problem

I would consider property $x to be initialized, there I would not expect it to become uninitialised again. I think it should NOT be allowed to unset value from object outer scope.

Any arguments for or against?

[ondrejmirtes]

I'd say if your code isn't special like https://github.com/Ocramius/ProxyManager, it's never valid to unset a property. So it'd be a good fit for phpstan-strict-rules.

But there are many things like that in PHP. One could say that using ReflectionProperty::setAccessible (https://www.php.net/manual/en/reflectionproperty.setaccessible.php) is also never a valid thing to do - but I don't think we need to write a rule for that...

[jkuchar]

ReflectionProperty::setAccessible() is explicitly stated what are you intending to do (you can always break things with reflection).

From what I have see – with unset() it is in most cases kind of stangely done nullability.

When developer see ?string, unset() can look like a valid way to unset the property to null. That is wrong obviously. In addition it works even with non-nullable fields, which basically adds |uninicialized to all types, which in most cases you do not need to count with.

From my perspective there are valid use-cases when in-class code uses unset() for property inside the class, it is trying to make something strange with the property lifecycle, however class itself should be aware what it is doing with itself. I would like to be must stricter with code that accesses public property of class from outside. In case someone want to enable unseting of property, providing class method for that seems much more valid pattern to me as it hides this strange internal API.

class A {public ?string $x;}
$a = new A();
unset($a->x); // invalid

class A {
    private string $x; 
    public function doSth() {
         unset($this->x); // valid, as it is internal state
    }
}
$a = new A();
$a->doSth();

---

Here I made a property public. Not sure if this makes sense to allow, as accessing $a->x can be uninitialised at any time and there is no type-hint for that in class public API.

class A {
    public string $x; 
    public function doSth() {
         unset($this->x); // (?!)
    }
}
$a = new A();
$a->doSth();

For the last example it looks to me that this behaviour should be disabled altogether on class public properties.

---

Note: Interesting enough, if you use stdClass() with ad-hoc properties, behaviour is different (and definitely more valid for the use-case). In case you unset a property, it disappears completely from the object. Therefore unset() with undeclared property is always valid. 🤷‍♂️

➜  ~ php -a
Interactive shell

php > class A{};
php > $a = new A();
php > var_dump($a);
object(A)#1 (0) {
}
php > $a->x = 'hello';
php > var_dump($a);
object(A)#1 (1) {
  ["x"]=>
  string(5) "hello"
}
php > unset($a->x);
php > var_dump($a);
object(A)#1 (0) {
}