Detect generators by scanning the body for yield, including unreachable code and excluding nested functions

src/Analyser/NodeScopeResolver.php

@@ -115,6 +115,7 @@
 use PHPStan\Parser\ImmediatelyInvokedClosureVisitor;
 use PHPStan\Parser\LineAttributesVisitor;
 use PHPStan\Parser\Parser;
+use PHPStan\Parser\YieldFindingVisitor;
 use PHPStan\PhpDoc\PhpDocInheritanceResolver;
 use PHPStan\PhpDoc\ResolvedPhpDocBlock;
 use PHPStan\PhpDoc\Tag\VarTag;
@@ -551,6 +552,11 @@ private function processStmtNodesInternalWithoutFlushingPendingFibers(
 			|| $parentNode instanceof PropertyHookStatementNode
 			|| $parentNode instanceof Expr\Closure;
 
+		// A `yield` anywhere in the body makes the function a generator, even when the
+		// `yield` is unreachable. $hasYield only tracks reachable yields, so it cannot be
+		// used to decide whether a missing return is acceptable for a generator.
+		$bodyHasYield = $shouldCheckLastStatement && YieldFindingVisitor::findInNodes($stmts) !== [];
+
 		foreach ($stmts as $i => $stmt) {
 			if ($alreadyTerminated && !($stmt instanceof Node\Stmt\Function_ || $stmt instanceof Node\Stmt\ClassLike || $stmt instanceof Node\Stmt\Label)) {
 				continue;
@@ -617,7 +623,7 @@ private function processStmtNodesInternalWithoutFlushingPendingFibers(
 							$endStatement->getStatement(),
 							(new InternalStatementResult(
 								$endStatementResult->getScope(),
-								$hasYield,
+								$hasYield || $bodyHasYield,
 								$endStatementResult->isAlwaysTerminating(),
 								$endStatementResult->getExitPoints(),
 								$endStatementResult->getThrowPoints(),
@@ -631,7 +637,7 @@ private function processStmtNodesInternalWithoutFlushingPendingFibers(
 						$stmt,
 						(new InternalStatementResult(
 							$scope,
-							$hasYield,
+							$hasYield || $bodyHasYield,
 							$statementResult->isAlwaysTerminating(),
 							$statementResult->getExitPoints(),
 							$statementResult->getThrowPoints(),
@@ -805,10 +811,10 @@ public function processStmtNode(
 			$this->callNodeCallback($nodeCallback, new InFunctionNode($functionReflection, $stmt), $functionScope, $storage);
 
 			$gatheredReturnStatements = [];
-			$gatheredYieldStatements = [];
+			$gatheredYieldStatements = YieldFindingVisitor::findInNodes($stmt->stmts);
 			$executionEnds = [];
 			$functionImpurePoints = [];
-			$statementResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $functionScope, $storage, static function (Node $node, Scope $scope) use ($nodeCallback, $functionScope, &$gatheredReturnStatements, &$gatheredYieldStatements, &$executionEnds, &$functionImpurePoints): void {
+			$statementResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $functionScope, $storage, static function (Node $node, Scope $scope) use ($nodeCallback, $functionScope, &$gatheredReturnStatements, &$executionEnds, &$functionImpurePoints): void {
 				$nodeCallback($node, $scope);
 				if ($scope->getFunction() !== $functionScope->getFunction()) {
 					return;
@@ -830,9 +836,6 @@ public function processStmtNode(
 					$executionEnds[] = $node;
 					return;
 				}
-				if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
-					$gatheredYieldStatements[] = $node;
-				}
 				if (!$node instanceof Return_) {
 					return;
 				}
@@ -954,10 +957,10 @@ public function processStmtNode(
 
 			if ($stmt->stmts !== null) {
 				$gatheredReturnStatements = [];
-				$gatheredYieldStatements = [];
+				$gatheredYieldStatements = YieldFindingVisitor::findInNodes($stmt->stmts);
 				$executionEnds = [];
 				$methodImpurePoints = [];
-				$statementResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $methodScope, $storage, static function (Node $node, Scope $scope) use ($nodeCallback, $methodScope, &$gatheredReturnStatements, &$gatheredYieldStatements, &$executionEnds, &$methodImpurePoints): void {
+				$statementResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $methodScope, $storage, static function (Node $node, Scope $scope) use ($nodeCallback, $methodScope, &$gatheredReturnStatements, &$executionEnds, &$methodImpurePoints): void {
 					$nodeCallback($node, $scope);
 					if ($scope->getFunction() !== $methodScope->getFunction()) {
 						return;
@@ -988,9 +991,6 @@ public function processStmtNode(
 						$executionEnds[] = $node;
 						return;
 					}
-					if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
-						$gatheredYieldStatements[] = $node;
-					}
 					if (!$node instanceof Return_) {
 						return;
 					}
@@ -2993,10 +2993,10 @@ public function processClosureNode(
 
 		$executionEnds = [];
 		$gatheredReturnStatements = [];
-		$gatheredYieldStatements = [];
+		$gatheredYieldStatements = YieldFindingVisitor::findInNodes($expr->stmts);
 		$closureImpurePoints = [];
 		$invalidateExpressions = [];
-		$closureStmtsCallback = static function (Node $node, Scope $scope) use ($nodeCallback, &$executionEnds, &$gatheredReturnStatements, &$gatheredYieldStatements, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
+		$closureStmtsCallback = static function (Node $node, Scope $scope) use ($nodeCallback, &$executionEnds, &$gatheredReturnStatements, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
 			$nodeCallback($node, $scope);
 			if ($scope->getAnonymousFunctionReflection() !== $closureScope->getAnonymousFunctionReflection()) {
 				return;
@@ -3020,9 +3020,6 @@ public function processClosureNode(
 				$invalidateExpressions[] = $node;
 				return;
 			}
-			if ($node instanceof Expr\Yield_ || $node instanceof Expr\YieldFrom) {
-				$gatheredYieldStatements[] = $node;
-			}
 			if (!$node instanceof Return_) {
 				return;
 			}

src/Parser/YieldFindingVisitor.php

@@ -0,0 +1,52 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Parser;
+
+use Override;
+use PhpParser\Node;
+use PhpParser\Node\Expr\Yield_;
+use PhpParser\Node\Expr\YieldFrom;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitor;
+use PhpParser\NodeVisitorAbstract;
+
+/**
+ * Collects all `yield`/`yield from` expressions that belong to a single function-like
+ * (a function/method/closure makes itself a generator), regardless of whether they are
+ * reachable. Nested function-likes are not descended into, because a `yield` inside a
+ * nested closure makes the nested closure a generator, not the outer scope.
+ */
+final class YieldFindingVisitor extends NodeVisitorAbstract
+{
+
+	/** @var list<Yield_|YieldFrom> */
+	private array $yieldNodes = [];
+
+	#[Override]
+	public function enterNode(Node $node): ?int
+	{
+		if ($node instanceof Node\FunctionLike) {
+			return NodeVisitor::DONT_TRAVERSE_CHILDREN;
+		}
+
+		if ($node instanceof Yield_ || $node instanceof YieldFrom) {
+			$this->yieldNodes[] = $node;
+		}
+
+		return null;
+	}
+
+	/**
+	 * @param Node[] $nodes
+	 * @return list<Yield_|YieldFrom>
+	 */
+	public static function findInNodes(array $nodes): array
+	{
+		$visitor = new self();
+		$traverser = new NodeTraverser($visitor);
+		$traverser->traverse($nodes);
+
+		return $visitor->yieldNodes;
+	}
+
+}

src/Reflection/Php/PhpFunctionFromParserNodeReflection.php

@@ -7,6 +7,7 @@
 use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Function_;
+use PHPStan\Parser\YieldFindingVisitor;
 use PHPStan\Reflection\Assertions;
 use PHPStan\Reflection\AttributeReflection;
 use PHPStan\Reflection\ExtendedFunctionVariant;
@@ -22,7 +23,6 @@
 use PHPStan\Type\Type;
 use PHPStan\Type\TypehintHelper;
 use function array_reverse;
-use function is_array;
 use function is_string;
 use function strtolower;
 
@@ -271,45 +271,19 @@ public function isBuiltin(): bool
 
 	public function isGenerator(): bool
 	{
-		return $this->nodeIsOrContainsYield($this->functionLike);
+		$stmts = $this->functionLike->getStmts();
+		if ($stmts === null) {
+			return false;
+		}
+
+		return YieldFindingVisitor::findInNodes($stmts) !== [];
 	}
 
 	public function acceptsNamedArguments(): TrinaryLogic
 	{
 		return TrinaryLogic::createFromBoolean($this->acceptsNamedArguments);
 	}
 
-	private function nodeIsOrContainsYield(Node $node): bool
-	{
-		if ($node instanceof Node\Expr\Yield_) {
-			return true;
-		}
-
-		if ($node instanceof Node\Expr\YieldFrom) {
-			return true;
-		}
-
-		foreach ($node->getSubNodeNames() as $nodeName) {
-			$nodeProperty = $node->$nodeName;
-
-			if ($nodeProperty instanceof Node && $this->nodeIsOrContainsYield($nodeProperty)) {
-				return true;
-			}
-
-			if (!is_array($nodeProperty)) {
-				continue;
-			}
-
-			foreach ($nodeProperty as $nodePropertyArrayItem) {
-				if ($nodePropertyArrayItem instanceof Node && $this->nodeIsOrContainsYield($nodePropertyArrayItem)) {
-					return true;
-				}
-			}
-		}
-
-		return false;
-	}
-
 	public function getAsserts(): Assertions
 	{
 		return $this->assertions;

tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php

@@ -451,4 +451,16 @@ public function testBug13565(): void
 		]);
 	}
 
+	public function testBug4100(): void
+	{
+		$this->checkNullables = true;
+		$this->checkExplicitMixed = true;
+		$this->analyse([__DIR__ . '/data/bug-4100.php'], [
+			[
+				'Function Bug4100ReturnType\returnsStringWithNestedGenerator() should return int but returns string.',
+				12,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Functions/data/bug-4100.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types = 1);
+
+namespace Bug4100ReturnType;
+
+function returnsStringWithNestedGenerator(): int
+{
+	$inner = function (): \Generator {
+		yield 1;
+	};
+	$inner;
+
+	return 'hello';
+}

tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php

@@ -386,4 +386,10 @@ public function testBug5681(): void
 		$this->analyse([__DIR__ . '/data/bug-5681.php'], []);
 	}
 
+	public function testBug4100(): void
+	{
+		$this->checkExplicitMixedMissingReturn = true;
+		$this->analyse([__DIR__ . '/data/bug-4100.php'], []);
+	}
+
 }

tests/PHPStan/Rules/Missing/data/bug-4100.php

@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace Bug4100;
+
+use Generator;
+
+function foo(): Generator
+{
+	while (false) {
+		return;
+		yield 2;
+	}
+}
+
+function withThrow(): Generator
+{
+	if (rand(0, 1)) {
+		throw new \Exception();
+		yield 1;
+	}
+}
+
+class Foo
+{
+
+	public function method(): Generator
+	{
+		while (false) {
+			return;
+			yield 2;
+		}
+	}
+
+}
+
+function closures(): void
+{
+	$c = function (): Generator {
+		while (false) {
+			return;
+			yield 2;
+		}
+	};
+	$c;
+}