> On 28 Feb 2024, at 06:17, Larry Garfield <la...@garfieldtech.com> wrote:
>
> On Sun, Feb 25, 2024, at 10:16 PM, Rowan Tommins [IMSoP] wrote:
>> [Including my full previous reply, since the list and gmail currently
>> aren't being friends. Apologies that this leads to rather a lot of
>> reading in one go...]
>
> Eh, I'd prefer a few big emails that come in slowly to lots of little emails
> that come in fast. :-)
>
>>> On 21/02/2024 18:55, Larry Garfield wrote:
>>>> Hello again, fine Internalians.
>>>>
>>>> After much on-again/off-again work, Ilija and I are back with a more
>>>> polished property access hooks/interface properties RFC.
>>>
>>>
>>> Hello, and a huge thanks to both you and Ilija for the continued work
>>> on this. I'd really like to see this feature make it into PHP, and
>>> agree with a lot of the RFC.
>>>
>>>
>>> My main concern is the proliferation of things that look the same but
>>> act differently, and things that look different but act the same:
>
> *snip*
>
>>> - a and b are both what we might call "traditional" properties, and
>>> equivalent to each other; a uses legacy syntax which we haven't
>>> removed for some reason
>
> I don't know why we haven't removed `var` either. I can't recall the last
> time I saw it in real code. But that's out of scope here.
>
> *snip*
>
>> I think there's some really great functionality in the RFC, and would
>> love for it to succeed in some form, but I think it would benefit from
>> removing some of the "magic".
>>
>>
>> Regards,
>>
>> --
>> Rowan Tommins
>> [IMSoP]
>
>
> I'm going to try and respond to a couple of different points together here,
> including from later in the thread, as it's just easier.
>
> == Re, design philosophy:
>
>> In C#, all "properties" are virtual - as soon as you have any
>> non-default "get", "set" or "init" definition, it's up to you to declare
>> a separate "field" to store the value in. Swift's "computed properties"
>> are similar: if you have a custom getter or setter, there is no backing
>> store; to add behaviour to a "stored property", you use the separate
>> "property observer" hooks.
>>
>> Kotlin's approach is philosophically the opposite: there are no fields,
>> only properties, but properties can access a hidden "backing field" via
>> the special keyword "field". Importantly, omitting the setter doesn't
>> make the property read-only, it implies set(value) { field = value }
>
> A little history here to help clarify how we ended up where we are: The
> original RFC as we designed it modeled very closely on Swift, with 4 hooks.
> Using get/set at all would create a virtual property and you were on your
> own, while the beforeSet/afterSet hooks would not. We ran that design by
> some PHP Foundation sponsors a year ago (I don't actually know who, Roman did
> it for us), and the general feedback was "we like the idea, but woof this is
> complicated with all these hooks and having to make my own backing property
> for all these little things. Couldn't this be simplified?" We thought a bit
> more, and I off-handedly suggested to Ilija "I mean, would it be possible to
> just detect if a get/set hook is using a backing store and make it
> automatically? Then we could get rid of the before/after hooks." He gave it
> a quick try and found that was straightforward, so we pivoted to that
> simplified version. We then realized that we had... mostly just recreated
> Kotlin's design, so shrugged happily and went on with life.
>
> As noted in an earlier email, C#, Kotlin, and Swift all have different
> stances on the variable name for the incoming value. We originally modeled
> on Swift so had that model (optional newVal name), and also because we liked
> how compact it was. When we switched to the simplified, incidentally
> Kotlin-esque approach, we just kept the optional variable as it works.
>
> I think where that ended up is pretty nice, personally, even if it is not a
> direct map of any particular other language.
>
> == Re asymmetric typing:
>
> This is capability already present today if using a setter method.
>
> class Person {
> private $name;
>
> public function setName(UnicodeString|string $name)
> {
> $this->name = $value instanceof UnicodeString ? $value : new
> UnicodeString($value);
> }
> }
>
> And widening the parameter type in a child class is also entirely legal. As
> the goal of the RFC is, essentially, "make most common getter/setter patterns
> easy to add to a property without making an API-breaking method, so people
> don't have to add redundant just-in-case getters and setters all the time,"
> covering an easy-to-cover use case seems like a good thing to do.
>
> It also ties into the question of the explict/implicit name, for the reason
> you mentioned earlier (unspecified means mixed), not by intent. More on that
> in another section.
>
> == Re virtual properties:
>
> Ilija and I talked this through, and there's pros and cons to a `virtual`
> keyword. Ilija also suggested a `backed` keyword, which forces a backed
> property to exist even if it's not used in the hook itself.
>
> * Adding `virtual` adds more work for the developer, but more clarity. It
> would also mean $this->$propName or $this->{__PROPERTY__} would work "as
> expected", since there's no auto-detection for virtual-ness. On the
> downside, if you have a could-be-virtual property but never actually use the
> backing value, you have an extra backing value hanging around in memory that
> is inaccessible normally, but will still show up in some serialization
> formats, which could be unexpected. If you omit one of the hooks and forget
> to mark it virtual, you'll still get the default of the other operation,
> which could be unexpected. (Mostly this would be for a virtual-get that
> accidentally has a default setter because you forgot to mark it `virtual`.)
> * Doing autodetection as now, but with an added "make a backing value anyway"
> flag would resolve the use case of "My set hook just calls a method, and that
> method sets the property, but since the hook doesn't mention the property it
> doesn't get created" problem. It would also allow for $this->$propName to
> work if a property is explicitly backed. On the flipside, it's one more
> thing to think about, and the above example it solves would be trivially
> solved by having the method just return the value to set and letting the set
> hook do the actual write, which is arguably better and more reliable code
> anyway.
> * The status quo (auto-detection based on the presence of $this->propName).
> This has the advantage it "just works" in the 95% case, without having to
> think about anything extra. The downside is it does have some odd edge
> cases, like needing $this->propName to be explicitly used.
>
> I don't think any is an obvious winner. My personal preference would be for
> status quo (auto-detect) or explicit-virtual always. I could probably live
> with either, though I think I'd personally favor status quo. Thoughts from
> others?
>
I agree that a flag to make the field *virtual* (thus disabling the backing
store) makes more sense than a flag to make it backed; It's also easier to
understand when comparing hooked properties with regular properties
(essentially, backed is the default, you have to opt-in to it being virtual). I
don't think the edge cases of "auto" make it worthwhile just to not need
"virtual".
> == Re reference-get
>
> Allowing backed properties to have a reference return creates a situation
> where any writes would then bypass the set hook, and thus any validation
> implemented there. That is, it makes the validation unreliable. A major
> footgun. The question is, do we favor caveat-emptor flexibility or
> correct-by-construction safety? Personally I always lead toward the latter,
> though PHP in general is... schizophrenic about it, I'd say. :-)
>
> At this point, we'd much rather leave it blocked to avoid the issue; it's
> easier to enable that loophole in the future if we really want it than to get
> rid of it if it turns out to have been a bad idea.
>
> There is one edge case that *might* make sense: If there is no set hook
> defined, then there's no set hook to worry about bypassing. So it may be
> safe to allow &get on backed properties IFF there is no set hook. I worry
> that is "one more quirky edge case", though, so as above it may be better to
> skip for now as it's easier to add later than remove. But if the consensus
> is to do that, we're open to it. (Question for everyone.)
>
I don't have strong feeling about this, but in general I usually tend to prefer
options that are consistent, and give power/options to the developer. If
references are opt-in anyway, I see that as accepting the trade-offs. If a
developer doesn't want to allow by-ref modifications of the property, why would
they make it referenceable in the first place? This sounds a bit like
disallowing regular public properties because they might be modified outside
the class - that's kind of the point, surely.
> == Re
>
> == Re arrays
>
>> Regarding arrays, have you considered allowing array-index writes if
> an &get hook is defined? i.e. "$x->foo['bar'] = 42;" could be treated
> as semantically equivalent to "$_temp =& $x->foo; $_temp['bar'] = 42;
> unset($_temp);"
>
> That's already discussed in the RFC:
>
>> The simplest approach would be to copy the array, modify it accordingly, and
>> pass it to set hook. This would have a large and obscure performance
>> penalty, and a set implementation would have no way of knowing which values
>> had changed, leading to, for instance, code needing to revalidate all
>> elements even if only one has changed.
>
> Unless we were OK with that bypassing the set hook entirely if defined,
> which, as noted above, means any safety guarantees provided by a set hook are
> bypassed, leading to untrustworthy code.
>
> == Re hook shorthands and return values
>
> Ilija and I have been discussing this for a bit, and we've both budged a
> little. :-) Here's our counter-proposal:
>
> - Drop the "top level" shorthand, for get-only hooks.
> - Keep the => shorthand for the get hook itself.
> - For a set hook, the {} form has no return value; set the value yourself
> however you want.
> - For a set hook, the => form implies a backed value and will set the
> property to whatever value that evaluates to.
>
> So these are equivalent:
>
> public $foo { set { $this->foo = $value; } }
> public $foo { set => $value; }
>
> These are equivalent:
>
> public string $foo {
> get {
> return strtoupper($this->foo);
> }
> }
> public string $foo { get => strtoupper($this->foo); }
>
> And this goes away:
>
> public string $foo => strtoupper($this->foo);
>
> That covers the common cases with an arrow-function-like syntax that behaves
> as you'd expect (it returns things), and allows a longer version with
> arbitrarily complex logic if desired. It also means that each syntax variant
> does mean something importantly different.
>
> Would that be an acceptable compromise? (Question for everyone.)
>
I think the examples given are clear, and the lack of the top-level short
closure-esque version makes it more obvious. Forgive me, I must have missed
some of the previous comments - is there a reason the 'full' setter can't
return a value, for the sake of consistency? I understand that you don't want
"return to set" to be the *only* option, for the sake of e.g. change/audit
logging type functionality (i.e. set and then some action to record that the
change was made), but it seems a little odd and inconsistent to me that the
return value of a short closure would be used when the return value of the long
version isn't. This isn't really a major issue, I'm just curious if there was
some explanation about it?
> == Re the $value variable in set
>
> Honestly, Rowan's earlier point here is the strongest argument in favor for
> me of the current RFC approach. Anywhere else in PHP, something that looks
> like a parameter and has no type, like `($param)`, means its type is `mixed`.
> It would be weird and confusing to be different here. That's above and
> beyond the issue of forcing people to retype something obvious every time.
> (I cite again, recent PHP's trend toward removing needless boilerplate, which
> is very good.) Requiring that the type be specified, for consistency, makes
> little sense if the type is not allowed to vary. You're just repeating a
> string from earlier on the same line, for no particular benefit.
>
> I genuinely don't understand the pushback on $value. It's something you
> learn once and never have to think about again. It's consistent.
>
> Ilija jokingly suggested making it always $value, unconditionally, and
> allowing only the type to be specified if widening:
>
> public int $foo { set(int|float) => floor($value); }
>
> Though I suspect that won't go over well, either. :-)
>
> So what makes the most sense to me is to keep $value optional, but IF you
> specify an alternate name, you must also specify a type (which may be wider).
> So these are equivalent:
>
> public int $foo { set (int $value) => $value + 1 }
> public int $foo { set => $value + 1 }
>
> And only those forms are legal. But you could also do this, if the situation
> called for it:
>
> public int $foo { set(int|float $num) => floor($num) + 1; }
>
> This "all or nothing" approach seems like it strikes the best balance, gives
> the most flexibility where needed while still having the least redundancy
> when not needed, and when a name/type is provided, its behavior is the same
> as for a method being inherited.
>
> Does that sound acceptable? (Again, question for everyone.)
>
My only question with this is the same as I had in an earlier reply (and I'm
not sure it was ever answered directly?), and you allude to this yourself:
everywhere *else*, `($var)` means a parameter with type `mixed`. Why is the
type *required* here, when you've specifically said you want to avoid
boilerplate? If we're going to assume people can understand that `(implicit
property-type $value) is implicit, surely we can also assume that they will
understand "specifying a parameter without a type" means the parameter has no
type (i.e. is `mixed`).
Again, for myself I'd be likely to type it (or regular parameters, properties,
etc) as `mixed` if that's what I want *anyway*, but the inconsistency here
seems odd, unless there's some until-now unknown drive to deprecate type-less
parameters/properties/etc.
> The alternative that gives the most future-flexibility is to do neither: The
> variable is called $value, period, you can't change it, and you can't change
> the type, either. There is no () after set, ever. Punt both of those to a
> later follow-up. I'd prefer to include both now, but including neither now
> is the next-safer option.
>
>
> ## Regarding $field
>
> Sigh, now y'all like it. :-P Most of the feedback on this has been negative,
> so I'm inclined to leave it out at this point, unless there's a major swing
> in feedback to bring it back. But the RFC seems more likely to pass without
> it than with right now.
>
> --Larry Garfield
>
Cheers
Stephen
