Adjust method's ReturnTypeRule for property hooks

Author: ondrejmirtes

src/Reflection/Php/PhpMethodFromParserNodeReflection.php

@@ -164,6 +164,38 @@ public function getName(): string
 		return sprintf('$%s::%s', $this->hookForProperty, $function->name->toString());
 	}
 
+	/**
+	 * @phpstan-assert-if-true !null $this->getHookedPropertyName()
+	 * @phpstan-assert-if-true !null $this->getPropertyHookName()
+	 */
+	public function isPropertyHook(): bool
+	{
+		return $this->hookForProperty !== null;
+	}
+
+	public function getHookedPropertyName(): ?string
+	{
+		return $this->hookForProperty;
+	}
+
+	/**
+	 * @return 'get'|'set'|null
+	 */
+	public function getPropertyHookName(): ?string
+	{
+		$function = $this->getFunctionLike();
+		if (!$function instanceof Node\PropertyHook) {
+			return null;
+		}
+
+		$name = $function->name->toLowerString();
+		if (!in_array($name, ['get', 'set'], true)) {
+			throw new ShouldNotHappenException(sprintf('Unknown property hook: %s', $name));
+		}
+
+		return $name;
+	}
+
 	public function isStatic(): bool
 	{
 		$method = $this->getClassMethod();

src/Rules/Methods/ReturnTypeRule.php

@@ -21,6 +21,7 @@
 use function count;
 use function sprintf;
 use function strtolower;
+use function ucfirst;
 
 /**
  * @implements Rule<Node\Stmt\Return_>
@@ -52,31 +53,38 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
+		if ($method->isPropertyHook()) {
+			$methodDescription = sprintf(
+				'%s hook for property %s::$%s',
+				ucfirst($method->getPropertyHookName()),
+				$method->getDeclaringClass()->getDisplayName(),
+				$method->getHookedPropertyName(),
+			);
+		} else {
+			$methodDescription = sprintf('Method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName());
+		}
+
 		$returnType = $method->getReturnType();
 		$errors = $this->returnTypeCheck->checkReturnType(
 			$scope,
 			$returnType,
 			$node->expr,
 			$node,
 			sprintf(
-				'Method %s::%s() should return %%s but empty return statement found.',
-				$method->getDeclaringClass()->getDisplayName(),
-				$method->getName(),
+				'%s should return %%s but empty return statement found.',
+				$methodDescription,
 			),
 			sprintf(
-				'Method %s::%s() with return type void returns %%s but should not return anything.',
-				$method->getDeclaringClass()->getDisplayName(),
-				$method->getName(),
+				'%s with return type void returns %%s but should not return anything.',
+				$methodDescription,
 			),
 			sprintf(
-				'Method %s::%s() should return %%s but returns %%s.',
-				$method->getDeclaringClass()->getDisplayName(),
-				$method->getName(),
+				'%s should return %%s but returns %%s.',
+				$methodDescription,
 			),
 			sprintf(
-				'Method %s::%s() should never return but return statement found.',
-				$method->getDeclaringClass()->getDisplayName(),
-				$method->getName(),
+				'%s should never return but return statement found.',
+				$methodDescription,
 			),
 			$method->isGenerator(),
 		);

tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php

@@ -1101,4 +1101,41 @@ public function testBug12223(): void
 		$this->analyse([__DIR__ . '/data/bug-12223.php'], []);
 	}
 
+	public function testPropertyHooks(): void
+	{
+		if (PHP_VERSION_ID < 80400) {
+			self::markTestSkipped('Test requires PHP 8.4.');
+		}
+
+		$this->analyse([__DIR__ . '/data/property-hooks-return.php'], [
+			[
+				'Get hook for property PropertyHooksReturn\Foo::$i should return int but returns string.',
+				11,
+			],
+			[
+				'Set hook for property PropertyHooksReturn\Foo::$i with return type void returns int but should not return anything.',
+				21,
+			],
+			[
+				'Get hook for property PropertyHooksReturn\Foo::$s should return non-empty-string but returns \'\'.',
+				29,
+			],
+			[
+				'Get hook for property PropertyHooksReturn\GenericFoo::$a should return T of PropertyHooksReturn\Foo but returns PropertyHooksReturn\Foo.',
+				48,
+				'Type PropertyHooksReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+			[
+				'Get hook for property PropertyHooksReturn\GenericFoo::$b should return T of PropertyHooksReturn\Foo but returns PropertyHooksReturn\Foo.',
+				63,
+				'Type PropertyHooksReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+			[
+				'Get hook for property PropertyHooksReturn\GenericFoo::$c should return T of PropertyHooksReturn\Foo but returns PropertyHooksReturn\Foo.',
+				73,
+				'Type PropertyHooksReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Methods/data/property-hooks-return.php

@@ -0,0 +1,83 @@
+<?php // lint >= 8.4
+
+namespace PropertyHooksReturn;
+
+class Foo
+{
+
+	public int $i {
+		get {
+			if (rand(0, 1)) {
+				return 'foo';
+			}
+
+			return 1;
+		}
+		set {
+			if (rand(0, 1)) {
+				return;
+			}
+
+			return 1;
+		}
+	}
+
+	/** @var non-empty-string */
+	public string $s {
+		get {
+			if (rand(0, 1)) {
+				return '';
+			}
+
+			return 'foo';
+		}
+	}
+
+}
+
+/**
+ * @template T of Foo
+ */
+class GenericFoo
+{
+
+	/** @var T */
+	public Foo $a {
+		get {
+			if (rand(0, 1)) {
+				return new Foo();
+			}
+
+			return $this->a;
+		}
+	}
+
+	/**
+	 * @param T $c
+	 */
+	public function __construct(
+		/** @var T */
+		public Foo $b {
+			get {
+				if (rand(0, 1)) {
+					return new Foo();
+				}
+
+				return $this->b;
+			}
+		},
+
+		public Foo $c {
+			get {
+				if (rand(0, 1)) {
+					return new Foo();
+				}
+
+				return $this->c;
+			}
+		}
+	)
+	{
+	}
+
+}