Potential scoping issue when hooks are defined in traits. Not sure if this is the expected behavior or not.

https://3v4l.org/8NILP/rfc#vrfc.property-hooks

@TRowbotham Thanks for letting me know. I added this to my todo list.

Here is another one: https://3v4l.org/7Al8b/rfc#vrfc.property-hooks

When hooks are defined in a class that implements the \Iterator interface the iterator returns the values of each property's get hook instead of the values that the iterator contains.

@TRowbotham Good catch! I'll have a look at that one as well.

Ran into a behavior difference between __get/__set and property hooks when a property hook's setter calls a function, which reads a property hook's value from a different scope. With __get/__set it works without error, however, the property hooks version reports a fatal error. I couldn't tell from the RFC if the difference in behavior was intentional.

(property hooks) https://3v4l.org/6W26c/rfc#vrfc.property-hooks
(__get/__set) https://3v4l.org/3pUtD/rfc#vrfc.property-hooks

@TRowbotham This is intended behavior. Inside functions called from a hook, $this->prop refers to the backing field, as it would in the hook. However, the compiler cannot infer that that a backing field must be added, since it isn't used directly in the hook. As such, the property is marked as virtual. When doSomething is called, it tries to access this backing field that doesn't exist.

An additional difference is that, for the __get/__set case, each magic method is "guarded" on it's own. Guarding in this context means that inside the magic method, it won't be called again, but the underlying property will be accessed. If you assign a variable, invoking __set, __get can still be invoked inside __set. The same is not true for hooks, once you're in the hook $this->prop always refers to the backing value. This is intentional, because otherwise read/write assignments such as ++ within set might unexpectedly call your getter. Furthermore, it allows for more flexibility in the future, where self::$prop::get() may still be used as an explicit way to invoke the other hook.

I hope that clears it up.

@iluuu1994 Thanks for the explanation. That makes it clear.

Congratulations!!! RFC passed! PHP 8.4 will be AWESOME!!!

@rodrigoslayertech The voting is open until 29 April. Let's not count our chickens before they hatch. ;-)

Awsome, this sounds like the dynamic properties we have in laravel models, voted.

Hi,

While playing with the feature on 3v4l.org, I found the following bug. Although I’ve not tested the PR in its current state, I suspect that the bug might be still here, because I didn’t find test against it.

Test case (https://3v4l.org/fuj6p/rfc#vrfcproperty-hooks):

abstract class A {
    abstract public $x { get; }
}

class C extends A {
    private $_x;
    public $x {
        get => $this->_x;
    }
}

// expected: bool(true); actual: bool(false)
var_dump((new ReflectionProperty(C::class, 'x'))->isVirtual());

// expected: Error: Property C::$x is read-only; actual: no error
$c = new C;
$c->x = 3;

The test case works as expected when I replace the abstract class with an interface (https://3v4l.org/ed1TG/rfc#vrfc.property-hooks).

(Background: I expect to use that pattern in order to approximate asymmetric visibility.)

@claudepache The 3v4l build is indeed outdated. In any case, I'll make sure to have a look at your example. Thanks!

While continuing to play with this, I came across a case where using the get hook as a generator results in a segfault.

<?php

class A {
    public Generator $prop {
        get {
            for ($i = 0; $i < 3; ++$i) {
                yield $i;
            }
        }
    }
}

$a = new A();
var_dump($a->prop->valid());

@TRowbotham Thanks! This wasn't really specified in the RFC. I fixed the issue, but also raised it to Larry to discuss over whether we even want to allow yield. Edit: Talked it over with Larry and Bob. There's no good reason not to allow this, so we will.

The implementation now solves all known issues, and CI (including nightly) passes. So I will be merging it in the coming days.

@iluuu1994 I know it's already merged, but I have question: wouldn't it be a good idea to introduce T_PROPERTY_HOOK_BRACE_OPEN and T_PROPERTY_HOOK_BRACE_CLOSE, so tools based on tokenizer could take direct advantage of it for their logic? Without this, PHP-CS-Fixer probably needs to introduce new custom token and transformer to make it easier for all the rules to hook into new syntax.

@Wirone Unfortunately, we are lacking the context in the lexer ourselves to do that.