[PHP-DEV] [RFC][Vote] Property Hooks

The vote for the Property Hooks RFC is now open:

https://wiki.php.net/rfc/property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

--
  Larry Garfield
  larry@garfieldtech.com

1 Like

On 15/04/2024 17:43, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

PHP: rfc:property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

I'm somewhat conflicted on this one. On the one hand, I think the feature will be very powerful, and it's clear a lot of effort has been put into designing something that fits with the language. On the other hand, however, I share the concerns some have expressed that it is a very complex proposal.

I would have more enthusiastically supported one which left out a few "bells and whistles", or moved them to Future Scope to be polished and agreed separately. I hope I was consistent in expressing that during the discussion phase.

For that reason, please consider this an "abstention": while I'm reasonably happy for the RFC as written to pass, I am not going to cast a Yes vote myself.

Regards,

--
Rowan Tommins
[IMSoP]

On 29/04/2024 20:58, Larry Garfield wrote:

On Mon, Apr 15, 2024, at 4:43 PM, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

PHP: rfc:property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

The vote has now been closed. The final tally is 42 Yes and 2 No, meaning it has passed.

Thank you everyone for your participation and feedback. Ilija will try to get the PR merged ASAP.

--Larry Garfield

Finally, a reason to upgrade this Winter! Congrats on the successful RFC, Larry and Ilija, et al.

Cheers,
Bilge

On Mon, Apr 15, 2024, at 4:43 PM, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

PHP: rfc:property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

The vote has now been closed. The final tally is 42 Yes and 2 No, meaning it has passed.

Thank you everyone for your participation and feedback. Ilija will try to get the PR merged ASAP.

--Larry Garfield

1 Like

On Mon, Apr 15, 2024, at 18:43, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

https://wiki.php.net/rfc/property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

I know I’m a few months late on this one, but I figured I’d still leave a couple of thoughts… Overall, the proposal is well thought out and does address many of the reservations I had about my original accessors proposal.

One of the more interesting choices in this proposal is to base whether the property is virtual or backed on the presence of a “$this->prop” reference in the accessor implementation. I think that, conceptually at least, this is a good choice.

What I find concerning though, is that the presence/absence of such a reference also affects the meaning of the get and set hooks. If the property is virtual and it only has get, then the property cannot be set. If the property is backed and only has get, then the property can be set. A no-op setter is implied. (Similar for only having a set hook.)

This has the unfortunate consequence that you actually have to look at the accessor implementation to determine the API of the class – only looking at the “prototypes” (i.e. signatures without implementation bodies) is no longer sufficient! This seems both unfortunate and unprecedented.

This could have been avoided by still requiring an explicit no-op “set;” at the expensive of some additional verbosity.

The other thing that stood out to me are the short-hand notations using =>. There was a prior RFC on the topic (https://wiki.php.net/rfc/short-functions), which has been declined. That RFC would have introduced => … as a general shorthand for { return …; }.

The shorthand notation for get is compatible with that formulation. However, the shorthand notation for set is not. In that case => … isn’t short for { return …; }, but rather for { $this->prop = …; }.

This seems pretty unfortunate to me, and possibly closes the door on revisiting a general short function syntax in the future. Mostly I’m scratching my head at why this was included in the proposal at all, as I would not expect this use of the set hook to be common enough to justify a shorthand. The common case is a guard that checks the value without modifying it.

Putting this to the “would this shorthand have passed if it were introduced by a separate RFC on top of the base implementation” test, I think the answer would have been a clear “no”.

Regards,

Nikita

Personally I still think we should have used TS/JS syntax for computed properties. PHP is much closer to typescript than Java/C# and we already have support for modifiers. Slightly more verbose but these issues would be non-existent.

On Sun, Jun 9, 2024 at 10:03 AM Nikita Popov <php@npopov.com> wrote:

On Mon, Apr 15, 2024, at 18:43, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

https://wiki.php.net/rfc/property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

I know I’m a few months late on this one, but I figured I’d still leave a couple of thoughts… Overall, the proposal is well thought out and does address many of the reservations I had about my original accessors proposal.

One of the more interesting choices in this proposal is to base whether the property is virtual or backed on the presence of a “$this->prop” reference in the accessor implementation. I think that, conceptually at least, this is a good choice.

What I find concerning though, is that the presence/absence of such a reference also affects the meaning of the get and set hooks. If the property is virtual and it only has get, then the property cannot be set. If the property is backed and only has get, then the property can be set. A no-op setter is implied. (Similar for only having a set hook.)

This has the unfortunate consequence that you actually have to look at the accessor implementation to determine the API of the class – only looking at the “prototypes” (i.e. signatures without implementation bodies) is no longer sufficient! This seems both unfortunate and unprecedented.

This could have been avoided by still requiring an explicit no-op “set;” at the expensive of some additional verbosity.

The other thing that stood out to me are the short-hand notations using =>. There was a prior RFC on the topic (https://wiki.php.net/rfc/short-functions), which has been declined. That RFC would have introduced => … as a general shorthand for { return …; }.

The shorthand notation for get is compatible with that formulation. However, the shorthand notation for set is not. In that case => … isn’t short for { return …; }, but rather for { $this->prop = …; }.

This seems pretty unfortunate to me, and possibly closes the door on revisiting a general short function syntax in the future. Mostly I’m scratching my head at why this was included in the proposal at all, as I would not expect this use of the set hook to be common enough to justify a shorthand. The common case is a guard that checks the value without modifying it.

Putting this to the “would this shorthand have passed if it were introduced by a separate RFC on top of the base implementation” test, I think the answer would have been a clear “no”.

Regards,

Nikita

Correct me if I’m wrong, but here’s how I understand the short-hand notation in setter hook in combination with https://wiki.php.net/rfc/short-functions:

final class Foo
{
// Before
public string $x {
set {
$this->x = $this->transformX($value);
}
}

private function transformX(string $value): string => trim($value);

// After
public string $x {
set => trim($value);
}
}

So it’s kinda consistent if you say that only the right side of the property assignment is replaced with short-hand notation, not the whole assignment. And then $this->x = is implicit.

···

Regards,
Valentin

On Mon, Jun 10, 2024, at 12:33 PM, Valentin Udaltsov wrote:

On Sunday, 9 June, 2024 at 19:03, Nikita Popov <php@npopov.com> wrote:

__
On Mon, Apr 15, 2024, at 18:43, Larry Garfield wrote:

The vote for the Property Hooks RFC is now open:

PHP: rfc:property-hooks

Voting will close on Monday 29 April, afternoonish Chicago time.

The other thing that stood out to me are the short-hand notations using =>. There was a prior RFC on the topic (PHP: rfc:short-functions), which has been declined. That RFC would have introduced => ... as a general shorthand for { return ...; }.

The shorthand notation for get is compatible with that formulation. However, the shorthand notation for set is not. In that case => ... isn't short for { return ...; }, but rather for { $this->prop = ...; }.

This seems pretty unfortunate to me, and possibly closes the door on revisiting a general short function syntax in the future. Mostly I'm scratching my head at why this was included in the proposal at all, as I would not expect this use of the set hook to be common enough to justify a shorthand. The common case is a guard that checks the value without modifying it.

I'm a little surprised to hear you say that, since you voted against short-functions at the time. :slight_smile: I would love to try again on those if there's enough change in opinion to make it worthwhile, as I do think it's useful.

The reason we included short-set, aside from consistency, is that people really seemed to want a "return to assign" behavior, and that was the most logical way to do it. It's also more compact for when a set hook is inlined into a promoted constructor property, which I suspect will be not uncommon. For "validate or error" use cases, a ternary isn't ideal but it will get the job done, and there are several examples of that in the RFC.

If we find that "validate but don't modify" is an especially common case, then we should look into a more dedicated syntax for property Constraints / Guards. (I have some ideas there, but nothing ready for public consumption.)

I don't think the current hook syntax would preclude short-functions generally, though. The syntax is the same, the behavior is the same, all that's really different from the typical case is the extra "and the thing it evaluates to gets assigned." Which is exactly what several people asked for. So I don't see how it is really going to cause a conflict.

(Side note: If anyone has warmed to short-functions since it was last brought up let me know. I have no idea how to gauge if the zeitgeist has changed since then to make another attempt worthwhile.)

Regards,
Nikita

Correct me if I'm wrong, but here's how I understand the short-hand
notation in setter hook in combination with
PHP: rfc:short-functions

final class Foo
{
    // Before
    public string $x {
        set {
            $this->x = $this->transformX($value);
        }
    }

    private function transformX(string $value): string => trim($value);

    // After
    public string $x {
        set => trim($value);
    }
}

So it's kinda consistent if you say that only the right side of the
property assignment is replaced with short-hand notation, not the whole
assignment. And then `$this->x =` is implicit.

Essentially that, yes.

--Larry Garfield