Property Hooks - reflectionProperty::getInitialized

[bkdotcom]

Description

The following code:

https://3v4l.org/KXI3l/rfc#vgit.master

<?php

class PropertyHooks
{
    public ?string $backedGetOnly {
        get => $this->backedGetOnly;
    }

    public ?string $backedSetOnly {
        set (?string $value) {
            $this->backedSetOnly = $value;
        }
    }

    public ?string $backedGetAndSet {
        set (?string $value) {
            $this->backedGetAndSet = $value;
        }

        get => $this->backedGetAndSet;
    }
}

$propertyHooks = new PropertyHooks();

$reflectionProperty = new ReflectionProperty($propertyHooks, 'backedGetOnly');

$reflectionProperty->isInitialized($propertyHooks); // returns true - I would expect false
                                      // backSetOnly reports  false
                                      // backedGetAndSet reports true - I would also expect false

if ($reflectionProperty->isInitialized($propertyHooks)) {
    $reflectionProperty->getRawValue($propertyHooks); // Uncaught exception 'Error' with message Typed property PropertyHooks::$backedGetOnly must not be accessed before initialization
} else {
    echo 'not initialized';
}

Resulted in this output:

Fatal error: Uncaught Error: Typed property PropertyHooks::$backedGetOnly must not be accessed before initialization in /in/KXI3l:33
Stack trace:
#0 [internal function]: PropertyHooks->$backedGetOnly::get()
#1 /in/KXI3l(33): ReflectionProperty->getRawValue(Object(PropertyHooks))

But I expected this output instead:

not initialized

ReflectionProperty::isInitialized() seems to be based on whether or not there's a get hook instead of the backing-value?
If that is expected behavior, should we also have a isRawValueInitialized() method?

(isInitialized() returns true for virtual properties)

PHP Version

PHP 8.4 (master - 2024-09-01)

Operating System

OSX & 3v4l

[cmb69]

Maybe @iluuu1994 or @Crell can clarify?

[iluuu1994]

This is a oversight. I agree that ReflectionProperty::isInitialized() should refer to the properties backing value. Semantically, hooked properties (with a get operation) cannot be uninitialized, as a function cannot return an uninitialized value. So there would be no point in checking for it.

[iluuu1994]

That does pose the question what isInitialized should return for virtual properties. There are only two sensible options:

1. Always return true, even though they have no concept of being initialized.
2. Throw, which makes ReflectionProperty::isInitialized() conditionally usable.

Not a fan of either of those. @Crell Thoughts?

[bkdotcom]

That does pose the question what isInitialized should return for virtual properties.

in the sense that ReflectionProperty::isInitialized() serves as check of whether or not one can access the property (via both the obj and reflection)

• $obj->prop, or ReflectionProperty::getValue($obj). both will work... it's "initialized"
• it's already documented that ReflectionProperty::getRawValue() will throw an exception for virtual properties

returning true (apparent current behavior) makes as much sense as anything

[iluuu1994]

in the sense that ReflectionProperty::isInitialized() serves as check of whether or not one can access the property

Under that definition, the current behavior is actually correct for backed properties too, since it's the responsibility of the getter to ensure not to access the uninitialized backing value. Otherwise, lazy getters would not be safe to access, according to isInitialized.

[bkdotcom]

I added the qualifier: "(via both the obj and reflection)"

as getRawValue() throws the "must not be accessed before initialization" : not initialized

it's a very "loose" interpretation of what it means to be "initialized" 😊

[iluuu1994]

Right. What I was trying to imply is that reflection can't know whether it's safe to call get on a property with an uninitialized backing value, as it entirely depends on its implementation. I.e. a lazy get may initialize the backing value during its execution. Hence, a "check of whether or not one can access the property" can't be implemented for backed properties. We can however check whether its backing value is set, but that is not meaningful for virtual properties. So essentially, either we have to throw, or use a different definition of what the function does for backed and unbacked properties.

Sorry if I'm being pedantic about terminology. I'm also trying to figure out what the correct behavior is.

[Crell]

(Thinking aloud...)

The user-expected behavior, I would think, is that "if it's initialized I can use it, if it's not initialized I can't." (We don't need to worry about readonly at the moment and its implications.) So we should try to get as close to that as possible. However, Ilija is very correct that there are ample cases where we cannot tell that directly. Any lazy-cached property would have that issue:

class Person {
  public string $fullName { get => $this->lastName ??= "$this->first $this->last"; }
}

That has an uninitialized backing value, but reading it is perfectly safe. OTOH, even a virtual property could be written such that the expectation doesn't hold.

class Person {
  public string $fullName { get => "$this->first $this->last"; }
}

There's no backing value, but it will still fail if $first or $last are not initialized.

So no matter what we do, there's going to be some situations where isInitialized() will be unreliable. The question is how do we minimize that.

If we think of hooks as methods, then we get an expectation of "calling the method is fine, but whether the method works or not is anyone's guess." Which would suggest always returning true, and it's the property's responsibility to not break. That would be somewhat consistent with the behavior if explicit getFoo() methods are defined. (There is no initialized for a method, so you have to just trust the method's return value is type safe and it doesn't access uninitialized values internally.)

On the flip side, what's the use case for basing it on a backing value? The backing value should be opaque, by design. In what situations would you want to determine the initialization status of a backed property's internal store? That feels like trying to access the status if a private property. Is there a use case for doing so? The only one I can think of is some kind of external lazy initialization, but... I cannot think of where you'd do that, and the lazy-cache pattern above isn't superior in every way.

So I think I'm leaning towards the status quo (the presence of a get hook always returns initialized true), and saying if that doesn't hold, the get hook is buggy and should feel bad.

(A valid use case for external determination would possibly change my view there, if one can be found.)

[iluuu1994]

the presence of a get hook always returns initialized true

That's not quite the current behavior. We currently actually invoke get, which isn't really necessary since it can't return false anyway.

But I tend to agree.