readonly properties

php.internals

Marc

5 years ago
Hi, As 8.1 adds readonly properties I wonder which build-in properties should be defined readonly. Currently I could find build-in readonly properties only on PDO and DOM. Very incomplete list where readonly properties could make sense: 1. Enum properties: enum Test:string {     case TEST = 'test'; } $case = TEST::TEST; $refl = (new ReflectionObject($case))->getProperty('value'); var_dump($refl->isReadOnly());  // false var_dump($refl->isPublic());  // true $case->value = 'foo'; // Fatal error: Uncaught Error: Enum properties are immutable 2. DateInterval->days $interval = (new DateTime())->diff(new DateTime()); var_dump($interval->days); // 0 $refl = (new ReflectionObject($interval))->getProperty('days'); var_dump($refl->isReadOnly()); // false var_dump($refl->isPublic()); // true $interval->days = 2; var_dump($interval->days);  // 0 3. Exception properties Exception properties are protected but does it really make sense to be able to modify an exception property after initialization? I know this would be a BC break :( Thanks for all the excellent work - Can't wait for 8.1 :) Marc

Nikita Popov

5 years ago
On Thu, Aug 12, 2021 at 9:16 PM Marc <marc@mabe.berlin> wrote:
> Hi, > > As 8.1 adds readonly properties I wonder which build-in properties > should be defined readonly. > > Currently I could find build-in readonly properties only on PDO and DOM. > > > Very incomplete list where readonly properties could make sense: > > 1. Enum properties: > > enum Test:string { > case TEST = 'test'; > } > > $case = TEST::TEST; > $refl = (new ReflectionObject($case))->getProperty('value'); > var_dump($refl->isReadOnly()); // false > var_dump($refl->isPublic()); // true > $case->value = 'foo'; // Fatal error: Uncaught Error: Enum properties > are immutable >
Yeah, these are a perfect use case for "readonly". Done in https://github.com/php/php-src/commit/caefc6a50789295b0993c4e657c825484650172a. This actually fixes a bug, because the homegrown "readonly" implementation for enums was not quite correct.
> 2. DateInterval->days > > $interval = (new DateTime())->diff(new DateTime()); > var_dump($interval->days); // 0 > $refl = (new ReflectionObject($interval))->getProperty('days'); > var_dump($refl->isReadOnly()); // false > var_dump($refl->isPublic()); // true > $interval->days = 2; > var_dump($interval->days); // 0 >
The DateInterval properties are currently implemented as getters/setters on some internal state. Some of those are getter-only, but probably not fully immutable. 3. Exception properties
> > Exception properties are protected but does it really make sense to be > able to modify an exception property after initialization? > > I know this would be a BC break :( >
There is definitely code out there relying on modifying both protected and private Exception properties, I don't think we want to touch these without cause. Regards, Nikita

Marc

5 years ago
On 8/13/21 4:56 PM, Nikita Popov wrote:
> On Thu, Aug 12, 2021 at 9:16 PM Marc <marc@mabe.berlin> wrote: > >> Hi, >> >> As 8.1 adds readonly properties I wonder which build-in properties >> should be defined readonly. >> >> Currently I could find build-in readonly properties only on PDO and DOM. >> >> >> Very incomplete list where readonly properties could make sense: >> >> 1. Enum properties: >> >> enum Test:string { >> case TEST = 'test'; >> } >> >> $case = TEST::TEST; >> $refl = (new ReflectionObject($case))->getProperty('value'); >> var_dump($refl->isReadOnly()); // false >> var_dump($refl->isPublic()); // true >> $case->value = 'foo'; // Fatal error: Uncaught Error: Enum properties >> are immutable >> > Yeah, these are a perfect use case for "readonly". Done in > https://github.com/php/php-src/commit/caefc6a50789295b0993c4e657c825484650172a. > This actually fixes a bug, because the homegrown "readonly" implementation > for enums was not quite correct.
WOW this was fast. Less code and even a bugfix +1
> >> 2. DateInterval->days >> >> $interval = (new DateTime())->diff(new DateTime()); >> var_dump($interval->days); // 0 >> $refl = (new ReflectionObject($interval))->getProperty('days'); >> var_dump($refl->isReadOnly()); // false >> var_dump($refl->isPublic()); // true >> $interval->days = 2; >> var_dump($interval->days); // 0 >> > The DateInterval properties are currently implemented as getters/setters on > some internal state. Some of those are getter-only, but probably not fully > immutable.
I was referring to the property "days" only as this is a non writable value generated on "DateTime->diff". As you can see in the snipped it doesn't allow to modify days but it also doesn't fail which seems wrong to me. I don't have much knowledge about internals but would it be possible to define "public readonly int|false $days" not using getters/setters and keep all other properties as it?
> > 3. Exception properties >> Exception properties are protected but does it really make sense to be >> able to modify an exception property after initialization? >> >> I know this would be a BC break :( >> > There is definitely code out there relying on modifying both protected and > private Exception properties, I don't think we want to touch these without > cause.
Yea you are probably right - it would be a heavy BC break for not much profit.
> Regards, > Nikita >
Thank you very much Nikita!

Guilliam Xavier

5 years ago
> > >> 2. DateInterval->days > >> > >> $interval = (new DateTime())->diff(new DateTime()); > >> var_dump($interval->days); // 0 > >> $refl = (new ReflectionObject($interval))->getProperty('days'); > >> var_dump($refl->isReadOnly()); // false > >> var_dump($refl->isPublic()); // true > >> $interval->days = 2; > >> var_dump($interval->days); // 0 > >> > > The DateInterval properties are currently implemented as getters/setters > on > > some internal state. Some of those are getter-only, but probably not > fully > > immutable. > > I was referring to the property "days" only as this is a non writable > value generated on "DateTime->diff". > > As you can see in the snipped it doesn't allow to modify days but it > also doesn't fail which seems wrong to me. > > I don't have much knowledge about internals but would it be possible to > define "public readonly int|false $days" not using getters/setters and > keep all other properties as it? >
Hi, FWIW, I too agree that [$interval->days, $interval->days = 2, $interval->days] giving [0, 2, 0] without any warning (or https://3v4l.org/h8Cju for a variation) seems wrong. Regards,
-- Guilliam Xavier