fix: check magic methods on final classes for allow dynamic properties

Author: calebdw

src/Reflection/ClassReflection.php

@@ -410,33 +410,6 @@ public function allowsDynamicProperties(): bool
 			return true;
 		}
 
-		if ($this->isReadOnly()) {
-			return false;
-		}
-
-		if (UniversalObjectCratesClassReflectionExtension::isUniversalObjectCrate(
-			$this->reflectionProvider,
-			$this,
-		)) {
-			return true;
-		}
-
-		$class = $this;
-		$attributes = $class->reflection->getAttributes('AllowDynamicProperties');
-		while (count($attributes) === 0 && $class->getParentClass() !== null) {
-			$attributes = $class->getParentClass()->reflection->getAttributes('AllowDynamicProperties');
-			$class = $class->getParentClass();
-		}
-
-		return count($attributes) > 0;
-	}
-
-	private function allowsDynamicPropertiesExtensions(): bool
-	{
-		if ($this->allowsDynamicProperties()) {
-			return true;
-		}
-
 		$hasMagicMethod = $this->hasNativeMethod('__get') || $this->hasNativeMethod('__set') || $this->hasNativeMethod('__isset');
 		if ($hasMagicMethod) {
 			return true;
@@ -449,18 +422,31 @@ private function allowsDynamicPropertiesExtensions(): bool
 			}
 
 			$reflection = $type->getClassReflection();
-			if ($reflection === null) {
+			if ($reflection === null || !$reflection->allowsDynamicProperties()) {
 				continue;
 			}
 
-			if (!$reflection->allowsDynamicPropertiesExtensions()) {
-				continue;
-			}
+			return true;
+		}
+
+		if ($this->isReadOnly()) {
+			return false;
+		}
 
+		if (UniversalObjectCratesClassReflectionExtension::isUniversalObjectCrate(
+			$this->reflectionProvider,
+			$this,
+		)) {
 			return true;
 		}
 
-		return false;
+		$class = $this;
+		do {
+			$attributes = $class->reflection->getAttributes('AllowDynamicProperties');
+			$class = $class->getParentClass();
+		} while ($attributes === [] && $class !== null);
+
+		return $attributes !== [];
 	}
 
 	public function hasProperty(string $propertyName): bool
@@ -474,7 +460,7 @@ public function hasProperty(string $propertyName): bool
 		}
 
 		foreach ($this->propertiesClassReflectionExtensions as $i => $extension) {
-			if ($i > 0 && !$this->allowsDynamicPropertiesExtensions()) {
+			if ($i > 0 && !$this->allowsDynamicProperties()) {
 				break;
 			}
 			if ($extension->hasProperty($this, $propertyName)) {
@@ -656,7 +642,7 @@ public function getProperty(string $propertyName, ClassMemberAccessAnswerer $sco
 
 		if (!isset($this->properties[$key])) {
 			foreach ($this->propertiesClassReflectionExtensions as $i => $extension) {
-				if ($i > 0 && !$this->allowsDynamicPropertiesExtensions()) {
+				if ($i > 0 && !$this->allowsDynamicProperties()) {
 					break;
 				}
 

tests/PHPStan/Analyser/nsrt/bug-13450.php

@@ -0,0 +1,72 @@
+<?php declare(strict_types = 1);
+
+namespace Bug13450;
+
+use function PHPStan\Testing\assertType;
+
+/**
+ * @template TRelated of Model
+ * @template TDeclaring of Model
+ * @template TResult
+ */
+abstract class Relation
+{
+	/** @return TResult */
+    public function getResults(): mixed
+	{
+		return []; // @phpstan-ignore return.type
+	}
+}
+
+/**
+ * @template TRelated of Model
+ * @template TDeclaring of Model
+ * @template TPivot of Pivot = Pivot
+ * @template TAccessor of string = 'pivot'
+ *
+ * @extends Relation<TRelated, TDeclaring, array<int, TRelated&object{pivot: TPivot}>>
+ */
+class BelongsToMany extends Relation {}
+
+abstract class Model
+{
+	/**
+	 * @template TRelated of Model
+	 * @param class-string<TRelated> $related
+	 * @return BelongsToMany<TRelated, $this>
+	 */
+	public function belongsToMany(string $related): BelongsToMany
+	{
+		return new BelongsToMany(); // @phpstan-ignore return.type
+	}
+
+	public function __get(string $name): mixed { return null; }
+	public function __set(string $name, mixed $value): void {}
+}
+
+class Pivot extends Model {}
+
+class User extends Model
+{
+	/** @return BelongsToMany<Team, $this> */
+	public function teams(): BelongsToMany
+	{
+		return $this->belongsToMany(Team::class);
+	}
+
+	/** @return BelongsToMany<TeamFinal, $this> */
+	public function teamsFinal(): BelongsToMany
+	{
+		return $this->belongsToMany(TeamFinal::class);
+	}
+}
+
+class Team extends Model {}
+
+final class TeamFinal extends Model {}
+
+function test(User $user): void
+{
+	assertType('array<int, Bug13450\Team&object{pivot: Bug13450\Pivot}>', $user->teams()->getResults());
+	assertType('array<int, Bug13450\TeamFinal&object{pivot: Bug13450\Pivot}>', $user->teamsFinal()->getResults());
+}

tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php

@@ -821,23 +821,28 @@ public function testPhp82AndDynamicProperties(bool $b): void
 				34,
 				$tipText,
 			];
-			$errors[] = [
-				'Access to an undefined property Php82DynamicProperties\HelloWorld::$world.',
-				71,
-				$tipText,
-			];
 			if ($b) {
+				$errors[] = [
+					'Access to an undefined property Php82DynamicProperties\HelloWorld::$world.',
+					71,
+					$tipText,
+				];
 				$errors[] = [
 					'Access to an undefined property Php82DynamicProperties\HelloWorld::$world.',
 					78,
 					$tipText,
 				];
+				$errors[] = [
+					'Access to an undefined property Php82DynamicProperties\FinalHelloWorld::$world.',
+					112,
+					$tipText,
+				];
+				$errors[] = [
+					'Access to an undefined property Php82DynamicProperties\ReadonlyWithMagic::$foo.',
+					133,
+					$tipText,
+				];
 			}
-			$errors[] = [
-				'Access to an undefined property Php82DynamicProperties\FinalHelloWorld::$world.',
-				112,
-				$tipText,
-			];
 		} elseif ($b) {
 			$errors[] = [
 				'Access to an undefined property Php82DynamicProperties\HelloWorld::$world.',

tests/PHPStan/Rules/Properties/data/php-82-dynamic-properties.php

@@ -114,3 +114,24 @@ function (): void {
 		echo $hello->world;
 	}
 };
+
+readonly class ReadonlyWithMagic
+{
+	public function __set(string $name, mixed $value): void
+	{
+		var_dump('here');
+	}
+
+	public function __get(string $name): mixed
+	{
+		return 1;
+	}
+}
+
+function (): void {
+	$class = new ReadonlyWithMagic();
+	if(isset($class->foo))
+	{
+		echo $class->foo;
+	}
+};