Skip to content

isset()/empty() not working correctly with chained references #373

@viktorasp

Description

@viktorasp

If you are are poor soul stuck with an old code migration, please note that when migrating from PHP5 to PHP7+, the checks isset()/empty() on chained references might stop working correctly:

$user = User::Load($someId);
if (isset($user->Group->name)) {
    echo "name is set"; // PHP5
} else {
    echo "name is missing"; // PHP7+
}

After spending way too many hours on this issue, the problem is a combination of a tiny change in PHP and Doctrine1 dynamic properties usage.

PHP changes explanation:

Sandbox URL: https://onlinephp.io/c/89306

<?php
class Registry
{
    protected $_items = array();
    public function __set($key, $value)
    {
    	echo "Set: $key\n";
        $this->_items[$key] = $value;
    }
    public function __get($key)
    {
    	echo "Get: $key\n";
    	return isset($this->_items[$key]) ? $this->_items[$key] : null;
    }
    public function __isset($key)
    {
    	echo "Isset: $key\n";
        return isset($this->_items[$key]) ? empty($this->_items[$key])) === false : null;
    }
}
class RegB extends Registry {}

$registry = new Registry();
$registry->one = new RegB();
$registry->one->two = 'value';


var_dump(empty($registry->one->two)); 

Running the code above, there's a small difference: when doing check empty($registry->test->secret), PHP7+ does isset() check for each element in a chain, starting from the left side. So, it goes like this
PHP7+:
Isset: one
Get: one
Isset: two
Get: two

PHP5:
Get: one
Isset: two
Get: two

Now we are getting to the Doctrine1 part!

Doctrine/Record.php:1782 has

if (isset($this->_references[$offset]) &&
    $this->_references[$offset] !== self::$_null) {
    return true;
}

which checks if a given offset exists among references, and in PHP5 that works just fine, as we are checking the right-most value in the chain for existence. In PHP7+ this check fails, as we are checking the left-most chain member, and if it's not loaded yet, then this check will fail.

As for the possible fixes, either user-land code could change isset()/empty() to a bunch of validity and instanceof checks, in case of the original example, something like:

$user = User::Load($someId);
if ($user->Group instanceof Group && isset($user->Group->name)) {
// if ($user->Group && $user->Group->name) {
    echo "name is set"; // hooray!
} else {
    echo "name is missing";
}

optionally, Doctrine/Record::contains() could be modified to attempt loading of a reference, if it was not found immediately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions