Fix MissingReturnRule for property hooks

Author: ondrejmirtes

src/Analyser/NodeScopeResolver.php

@@ -114,6 +114,7 @@
 use PHPStan\Node\MethodReturnStatementsNode;
 use PHPStan\Node\NoopExpressionNode;
 use PHPStan\Node\PropertyAssignNode;
+use PHPStan\Node\PropertyHookStatementNode;
 use PHPStan\Node\ReturnStatement;
 use PHPStan\Node\StaticMethodCallableNode;
 use PHPStan\Node\UnreachableStatementNode;
@@ -343,6 +344,7 @@ public function processStmtNodes(
 		$stmtCount = count($stmts);
 		$shouldCheckLastStatement = $parentNode instanceof Node\Stmt\Function_
 			|| $parentNode instanceof Node\Stmt\ClassMethod
+			|| $parentNode instanceof PropertyHookStatementNode
 			|| $parentNode instanceof Expr\Closure;
 		foreach ($stmts as $i => $stmt) {
 			if ($alreadyTerminated && !($stmt instanceof Node\Stmt\Function_ || $stmt instanceof Node\Stmt\ClassLike)) {
@@ -360,7 +362,7 @@ public function processStmtNodes(
 			$hasYield = $hasYield || $statementResult->hasYield();
 
 			if ($shouldCheckLastStatement && $isLast) {
-				/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|Expr\Closure $parentNode */
+				/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|PropertyHookStatementNode|Expr\Closure $parentNode */
 				$parentNode = $parentNode;
 
 				$endStatements = $statementResult->getEndStatements();
@@ -377,7 +379,7 @@ public function processStmtNodes(
 								$endStatementResult->getThrowPoints(),
 								$endStatementResult->getImpurePoints(),
 							),
-							$parentNode->returnType !== null,
+							$parentNode->getReturnType() !== null,
 						), $endStatementResult->getScope());
 					}
 				} else {
@@ -391,7 +393,7 @@ public function processStmtNodes(
 							$statementResult->getThrowPoints(),
 							$statementResult->getImpurePoints(),
 						),
-						$parentNode->returnType !== null,
+						$parentNode->getReturnType() !== null,
 					), $scope);
 				}
 			}
@@ -414,9 +416,9 @@ public function processStmtNodes(
 
 		$statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints, $impurePoints);
 		if ($stmtCount === 0 && $shouldCheckLastStatement) {
-			/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|Expr\Closure $parentNode */
+			/** @var Node\Stmt\Function_|Node\Stmt\ClassMethod|PropertyHookStatementNode|Expr\Closure $parentNode */
 			$parentNode = $parentNode;
-			$returnTypeNode = $parentNode->returnType;
+			$returnTypeNode = $parentNode->getReturnType();
 			if ($parentNode instanceof Expr\Closure) {
 				$parentNode = new Node\Stmt\Expression($parentNode, $parentNode->getAttributes());
 			}
@@ -4700,7 +4702,7 @@ private function processPropertyHooks(
 				$this->processExprNode($stmt, $hook->body, $hookScope, $nodeCallback, ExpressionContext::createTopLevel());
 				$nodeCallback(new PropertyAssignNode(new PropertyFetch(new Variable('this'), $propertyName, $hook->body->getAttributes()), $hook->body, false), $hookScope);
 			} elseif (is_array($hook->body)) {
-				$this->processStmtNodes($stmt, $hook->body, $hookScope, $nodeCallback, StatementContext::createTopLevel());
+				$this->processStmtNodes(new PropertyHookStatementNode($hook), $hook->body, $hookScope, $nodeCallback, StatementContext::createTopLevel());
 			}
 
 		}

src/Node/PropertyHookStatementNode.php

@@ -0,0 +1,52 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node;
+
+use PhpParser\Node\PropertyHook;
+use PhpParser\Node\Stmt;
+
+/**
+ * This class exists because PhpParser\Node\PropertyHook
+ * is not a Stmt, but we need to pass it to
+ * a few places that expect a Stmt.
+ *
+ * This is because PhpParser\Node\PropertyHook
+ * is likely the one of two PhpParser nodes which contains Stmt[]
+ * but itself is not a Stmt.
+ *
+ * The other one is Expr\Closure, but that one can
+ * at least be wrapped in Stmt\Expression.
+ */
+final class PropertyHookStatementNode extends Stmt implements VirtualNode
+{
+
+	public function __construct(private PropertyHook $propertyHook)
+	{
+		parent::__construct($this->propertyHook->getAttributes());
+	}
+
+	public function getPropertyHook(): PropertyHook
+	{
+		return $this->propertyHook;
+	}
+
+	/**
+	 * @return null
+	 */
+	public function getReturnType()
+	{
+		return null;
+	}
+
+	public function getType(): string
+	{
+		return 'PHPStan_Node_PropertyHookStatementNode';
+	}
+
+	public function getSubNodeNames(): array
+	{
+		return [];
+	}
+
+
+}

src/Rules/Missing/MissingReturnRule.php

@@ -6,7 +6,7 @@
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
 use PHPStan\Node\ExecutionEndNode;
-use PHPStan\Reflection\MethodReflection;
+use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\ShouldNotHappenException;
@@ -19,6 +19,7 @@
 use PHPStan\Type\VerbosityLevel;
 use PHPStan\Type\VoidType;
 use function sprintf;
+use function ucfirst;
 
 /**
  * @implements Rule<ExecutionEndNode>
@@ -55,8 +56,12 @@ public function processNode(Node $node, Scope $scope): array
 			}
 		} elseif ($scopeFunction !== null) {
 			$returnType = $scopeFunction->getReturnType();
-			if ($scopeFunction instanceof MethodReflection) {
-				$description = sprintf('Method %s::%s()', $scopeFunction->getDeclaringClass()->getDisplayName(), $scopeFunction->getName());
+			if ($scopeFunction instanceof PhpMethodFromParserNodeReflection) {
+				if (!$scopeFunction->isPropertyHook()) {
+					$description = sprintf('Method %s::%s()', $scopeFunction->getDeclaringClass()->getDisplayName(), $scopeFunction->getName());
+				} else {
+					$description = sprintf('%s hook for property %s::$%s', ucfirst($scopeFunction->getPropertyHookName()), $scopeFunction->getDeclaringClass()->getDisplayName(), $scopeFunction->getHookedPropertyName());
+				}
 			} else {
 				$description = sprintf('Function %s()', $scopeFunction->getName());
 			}

tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php

@@ -351,4 +351,23 @@ public function testBug9374(): void
 		$this->analyse([__DIR__ . '/data/bug-9374.php'], []);
 	}
 
+	public function testPropertyHooks(): void
+	{
+		if (PHP_VERSION_ID < 80400) {
+			$this->markTestSkipped('Test requires PHP 8.4.');
+		}
+
+		$this->checkExplicitMixedMissingReturn = true;
+		$this->analyse([__DIR__ . '/data/property-hooks-missing-return.php'], [
+			[
+				'Get hook for property PropertyHooksMissingReturn\Foo::$i should return int but return statement is missing.',
+				10,
+			],
+			[
+				'Get hook for property PropertyHooksMissingReturn\Foo::$j should return int but return statement is missing.',
+				23,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Missing/data/property-hooks-missing-return.php

@@ -0,0 +1,34 @@
+<?php // lint >= 8.4
+
+namespace PropertyHooksMissingReturn;
+
+class Foo
+{
+
+	public int $i {
+		get {
+			if (rand(0, 1)) {
+
+			} else {
+				return 1;
+			}
+		}
+
+		set {
+			// set hook returns void
+		}
+	}
+
+	public int $j {
+		get {
+
+		}
+	}
+
+	public int $ok {
+		get {
+			return $this->ok + 1;
+		}
+	}
+
+}