Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118360
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-758-ge0d20a54e1-fm-20220729.001-ge0d20a54
Mime-Version: 1.0
Message-ID: <0923bd88-8e62-4386-bf6b-4ef9456780b4@www.fastmail.com>
In-Reply-To: 
 <CAJp_myVOPbe9B3YPNtNC79Avj1BN4d5eSqXKomiDVOutd00w9g@mail.gmail.com>
References: <ec4bc6e5-39ac-4a36-9d63-cca8dc6a29a6@www.fastmail.com>
 <CAJp_myVOPbe9B3YPNtNC79Avj1BN4d5eSqXKomiDVOutd00w9g@mail.gmail.com>
Date: Fri, 05 Aug 2022 15:25:03 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] [RFC] Asymmetric visibility
From: larry@garfieldtech.com ("Larry Garfield")

On Fri, Aug 5, 2022, at 1:08 PM, Matthew Weier O'Phinney wrote:
> On Fri, Aug 5, 2022, 12:09 PM Larry Garfield <larry@garfieldtech.com> wrote:
>
>> Ilija Tovilo and I are happy to present the first new RFC for PHP 8.3:
>> Asymmetric Visibility.
>>
>> https://wiki.php.net/rfc/asymmetric-visibility
>>
>> Details are in the RFC, but it's largely a copy of Swift's support for the
>> same.
>>
>
> I have two comments:
>
> - For reflection purposes, having two separate methods feels like it will
> be cumbersome; you'd need to check both to determine if you'd need to make
> the reflection property accessible before changing the value. An
> isPublicSet() would alleviate that, or potentially a getSetFlags() method
> against which you could apply a bit mask. You'd need to add a constant for
> public set I both cases.

Reflection is a mess, unfortunately. :-(

There's already separate isPrivate(), isProtected(), and isPublic() methods on ReflectionProperty, so this is consistent.  Also, there's getModifiers() which returns a bitmask; these new flags would be included there.  All these methods all do is check what is syntactically present, both the current ones and proposed ones.

There's definitely room in the Reflection API for a "couldItBeModifiedFromScope($scope)" type method (and similar) that aggregates together symmetric visibility, readonly, asymmetric visibility, etc. into nice booleans.  However, that is notably more work than just adding more syntax flag methods.  It would probably be better as its own RFC, though I'd support that.

If there's consensus that it has to be done here that could be done, but we'd prefer not as it feels like scope creep.

> - The number of items that appear to the left of a property is growing,
> making understanding a declaration increasingly difficult, and your
> discussion of future scope indicates that this is likely to get worse. I'm
> wondering if this sort of behavior could be indicated via attributes
> instead? Something like `#[PropertySetBehavior(PROPERTY_SET_PRIVATE)]`.
> Attributes have the benefit of being separate from the property
> declaration, arguably more readable (one per line), and composable. This
> might play into some of the future scope items as well.

The list indeed groweth, but there's a couple of reasons it wouldn't be appropriate here to use an attribute.

* It would mean get-visibility is a keyword and set-visibility is an attribute.  That's weird and inconsistent.
* There's a strong sense from many on the list that attributes should not be used for language behavior.  Ilija agrees with that position, and I don't care enough to fight it one way or the other.
* An attribute would be ignored in older PHP versions.  For most attributes that's OK, but in this case it would be a potential bug source.  Specifically, it would mean `#[PrivateSet] \n public string $value` would be publicly settable on PHP 8.2, but not publicly settable in 8.3.  That means the integrity of the object is compromised on older versions.  A keyword would make that a syntax error, which in this case is preferable.

> Overall, though, love the design simplicity!

Yay!

--Larry Garfield