[RFC[ Property accessor hooks, take 2

> On 23 Feb 2024, at 06:56, Larry Garfield <larry@garfieldtech.com> wrote: > > On Wed, Feb 21, 2024, at 11:02 PM, Matthew Weier O'Phinney wrote: >> On Wed, Feb 21, 2024 at 12:57 PM Larry Garfield <larry@garfieldtech.com> wrote: >>> After much on-again/off-again work, Ilija and I are back with a more polished property access hooks/interface properties RFC. It’s 99% unchanged from last summer; the PR is now essentially complete and more robust, and we were able to squish the last remaining edge cases. >>> >>> Baring any major changes, we plan to bring this to a vote in mid-March. >>> >>> https://wiki.php.net/rfc/property-hooks >>> >>> It’s long, but that’s because we’re handling every edge case we could think of. Properties involve dealing with both references and inheritance, both of which have complex implications. We believe we’ve identified the most logical handling for all cases, though. >> >> Once again in reading the proposal, the first thing I'm struck by are >> the magic "$field" and "$value" variables inside accessors. The first >> time they are used, they're used without explanation, and they're >> jarring. >> >> Additionally, once you start defining the behavior of accessors... you >> don't start with the basics, but instead jump into some of the more >> esoteric usage, which does nothing to help with the questions I have. >> >> So, first: >> >> - Start with the most basic, most expected usage for each of reading >> and writing properties. >> - I need a better argument for why the $field and $value variables >> exist. Saying they're macros doesn't help those not deep into >> internals. As a user, why do they exist? > > For $field, it's not a requirement. It's mostly for copy-paste convenience. A number of people have struggled on this point so if the consensus is to leave out $field and just use $this->propName directly, we can accept that. They can be re-added if reusable hook packages are added in the future (as noted in Future Scope). > > For $value, it's to avoid boilerplate. For the majority case, you'll be just operating on an individual value trivially. Checking it's range, or uppercasing it, or whatever. Requiring the developer to provide a name explicitly is just extra work; it's much the same as how PHP doesn't require you to pass $this as the first argument to a method explicitly, the way Python and Rust do. It's just understood that $this exists, and once you learn that it's obvious. > > On the occasions where you do want to specify an alternate name for some reason, or more likely you're providing a wider type, you can. But in the typical case it would just be one more thing for the dev to have to type out. This is especially true in what I expect to be a common case, which is promoted constructor arguments with an extra validator set hook on them. > > It also introduces some ambiguity. If I specify only the name, does that mean I'm widening the type to mixed? Or just that I'm omitting the name? If specifying the name is rare, that's not really a big deal. If it's required in every case, it's a confusion point in every case. > > In the interest of transparency. for comparison: > > * Kotlin appears to require an argument name, but by convention recommends using "value". > * Swift makes it optional, with a default name of "newValue". (Same logic as in the RFC.) > * C# ... as far as I can tell, doesn't support a custom name at all; it's always called "value", implicitly. > >> Second: you don't have examples of defining BOTH get and set OTHER than >> when using expressions for both accessors or a mix. I'm actually >> unclear what the syntax is when both are defined. Is there supposed to >> be a `;` terminating each? Or a `,`? Or just an empty line? Again, >> this is one of the more common scenarios. It needs to be covered early, >> and clearly. > > ... huh. I thought we had one in there somewhere. I will add one, thanks. Though to clarify, there's no separator character. > > public string $foo { > get { > // ... > } > set { > // ... > } > } > >> Third: the caveats around usage with arrays... give me pause. While I'm >> personally trying to not use arrays as much as possible, a lot of code >> I see or contribute to still does, and the fact that an array property >> that uses a write accessor doesn't allow the same level of access as a >> normal array property is something I see leading to a lot of confusion >> and errors. I don't have a solution, but I worry that this one thing >> alone could be enough to prevent the passage of the RFC. > > We completely agree that it's a suboptimal situation. But as explained, it is the way it is because it's not possible (as far as we can tell) to fully support hooks on array properties. If you can think of one, please share, because we'd love to make this part better. I don't like it either, but we haven't found a way around it. And that caveat doesn't seem like a good enough reason to not support hooks everywhere we actually can. > > >> Fourth: the syntax around inheritance is not intuitive, as it does not >> work in the same way as the rest of the language. I'm talking about >> this: >> >> public int $x { >> get: 2 * parent::$x::get() >> } >> >> Why do we need to use the accessors here? Why wouldn't it just be `parent::$x`? > > Almost. Ilija spent some time looking into this, and it's possible with caveats. > > First, there's then no way to differentiate between "access parent hook" and "read the static property $x on the parent". Arguably that's not a common case, but it is a point of confusion. > > The larger issue is that parent::$x can't be just a stand-in for the backing value in all cases. While supporting that for the = operator is straightforward enough, it wouldn't give us access to ++, --, <=, and the dozen or so other operators that could conceivably apply. In theory those could all be implemented manually, but Ilija described that as "hundreds to thousands of lines of code" to do, which... is not time or code well spent. :-) Especially as this is a very edge-case situation to begin with. (In practice, I expect auto-generated ORM proxy code to be the primary user of accessing a parent property from a child hook. I can think of very few other cases where I'd want to use it.) > > So we have the choice between making $a = parent::$prop and parent::$prop = $a work, but *nothing else*, inexplicably (creating confusion) or the slightly longer syntax that wouldn't support those other operations anyway so there's no confusion. > > We feel the current approach is the better trade off, but if the consensus generally is for the shorter-but-inconsistent syntax, that can be changed. > >> I want to be clear: I really like the idea behind this feature, and >> overall, I appreciate the design. From a user perspective, though, the >> above are things that I found jarring as they vary quite a bit from our >> current language design. >>> >>> Note the FAQ question at the end, which explains some design choices. >>> >>> There’s one outstanding question, which is slightly painful to ask: Originally, this RFC was called “property accessors,” which is the terminology used by most languages. During early development, when we had 4 accessors like Swift, we changed the name to “hooks” to better indicate that one was “hooking into” the property lifecycle. However, later refinement brought it back down to 2 operations, get and set. That makes the “hooks” name less applicable, and inconsistent with what other languages call it. >>> >>> However, changing it back at this point would be a non-small amount of grunt work. There would be no functional changes from doing so, but it’s lots of renaming things both in the PR and the RFC. We are willing to do so if the consensus is that it would be beneficial, but want to ask before putting in the effort. >> >> I personally would go with just "accessors". > > On Thu, Feb 22, 2024, at 12:06 AM, Deleu wrote: > >> This is a long, huge and comprehensive work, congratz to the authors. >> >> It clearly shows that so much thought and work has been put into it >> that it makes me cautious to even ask for further clarification. >> >>> Javascript have similar features via a different syntax, although that syntax would not be viable for PHP >> >> Why not? > >> It feels quite a natural syntax for PHP and from someone oblivious to >> the internal work, it appears to be a slight marginal change to the >> existing RFC. Given the extensive work of this RFC, it seems pretty >> obvious that this syntax will not work, I just don't know why. > > I've added an FAQ section explaining why the Python/JS approach wouldn't really work. To be clear, Ilija and I spent 100+ hours doing research and design before we started implementation (back in mid-late 2022). We did seriously consider the JS-style syntax, but in the end we found it created more problems than it solves. For the type of language PHP is (explicit typed properties), doing it on the property itself is a much cleaner approach. > > On Thu, Feb 22, 2024, at 1:14 AM, Robert Landers wrote: > > >> I apologize if this has already been covered: >> >>> There are two shorthand notations supported, beyond the optional argument to set. >>> First, if a hook's body is a single expression, then the { } and return statement may be omitted and replaced with =>, just like with arrow functions. >> >> Does => do any special auto-capturing of variables like arrow >> functions or is it just a shorthand? > > No, there is nothing to capture. Inside the hook body you have $this the same as any other method. > >> Also, is this a meaningful >> shorthand to the example a little further down: >> >> public string $phone { >> set = $this->sanitizePhone(...); >> } >> >> or do we always have to write it out? >> >> public string $phone { >> set => $field = $this->sanitizePhone($value); >> } > > Currently no, the set is always void; you have to write the value yourself. See the FAQ section on the topic for a detailed explanation. > > However, I just had a long discussion with Ilija and there is one possibility we could consider: Use the return value only on the shorthand (arrow-function-like) syntax. > > So you could do either of these, which would be equivalent: > > set { > $this->phone = $this->santizePhone($value); > } > > set => $this->santizePhone($value); > > This would have the advantage of offering a return-to-set mechanism, as well as even shorter syntax in the simple case (and no question of $field vs $this->propName). But it would have the disadvantage of being potentially inconsistent between the short and long version. It also would mean the short version is incompatible with virtual properties; using a short-set would create a backing value, so it's non-virtual. But since "simple validation, maybe in a promoted constructor property" is likely to be one of the main use cases, it would simplify that case. > > Not sure if that's a trade up or not. Thoughts from the list? > >> Would __PROPERTY__ be set inside sanitizePhone() as well? > > No. Like __CLASS__, it is materialized at compile time and has no meaning outside of its intended scope. > >> You mention several ways values are displayed (whether or not they use >> the get-hook), but what does the default implementation of >> `__debugInfo()` look like now (or is that out of scope or a silly >> question?) > > var_dump() shows the underlying backing value, bypassing get, since it's debugging the object state. If you implement __debugInfo(), that's a method like any other and you can do what you like, though you'll be reading through get hooks (just like using __serialize()). > >> For attributes, it would be nice to be able to target hooks >> specifically with attributes instead of also all methods (e.g., a >> Attribute::TARGET_GET_HOOK const). For example, if I were writing a >> serialization library, I may want to specify #[UseRawValue] only on >> getters to ensure that only the raw value is serialized instead of the >> getter (which may be specific to the application logic, or >> #[GetFromMethod] to tell the serialization library to get the value >> from a completely different method. It wouldn't make sense to target >> just any method with that attribute. > > This feels very niche, honestly. Those would naturally have to be sub-cases of TARGET_METHOD anyway, so method-targeted attributes would need to be supported regardless. That makes hook-specific-targeting an easy and non-breaking add-on for a future RFC if it turns out to be useful in practice. > > --Larry Garfield

> On Wed, Feb 21, 2024, at 11:02 PM, Matthew Weier O'Phinney wrote: >> On Wed, Feb 21, 2024 at 12:57 PM Larry Garfield <larry@garfieldtech.com> wrote: >>> After much on-again/off-again work, Ilija and I are back with a more polished property access hooks/interface properties RFC. It’s 99% unchanged from last summer; the PR is now essentially complete and more robust, and we were able to squish the last remaining edge cases. >>> >>> Baring any major changes, we plan to bring this to a vote in mid-March. >>> >>> https://wiki.php.net/rfc/property-hooks >>> >>> It’s long, but that’s because we’re handling every edge case we could think of. Properties involve dealing with both references and inheritance, both of which have complex implications. We believe we’ve identified the most logical handling for all cases, though. >> Once again in reading the proposal, the first thing I'm struck by are >> the magic "$field" and "$value" variables inside accessors. The first >> time they are used, they're used without explanation, and they're >> jarring. >> >> Additionally, once you start defining the behavior of accessors... you >> don't start with the basics, but instead jump into some of the more >> esoteric usage, which does nothing to help with the questions I have. >> >> So, first: >> >> - Start with the most basic, most expected usage for each of reading >> and writing properties. >> - I need a better argument for why the $field and $value variables >> exist. Saying they're macros doesn't help those not deep into >> internals. As a user, why do they exist? > For $field, it's not a requirement. It's mostly for copy-paste convenience. A number of people have struggled on this point so if the consensus is to leave out $field and just use $this->propName directly, we can accept that. They can be re-added if reusable hook packages are added in the future (as noted in Future Scope). >

> However, I just had a long discussion with Ilija and there is one > possibility we could consider: Use the return value only on the > shorthand (arrow-function-like) syntax. > > So you could do either of these, which would be equivalent: > > set { > $this->phone = $this->santizePhone($value); > } > > set => $this->santizePhone($value);

> > > I've added an FAQ section explaining why the Python/JS approach wouldn't really work. To be clear, Ilija and I spent 100+ hours doing research and design before we started implementation (back in mid-late 2022). We did seriously consider the JS-style syntax, but in the end we found it created more problems than it solves. For the type of language PHP is (explicit typed properties), doing it on the property itself is a much cleaner approach.

> What is the property type of the $firstName property?

> but there's nothing inherent that forces, public string $firstName, get firstName()s return and set firstName()s parameter to be the same. Even if we could detect it as a compile error, it means one more thing that the developer has to keep track of and get right, in three different places.

> What about visibility? Do the get/set methods need to have the same visibility as the property?

> If not, does that become a way to do asymmetric visibility?

> What about inconsistency between the method's visibility and the property visibility? How is that handled?

> How do you differentiate between virtual and non-virtual properties?

> For non-virtual properties, if you need to triple-enter everything, we're back to constructors pre-promotion. Plus, the accessor methods could be anywhere in the class, potentially hundreds of lines away. That means just looking at the property declaration doesn't tell you what its logic is; the logic may be on line 960, which only makes keeping its type/visibility in sync with the property harder.

> Hi Larry, > > pt., 8 mar 2024 o 16:55 Larry Garfield <larry@garfieldtech.com> napisał(a): >> Hi folks. Based on earlier discussions, we've made a number of changes to the RFC that should address some of the concerns people raised. We also had some very fruitful discussions off-list with several developers from the Foundation, which led to what we feel are some solid improvements. >> >> https://wiki.php.net/rfc/property-hooks >> > > This RFC looks awesome, thanks Larry and Ilija I love the functionality > in its current shape. > >> Thank you everyone for the feedback so far, and if you still have some, please say so. (Even if it's just to say that you're happy with the RFC now so we feel more comfortable bringing it to a vote.) > > The only thing I don't like and can still be worked on is the > reflection mechanism changes. > The proposed methods isVirtual and getRawValue, setRawValue pair > introduces a need to catch exceptions which could be eliminated by > subtyping ReflectionProperty. > Having these methods on a separate subtype allows returning a valid > value. > I realize this isn't trivial because for the last 2 days, I was > thinking about giving it a name and TBH cannot figure out anything > feasible. > If this is not possible to put in understandable words then at least > mention it in FAQ and why not. > > Cheers, > Michał Marcin Brzuchalski

> Hi Larry > > Thanks again for both of your work on this, I'm really hopeful this passes. > > Was there ever any further discussion/resolution/decision about the use > an explicit `virtual` keyword, and the related flag for creation of a > backing store? I thought it was discussed by several people but I don't > recall seeing any eventual consensus, and it looks to my eye that it > hasn't changed from the original proposal: i.e. it's 'magic' and > `$this->{__PROPERTY__}` won't work? > > Is that correct?

>> if a `get` hook for property `$foo` calls method `bar()`, then inside that method `$this->foo` will refer to the raw property, both read and write. If `bar()` is called from somewhere other than the hook, reading from `$this->foo` will trigger the `get` hook. This behavior is identical to that already used by `__get` and `__set` today. > > > > I'm slightly confused by this. > > If there is an actual property called $foo, then __get and __set will > be called only when it is out of visibility, regardless of the call > stack - e.g. a private property will always trigger __get from public > scope, and always access it directly from private scope: > https://3v4l.org/R5Yos That seems differ from what's proposed, where > even a private call to bar() would trigger the hook. > > The protection against recursion appears to only be relevant for > completely undefined properties. For __get, the direct access can never > do anything useful - there's nothing to access: https://3v4l.org/2nDZS > For __set, it is at least possible for the non-recursive write to > succeed, but only in the niche case of creating a dynamic property: > https://3v4l.org/dpYOj I'm not sure that there's any equivalent to this > scenario for property hooks, since they can never be undefined/dynamic.

>> There is one exception to the above: if a property is virtual, then there is no presumed connection between the get and set operations. [...] For that reason, `&get` by reference is allowed for virtual properties, regardless of whether or not there is a `set` hook. > > > I don't agree with this, and the example immediately following it > demonstrates the exact opposite: the &get and set hooks are both > proxying to the same backing value, and have all the same problems as > if the property was non-virtual. I would imagine a lot of real-life > virtual properties would be doing something similar: converting to/from > a different type, proxying to another object, etc. > > I think this exception is unnecessarily complicated: either trust users > to handle the implications of combining &get with set, or forbid it.

>> Additionally, `&get` hooks are allowed for arrays as well, provided there is no `set` hook. > > > I mentioned in a previous e-mail the possibility of using the &get hook > for array writes. Has this been considered? > > That is: > > $c->arr['beep'] = 'boop'; > > Would be equivalent to: > > $temp =& $c->arr; > $temp['beep'] = 'boop'; > unset($temp); > > Which would be valid if $arr had an &get hook defined.

>> A `set` hook on a typed property must declare a parameter type that is the same as or contravariant (wider) from the type of the property. > >> Once a property has both a `get` and `set` operation, however, it is no longer covariant or contravariant for further extension. > > > > How do these two rules interact? > > Could this: > > public string $foo { > get => $this->_foo; > set(string|Stringable $value) { > $this->_foo = (string)$value; > } > } > > be over-ridden by this, where the property's "main type" remains > invariant but its "settable type" is contravariant? > > public string $foo { > get => $this->_foo; > set(string|Stringable|SomethingElse $value) { > $this->_foo = $value instanceof SomethingElse ? > $value->asString() : (string)$value; > } > }

>> ReflectionProperty has several new methods to work with hooks. > > > There should be some way to reliably determine the "settable type" of a > property. At the moment, I think you would have to do something like > this: > > $setHook = $property->getHook(PropertyHookType::Set); > $writeType = $setHook === null ? $property->getType() : > $setHook->getParameters()[0]->getType();

> On Fri, 8 Mar 2024, Larry Garfield wrote: > >> Hi folks. Based on earlier discussions, we've made a number of >> changes to the RFC that should address some of the concerns people >> raised. We also had some very fruitful discussions off-list with >> several developers from the Foundation, which led to what we feel are >> some solid improvements. >> >> https://wiki.php.net/rfc/property-hooks > > Some comments and questions: > > Be aware, the detection logic works on $this->[propertyName] directly at > compile time, not on dynamic forms of it like $prop = 'beep'; > $this->$prop. That will not trigger a backing value. > > How can that not cause issues?

> The set hook's return type is unspecified, and will silently be > treated as void. > > What happens if you *do* specify a return type? Will it Error?

> Implicit ''set'' parameter > > If the write-type of a property is the same as its defined type > (this is the common case), then the argument may be omitted > entirely. > > … > > If the parameter is not specified, it defaults to $value. > > I am not a fan of this "magical" behaviour. Do we *need* this short cut, > and the following "Short-set"?

> With asymmetric visibility that was previously proposed, the > example can be further simplified. > > But it isn't here, so why is this example (and the next one) in the RFC? > :-)

> Interaction with constructor property promotion > > … In particular, the shorthand version of hook bodies and the > ability to call out to private methods if they get complicated > partially obviate the concern about syntactic complexity. > > Although that is true, it *does* add more complexity in tools that needs > to parse PHP, as there is now another piece of new syntax that needs to > be added (and tested with).

> ReflectionProperty has several new methods to work with hooks. > > getHooks(): array returns an array of \ReflectionMethod objects > keyed by the hook they are for. > > What will the name for the &get hook be? And shouldn't there be an enum > case for that as well?