Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:125796
Precedence: bulk
MIME-Version: 1.0
References: <CAPyeZ4N8GMiLNVCDGxdofcy6txBhng_i2UKUGpiS5bnZ-YcYvA@mail.gmail.com>
 <2A7CF24F-3AE3-4125-965F-C65431C42DFB@gmail.com> <CAPyeZ4MSc=Sw9UE0soCdBEnVAJ8wXHHhYqm88OffkvNLrHPxFw@mail.gmail.com>
 <e966cf16-794d-4827-9a5e-b0de780a5b91@app.fastmail.com> <30a41608-a1ea-40a9-8d2a-c53c508cd89f@jnvsor.net>
 <a6cdfe39-2a18-40b1-8da5-f49e9e7a7918@app.fastmail.com> <CAMrTa2FM4o6q8TXdBbuHfnD66vtqFBVL2Hw6_412Bf1r2T1QMg@mail.gmail.com>
 <CAPyeZ4M0LDzPBTjT_Qa-ccB73DNatGWdARg-mKMO1T1XmnTzUg@mail.gmail.com>
 <CAMrTa2HMx0ZQJ8OMAcyjkGjQfdbCZ5h5u7seS9gP0pzFNYUCsg@mail.gmail.com>
 <CAPyeZ4O0Dc9ggrhCLBsnihqQcuBTr9EMSAXFBf6N+vwAy49uiw@mail.gmail.com> <c1eb871f-4991-4794-9a29-e8ee244c0d7c@app.fastmail.com>
In-Reply-To: <c1eb871f-4991-4794-9a29-e8ee244c0d7c@app.fastmail.com>
Date: Mon, 14 Oct 2024 06:22:13 +0300
Message-ID: <CAPyeZ4O8KbkHHm77BKjAmWiMZKoafWWNogLUOT1EJ33KbDWUdg@mail.gmail.com>
Subject: Re: [PHP-DEV] Asymmetric visibility is a BC break
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: udaltsov.valentin@gmail.com (Valentin Udaltsov)

On Mon, 14/10/2024=E2=80=AFat 06:01, Larry Garfield <larry@garfieldtech.com=
> wrote:
>
> On Sun, Oct 13, 2024, at 9:37 PM, Valentin Udaltsov wrote:
>
>
> > First of all, I have already agreed above that PHP does not have a BC
> > break here. Now we are discussing the potential problems in the PHP
> > ecosystem and how they could be mitigated.
>
> Ilija and I have discussed this issue a bit.
>
> The first issue is that isPublic() technically means "does this property =
have the public flag set," and nothing more. Prior to 8.1, that implicitly =
also meant "can the property be read and written to from public scope," bec=
ause of how properties worked. (And same for isProtected().) That implicit =
assumption became invalid in 8.1 with readonly, which stealth introduced li=
mited and not fully designed asymmetric visibility as well as properties th=
at could not be set multiple times from any scope. Full aviz in 8.4 doesn't=
 change that. It just makes the previous assumption change more apparent. T=
he fact that no one seems to have reported it as an issue until now suggest=
s it's not a particularly widespread problem.  In practice, if someone is u=
sing reflection to determine the visibility of a property, they'll be writi=
ng to it through reflection as well if at all.
>
> The best solution here is probably to just clarify the docs, which I will=
 do as part of the aviz docs that I have already been working on.  cf: http=
s://github.com/php/doc-en/pull/3828
>
> The second issue is that the behavior of isProtectedSet() / isPrivateSet(=
) was not as clearly defined in the RFC as it should have been. That's on u=
s for not being clearer, as we apologize for the oversight.
>
> Those methods follow the low level pattern of isPublic() , that is, they =
just report of a given flag is set, not what the implications of that flag =
in various contexts are. That is consistent with the rest of the reflection=
 API, so we feel that is best left as-is.
>
> That still means the "so can I  read/write this property or not?" questio=
n has no simple operation for it. Again: it never did, we just kinda sorta =
had it indirectly and implicitly. For that we feel the best answer, as well=
 as least disruptive given we're in RCs, is dedicated methods as Ilija has =
already described that take all property behavior and context into account.=
 (isReadable and isWriteable.)
>
> As a reminder, the concept is:
>
> $rProp->isReadable($obj); // Can this property on $obj be read from the c=
alling scope?
> $rProp->isReadable($obj, 'static'); // Same as previous.
> $rProp->isReadable($obj, null); // Can this property on $obj be read from=
 global scope?
> $rProp->isReadable($obj, Foo::class); // Can this property on $obj be rea=
d from code inside class Foo?
>
> $rProp->isWriteable($obj); // Can this property on object $obj be written=
 from the calling scope?
> $rProp->isWriteable($obj, 'static'); // Same as previous.
> $rProp->isWriteable($obj, null); // Can this property on object $obj be w=
ritten from global scope?
> $rProp->isWriteable($obj, Foo::class); // Can this property on object $ob=
j be written from code inside class Foo?
>
> cf: https://github.com/php/php-src/pull/16209
>
> (The use of null to indicate global scope is borrowed from Closure::bind(=
), which does the same.)
>
> These methods do runtime analysis to see if a property should be readable=
/writeable.  Specifically:
>
> isReadable()
> * Checks that the property is readable from the passed scope
> * Checks that the property is initialized (i.e. not typed and never writt=
en to)
> * Checks that the property is not virtual or has a get hook
> isWritable()
> * Checks that the property is writable (respecting symmetric and asymmetr=
ic properties) from the passed scope
> * Checks that the property is not readonly, is not yet initialized, or is=
 reinitializable (__clone)
> * Checks that the property is not virtual or has a set hook
>
> Of note, this does not absolutely guarantee that a read/write will succee=
d.  There's at least two exceptions: One, some PHP built-in classes have ef=
fectively immutable properties but do not use `readonly` or `private(set)`.=
  Those would not be detected here, until and unless they are updated to us=
e the now-available mechanisms.  (See, eg: https://github.com/php/php-src/i=
ssues/15309)  The other is that a get or set hook may throw an exception un=
der various circumstances.  There is no way to evaluate that via reflection=
, so it's a gap that will necessarily always be there.
>
> Whether those methods are OK to add in the RC phase or if they should be =
left to early 8.5, and if they would need a formal RFC, is up to the RMs to=
 decide. RMs, what is your preference?
>
> --Larry Garfield

Hi, Larry!

Thank you very much for this response. I agree with every point.

I have only one comment about the methods' signatures. They should not
require an object, because properties can be static or the developer
might want to check writable/readable without instantiating an object
(in a code generator, for instance).
Then it makes sense to make `$object` the 2nd parameter. This will
also be consistent with Closure::bind().

isReadable(?string $scope =3D 'static', ?object $object =3D null): bool
isWritable(?string $scope =3D 'static', ?object $object =3D null): bool

--=20
Best regards,
Valentin