From 823d6e751ea59dca16e872513ac4a8c0b68c8716 Mon Sep 17 00:00:00 2001 From: Jacob Tobiasz Date: Sun, 2 Feb 2025 19:21:22 +0100 Subject: [PATCH 1/4] Prevent declaring hooked properties as readonly --- Makefile | 1 + src/Rules/Properties/PropertyInClassRule.php | 11 ++++++++++ .../Properties/PropertyInClassRuleTest.php | 22 +++++++++++++++++++ .../data/readonly-property-hooks.php | 15 +++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php diff --git a/Makefile b/Makefile index b0379d18e7..71fadadf9b 100644 --- a/Makefile +++ b/Makefile @@ -88,6 +88,7 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \ + --exclude tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php \ --exclude tests/PHPStan/Rules/Classes/data/bug-12281.php \ --exclude tests/PHPStan/Rules/Traits/data/bug-12281.php \ --exclude tests/PHPStan/Rules/Classes/data/invalid-hooked-properties.php \ diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index 3500959541..e4df9f5341 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -76,6 +76,17 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->isReadOnly()) { + if ($node->hasHooks()) { + return [ + RuleErrorBuilder::message('Hooked properties cannot be readonly.') + ->nonIgnorable() + ->identifier('property.hookReadOnly') + ->build(), + ]; + } + } + if (!$this->doAllHooksHaveBody($node)) { return [ RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index f6fde1e51c..a076f9613b 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -151,4 +151,26 @@ public function testPhp84AndAbstractHookedPropertiesWithBodies(): void ]); } + public function testPhp84AndReadonlyHookedProperties(): void + { + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } + + $this->analyse([__DIR__ . '/data/readonly-property-hooks.php'], [ + [ + 'Hooked properties cannot be readonly.', + 7, + ], + [ + 'Hooked properties cannot be readonly.', + 12, + ], + [ + 'Hooked properties cannot be readonly.', + 14, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php new file mode 100644 index 0000000000..2b64060bea --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php @@ -0,0 +1,15 @@ + $this->firstName; + set => $this->firstName; + } + + public readonly string $middleName { get => $this->middleName; } + + public readonly string $lastName { set => $this->lastName; } +} From 2d365263477d88a9c1aa4715a8514b0886a1bbd1 Mon Sep 17 00:00:00 2001 From: Jacob Tobiasz Date: Mon, 3 Feb 2025 08:02:38 +0100 Subject: [PATCH 2/4] Cover readonly hooked properties in abstract classes --- src/Rules/Properties/PropertyInClassRule.php | 18 +++++++++--------- .../Properties/PropertyInClassRuleTest.php | 4 ++++ ...oked-properties-without-bodies-in-class.php | 2 +- .../data/readonly-property-hooks.php | 5 +++++ 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index e4df9f5341..8d598bc525 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -72,8 +72,6 @@ public function processNode(Node $node, Scope $scope): array ->build(), ]; } - - return []; } if ($node->isReadOnly()) { @@ -87,13 +85,15 @@ public function processNode(Node $node, Scope $scope): array } } - if (!$this->doAllHooksHaveBody($node)) { - return [ - RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') - ->nonIgnorable() - ->identifier('property.hookWithoutBody') - ->build(), - ]; + if (!$node->isAbstract() && !$node->isReadOnly()) { + if (!$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + ->nonIgnorable() + ->identifier('property.hookWithoutBody') + ->build(), + ]; + } } return []; diff --git a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php index a076f9613b..6d61e5c7cc 100644 --- a/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertyInClassRuleTest.php @@ -170,6 +170,10 @@ public function testPhp84AndReadonlyHookedProperties(): void 'Hooked properties cannot be readonly.', 14, ], + [ + 'Hooked properties cannot be readonly.', + 19, + ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php b/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php index fb0edcc3ce..24238e5c14 100644 --- a/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php +++ b/tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php @@ -12,7 +12,7 @@ class AbstractPerson class PromotedHookedPropertyWithoutVisibility { - public function __construct(mixed $test { get; }) + public function __construct(public mixed $test { get; }) { } diff --git a/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php index 2b64060bea..f727d148b3 100644 --- a/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php +++ b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php @@ -13,3 +13,8 @@ class HelloWorld public readonly string $lastName { set => $this->lastName; } } + +abstract class HiWorld +{ + public abstract readonly string $firstName { get { return 'jake'; } set; } +} From ca26ca4005d82bf10fee910bd45f320e09c2396b Mon Sep 17 00:00:00 2001 From: Jacob Tobiasz Date: Mon, 3 Feb 2025 08:08:06 +0100 Subject: [PATCH 3/4] Cover readonly hooked properties in interfaces --- Makefile | 1 + .../Properties/PropertiesInInterfaceRule.php | 9 ++++++++ .../PropertiesInInterfaceRuleTest.php | 22 +++++++++++++++++++ .../readonly-property-hooks-in-interface.php | 12 ++++++++++ 4 files changed, 44 insertions(+) create mode 100644 tests/PHPStan/Rules/Properties/data/readonly-property-hooks-in-interface.php diff --git a/Makefile b/Makefile index 71fadadf9b..ab0a439c3b 100644 --- a/Makefile +++ b/Makefile @@ -89,6 +89,7 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/readonly-property-hooks.php \ + --exclude tests/PHPStan/Rules/Properties/data/readonly-property-hooks-in-interface.php \ --exclude tests/PHPStan/Rules/Classes/data/bug-12281.php \ --exclude tests/PHPStan/Rules/Traits/data/bug-12281.php \ --exclude tests/PHPStan/Rules/Classes/data/invalid-hooked-properties.php \ diff --git a/src/Rules/Properties/PropertiesInInterfaceRule.php b/src/Rules/Properties/PropertiesInInterfaceRule.php index df5354adb3..b6a3c7d493 100644 --- a/src/Rules/Properties/PropertiesInInterfaceRule.php +++ b/src/Rules/Properties/PropertiesInInterfaceRule.php @@ -57,6 +57,15 @@ public function processNode(Node $node, Scope $scope): array ]; } + if ($node->isReadOnly()) { + return [ + RuleErrorBuilder::message('Interfaces cannot include readonly hooked properties.') + ->nonIgnorable() + ->identifier('property.readOnlyInInterface') + ->build(), + ]; + } + if ($this->hasAnyHookBody($node)) { return [ RuleErrorBuilder::message('Interfaces cannot include property hooks with bodies.') diff --git a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php index de425931da..dc011f8a82 100644 --- a/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php +++ b/tests/PHPStan/Rules/Properties/PropertiesInInterfaceRuleTest.php @@ -118,4 +118,26 @@ public function testPhp84AndPropertyHooksWithBodiesInInterface(): void ]); } + public function testPhp84AndReadonlyPropertyHooksInInterface(): void + { + if (PHP_VERSION_ID < 80400) { + $this->markTestSkipped('Test requires PHP 8.4 or later.'); + } + + $this->analyse([__DIR__ . '/data/readonly-property-hooks-in-interface.php'], [ + [ + 'Interfaces cannot include readonly hooked properties.', + 7, + ], + [ + 'Interfaces cannot include readonly hooked properties.', + 9, + ], + [ + 'Interfaces cannot include readonly hooked properties.', + 11, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/readonly-property-hooks-in-interface.php b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks-in-interface.php new file mode 100644 index 0000000000..53aa244fa9 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/readonly-property-hooks-in-interface.php @@ -0,0 +1,12 @@ + Date: Tue, 4 Feb 2025 13:32:52 +0100 Subject: [PATCH 4/4] Move code --- src/Rules/Properties/PropertyInClassRule.php | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/Rules/Properties/PropertyInClassRule.php b/src/Rules/Properties/PropertyInClassRule.php index 8d598bc525..50e3880013 100644 --- a/src/Rules/Properties/PropertyInClassRule.php +++ b/src/Rules/Properties/PropertyInClassRule.php @@ -72,6 +72,13 @@ public function processNode(Node $node, Scope $scope): array ->build(), ]; } + } elseif (!$this->doAllHooksHaveBody($node)) { + return [ + RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') + ->nonIgnorable() + ->identifier('property.hookWithoutBody') + ->build(), + ]; } if ($node->isReadOnly()) { @@ -85,17 +92,6 @@ public function processNode(Node $node, Scope $scope): array } } - if (!$node->isAbstract() && !$node->isReadOnly()) { - if (!$this->doAllHooksHaveBody($node)) { - return [ - RuleErrorBuilder::message('Non-abstract properties cannot include hooks without bodies.') - ->nonIgnorable() - ->identifier('property.hookWithoutBody') - ->build(), - ]; - } - } - return []; }