Skip function-like and anonymous class nodes when checking if a function or method is a generator

src/Parser/CleaningVisitor.php

@@ -3,102 +3,172 @@
 namespace PHPStan\Parser;
 
 use PhpParser\Node;
-use PhpParser\NodeFinder;
+use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitorAbstract;
 use PHPStan\Reflection\ParametersAcceptor;
+use PHPStan\ShouldNotHappenException;
+use function array_filter;
+use function array_map;
 use function in_array;
 use function is_array;
 
 final class CleaningVisitor extends NodeVisitorAbstract
 {
 
-	private NodeFinder $nodeFinder;
+	private const CONTEXT_DEFAULT = 0;
 
-	public function __construct()
+	private const CONTEXT_FUNCTION_OR_METHOD = 1;
+
+	private const CONTEXT_PROPERTY_HOOK = 2;
+
+	/** @var self::CONTEXT_* */
+	private int $context = self::CONTEXT_DEFAULT;
+
+	private string|null $propertyName = null;
+
+	/**
+	 * @return int|Node[]|null
+	 */
+	public function enterNode(Node $node): int|array|null
 	{
-		$this->nodeFinder = new NodeFinder();
+		switch ($this->context) {
+			case self::CONTEXT_DEFAULT:
+				return $this->clean($node);
+			case self::CONTEXT_FUNCTION_OR_METHOD:
+				return $this->cleanFunctionOrMethod($node);
+			case self::CONTEXT_PROPERTY_HOOK:
+				return $this->cleanPropertyHook($node);
+		}
 	}
 
-	public function enterNode(Node $node): ?Node
+	private function clean(Node $node): int|null
 	{
-		if ($node instanceof Node\Stmt\Function_) {
-			$node->stmts = $this->keepVariadicsAndYields($node->stmts, null);
-			return $node;
-		}
+		if (($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassMethod) && $node->stmts !== null) {
+			$params = [];
+			foreach ($this->traverse($node->params, self::CONTEXT_DEFAULT) as $param) {
+				if (!($param instanceof Node\Param)) {
+					continue;
+				}
 
-		if ($node instanceof Node\Stmt\ClassMethod && $node->stmts !== null) {
-			$node->stmts = $this->keepVariadicsAndYields($node->stmts, null);
-			return $node;
-		}
+				$params[] = $param;
+			}
+			$node->params = $params;
 
-		if ($node instanceof Node\Expr\Closure) {
-			$node->stmts = $this->keepVariadicsAndYields($node->stmts, null);
-			return $node;
+			$stmts = [];
+			foreach ($this->traverse($node->stmts, self::CONTEXT_FUNCTION_OR_METHOD) as $stmt) {
+				if (!($stmt instanceof Node\Stmt)) {
+					continue;
+				}
+
+				$stmts[] = $stmt;
+			}
+			$node->stmts = $stmts;
+
+			return self::DONT_TRAVERSE_CHILDREN;
 		}
 
 		if ($node instanceof Node\PropertyHook && is_array($node->body)) {
 			$propertyName = $node->getAttribute('propertyName');
 			if ($propertyName !== null) {
-				$node->body = $this->keepVariadicsAndYields($node->body, $propertyName);
-				return $node;
+				$body = [];
+				foreach ($this->traverse($node->body, self::CONTEXT_PROPERTY_HOOK, $propertyName) as $stmt) {
+					if (!($stmt instanceof Node\Stmt)) {
+						continue;
+					}
+
+					$body[] = $stmt;
+				}
+				$node->body = $body;
+
+				return self::DONT_TRAVERSE_CHILDREN;
 			}
 		}
 
 		return null;
 	}
 
 	/**
-	 * @param Node\Stmt[] $stmts
-	 * @return Node\Stmt[]
+	 * @return int|Node[]
 	 */
-	private function keepVariadicsAndYields(array $stmts, ?string $hookedPropertyName): array
+	private function cleanFunctionOrMethod(Node $node): int|array
 	{
-		$results = $this->nodeFinder->find($stmts, static function (Node $node) use ($hookedPropertyName): bool {
-			if ($node instanceof Node\Expr\YieldFrom || $node instanceof Node\Expr\Yield_) {
-				return true;
-			}
-			if ($node instanceof Node\Expr\FuncCall && $node->name instanceof Node\Name) {
-				return in_array($node->name->toLowerString(), ParametersAcceptor::VARIADIC_FUNCTIONS, true);
-			}
+		if ($node instanceof Node\Expr\YieldFrom || $node instanceof Node\Expr\Yield_) {
+			return self::DONT_TRAVERSE_CHILDREN;
+		}
 
-			if ($node instanceof Node\Expr\Closure || $node instanceof Node\Expr\ArrowFunction) {
-				return true;
-			}
+		if ($node instanceof Node\Expr\FuncCall && $node->name instanceof Node\Name
+			&& in_array($node->name->toLowerString(), ParametersAcceptor::VARIADIC_FUNCTIONS, true)
+		) {
+			$node->name = new Node\Name\FullyQualified('func_get_args');
+			return self::DONT_TRAVERSE_CHILDREN;
+		}
 
-			if ($hookedPropertyName !== null) {
-				if (
-					$node instanceof Node\Expr\PropertyFetch
-					&& $node->var instanceof Node\Expr\Variable
-					&& $node->var->name === 'this'
-					&& $node->name instanceof Node\Identifier
-					&& $node->name->toString() === $hookedPropertyName
-				) {
-					return true;
-				}
-			}
+		if ($node instanceof Node\Expr\Closure || $node instanceof Node\Expr\ArrowFunction) {
+			return self::REMOVE_NODE;
+		}
 
-			return false;
-		});
-		$newStmts = [];
-		foreach ($results as $result) {
-			if (
-				$result instanceof Node\Expr\Yield_
-				|| $result instanceof Node\Expr\YieldFrom
-				|| $result instanceof Node\Expr\Closure
-				|| $result instanceof Node\Expr\ArrowFunction
-				|| $result instanceof Node\Expr\PropertyFetch
-			) {
-				$newStmts[] = new Node\Stmt\Expression($result);
-				continue;
-			}
-			if (!$result instanceof Node\Expr\FuncCall) {
-				continue;
-			}
+		return $this->cleanSubnodes($node);
+	}
+
+	/**
+	 * @param Node[] $nodes
+	 * @param self::CONTEXT_* $context
+	 * @return Node[]
+	 */
+	private function traverse(
+		array $nodes,
+		int $context = self::CONTEXT_DEFAULT,
+		string|null $propertyName = null,
+	): array
+	{
+		$visitor = new self();
+		$visitor->context = $context;
+		$visitor->propertyName = $propertyName;
+
+		return (new NodeTraverser($visitor))->traverse($nodes);
+	}
 
-			$newStmts[] = new Node\Stmt\Expression(new Node\Expr\FuncCall(new Node\Name\FullyQualified('func_get_args')));
+	/**
+	 * @return int|Node[]
+	 */
+	private function cleanPropertyHook(Node $node): int|array
+	{
+		if (
+			$node instanceof Node\Expr\PropertyFetch
+			&& $node->var instanceof Node\Expr\Variable
+			&& $node->var->name === 'this'
+			&& $node->name instanceof Node\Identifier
+			&& $node->name->toString() === $this->propertyName
+		) {
+			return self::DONT_TRAVERSE_CHILDREN;
 		}
 
-		return $newStmts;
+		return $this->cleanSubnodes($node);
+	}
+
+	/**
+	 * @return Node[]
+	 */
+	private function cleanSubnodes(Node $node): array
+	{
+		$subnodes = [];
+		foreach ($node->getSubNodeNames() as $subnodeName) {
+			$subnodes = [...$subnodes, ...array_filter(
+				is_array($node->$subnodeName) ? $node->$subnodeName : [$node->$subnodeName],
+				static fn ($subnode) => $subnode instanceof Node,
+			)];
+		}
+
+		return array_map(static function ($node) {
+			switch (true) {
+				case $node instanceof Node\Stmt:
+					return $node;
+				case $node instanceof Node\Expr:
+					return new Node\Stmt\Expression($node);
+				default:
+					throw new ShouldNotHappenException();
+			}
+		}, $this->traverse($subnodes, $this->context, $this->propertyName));
 	}
 
 }

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\Node\AnonymousClassNode;
 use PHPStan\Reflection\Assertions;
 use PHPStan\Reflection\ExtendedFunctionVariant;
 use PHPStan\Reflection\ExtendedParameterReflection;
@@ -281,7 +282,11 @@ private function nodeIsOrContainsYield(Node $node): bool
 		foreach ($node->getSubNodeNames() as $nodeName) {
 			$nodeProperty = $node->$nodeName;
 
-			if ($nodeProperty instanceof Node && $this->nodeIsOrContainsYield($nodeProperty)) {
+			if ($nodeProperty instanceof Node &&
+				!$nodeProperty instanceof FunctionLike &&
+				!$nodeProperty instanceof AnonymousClassNode &&
+				$this->nodeIsOrContainsYield($nodeProperty)
+			) {
 				return true;
 			}
 

tests/PHPStan/Parser/data/cleaning-1-after.php

@@ -21,6 +21,10 @@ public function someGenerator2()
     {
         yield from [1, 2, 3];
     }
+    public function someGenerator3()
+    {
+        yield;
+    }
     public function someVariadics()
     {
         \func_get_args();
@@ -43,9 +47,8 @@ class ContainsClosure
 {
     public function doFoo()
     {
-        static function () {
-            yield;
-        };
-        yield;
+    }
+    public function doBar()
+    {
     }
 }

tests/PHPStan/Parser/data/cleaning-1-before.php

@@ -36,6 +36,11 @@ public function someGenerator2()
 		}
 	}
 
+	public function someGenerator3()
+	{
+		echo yield;
+	}
+
 	public function someVariadics()
 	{
 		if (rand(0, 1)) {
@@ -82,4 +87,9 @@ public function doFoo()
 		};
 	}
 
+	public function doBar()
+	{
+		$fn = fn() => yield;
+	}
+
 }

tests/PHPStan/Rules/Functions/ReturnTypeRuleTest.php

@@ -341,4 +341,24 @@ public function testBug11301(): void
 		]);
 	}
 
+	public function testBug12462(): void
+	{
+		$this->checkExplicitMixed = true;
+		$this->checkNullables = true;
+		$this->analyse([__DIR__ . '/data/bug-12462.php'], [
+			[
+				'Function Bug12462\functionReturningYieldingClosure() should return int but returns Closure.',
+				7,
+			],
+			[
+				'Function Bug12462\functionReturningYieldingArrowFunction() should return int but returns Closure.',
+				12,
+			],
+			[
+				'Function Bug12462\functionRetuningYieldingAnonymousClass() should return int but returns class@anonymous/tests/PHPStan/Rules/Functions/data/bug-12462.php:17.',
+				17,
+			],
+		]);
+	}
+
 }

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

@@ -0,0 +1,22 @@
+<?php declare(strict_types = 1);
+
+namespace Bug12462;
+
+function functionReturningYieldingClosure (): int
+{
+	return function () { yield ''; };
+}
+
+function functionReturningYieldingArrowFunction (): int
+{
+	return fn () => yield '';
+}
+
+function functionRetuningYieldingAnonymousClass (): int
+{
+	return new class () {
+		public function f(): \Generator {
+			yield '';
+		}
+	};
+}

tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php

@@ -1227,4 +1227,22 @@ public function testBug1O580(): void
 		]);
 	}
 
+	public function testBug12462(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-12462.php'], [
+			[
+				'Method Bug12462\A::methodReturningYieldingClosure() should return int but returns Closure.',
+				8,
+			],
+			[
+				'Method Bug12462\A::methodReturningYieldingArrowFunction() should return int but returns Closure.',
+				13,
+			],
+			[
+				'Method Bug12462\A::methodRetuningYieldingAnonymousClass() should return int but returns class@anonymous/tests/PHPStan/Rules/Methods/data/bug-12462.php:18.',
+				18,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Methods/data/bug-12462.php

@@ -0,0 +1,24 @@
+<?php declare(strict_types = 1);
+
+namespace Bug12462;
+
+class A {
+	function methodReturningYieldingClosure (): int
+	{
+		return function () { yield ''; };
+	}
+
+	function methodReturningYieldingArrowFunction (): int
+	{
+		return fn () => yield '';
+	}
+
+	function methodRetuningYieldingAnonymousClass (): int
+	{
+		return new class () {
+			public function f(): \Generator {
+				yield '';
+			}
+		};
+	}
+}