> Le 9 oct. 2024 à 17:01, Valentin Udaltsov <udaltsov.valen...@gmail.com> a > écrit : > > 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 <=8.0 this code is valid for an object of any user class: > > ```php > class User > { > public string $name = 'Bob'; > } > > $object = new User(); > > foreach ($object as $property => $value) { > $object->{$property} = '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 = 'Bob', > ) {} > } > > $object = new User(); > > foreach ($object as $property => $value) { > // Fatal error: Uncaught Error: Cannot modify readonly property User::$name > $object->{$property} = '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 = 'Bob', > ) {} > } > > $object = new User(); > > foreach ($object as $property => $value) { > // Fatal error: Uncaught Error: Cannot modify private(set) > property User::$name from global scope > $object->{$property} = '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 <=8.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
Hi Valentin, There is no BC break, in the sense that code that worked under PHP 8.3 (and therefore use PHP 8.3 features only) will not break when run under PHP 8.4. Of course, code that makes assumptions that are true when using PHP 8.3 features only, will need to be adapted as soon as PHP 8.4 features are used. This is unsurprising and expected. Yes, it means that libraries that make use of Reflection are not automatically compatible with PHP 8.4+ without amendment. By nature, such libraries cannot claim to be compatible with arbitrary future versions of PHP. That said, https://github.com/php/php-src/pull/16209 is interesting to have, but not mandatory. The current `ReflectionProperty::isPropertySet()` and `ReflectionProperty::isPrivateSet()` might be somewhat confusing at first, but they are sufficient in order to obtain the needed information. —Claude
