Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:125773 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by qa.php.net (Postfix) with ESMTPS id E1A371A00BD for ; Wed, 9 Oct 2024 16:08:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1728490252; bh=xbUlSFIP2stOeSJ6Sj0t6JCQ8XKypSBV8lkdP5g2Vns=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=cFwKsrDO0fKBauSZhLw1HsJFvw+8703YZZODOLL/GbWbsLtAk3OAfOkZ6qXDMc67z rgYV61yAJKXY7N7TynvJhdyt3lVJmfW4ta3QdUMpmD5l61l95tDdfoZLVJ3XjjiRJl ThRd3RzcKpEE0TlqfnYBqDprQ6yeVwr1NBIrx1N9r3QBDosxySXUYzy8DYzWi9CNe0 t57LerHjdIG8z4CNAni6lIp0PAs/uHkkMGsi5FrXRHLwbZ68CZkuByG8NhFMIGxn2i YYAymZaF0y2KpGo8B7hr55qEjZfTu2lNJDNvXtHJhJ1j52ip5Ix7ToMpAMTDADpUo3 CfvmXQXJtRflw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 3C095180069 for ; Wed, 9 Oct 2024 16:10:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=0.6 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_PASS,FREEMAIL_FROM, HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: No X-Envelope-From: Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Wed, 9 Oct 2024 16:10:51 +0000 (UTC) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2fadc95ccfcso79012941fa.1 for ; Wed, 09 Oct 2024 09:08:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728490113; x=1729094913; darn=lists.php.net; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=oussS9MLE8WCp/4kPa9LurqKHAJvciXq9n/HJWQQvxQ=; b=IkMBp94GJdExsrf4RMjzx1DsOMKzyrKoUnkeOmaS+xdoIiibpgjqHFTsdM3T03FWUk AXWqBZuo2/0SqZyYYIGlSZcw+FpCxZQokzCWfhJ1AWcghGpZqNjBE8S61radWxabg6TL DWifsuStPFdMLsfVpiz3KxLDZO4OZbTaHSQCT74WdEmhvtxW1cTnHSxz5SEDf4c3lLB1 jOurn2pJ2IJRTbMjkFE2koyIjnn+NbJU/6Y7By52njwpLETNhVZF2VA3lvDEJEvD/NQu 5nLWhbnqhwAg5cIuLsXdhst2Il81lPxEVsnS/ZkXTsoGU4MWzBX5dqwLPq+590MStO/W fRvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728490113; x=1729094913; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oussS9MLE8WCp/4kPa9LurqKHAJvciXq9n/HJWQQvxQ=; b=dzlrJLP182uHIDo0A50/K2j6cqWm8yvn+ynO48uYfVYeWFBMmbwCFigSrseYAKTCsO 1DIYMHiS79vin1EKnG6gZMJeBVAx30MNG39Nbsm+hSnLa0zWAFrByhmGGwX44/YnOQjs XJOK69m0G11YU2B6rNUp2bkb3By+swxCXHfPghoFje+G6HCI7RnlXhEoN0CmZmeC1ip+ /xvIctsqVkdJDz9RlMOvblgkd+wlceOEt0TDzeVAxkP8qbB3tIdhJo5D/804UO92sc1v cBuZ+UDpcNUeInChbH5o5B12lRgjqkek/sazj8OUPGDI06Sdd1y6hvhx0O0Z++Xvi6bE gXow== X-Gm-Message-State: AOJu0YwSfIZ1cdahZPLSqAQNR1gGBnSVvjp9YhXq+vZZIgqhenKV2lcJ jvkNQIzrqwdn0+w1/4lZsWDKHrkR1Tkk+c/Ep0iBGtSy0lZLZZ+dGPuu3L6siaGfyFwpcvZqnw3 Zq7ms1QC9fjqu1CmEZBGq1A30/3o= X-Google-Smtp-Source: AGHT+IEsLTEcNVCnmSnvhCcUtsrftL3Bv0GMkhoU4RZ51wzQ9N/emurGQaSgRbqSelQK5YeveG+0HFoga4b4gEOhdWs= X-Received: by 2002:a05:651c:503:b0:2fa:dce8:7387 with SMTP id 38308e7fff4ca-2fb187cd8b8mr20825801fa.32.1728490112516; Wed, 09 Oct 2024 09:08:32 -0700 (PDT) Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 References: In-Reply-To: Date: Wed, 9 Oct 2024 10:08:21 -0600 Message-ID: Subject: Re: [PHP-DEV] Asymmetric visibility is a BC break To: Valentin Udaltsov Cc: php internals Content-Type: multipart/alternative; boundary="000000000000278ca306240d776e" From: hamiegold@gmail.com (Hammed Ajao) --000000000000278ca306240d776e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Oct 9, 2024, 9:02=E2=80=AFa.m. Valentin Udaltsov < udaltsov.valentin@gmail.com> wrote: > Hi, internals! > > Since writing https://externals.io/message/125740 I've realized that > the major problem with aviz is actually simple but fundamental. > > In PHP <=3D8.0 this code is valid for an object of any user class: > > ```php > class User > { > public string $name =3D 'Bob'; > } > > $object =3D new User(); > > foreach ($object as $property =3D> $value) { > $object->{$property} =3D 'Jack'; > } > ``` > > The same is true for Reflection API: once you check that `(new > ReflectionProperty(Foo::class, 'property'))->isPublic()`, you can > safely read property and write to it from any scope. > > Now let's jump to PHP 8.1+ with support for readonly properties. > > A readonly property is two functionalities in one: write-once and > private set. This means that `public readonly $property` is actually > `public(get) private(set) readonly $property`. Although it is marked > as `public`, it is not public because it is not a symmetric public > property! > > In PHP 8.1+, the following User class suddenly breaks the code above: > > ```php > class User > { > public function __construct( > public readonly string $name =3D 'Bob', > ) {} > } > > $object =3D new User(); > > foreach ($object as $property =3D> $value) { > // Fatal error: Uncaught Error: Cannot modify readonly property > User::$name > $object->{$property} =3D 'Jack'; > } > ``` > > In other words, the meaning of `public` has changed in PHP 8.1. > Before, it used to mean "symmetric", now it means "symmetric unless > readonly". > > While not explicitly stated in changelogs, this was a BC break, > because a changed semantic of smth that existed before is a BC break. > > Did it break anything? Of course it did! See: > - https://github.com/doctrine/orm/issues/10049 > - https://github.com/symfony/symfony/pull/46840 > - https://github.com/Ocramius/GeneratedHydrator/issues/656 > - https://github.com/opis/closure/issues/129 > > I believe there are still many places where the concept of "public" > needs to be adjusted to fully support readonly properties. > > Now in PHP 8.4 asymmetry will be made explicit and will allow users to > specify visibility for setters. However, the core issue remains > unresolved: > > ```php > final class User > { > public function __construct( > public private(set) string $name =3D 'Bob', > ) {} > } > > $object =3D new User(); > > foreach ($object as $property =3D> $value) { > // Fatal error: Uncaught Error: Cannot modify private(set) > property User::$name from global scope > $object->{$property} =3D 'Jack'; > } > ``` > > I'd like to draw your attention to the fact that aviz introduces a BC > break, despite saying "Backward Incompatible Changes: None. This > syntax would have been a parse error before." While the syntax is new, > it allows one to alter the old concept of public by changing set > visibility. > > What can we do about it: > 1. Explicitly introduce the concept of getter and setter visibility, > preserve `ReflectionProperty::isPublic()` behavior from PHP <=3D8.0 and > add `ReflectionProperty::(get|set)Is(Public|Protected|Private)` > methods. I have explained all these ideas in > https://externals.io/message/125740 . If this option is chosen, aviz > will likely need to be reverted and reintroduced in PHP 8.5, since > we're already in the feature freeze period. > 2. Proceed with the current approach, but clearly explain the BC break > in the changelog, and merge this PR > https://github.com/php/php-src/pull/16209 to mitigate reflection > issues as outlined in https://externals.io/message/125740. > > -- > Best regards, > Valentin > Tbh we should consider voting to get rid of it, the costs are starting to outweigh the benefits (which aren't super clear to me). Cheers. > --000000000000278ca306240d776e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Wed, Oct 9, 2024, 9:02=E2=80=AFa.m. Valentin Udalts= ov <udaltsov.valentin@gma= il.com> wrote:
Hi, internals= !

Since writing https://externals.io/message/125740 = I've realized that
the major problem with aviz is actually simple but fundamental.

In PHP <=3D8.0 this code is valid for an object of any user class:

```php
class User
{
=C2=A0 =C2=A0public string $name =3D 'Bob';
}

$object =3D new User();

foreach ($object as $property =3D> $value) {
=C2=A0 =C2=A0 $object->{$property} =3D 'Jack';
}
```

The same is true for Reflection API: once you check that `(new
ReflectionProperty(Foo::class, 'property'))->isPublic()`, you ca= n
safely read property and write to it from any scope.

Now let's jump to PHP 8.1+ with support for readonly properties.

A readonly property is two functionalities in one: write-once and
private set. This means that `public readonly $property` is actually
`public(get) private(set) readonly $property`. Although it is marked
as `public`, it is not public because it is not a symmetric public
property!

In PHP 8.1+, the following User class suddenly breaks the code above:

```php
class User
{
=C2=A0 =C2=A0public function __construct(
=C2=A0 =C2=A0 =C2=A0 =C2=A0public readonly string $name =3D 'Bob',<= br> =C2=A0 =C2=A0) {}
}

$object =3D new User();

foreach ($object as $property =3D> $value) {
=C2=A0 =C2=A0// Fatal error: Uncaught Error: Cannot modify readonly propert= y User::$name
=C2=A0 =C2=A0$object->{$property} =3D 'Jack';
}
```

In other words, the meaning of `public` has changed in PHP 8.1.
Before, it used to mean "symmetric", now it means "symmetric= unless
readonly".

While not explicitly stated in changelogs, this was a BC break,
because a changed semantic of smth that existed before is a BC break.

Did it break anything? Of course it did! See:
- https://github.com/doctrine/orm/issues/1004= 9
- https://github.com/symfony/symfony/pull/46= 840
- https://github.com/Ocramius/Ge= neratedHydrator/issues/656
- https://github.com/opis/closure/issues/129

I believe there are still many places where the concept of "public&quo= t;
needs to be adjusted to fully support readonly properties.

Now in PHP 8.4 asymmetry will be made explicit and will allow users to
specify visibility for setters. However, the core issue remains
unresolved:

```php
final class User
{
=C2=A0 =C2=A0public function __construct(
=C2=A0 =C2=A0 =C2=A0 =C2=A0public private(set) string $name =3D 'Bob= 9;,
=C2=A0 =C2=A0) {}
}

$object =3D new User();

foreach ($object as $property =3D> $value) {
=C2=A0 =C2=A0// Fatal error:=C2=A0 Uncaught Error: Cannot modify private(se= t)
property User::$name from global scope
=C2=A0 =C2=A0$object->{$property} =3D 'Jack';
}
```

I'd like to draw your attention to the fact that aviz introduces a BC break, despite saying "Backward Incompatible Changes: None. This
syntax would have been a parse error before." While the syntax is new,=
it allows one to alter the old concept of public by changing set
visibility.

What can we do about it:
1. Explicitly introduce the concept of getter and setter visibility,
preserve `ReflectionProperty::isPublic()` behavior from PHP <=3D8.0 and<= br> add `ReflectionProperty::(get|set)Is(Public|Protected|Private)`
methods. I have explained all these ideas in
https://externals.io/message/125740 . If this opti= on is chosen, aviz
will likely need to be reverted and reintroduced in PHP 8.5, since
we're already in the feature freeze period.
2. Proceed with the current approach, but clearly explain the BC break
in the changelog, and merge this PR
https://github.com/php/php-src/pull/16209 to= mitigate reflection
issues as outlined in https://externals.io/message/1= 25740.

--
Best regards,
Valentin

Tbh we should consider voting to get rid of it, the costs are sta= rting to outweigh the=C2=A0benefits (which aren't super clear to me).

Cheers.
--000000000000278ca306240d776e--