-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[RFC] Allow hooks for backed readonly
properties
#18757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
NickSdot
wants to merge
13
commits into
php:master
Choose a base branch
from
NickSdot:allow-readonly-hooks
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
12b986a
feat: allow hooks for backed readonly properties
NickSdot 8c177da
refactor: added tests for rfc examples
NickSdot c9ebae4
refactor: added trailing newlines at EOF in tests
NickSdot b4df02d
refactor: adjusted test error message formatting
NickSdot 92d70bc
refactor: improved misleading error handling
NickSdot 43f3130
chore: removed unrelated tests
NickSdot 29520ed
refactor: added trailing newline at EOF in tests
NickSdot bfee62d
refactor: renamed "may not" back to "cannot", because it's more commo…
NickSdot d7bf39b
formatting
NickSdot 71474c3
fix: address review comment about "consumed too much"
NickSdot 324e61e
refactor: tidied up test
NickSdot 077c940
refactor: addressed https://github.com/php/php-src/pull/18757#discuss…
NickSdot 3ec4f91
refactor: addressed https://github.com/php/php-src/pull/18757#discuss…
NickSdot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
41 changes: 41 additions & 0 deletions
41
Zend/tests/property_hooks/readonly_class_property_backed.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
--TEST-- | ||
Backed property in readonly class may have hooks | ||
--FILE-- | ||
<?php | ||
|
||
// readonly class | ||
readonly class Test { | ||
public int $prop { | ||
get => $this->prop; | ||
set => $value; | ||
} | ||
|
||
public function __construct(int $v) { | ||
$this->prop = $v; | ||
} | ||
|
||
public function set($v) | ||
{ | ||
$this->prop = $v; | ||
} | ||
} | ||
|
||
$t = new Test(42); | ||
var_dump($t->prop); | ||
try { | ||
$t->set(43); | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
try { | ||
$t->prop = 43; | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
var_dump($t->prop); | ||
?> | ||
--EXPECT-- | ||
int(42) | ||
Error: Cannot modify readonly property Test::$prop | ||
Error: Cannot modify protected(set) readonly property Test::$prop from global scope | ||
int(42) |
39 changes: 39 additions & 0 deletions
39
Zend/tests/property_hooks/readonly_class_property_backed_promoted.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
--TEST-- | ||
Backed promoted property in readonly class may have hooks | ||
--FILE-- | ||
<?php | ||
|
||
// readonly class, promoted | ||
readonly class Test { | ||
public function __construct( | ||
public int $prop { | ||
get => $this->prop; | ||
set => $value; | ||
} | ||
) {} | ||
|
||
public function set($v) | ||
{ | ||
$this->prop = $v; | ||
} | ||
} | ||
|
||
$t = new Test(42); | ||
var_dump($t->prop); | ||
try { | ||
$t->set(43); | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
try { | ||
$t->prop = 43; | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
var_dump($t->prop); | ||
?> | ||
--EXPECT-- | ||
int(42) | ||
Error: Cannot modify readonly property Test::$prop | ||
Error: Cannot modify protected(set) readonly property Test::$prop from global scope | ||
int(42) |
16 changes: 16 additions & 0 deletions
16
Zend/tests/property_hooks/readonly_class_property_virtual_promoted.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
--TEST-- | ||
Virtual promoted property in readonly class cannot have hooks | ||
--FILE-- | ||
<?php | ||
|
||
readonly class Test { | ||
public function __construct( | ||
public int $prop { | ||
get => 42; | ||
} | ||
) {} | ||
} | ||
|
||
?> | ||
--EXPECTF-- | ||
Fatal error: Hooked virtual properties cannot be declared readonly in %s on line %d |
108 changes: 108 additions & 0 deletions
108
Zend/tests/property_hooks/readonly_property_backed.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
--TEST-- | ||
Backed readonly property may have hooks | ||
--FILE-- | ||
<?php | ||
|
||
// readonly property | ||
class Test { | ||
public readonly int $prop { | ||
get => $this->prop; | ||
set => $value; | ||
} | ||
|
||
public function __construct(int $v) { | ||
$this->prop = $v; | ||
} | ||
|
||
public function set($v) | ||
{ | ||
$this->prop = $v; | ||
} | ||
} | ||
|
||
$t = new Test(42); | ||
var_dump($t->prop); | ||
try { | ||
$t->set(43); | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
try { | ||
$t->prop = 43; | ||
} catch (Error $e) { | ||
echo $e::class, ': ', $e->getMessage(), PHP_EOL; | ||
} | ||
var_dump($t->prop); | ||
|
||
// class readonly | ||
final readonly class Foo | ||
{ | ||
public function __construct( | ||
public array $values { | ||
set(array $value) => array_map(strtoupper(...), $value); | ||
}, | ||
) {} | ||
} | ||
|
||
// property readonly | ||
final class Foo2 | ||
{ | ||
public function __construct( | ||
public readonly array $values { | ||
set(array $value) => array_map(strtoupper(...), $value); | ||
}, | ||
) {} | ||
} | ||
|
||
// redundant readonly | ||
final readonly class Foo3 | ||
{ | ||
public function __construct( | ||
public readonly array $values { | ||
set(array $value) => array_map(strtoupper(...), $value); | ||
get => $this->makeNicer($this->values); | ||
}, | ||
) {} | ||
|
||
public function makeNicer(array $entries): array | ||
{ | ||
return array_map( | ||
fn($i, $entry) => $entry . strtoupper(['', 'r', 'st'][$i]), array_keys($entries), | ||
$entries | ||
); | ||
} | ||
} | ||
|
||
\var_dump(new Foo(['yo,', 'you', 'can'])->values); | ||
\var_dump(new Foo2(['just', 'do', 'things'])->values); | ||
\var_dump(new Foo3(['nice', 'nice', 'nice'])->values); | ||
?> | ||
--EXPECT-- | ||
int(42) | ||
Error: Cannot modify readonly property Test::$prop | ||
Error: Cannot modify protected(set) readonly property Test::$prop from global scope | ||
int(42) | ||
array(3) { | ||
[0]=> | ||
string(3) "YO," | ||
[1]=> | ||
string(3) "YOU" | ||
[2]=> | ||
string(3) "CAN" | ||
} | ||
array(3) { | ||
[0]=> | ||
string(4) "JUST" | ||
[1]=> | ||
string(2) "DO" | ||
[2]=> | ||
string(6) "THINGS" | ||
} | ||
array(3) { | ||
[0]=> | ||
string(4) "NICE" | ||
[1]=> | ||
string(5) "NICER" | ||
[2]=> | ||
string(6) "NICEST" | ||
} |
21 changes: 21 additions & 0 deletions
21
Zend/tests/property_hooks/readonly_property_backed_inheritance_1.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
--TEST-- | ||
Backed property cannot redeclare readonly as non-readonly property | ||
--FILE-- | ||
<?php | ||
|
||
class ParentClass { | ||
public readonly int $prop; | ||
} | ||
|
||
class Test extends ParentClass { | ||
public function __construct( | ||
public int $prop { | ||
get => $this->prop; | ||
set => $value; | ||
} | ||
) {} | ||
} | ||
|
||
?> | ||
--EXPECTF-- | ||
Fatal error: Cannot redeclare readonly property ParentClass::$prop as non-readonly Test::$prop %s on line %d | ||
21 changes: 21 additions & 0 deletions
21
Zend/tests/property_hooks/readonly_property_backed_inheritance_2.phpt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
--TEST-- | ||
Backed property cannot redeclare non-readonly as readonly property | ||
--FILE-- | ||
<?php | ||
|
||
class ParentClass { | ||
public int $prop; | ||
} | ||
|
||
class Test extends ParentClass { | ||
public function __construct( | ||
public readonly int $prop { | ||
get => $this->prop; | ||
set => $value; | ||
} | ||
) {} | ||
} | ||
|
||
?> | ||
--EXPECTF-- | ||
Fatal error: Cannot redeclare non-readonly property ParentClass::$prop as readonly Test::$prop in %s on line %d |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.