Fix :Generator without yield rejecting scalar return (#10333)

lib/Ast/GeneratorYieldSourceMarker.php

@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PHPCompiler\Ast;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\ArrowFunction;
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Expr\Yield_;
+use PhpParser\Node\Expr\YieldFrom;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
+use PhpParser\NodeVisitorAbstract;
+
+/**
+ * Mark user callables that contain `yield` / `yield from` in source (#10333, #3350).
+ *
+ * php-cfg may DCE unreachable yield after return; Zend still treats the callable as a
+ * generator when yield appears in source (zend_compile.c / zend_generators.c).
+ */
+final class GeneratorYieldSourceMarker extends NodeVisitorAbstract
+{
+    public const ATTRIBUTE = 'compilerSourceHasYield';
+
+    /** @var list<Node\FunctionLike> */
+    private array $stack = [];
+
+    private bool $currentHasYield = false;
+
+    public function enterNode(Node $node)
+    {
+        if ($node instanceof Function_ || $node instanceof ClassMethod || $node instanceof Closure || $node instanceof ArrowFunction) {
+            $this->stack[] = $node;
+            $this->currentHasYield = false;
+
+            return null;
+        }
+        if ([] !== $this->stack && ($node instanceof Yield_ || $node instanceof YieldFrom)) {
+            $this->currentHasYield = true;
+        }
+
+        return null;
+    }
+
+    public function leaveNode(Node $node)
+    {
+        if ([] === $this->stack || end($this->stack) !== $node) {
+            return null;
+        }
+        array_pop($this->stack);
+        if ($this->currentHasYield) {
+            $node->setAttribute(self::ATTRIBUTE, true);
+        }
+        $this->currentHasYield = false;
+
+        return null;
+    }
+}

lib/Block.php

@@ -125,6 +125,12 @@ class Block {
     /** Declared scalar return type for this function (issue #205), or null when untyped. */
     public ?int $returnTypeConstraint = null;
 
+    /** Declared object return class name (issue #10333), or null when untyped / non-class. */
+    public ?string $returnClassConstraint = null;
+
+    /** Declared object return type label for errors (#10333), or null. */
+    public ?string $returnDeclaredTypeLabel = null;
+
     /** Standalone `: true` / `: false` return type (#4784), or null. */
     public ?string $returnLiteralBoolType = null;
 
@@ -521,6 +527,8 @@ public function inheritScopeFrom(Block $parent): void
             $this->func = $parent->func;
             $this->strictTypes = $parent->strictTypes;
             $this->returnTypeConstraint = $parent->returnTypeConstraint;
+            $this->returnClassConstraint = $parent->returnClassConstraint;
+            $this->returnDeclaredTypeLabel = $parent->returnDeclaredTypeLabel;
             $this->returnDnfConstraints = $parent->returnDnfConstraints;
             $this->returnTypeVoid = $parent->returnTypeVoid;
             $this->returnTypeNever = $parent->returnTypeNever;

lib/Compiler.php

@@ -34,6 +34,7 @@
 use PHPCompiler\VM\ClassReadonly;
 use PHPCompiler\JIT\OperandName;
 use PHPCompiler\Ast\AsymmetricVisibilityRewriter;
+use PHPCompiler\Ast\GeneratorYieldSourceMarker;
 use PHPCompiler\Compiler\AbstractMethodVisibilityCheck;
 use PHPCompiler\Compiler\InterfaceConstVisibilityCheck;
 use PHPCompiler\Compiler\InterfaceMethodVisibilityCheck;
@@ -577,6 +578,13 @@ protected function applyReturnTypeFromFunc(Block $block, CfgFunc $func): void
             if ('static' === strtolower((string) $refName)) {
                 $block->returnTypeStatic = true;
 
+                return;
+            }
+            if (null !== $refName && '' !== $refName) {
+                $block->returnTypeConstraint = Variable::TYPE_OBJECT;
+                $block->returnClassConstraint = $refName;
+                $block->returnDeclaredTypeLabel = ltrim($refName, '\\');
+
                 return;
             }
         }
@@ -619,7 +627,16 @@ protected function applyReturnTypeFromFunc(Block $block, CfgFunc $func): void
 
                 return;
             }
-            $mapped = Variable::mapFromType(Type::fromDecl($returnType->name));
+            $declType = Type::fromDecl($returnType->name);
+            $mapped = Variable::mapFromType($declType);
+            if (Variable::TYPE_OBJECT === $mapped) {
+                $className = '' !== (string) $declType->userType ? $declType->userType : $returnType->name;
+                $block->returnTypeConstraint = Variable::TYPE_OBJECT;
+                $block->returnClassConstraint = $className;
+                $block->returnDeclaredTypeLabel = ltrim($className, '\\');
+
+                return;
+            }
             if (Variable::TYPE_UNDEFINED !== $mapped) {
                 $block->returnTypeConstraint = $mapped;
             }
@@ -820,6 +837,8 @@ private function inheritFuncFromParent(Block $child, Block $parent): void
             // Merge blocks skip inheritScopeFrom when parents>=2 (#3790); still need
             // return-type flags so :never epilogue checks run on implicit fall-off (#9240).
             $child->returnTypeConstraint = $parent->returnTypeConstraint;
+            $child->returnClassConstraint = $parent->returnClassConstraint;
+            $child->returnDeclaredTypeLabel = $parent->returnDeclaredTypeLabel;
             $child->returnDnfConstraints = $parent->returnDnfConstraints;
             $child->returnTypeVoid = $parent->returnTypeVoid;
             $child->returnTypeNever = $parent->returnTypeNever;
@@ -3399,6 +3418,7 @@ protected function compileClassMethodDeclaration(Op\Stmt\ClassMethod $child, Blo
         if (null !== $child->func->cfg) {
             $methodBlock = $this->compileCfgBlock($child->func->cfg, $child->func->params, $child->func);
             NoDiscardMetadata::applyToBlock($methodBlock, $child);
+            $this->markGeneratorIfNeeded($child, $methodBlock);
             $declare->block1 = $methodBlock;
         } elseif (0 === ($child->func->flags & CfgFunc::FLAG_ABSTRACT)) {
             // php-cfg omits cfg for `{}` method bodies; concrete methods still need block1 (#4758).
@@ -6385,10 +6405,7 @@ protected function compileParam(Op\Expr\Param $param, Block $block, int $paramId
     protected function compileFunction(Op\Stmt\Function_ $function, Block $block): OpCode {
         $funcBlock = $this->compileCfgBlock($function->func->cfg, $function->func->params, $function->func);
         NoDiscardMetadata::applyToBlock($funcBlock, $function);
-        // php-cfg may DCE unreachable yield after return; :Generator still implies generator (#3350).
-        if ($this->funcDeclReturnTypeIsGenerator($function->func)) {
-            $this->markFunctionGenerator($funcBlock);
-        }
+        $this->markGeneratorIfNeeded($function, $funcBlock);
         if ($this->funcDeclReturnTypeIsNever($function->func)) {
             $this->neverFunctionNames[strtolower($function->func->name)] = true;
         }
@@ -7395,6 +7412,7 @@ protected function compileAnonymousFunctionExpr($expr, Block $block): array
         } finally {
             $this->compilingArrowAutoCapture = $wasArrowAutoCapture;
         }
+        $this->markGeneratorIfNeeded($expr, $funcBlock);
         $op = new OpCode(
             OpCode::TYPE_CLOSURE,
             $this->compileOperand($expr->result, $block, false),
@@ -7481,6 +7499,24 @@ protected function markFunctionGenerator(Block $block): void
         }
     }
 
+    protected function markGeneratorIfNeeded(Op\CallableOp $callable, Block $funcBlock): void
+    {
+        if (Block::containsGeneratorOpcodes($funcBlock) || $this->callableOpHasSourceYield($callable)) {
+            $this->markFunctionGenerator($funcBlock);
+        }
+    }
+
+    protected function callableOpHasSourceYield(Op\CallableOp $callable): bool
+    {
+        if (!$callable instanceof Op) {
+            return false;
+        }
+        $attrs = $callable->getAttributes();
+
+        return isset($attrs[GeneratorYieldSourceMarker::ATTRIBUTE])
+            && $attrs[GeneratorYieldSourceMarker::ATTRIBUTE];
+    }
+
     protected function funcDeclReturnTypeIsGenerator(CfgFunc $func): bool
     {
         $returnType = $func->returnType;

lib/Runtime.php

@@ -129,6 +129,7 @@ private function initParsePipeline(): void {
         $this->staticClassAnnotator = new StaticClassAnnotator();
         $astTraverser->addVisitor($this->staticClassAnnotator);
         $astTraverser->addVisitor(new Ast\EnumPropertyCompileCheck());
+        $astTraverser->addVisitor(new Ast\GeneratorYieldSourceMarker());
         $this->parser = new Parser(
             (new ParserFactory)->create(ParserFactory::ONLY_PHP7),
             $astTraverser

lib/VM.php

@@ -13633,6 +13633,19 @@ private function enforceReturnType(Frame $frame, ?Variable $value): void
 
             return;
         }
+        if (null !== $block->returnClassConstraint && null !== $value) {
+            $returnLabel = ltrim($block->returnDeclaredTypeLabel ?? $block->returnClassConstraint, '\\');
+            if (!($block->isGenerator && 'Generator' === $returnLabel)) {
+                TypeCheck::assertObjectReturn(
+                    $value,
+                    $block->returnClassConstraint,
+                    $block->returnDeclaredTypeLabel ?? $block->returnClassConstraint,
+                    $this->returnTypeCallableName($block->func)
+                );
+            }
+
+            return;
+        }
         if (null === $block->returnTypeConstraint || null === $value) {
             return;
         }
@@ -13645,6 +13658,18 @@ private function enforceReturnType(Frame $frame, ?Variable $value): void
         );
     }
 
+    private function returnTypeCallableName(?\PHPCfg\Func $func): ?string
+    {
+        if (null === $func) {
+            return null;
+        }
+        if (null !== $func->class && '' !== $func->class) {
+            return $func->class.'::'.$func->name;
+        }
+
+        return $func->name;
+    }
+
     private function emitCallDeprecationNotice(Frame $frame): void
     {
         if (null === $frame->call || !($frame->call instanceof Func\PHP)) {

lib/VM/TypeCheck.php

@@ -320,6 +320,25 @@ public static function assertStaticReturn(
         }
     }
 
+    public static function assertObjectReturn(
+        Variable $value,
+        string $classConstraint,
+        string $declaredLabel,
+        ?string $callableName = null
+    ): void {
+        if (self::matchesClassTypeHint($value, $classConstraint)) {
+            return;
+        }
+        $expected = ltrim($declaredLabel, '\\');
+        $given = self::valueTypeLabel($value);
+        $message = "Return value must be of type {$expected}, {$given} returned";
+        if (null !== $callableName && '' !== $callableName) {
+            $message = "{$callableName}(): {$message}";
+        }
+
+        throw new \TypeError($message);
+    }
+
     private static function coerceUnionPropertyWrite(Variable $target, bool $strict): void
     {
         $constraints = $target->unionTypeConstraints ?? [];

test/compliance/cases/language/generator_return_type_scalar.phpt

@@ -0,0 +1,26 @@
+--TEST--
+Generator return type without yield rejects scalar return (issue #10333)
+--FILE--
+<?php
+declare(strict_types=1);
+
+function g(): Generator {
+    return 1;
+}
+
+try {
+    g();
+    echo "no error\n";
+} catch (Throwable $e) {
+    echo get_class($e), ': ', $e->getMessage(), "\n";
+}
+
+function ok(): Generator {
+    yield 1;
+}
+
+$gen = ok();
+echo $gen->current(), "\n";
+--EXPECT--
+TypeError: g(): Return value must be of type Generator, int returned
+1

test/repro/maintainer_gap_generator_return_scalar.php

@@ -0,0 +1,12 @@
+<?php
+declare(strict_types=1);
+
+function g(): Generator {
+    return 1;
+}
+
+$gen = g();
+var_export($gen);
+echo "\n";
+var_export($gen->getReturn());
+echo "\n";