Cannot unset property which might get hooked in subclass.

Author: staabm

src/Rules/Variables/UnsetRule.php

@@ -4,6 +4,7 @@
 
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
+use PHPStan\Php\PhpVersion;
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\Rules\Properties\PropertyReflectionFinder;
 use PHPStan\Rules\Rule;
@@ -20,6 +21,7 @@ final class UnsetRule implements Rule
 
 	public function __construct(
 		private PropertyReflectionFinder $propertyReflectionFinder,
+		private PhpVersion $phpVersion,
 	)
 	{
 	}
@@ -115,6 +117,23 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError
 					->line($node->getStartLine())
 					->identifier('unset.hookedProperty')
 					->build();
+			} elseif ($this->phpVersion->supportsPropertyHooks()) {
+				if (
+					!$propertyReflection->isPrivate()
+					&& !$propertyReflection->isFinal()->yes()
+					&& !$propertyReflection->getDeclaringClass()->isFinal()
+				) {
+					return RuleErrorBuilder::message(
+						sprintf(
+							'Cannot unset %s::$%s property which might get hooked in subclass.',
+							$propertyReflection->getDeclaringClass()->getDisplayName(),
+							$foundPropertyReflection->getName(),
+						),
+					)
+						->line($node->getStartLine())
+						->identifier('unset.maybeHookedProperty')
+						->build();
+				}
 			}
 		}
 

tests/PHPStan/Rules/Variables/UnsetRuleTest.php

@@ -2,6 +2,7 @@
 
 namespace PHPStan\Rules\Variables;
 
+use PHPStan\Php\PhpVersion;
 use PHPStan\Rules\Properties\PropertyReflectionFinder;
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
@@ -15,7 +16,10 @@ class UnsetRuleTest extends RuleTestCase
 
 	protected function getRule(): Rule
 	{
-		return new UnsetRule(self::getContainer()->getByType(PropertyReflectionFinder::class));
+		return new UnsetRule(
+			self::getContainer()->getByType(PropertyReflectionFinder::class),
+			self::getContainer()->getByType(PhpVersion::class),
+		);
 	}
 
 	public function testUnsetRule(): void
@@ -56,7 +60,12 @@ public function testBug2752(): void
 
 	public function testBug4289(): void
 	{
-		$this->analyse([__DIR__ . '/data/bug-4289.php'], []);
+		$this->analyse([__DIR__ . '/data/bug-4289.php'], [
+			[
+				'Cannot unset Bug4289\BaseClass::$fields property which might get hooked in subclass.',
+				25,
+			],
+		]);
 	}
 
 	public function testBug5223(): void
@@ -96,6 +105,10 @@ public function testBug4565(): void
 	public function testBug12421(): void
 	{
 		$this->analyse([__DIR__ . '/data/bug-12421.php'], [
+			[
+				'Cannot unset Bug12421\RegularProperty::$y property which might get hooked in subclass.',
+				7,
+			],
 			[
 				'Cannot unset readonly Bug12421\NativeReadonlyClass::$y property.',
 				11,
@@ -146,6 +159,10 @@ public function testUnsetHookedProperty(): void
 				'Cannot unset hooked UnsetHookedProperty\Foo::$iii property.',
 				10,
 			],
+			[
+				'Cannot unset UnsetHookedProperty\NonFinalClass::$publicProperty property which might get hooked in subclass.',
+				13,
+			],
 		]);
 	}
 

tests/PHPStan/Rules/Variables/data/unset-hooked-property.php

@@ -2,12 +2,18 @@
 
 namespace UnsetHookedProperty;
 
-function doUnset(Foo $foo, User $user): void {
+function doUnset(Foo $foo, User $user, NonFinalClass $nonFinalClass, FinalClass $finalClass): void {
 	unset($user->name);
 	unset($user->fullName);
 
 	unset($foo->ii);
 	unset($foo->iii);
+
+	unset($nonFinalClass->publicFinalProperty);
+	unset($nonFinalClass->publicProperty);
+
+	unset($finalClass->publicFinalProperty);
+	unset($finalClass->publicProperty);
 }
 
 class User
@@ -39,3 +45,22 @@ abstract class Foo
 	abstract public int $iii { get; }
 }
 
+class NonFinalClass {
+	private string $privateProperty;
+	public string $publicProperty;
+	final public string $publicFinalProperty;
+
+	function doFoo() {
+		unset($this->privateProperty);
+	}
+}
+
+final class FinalClass {
+	private string $privateProperty;
+	public string $publicProperty;
+	final public string $publicFinalProperty;
+
+	function doFoo() {
+		unset($this->privateProperty);
+	}
+}