From 45f39990e8d2c13523426ee49561dcf47d74ffdd Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 00:18:27 +1000 Subject: [PATCH 1/8] Resolve #12999 --- .../Php/PhpClassReflectionExtension.php | 5 +++- src/Reflection/Php/PhpPropertyReflection.php | 6 +++++ src/Rules/Variables/UnsetRule.php | 1 + .../PHPStan/Rules/Variables/UnsetRuleTest.php | 26 +++++++++---------- .../Variables/data/unset-hooked-property.php | 4 +++ 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index fc0dd70dbe..6ebe71798a 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -218,7 +218,7 @@ private function createProperty( $types[] = $value; } - return new PhpPropertyReflection($declaringClassReflection, null, null, TypeCombinator::union(...$types), $classReflection->getNativeReflection()->getProperty($propertyName), null, null, null, false, false, false, false, []); + return new PhpPropertyReflection($declaringClassReflection, null, null, TypeCombinator::union(...$types), $classReflection->getNativeReflection()->getProperty($propertyName), null, null, null, false, false, false, false, [], false); } } @@ -227,6 +227,7 @@ private function createProperty( $isDeprecated = $deprecation !== null; $isInternal = false; $isReadOnlyByPhpDoc = $classReflection->isImmutable(); + $isFinalByPhpDoc = $classReflection->isFinal(); $isAllowedPrivateMutation = false; if ( @@ -308,6 +309,7 @@ private function createProperty( } $isInternal = $resolvedPhpDoc->isInternal(); $isReadOnlyByPhpDoc = $isReadOnlyByPhpDoc || $resolvedPhpDoc->isReadOnly(); + $isFinalByPhpDoc = $isFinalByPhpDoc || $resolvedPhpDoc->isFinal(); $isAllowedPrivateMutation = $resolvedPhpDoc->isAllowedPrivateMutation(); } @@ -435,6 +437,7 @@ private function createProperty( $isReadOnlyByPhpDoc, $isAllowedPrivateMutation, $this->attributeReflectionFactory->fromNativeReflection($propertyReflection->getAttributes(), InitializerExprContext::fromClass($declaringClassReflection->getName(), $declaringClassReflection->getFileName())), + $isFinalByPhpDoc, ); } diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index 8307f36d7b..a7bd669afe 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -44,6 +44,7 @@ public function __construct( private bool $isReadOnlyByPhpDoc, private bool $isAllowedPrivateMutation, private array $attributes, + private bool $isFinalByPhpDoc, ) { } @@ -98,6 +99,11 @@ public function isReadOnlyByPhpDoc(): bool return $this->isReadOnlyByPhpDoc; } + public function isFinalByPhpDoc(): bool + { + return $this->isFinalByPhpDoc; + } + public function getReadableType(): Type { if ($this->type === null) { diff --git a/src/Rules/Variables/UnsetRule.php b/src/Rules/Variables/UnsetRule.php index 91e5260488..225a76237b 100644 --- a/src/Rules/Variables/UnsetRule.php +++ b/src/Rules/Variables/UnsetRule.php @@ -82,6 +82,7 @@ public function processNode(Node $node, Scope $scope): array if ( !$propertyReflection->isPrivate() && !$propertyReflection->isFinal()->yes() + && !$propertyReflection->isFinalByPhpDoc() && !$propertyReflection->getDeclaringClass()->isFinal() ) { $errors[] = RuleErrorBuilder::message( diff --git a/tests/PHPStan/Rules/Variables/UnsetRuleTest.php b/tests/PHPStan/Rules/Variables/UnsetRuleTest.php index df34c15d50..cfe63c0dd7 100644 --- a/tests/PHPStan/Rules/Variables/UnsetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/UnsetRuleTest.php @@ -167,55 +167,55 @@ public function testUnsetHookedProperty(): void ], [ 'Cannot unset property UnsetHookedProperty\NonFinalClass::$publicProperty because it might have hooks in a subclass.', - 13, + 14, ], [ 'Cannot unset property UnsetHookedProperty\ContainerClass::$finalClass because it might have hooks in a subclass.', - 83, + 86, ], [ 'Cannot unset property UnsetHookedProperty\ContainerClass::$nonFinalClass because it might have hooks in a subclass.', - 87, + 91, ], [ 'Cannot unset hooked UnsetHookedProperty\Foo::$iii property.', - 89, + 93, ], [ 'Cannot unset property UnsetHookedProperty\ContainerClass::$foo because it might have hooks in a subclass.', - 90, + 94, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$name property.', - 92, + 96, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$fullName property.', - 93, + 97, ], [ 'Cannot unset property UnsetHookedProperty\ContainerClass::$user because it might have hooks in a subclass.', - 94, + 98, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$name property.', - 96, + 100, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$name property.', - 97, + 101, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$fullName property.', - 98, + 102, ], [ 'Cannot unset hooked UnsetHookedProperty\User::$fullName property.', - 99, + 103, ], [ 'Cannot unset property UnsetHookedProperty\ContainerClass::$arrayOfUsers because it might have hooks in a subclass.', - 100, + 104, ], ]); } diff --git a/tests/PHPStan/Rules/Variables/data/unset-hooked-property.php b/tests/PHPStan/Rules/Variables/data/unset-hooked-property.php index d98eed672a..97ba5781cd 100644 --- a/tests/PHPStan/Rules/Variables/data/unset-hooked-property.php +++ b/tests/PHPStan/Rules/Variables/data/unset-hooked-property.php @@ -9,6 +9,7 @@ function doUnset(Foo $foo, User $user, NonFinalClass $nonFinalClass, FinalClass unset($foo->ii); unset($foo->iii); + unset($nonFinalClass->publicAnnotatedFinalProperty); unset($nonFinalClass->publicFinalProperty); unset($nonFinalClass->publicProperty); @@ -49,6 +50,8 @@ class NonFinalClass { private string $privateProperty; public string $publicProperty; final public string $publicFinalProperty; + /** @final */ + public string $publicAnnotatedFinalProperty; function doFoo() { unset($this->privateProperty); @@ -82,6 +85,7 @@ function dooNestedUnset(ContainerClass $containerClass) { unset($containerClass->finalClass->publicProperty); unset($containerClass->finalClass); + unset($containerClass->nonFinalClass->publicAnnotatedFinalProperty); unset($containerClass->nonFinalClass->publicFinalProperty); unset($containerClass->nonFinalClass->publicProperty); unset($containerClass->nonFinalClass); From c0d56bc043ed0e1f2343c93732f3787a60770bed Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 00:44:14 +1000 Subject: [PATCH 2/8] Rename isFinal to isFinalByKeyword and isFinalByPhpDoc to isFinal --- src/Analyser/NodeScopeResolver.php | 2 +- .../Annotations/AnnotationPropertyReflection.php | 2 +- src/Reflection/Dummy/ChangedTypePropertyReflection.php | 4 ++-- src/Reflection/Dummy/DummyPropertyReflection.php | 2 +- src/Reflection/ExtendedPropertyReflection.php | 2 +- src/Reflection/Php/EnumPropertyReflection.php | 2 +- src/Reflection/Php/PhpClassReflectionExtension.php | 6 +++--- src/Reflection/Php/PhpPropertyReflection.php | 8 ++++---- src/Reflection/Php/SimpleXMLElementProperty.php | 2 +- src/Reflection/Php/UniversalObjectCrateProperty.php | 2 +- src/Reflection/ResolvedPropertyReflection.php | 4 ++-- .../Type/IntersectionTypePropertyReflection.php | 4 ++-- src/Reflection/Type/UnionTypePropertyReflection.php | 4 ++-- src/Reflection/WrappedExtendedPropertyReflection.php | 2 +- src/Rules/Properties/FoundPropertyReflection.php | 4 ++-- src/Rules/Properties/OverridingPropertyRule.php | 2 +- src/Rules/Variables/UnsetRule.php | 3 +-- src/Type/ObjectShapePropertyReflection.php | 2 +- 18 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index d979a0f24a..ee2eed6a85 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4373,7 +4373,7 @@ private function getThrowPointsFromPropertyHook( if (!$propertyReflection->hasHook($hookName)) { if ( $propertyReflection->isPrivate() - || $propertyReflection->isFinal()->yes() + || $propertyReflection->isFinalByKeyword()->yes() || $declaringClass->isFinal() ) { return []; diff --git a/src/Reflection/Annotations/AnnotationPropertyReflection.php b/src/Reflection/Annotations/AnnotationPropertyReflection.php index 8f4f322d08..1b45fb83ec 100644 --- a/src/Reflection/Annotations/AnnotationPropertyReflection.php +++ b/src/Reflection/Annotations/AnnotationPropertyReflection.php @@ -119,7 +119,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Reflection/Dummy/ChangedTypePropertyReflection.php b/src/Reflection/Dummy/ChangedTypePropertyReflection.php index 3bf6a6eb84..65c7fff11c 100644 --- a/src/Reflection/Dummy/ChangedTypePropertyReflection.php +++ b/src/Reflection/Dummy/ChangedTypePropertyReflection.php @@ -116,9 +116,9 @@ public function isAbstract(): TrinaryLogic return $this->reflection->isAbstract(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { - return $this->reflection->isFinal(); + return $this->reflection->isFinalByKeyword(); } public function isVirtual(): TrinaryLogic diff --git a/src/Reflection/Dummy/DummyPropertyReflection.php b/src/Reflection/Dummy/DummyPropertyReflection.php index ca828a53fe..3625413d62 100644 --- a/src/Reflection/Dummy/DummyPropertyReflection.php +++ b/src/Reflection/Dummy/DummyPropertyReflection.php @@ -116,7 +116,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Reflection/ExtendedPropertyReflection.php b/src/Reflection/ExtendedPropertyReflection.php index 1027c193f1..3ec35218b7 100644 --- a/src/Reflection/ExtendedPropertyReflection.php +++ b/src/Reflection/ExtendedPropertyReflection.php @@ -38,7 +38,7 @@ public function getNativeType(): Type; public function isAbstract(): TrinaryLogic; - public function isFinal(): TrinaryLogic; + public function isFinalByKeyword(): TrinaryLogic; public function isVirtual(): TrinaryLogic; diff --git a/src/Reflection/Php/EnumPropertyReflection.php b/src/Reflection/Php/EnumPropertyReflection.php index 912b779e67..bcddaab569 100644 --- a/src/Reflection/Php/EnumPropertyReflection.php +++ b/src/Reflection/Php/EnumPropertyReflection.php @@ -112,7 +112,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index 6ebe71798a..19144986c4 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -227,7 +227,7 @@ private function createProperty( $isDeprecated = $deprecation !== null; $isInternal = false; $isReadOnlyByPhpDoc = $classReflection->isImmutable(); - $isFinalByPhpDoc = $classReflection->isFinal(); + $isFinal = $classReflection->isFinal() || $propertyReflection->isFinal(); $isAllowedPrivateMutation = false; if ( @@ -309,7 +309,7 @@ private function createProperty( } $isInternal = $resolvedPhpDoc->isInternal(); $isReadOnlyByPhpDoc = $isReadOnlyByPhpDoc || $resolvedPhpDoc->isReadOnly(); - $isFinalByPhpDoc = $isFinalByPhpDoc || $resolvedPhpDoc->isFinal(); + $isFinal = $isFinal || $resolvedPhpDoc->isFinal(); $isAllowedPrivateMutation = $resolvedPhpDoc->isAllowedPrivateMutation(); } @@ -437,7 +437,7 @@ private function createProperty( $isReadOnlyByPhpDoc, $isAllowedPrivateMutation, $this->attributeReflectionFactory->fromNativeReflection($propertyReflection->getAttributes(), InitializerExprContext::fromClass($declaringClassReflection->getName(), $declaringClassReflection->getFileName())), - $isFinalByPhpDoc, + $isFinal, ); } diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index a7bd669afe..5e8518b867 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -44,7 +44,7 @@ public function __construct( private bool $isReadOnlyByPhpDoc, private bool $isAllowedPrivateMutation, private array $attributes, - private bool $isFinalByPhpDoc, + private bool $isFinal, ) { } @@ -99,9 +99,9 @@ public function isReadOnlyByPhpDoc(): bool return $this->isReadOnlyByPhpDoc; } - public function isFinalByPhpDoc(): bool + public function isFinal(): bool { - return $this->isFinalByPhpDoc; + return $this->isFinal; } public function getReadableType(): Type @@ -248,7 +248,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createFromBoolean($this->reflection->isAbstract()); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createFromBoolean($this->reflection->isFinal()); } diff --git a/src/Reflection/Php/SimpleXMLElementProperty.php b/src/Reflection/Php/SimpleXMLElementProperty.php index 29975a5e3f..5a42996565 100644 --- a/src/Reflection/Php/SimpleXMLElementProperty.php +++ b/src/Reflection/Php/SimpleXMLElementProperty.php @@ -127,7 +127,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Reflection/Php/UniversalObjectCrateProperty.php b/src/Reflection/Php/UniversalObjectCrateProperty.php index 564613e219..eece81b69a 100644 --- a/src/Reflection/Php/UniversalObjectCrateProperty.php +++ b/src/Reflection/Php/UniversalObjectCrateProperty.php @@ -117,7 +117,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Reflection/ResolvedPropertyReflection.php b/src/Reflection/ResolvedPropertyReflection.php index df7a33f84d..240b04da6d 100644 --- a/src/Reflection/ResolvedPropertyReflection.php +++ b/src/Reflection/ResolvedPropertyReflection.php @@ -174,9 +174,9 @@ public function isAbstract(): TrinaryLogic return $this->reflection->isAbstract(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { - return $this->reflection->isFinal(); + return $this->reflection->isFinalByKeyword(); } public function isVirtual(): TrinaryLogic diff --git a/src/Reflection/Type/IntersectionTypePropertyReflection.php b/src/Reflection/Type/IntersectionTypePropertyReflection.php index 71de28e1e6..99535e2851 100644 --- a/src/Reflection/Type/IntersectionTypePropertyReflection.php +++ b/src/Reflection/Type/IntersectionTypePropertyReflection.php @@ -148,9 +148,9 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isAbstract()); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { - return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinal()); + return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinalByKeyword()); } public function isVirtual(): TrinaryLogic diff --git a/src/Reflection/Type/UnionTypePropertyReflection.php b/src/Reflection/Type/UnionTypePropertyReflection.php index 77e1ed0397..3fb2a66aa9 100644 --- a/src/Reflection/Type/UnionTypePropertyReflection.php +++ b/src/Reflection/Type/UnionTypePropertyReflection.php @@ -148,9 +148,9 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isAbstract()); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { - return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinal()); + return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinalByKeyword()); } public function isVirtual(): TrinaryLogic diff --git a/src/Reflection/WrappedExtendedPropertyReflection.php b/src/Reflection/WrappedExtendedPropertyReflection.php index 04eceb4848..b3072a8cd8 100644 --- a/src/Reflection/WrappedExtendedPropertyReflection.php +++ b/src/Reflection/WrappedExtendedPropertyReflection.php @@ -109,7 +109,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } diff --git a/src/Rules/Properties/FoundPropertyReflection.php b/src/Rules/Properties/FoundPropertyReflection.php index 1b14b785aa..b675bd7297 100644 --- a/src/Rules/Properties/FoundPropertyReflection.php +++ b/src/Rules/Properties/FoundPropertyReflection.php @@ -143,9 +143,9 @@ public function isAbstract(): TrinaryLogic return $this->originalPropertyReflection->isAbstract(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { - return $this->originalPropertyReflection->isFinal(); + return $this->originalPropertyReflection->isFinalByKeyword(); } public function isVirtual(): TrinaryLogic diff --git a/src/Rules/Properties/OverridingPropertyRule.php b/src/Rules/Properties/OverridingPropertyRule.php index c1806565e2..be3550f914 100644 --- a/src/Rules/Properties/OverridingPropertyRule.php +++ b/src/Rules/Properties/OverridingPropertyRule.php @@ -135,7 +135,7 @@ public function processNode(Node $node, Scope $scope): array ))->identifier('property.visibility')->nonIgnorable()->build(); } - if ($prototype->isFinal()->yes()) { + if ($prototype->isFinalByKeyword()->yes()) { $errors[] = RuleErrorBuilder::message(sprintf( 'Property %s::$%s overrides final property %s::$%s.', $classReflection->getDisplayName(), diff --git a/src/Rules/Variables/UnsetRule.php b/src/Rules/Variables/UnsetRule.php index 225a76237b..3f8f187aef 100644 --- a/src/Rules/Variables/UnsetRule.php +++ b/src/Rules/Variables/UnsetRule.php @@ -81,8 +81,7 @@ public function processNode(Node $node, Scope $scope): array } elseif ($this->phpVersion->supportsPropertyHooks()) { if ( !$propertyReflection->isPrivate() - && !$propertyReflection->isFinal()->yes() - && !$propertyReflection->isFinalByPhpDoc() + && !$propertyReflection->isFinal() && !$propertyReflection->getDeclaringClass()->isFinal() ) { $errors[] = RuleErrorBuilder::message( diff --git a/src/Type/ObjectShapePropertyReflection.php b/src/Type/ObjectShapePropertyReflection.php index 594c3278ca..e97fe62edf 100644 --- a/src/Type/ObjectShapePropertyReflection.php +++ b/src/Type/ObjectShapePropertyReflection.php @@ -114,7 +114,7 @@ public function isAbstract(): TrinaryLogic return TrinaryLogic::createNo(); } - public function isFinal(): TrinaryLogic + public function isFinalByKeyword(): TrinaryLogic { return TrinaryLogic::createNo(); } From 44cbb1711f2cd6633f00ee0fdab49056f6368be0 Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 01:08:59 +1000 Subject: [PATCH 3/8] Check if @final property is overridden --- src/Rules/Properties/OverridingPropertyRule.php | 10 ++++++++++ .../Properties/OverridingPropertyRuleTest.php | 16 ++++++++++++---- .../data/overriding-final-property.php | 10 ++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/Rules/Properties/OverridingPropertyRule.php b/src/Rules/Properties/OverridingPropertyRule.php index be3550f914..bc7f0c4c68 100644 --- a/src/Rules/Properties/OverridingPropertyRule.php +++ b/src/Rules/Properties/OverridingPropertyRule.php @@ -145,6 +145,16 @@ public function processNode(Node $node, Scope $scope): array ))->identifier('property.parentPropertyFinal') ->nonIgnorable() ->build(); + } elseif ($prototype->isFinal()) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Property %s::$%s overrides @final property %s::$%s.', + $classReflection->getDisplayName(), + $node->getName(), + $prototype->getDeclaringClass()->getDisplayName(), + $node->getName(), + ))->identifier('property.parentPropertyAnnotatedFinal') + ->nonIgnorable() + ->build(); } $typeErrors = []; diff --git a/tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php b/tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php index c5f5cd929c..dd9749e079 100644 --- a/tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php +++ b/tests/PHPStan/Rules/Properties/OverridingPropertyRuleTest.php @@ -187,19 +187,27 @@ public function testFinal(): void $this->analyse([__DIR__ . '/data/overriding-final-property.php'], [ [ 'Property OverridingFinalProperty\Bar::$a overrides final property OverridingFinalProperty\Foo::$a.', - 21, + 27, ], [ 'Property OverridingFinalProperty\Bar::$b overrides final property OverridingFinalProperty\Foo::$b.', - 23, + 29, ], [ 'Property OverridingFinalProperty\Bar::$c overrides final property OverridingFinalProperty\Foo::$c.', - 25, + 31, ], [ 'Property OverridingFinalProperty\Bar::$d overrides final property OverridingFinalProperty\Foo::$d.', - 27, + 33, + ], + [ + 'Property OverridingFinalProperty\Bar::$e overrides @final property OverridingFinalProperty\Foo::$e.', + 35, + ], + [ + 'Property OverridingFinalProperty\Bar::$f overrides @final property OverridingFinalProperty\Foo::$f.', + 37, ], ]); } diff --git a/tests/PHPStan/Rules/Properties/data/overriding-final-property.php b/tests/PHPStan/Rules/Properties/data/overriding-final-property.php index 02d7467e57..b41b531c98 100644 --- a/tests/PHPStan/Rules/Properties/data/overriding-final-property.php +++ b/tests/PHPStan/Rules/Properties/data/overriding-final-property.php @@ -13,6 +13,12 @@ class Foo protected private(set) int $d; + /** @final */ + public $e; + + /** @final */ + protected $f; + } class Bar extends Foo @@ -26,4 +32,8 @@ class Bar extends Foo public int $d; + public $e; + + protected $f; + } From d8d32336400675b8e683f1c17c7f9e28b4b51359 Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 21:28:16 +1000 Subject: [PATCH 4/8] Change isFinal return type --- src/Reflection/Php/PhpPropertyReflection.php | 4 ++-- src/Rules/Properties/OverridingPropertyRule.php | 2 +- src/Rules/Variables/UnsetRule.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index 5e8518b867..2cbc9ec100 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -99,9 +99,9 @@ public function isReadOnlyByPhpDoc(): bool return $this->isReadOnlyByPhpDoc; } - public function isFinal(): bool + public function isFinal(): TrinaryLogic { - return $this->isFinal; + return TrinaryLogic::createFromBoolean($this->isFinal); } public function getReadableType(): Type diff --git a/src/Rules/Properties/OverridingPropertyRule.php b/src/Rules/Properties/OverridingPropertyRule.php index bc7f0c4c68..8b50f18893 100644 --- a/src/Rules/Properties/OverridingPropertyRule.php +++ b/src/Rules/Properties/OverridingPropertyRule.php @@ -145,7 +145,7 @@ public function processNode(Node $node, Scope $scope): array ))->identifier('property.parentPropertyFinal') ->nonIgnorable() ->build(); - } elseif ($prototype->isFinal()) { + } elseif ($prototype->isFinal()->yes()) { $errors[] = RuleErrorBuilder::message(sprintf( 'Property %s::$%s overrides @final property %s::$%s.', $classReflection->getDisplayName(), diff --git a/src/Rules/Variables/UnsetRule.php b/src/Rules/Variables/UnsetRule.php index 3f8f187aef..91e5260488 100644 --- a/src/Rules/Variables/UnsetRule.php +++ b/src/Rules/Variables/UnsetRule.php @@ -81,7 +81,7 @@ public function processNode(Node $node, Scope $scope): array } elseif ($this->phpVersion->supportsPropertyHooks()) { if ( !$propertyReflection->isPrivate() - && !$propertyReflection->isFinal() + && !$propertyReflection->isFinal()->yes() && !$propertyReflection->getDeclaringClass()->isFinal() ) { $errors[] = RuleErrorBuilder::message( From 9f6deb0088836644db9c9b4d51d6c69fc14c1686 Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 21:28:53 +1000 Subject: [PATCH 5/8] Reorder methods --- src/Reflection/Php/PhpPropertyReflection.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Reflection/Php/PhpPropertyReflection.php b/src/Reflection/Php/PhpPropertyReflection.php index 2cbc9ec100..8c4ea07439 100644 --- a/src/Reflection/Php/PhpPropertyReflection.php +++ b/src/Reflection/Php/PhpPropertyReflection.php @@ -99,11 +99,6 @@ public function isReadOnlyByPhpDoc(): bool return $this->isReadOnlyByPhpDoc; } - public function isFinal(): TrinaryLogic - { - return TrinaryLogic::createFromBoolean($this->isFinal); - } - public function getReadableType(): Type { if ($this->type === null) { @@ -253,6 +248,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createFromBoolean($this->reflection->isFinal()); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createFromBoolean($this->isFinal); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createFromBoolean($this->reflection->isVirtual()); From 93c9f3a880b877972b91a830688306f49ec03c71 Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 21:30:17 +1000 Subject: [PATCH 6/8] Review feedback --- src/Analyser/NodeScopeResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index ee2eed6a85..d979a0f24a 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4373,7 +4373,7 @@ private function getThrowPointsFromPropertyHook( if (!$propertyReflection->hasHook($hookName)) { if ( $propertyReflection->isPrivate() - || $propertyReflection->isFinalByKeyword()->yes() + || $propertyReflection->isFinal()->yes() || $declaringClass->isFinal() ) { return []; From c674ef3ac83158070a96d215c8fd6f6dc36d874f Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 21:55:46 +1000 Subject: [PATCH 7/8] Add isFinal to ExtendedPropertyReflection interface --- src/Reflection/Annotations/AnnotationPropertyReflection.php | 5 +++++ src/Reflection/Dummy/ChangedTypePropertyReflection.php | 5 +++++ src/Reflection/Dummy/DummyPropertyReflection.php | 5 +++++ src/Reflection/ExtendedPropertyReflection.php | 2 ++ src/Reflection/Php/EnumPropertyReflection.php | 5 +++++ src/Reflection/Php/SimpleXMLElementProperty.php | 5 +++++ src/Reflection/Php/UniversalObjectCrateProperty.php | 5 +++++ src/Reflection/ResolvedPropertyReflection.php | 5 +++++ src/Reflection/Type/IntersectionTypePropertyReflection.php | 5 +++++ src/Reflection/Type/UnionTypePropertyReflection.php | 5 +++++ src/Reflection/WrappedExtendedPropertyReflection.php | 5 +++++ src/Rules/Properties/FoundPropertyReflection.php | 5 +++++ .../RewrittenDeclaringClassPropertyReflection.php | 5 +++++ src/Type/ObjectShapePropertyReflection.php | 5 +++++ 14 files changed, 67 insertions(+) diff --git a/src/Reflection/Annotations/AnnotationPropertyReflection.php b/src/Reflection/Annotations/AnnotationPropertyReflection.php index 1b45fb83ec..a79ffaaf3b 100644 --- a/src/Reflection/Annotations/AnnotationPropertyReflection.php +++ b/src/Reflection/Annotations/AnnotationPropertyReflection.php @@ -124,6 +124,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Reflection/Dummy/ChangedTypePropertyReflection.php b/src/Reflection/Dummy/ChangedTypePropertyReflection.php index 65c7fff11c..b43cf53453 100644 --- a/src/Reflection/Dummy/ChangedTypePropertyReflection.php +++ b/src/Reflection/Dummy/ChangedTypePropertyReflection.php @@ -121,6 +121,11 @@ public function isFinalByKeyword(): TrinaryLogic return $this->reflection->isFinalByKeyword(); } + public function isFinal(): TrinaryLogic + { + return $this->reflection->isFinal(); + } + public function isVirtual(): TrinaryLogic { return $this->reflection->isVirtual(); diff --git a/src/Reflection/Dummy/DummyPropertyReflection.php b/src/Reflection/Dummy/DummyPropertyReflection.php index 3625413d62..c90f546344 100644 --- a/src/Reflection/Dummy/DummyPropertyReflection.php +++ b/src/Reflection/Dummy/DummyPropertyReflection.php @@ -121,6 +121,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Reflection/ExtendedPropertyReflection.php b/src/Reflection/ExtendedPropertyReflection.php index 3ec35218b7..ad8b5a26d8 100644 --- a/src/Reflection/ExtendedPropertyReflection.php +++ b/src/Reflection/ExtendedPropertyReflection.php @@ -40,6 +40,8 @@ public function isAbstract(): TrinaryLogic; public function isFinalByKeyword(): TrinaryLogic; + public function isFinal(): TrinaryLogic; + public function isVirtual(): TrinaryLogic; /** diff --git a/src/Reflection/Php/EnumPropertyReflection.php b/src/Reflection/Php/EnumPropertyReflection.php index bcddaab569..891491a1c5 100644 --- a/src/Reflection/Php/EnumPropertyReflection.php +++ b/src/Reflection/Php/EnumPropertyReflection.php @@ -117,6 +117,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Reflection/Php/SimpleXMLElementProperty.php b/src/Reflection/Php/SimpleXMLElementProperty.php index 5a42996565..6fb9316fb5 100644 --- a/src/Reflection/Php/SimpleXMLElementProperty.php +++ b/src/Reflection/Php/SimpleXMLElementProperty.php @@ -132,6 +132,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Reflection/Php/UniversalObjectCrateProperty.php b/src/Reflection/Php/UniversalObjectCrateProperty.php index eece81b69a..924b249e6f 100644 --- a/src/Reflection/Php/UniversalObjectCrateProperty.php +++ b/src/Reflection/Php/UniversalObjectCrateProperty.php @@ -122,6 +122,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Reflection/ResolvedPropertyReflection.php b/src/Reflection/ResolvedPropertyReflection.php index 240b04da6d..8b54c0785a 100644 --- a/src/Reflection/ResolvedPropertyReflection.php +++ b/src/Reflection/ResolvedPropertyReflection.php @@ -179,6 +179,11 @@ public function isFinalByKeyword(): TrinaryLogic return $this->reflection->isFinalByKeyword(); } + public function isFinal(): TrinaryLogic + { + return $this->reflection->isFinal(); + } + public function isVirtual(): TrinaryLogic { return $this->reflection->isVirtual(); diff --git a/src/Reflection/Type/IntersectionTypePropertyReflection.php b/src/Reflection/Type/IntersectionTypePropertyReflection.php index 99535e2851..1e3e4d3179 100644 --- a/src/Reflection/Type/IntersectionTypePropertyReflection.php +++ b/src/Reflection/Type/IntersectionTypePropertyReflection.php @@ -153,6 +153,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinalByKeyword()); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinal()); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::lazyMaxMin($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isVirtual()); diff --git a/src/Reflection/Type/UnionTypePropertyReflection.php b/src/Reflection/Type/UnionTypePropertyReflection.php index 3fb2a66aa9..1862d3059f 100644 --- a/src/Reflection/Type/UnionTypePropertyReflection.php +++ b/src/Reflection/Type/UnionTypePropertyReflection.php @@ -153,6 +153,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinalByKeyword()); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isFinal()); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::lazyExtremeIdentity($this->properties, static fn (ExtendedPropertyReflection $propertyReflection): TrinaryLogic => $propertyReflection->isVirtual()); diff --git a/src/Reflection/WrappedExtendedPropertyReflection.php b/src/Reflection/WrappedExtendedPropertyReflection.php index b3072a8cd8..ffb2a34363 100644 --- a/src/Reflection/WrappedExtendedPropertyReflection.php +++ b/src/Reflection/WrappedExtendedPropertyReflection.php @@ -114,6 +114,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Rules/Properties/FoundPropertyReflection.php b/src/Rules/Properties/FoundPropertyReflection.php index b675bd7297..b1a890b6be 100644 --- a/src/Rules/Properties/FoundPropertyReflection.php +++ b/src/Rules/Properties/FoundPropertyReflection.php @@ -148,6 +148,11 @@ public function isFinalByKeyword(): TrinaryLogic return $this->originalPropertyReflection->isFinalByKeyword(); } + public function isFinal(): TrinaryLogic + { + return $this->originalPropertyReflection->isFinal(); + } + public function isVirtual(): TrinaryLogic { return $this->originalPropertyReflection->isVirtual(); diff --git a/src/Rules/RestrictedUsage/RewrittenDeclaringClassPropertyReflection.php b/src/Rules/RestrictedUsage/RewrittenDeclaringClassPropertyReflection.php index 69eb2dbf4f..d6b0424be9 100644 --- a/src/Rules/RestrictedUsage/RewrittenDeclaringClassPropertyReflection.php +++ b/src/Rules/RestrictedUsage/RewrittenDeclaringClassPropertyReflection.php @@ -73,6 +73,11 @@ public function isAbstract(): TrinaryLogic return $this->propertyReflection->isAbstract(); } + public function isFinalByKeyword(): TrinaryLogic + { + return $this->propertyReflection->isFinalByKeyword(); + } + public function isFinal(): TrinaryLogic { return $this->propertyReflection->isFinal(); diff --git a/src/Type/ObjectShapePropertyReflection.php b/src/Type/ObjectShapePropertyReflection.php index e97fe62edf..1d022cb726 100644 --- a/src/Type/ObjectShapePropertyReflection.php +++ b/src/Type/ObjectShapePropertyReflection.php @@ -119,6 +119,11 @@ public function isFinalByKeyword(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isFinal(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isVirtual(): TrinaryLogic { return TrinaryLogic::createNo(); From 0742cb393e5a3043aaba79730f89bfa4e5cae437 Mon Sep 17 00:00:00 2001 From: Luke Diggins Date: Wed, 14 May 2025 22:52:56 +1000 Subject: [PATCH 8/8] Rename property.parentPropertyFinalByPhpDoc and make it ignorable --- src/Rules/Properties/OverridingPropertyRule.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Rules/Properties/OverridingPropertyRule.php b/src/Rules/Properties/OverridingPropertyRule.php index 8b50f18893..8f4e23a861 100644 --- a/src/Rules/Properties/OverridingPropertyRule.php +++ b/src/Rules/Properties/OverridingPropertyRule.php @@ -152,8 +152,7 @@ public function processNode(Node $node, Scope $scope): array $node->getName(), $prototype->getDeclaringClass()->getDisplayName(), $node->getName(), - ))->identifier('property.parentPropertyAnnotatedFinal') - ->nonIgnorable() + ))->identifier('property.parentPropertyFinalByPhpDoc') ->build(); }