Erroneous call of magic functions on class variables, erroneous assignment of data to a variable

[AlexeyZamorov]

Description

The following code:

<?php
class TClass
{
	public function something() : array
	{
		unset(self::$Cache->Content);

		return [];
	}
	public static TCache $Cache;
}

TClass::$Cache = new TCache();

class TCache
{
	public function __destruct()
	{
		if(isset($this->Hash, $this->Content)) {}
	}

	public function &__get(string $name)
	{
		if(!isset($this->Content))
		{
			$this->Content = [];
			$this->Hash = false;
		}

		return $this->Content;
	}

	public function __set(string $name, string|array|false $value) : void
	{
		$this->Content = $value;

		// this code is failing
		$this->Hash ??= false;

		//this code works correctly
		//if(!isset($this->Hash)) { $this->Hash = false; }
	}

	public function __isset(string $name) : bool
	{
		if(!isset($this->Content))
		{
			$this->__get($name);
		}

		return isset($this->Content);
	}

	public function __unset(string $name) : void
	{
		unset($this->Content, $this->Hash);
	}

	protected string|array|false $Content;
	protected string|false $Hash;
}

(new TClass())->something();

Resulted in this output:

PHP Fatal error:  Uncaught TypeError: Cannot assign array to property TCache::$Hash of type string|false in test.php:38
Stack trace:
#0 test.php(26): TCache->__set('Content', Array)
#1 test.php(48): TCache->__get('Content')
#2 test.php(46): TCache->__isset('Content')
#3 test.php(19): TCache->__isset('Hash')
#4 [internal function]: TCache->__destruct()
#5 {main}
  thrown in test.php on line 38

Process finished with exit code 255

But I expected this output instead:

Process finished with exit code 0

PHP Version

8.1.21

Operating System

FreeBSD 13.2

[Girgias]

@iluuu1994 didn't you change something recently in regards to that?

[damianwadley]

Seems like the error message is wrong: the problem is that Hash is uninitialized, so when $this->Hash ??= false implicitly gets the value of Hash it fails; if you replace the line with simply $this->Hash; you get the same error message.

[AlexeyZamorov]

Seems like the error message is wrong: the problem is that Hash is uninitialized, so when $this->Hash ??= false implicitly gets the value of Hash it fails; if you replace the line with simply $this->Hash; you get the same error message.

There are two mistakes in my opinion.

The first problem is that the code calls magic functions in __destruct() on if(isset($this->Hash, $this->Content)) {}.

The second problem is that the code gives an error on $this->Hash ??= false;, but in this version if(!isset($this->Hash)) { $this->Hash = false; } everything is fine.

[damianwadley]

The first problem is that the code calls magic functions in __destruct()

Yeah, TCache is doing a number of things with magic that it really needs to not be doing, and as I see it, most of the problems stem from it unsetting Content and Hash instead of just using them as normal properties.

but in this version if(!isset($this->Hash)) { $this->Hash = false; } everything is fine.

There's something complicated going on here that's not obvious because PHP isn't recursively calling magic methods - it detects the recursion and will bypass things like __get/set if they have already been invoked. But that's hiding the actual underlying behavior; for instance, some quick tests suggest the Content= line above isn't a simple assignment but apparently triggering even more magic, and with all of its side effects...

[iluuu1994]

@iluuu1994 didn't you change something recently in regards to that?

Nope, these changes only landed in master.

Tbh your code is very convoluted. The issue is that you are returning a value from __get that is not compatible with the property type. The error message is confusing but was improved in master in #9436. If you change the line $this->Content = []; to $this->Content = false; which is valid for Hash then the error goes away. This check is there to satisfy lsp, as users of the property should be able to rely on the property type.

??= is different from isset in that it calls __get instead of __isset, thus triggering the error.