feat(analyzer): add HiddenGeneratorReturn issue

config.xsd

@@ -529,6 +529,7 @@
             <xs:element name="UnusedPsalmSuppress" type="FunctionIssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedReturnValue" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="UnusedVariable" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="HiddenGeneratorReturn" type="IssueHandlerType" minOccurs="0" />
         </xs:choice>
         <xs:anyAttribute processContents="skip" />
     </xs:complexType>

docs/running_psalm/issues.md

@@ -313,3 +313,4 @@
  - [UnusedPsalmSuppress](issues/UnusedPsalmSuppress.md)
  - [UnusedReturnValue](issues/UnusedReturnValue.md)
  - [UnusedVariable](issues/UnusedVariable.md)
+ - [HiddenGeneratorReturn](issues/HiddenGeneratorReturn.md)

docs/running_psalm/issues/HiddenGeneratorReturn.md

@@ -0,0 +1,17 @@
+# HiddenGeneratorReturn
+
+Emitted when trying to return a value from a generator function that does not have a `Generator` return type.
+
+```php
+<?php
+
+/**
+ * @return iterable<"foo">
+ */
+function generator(): iterable
+{
+    yield "foo";
+
+    return 1;
+}
+```

src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php

@@ -28,6 +28,7 @@
 use Psalm\Internal\Type\TemplateResult;
 use Psalm\Internal\Type\TypeExpander;
 use Psalm\Issue\FalsableReturnStatement;
+use Psalm\Issue\HiddenGeneratorReturn;
 use Psalm\Issue\InvalidDocblock;
 use Psalm\Issue\InvalidReturnStatement;
 use Psalm\Issue\LessSpecificReturnStatement;
@@ -330,10 +331,28 @@ public static function analyze(
                         }
                     }
 
-                    if ($local_return_type->isGenerator() && $storage->has_yield) {
-                        return;
+                    if ($storage->has_yield) {
+                        $generator_return = self::extractGeneratorReturnType($codebase, $local_return_type);
+                        if (null !== $generator_return) {
+                            [$generator_return_type, $is_from_generator] = $generator_return;
+                            if (!$is_from_generator) {
+                                if (IssueBuffer::accepts(
+                                    new HiddenGeneratorReturn(
+                                        'The value returned by generator ' . $cased_method_id
+                                            . ' may be inaccessible to callers.',
+                                        new CodeLocation($source, $stmt->expr),
+                                    ),
+                                    $statements_analyzer->getSuppressedIssues(),
+                                )) {
+                                    return;
+                                }
+                            }
+
+                            $local_return_type = $generator_return_type;
+                        }
                     }
 
+
                     if ($stmt_type->hasMixed()) {
                         if ($local_return_type->isVoid() || $local_return_type->isNever()) {
                             if (IssueBuffer::accepts(
@@ -716,4 +735,54 @@ private static function inferInnerClosureTypeFromParent(
         }
         return $return_type;
     }
+
+    /**
+     * @return null|array{0: Union, 1: bool}
+     */
+    private static function extractGeneratorReturnType(Codebase $codebase, Union $actual_return_type): ?array
+    {
+        $generator_return = null;
+        $could_be_array_or_traversable = false;
+
+        foreach ($actual_return_type->getAtomicTypes() as $atomic) {
+            if (!$atomic instanceof Type\Atomic\TArray &&
+                !$atomic instanceof Type\Atomic\TIterable &&
+                !$atomic instanceof Type\Atomic\TMixed &&
+                !$atomic->hasTraversableInterface($codebase)
+            ) {
+                continue;
+            }
+
+            $could_be_array_or_traversable = true;
+            if (!$atomic instanceof Type\Atomic\TNamedObject) {
+                continue;
+            }
+
+            if ($atomic->value !== 'Generator') {
+                continue;
+            }
+
+            if ($atomic instanceof Type\Atomic\TGenericObject) {
+                $atomic_generator_return = $atomic->type_params[3] ?? type::getMixed();
+            } else {
+                $atomic_generator_return = Type::getMixed();
+            }
+
+            $generator_return = Type::combineUnionTypes(
+                $generator_return,
+                $atomic_generator_return,
+                $codebase,
+            );
+        }
+
+        if (!$could_be_array_or_traversable) {
+            return null;
+        }
+
+        if (null === $generator_return) {
+            return [Type::getMixed(), false];
+        }
+
+        return [$generator_return, true];
+    }
 }

src/Psalm/Issue/HiddenGeneratorReturn.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\Issue;
+
+final class HiddenGeneratorReturn extends CodeIssue
+{
+    public const ERROR_LEVEL = 4;
+    public const SHORTCODE = 284;
+}

tests/Loop/ForeachTest.php

@@ -1176,7 +1176,10 @@ function f(array $a): array {
             'generatorWithUnspecifiedSend' => [
                 'code' => <<<'PHP'
                     <?php
-                    /** @return Generator<int,int> */
+                    /**
+                     * @return Generator<int,int>
+                     * @psalm-suppress MixedReturnStatement
+                     */
                     function gen() : Generator {
                         return yield 1;
                     }
@@ -1187,7 +1190,10 @@ function gen() : Generator {
             'generatorWithMixedSend' => [
                 'code' => <<<'PHP'
                     <?php
-                    /** @return Generator<int,int, mixed, mixed> */
+                    /**
+                     * @return Generator<int,int, mixed, mixed>
+                     * @psalm-suppress MixedReturnStatement
+                     */
                     function gen() : Generator {
                         return yield 1;
                     }

tests/ReturnTypeTest.php

@@ -19,6 +19,20 @@ final class ReturnTypeTest extends TestCase
     public function providerValidCodeParse(): iterable
     {
         return [
+            'suppressHiddenGeneratorReturn' => [
+                'code' => '<?php
+                    /**
+                     * @return iterable<"f">
+                     * @psalm-suppress HiddenGeneratorReturn
+                     */
+                    function generator(): iterable
+                    {
+                        yield "f";
+
+                        return 1;
+                    }
+                ',
+            ],
             'arrayCombine' => [
                 'code' => '<?php
                     class a {}
@@ -1350,6 +1364,20 @@ function throwError(): never
     public function providerInvalidCodeParse(): iterable
     {
         return [
+            'hiddenGeneratorReturn' => [
+                'code' => '<?php
+                    /**
+                     * @return iterable<"f">
+                     */
+                    function generator(): iterable
+                    {
+                        yield "f";
+
+                        return 1;
+                    }
+                ',
+                'error_message' => 'HiddenGeneratorReturn',
+            ],
             'wrongReturnType1' => [
                 'code' => '<?php
                     function fooFoo(): string {