From eb373717e020367952cf4ccf81bb914fe859e15d Mon Sep 17 00:00:00 2001 From: azjezz Date: Fri, 30 May 2025 15:13:40 +0100 Subject: [PATCH 1/5] feat(analyzer): add `HiddenGeneratorReturn` issue closes #11458 Signed-off-by: azjezz --- docs/running_psalm/issues.md | 1 + .../issues/HiddenGeneratorReturn.md | 17 +++++ .../Analyzer/Statements/ReturnAnalyzer.php | 66 ++++++++++++++++++- src/Psalm/Issue/HiddenGeneratorReturn.php | 11 ++++ tests/ReturnTypeTest.php | 28 ++++++++ 5 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 docs/running_psalm/issues/HiddenGeneratorReturn.md create mode 100644 src/Psalm/Issue/HiddenGeneratorReturn.php diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index 62614decd43..2cdec50f375 100644 --- a/docs/running_psalm/issues.md +++ b/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) diff --git a/docs/running_psalm/issues/HiddenGeneratorReturn.md b/docs/running_psalm/issues/HiddenGeneratorReturn.md new file mode 100644 index 00000000000..7903c5a05f7 --- /dev/null +++ b/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 + + */ +function generator(): iterable +{ + yield "foo"; + + return 1; +} +``` diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 7d70252fc3b..37a420a1268 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/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,27 @@ 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 +734,48 @@ 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\TGenericObject) { + continue; + } + + if ($atomic->value !== 'Generator') { + continue; + } + + $generator_return = Type::combineUnionTypes( + $generator_return, + $atomic->type_params[3] ?? type::getMixed(), + $codebase, + ); + } + + if (!$could_be_array_or_traversable) { + return null; + } + + if (null === $generator_return) { + return [Type::getMixed(), false]; + } + + return [$generator_return, true]; + } } diff --git a/src/Psalm/Issue/HiddenGeneratorReturn.php b/src/Psalm/Issue/HiddenGeneratorReturn.php new file mode 100644 index 00000000000..5155c81a5f6 --- /dev/null +++ b/src/Psalm/Issue/HiddenGeneratorReturn.php @@ -0,0 +1,11 @@ + [ + 'code' => ' + * @psalm-suppress HiddenGeneratorReturn + */ + function generator(): iterable + { + yield "f"; + + return 1; + } + ', + ], 'arrayCombine' => [ 'code' => ' [ + 'code' => ' + */ + function generator(): iterable + { + yield "f"; + + return 1; + } + ', + 'error_message' => 'HiddenGeneratorReturn', + ], 'wrongReturnType1' => [ 'code' => ' Date: Fri, 30 May 2025 15:19:15 +0100 Subject: [PATCH 2/5] chore(config): add new `HiddenGeneratorReturn` issue code to config Signed-off-by: azjezz --- config.xsd | 1 + 1 file changed, 1 insertion(+) diff --git a/config.xsd b/config.xsd index cad124396bb..ef22217b886 100644 --- a/config.xsd +++ b/config.xsd @@ -529,6 +529,7 @@ + From 2ce149c3051edb95aa804627c7bce745dd53fce6 Mon Sep 17 00:00:00 2001 From: azjezz Date: Fri, 30 May 2025 15:19:27 +0100 Subject: [PATCH 3/5] chore(analyzer): apply cs fixes Signed-off-by: azjezz --- src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 37a420a1268..086dc4f95f6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -338,7 +338,8 @@ public static function analyze( if (!$is_from_generator) { if (IssueBuffer::accepts( new HiddenGeneratorReturn( - 'The value returned by generator ' . $cased_method_id . ' may be inaccessible to callers.', + 'The value returned by generator ' . $cased_method_id + . ' may be inaccessible to callers.', new CodeLocation($source, $stmt->expr), ), $statements_analyzer->getSuppressedIssues(), From 59cd9ff61e80e4cbb9207e158afea5354f78cbc5 Mon Sep 17 00:00:00 2001 From: azjezz Date: Fri, 30 May 2025 15:30:12 +0100 Subject: [PATCH 4/5] fix(analyzer): handle non-generic `Generator` type Signed-off-by: azjezz --- .../Internal/Analyzer/Statements/ReturnAnalyzer.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php index 086dc4f95f6..5723b822842 100644 --- a/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/ReturnAnalyzer.php @@ -754,7 +754,7 @@ private static function extractGeneratorReturnType(Codebase $codebase, Union $ac } $could_be_array_or_traversable = true; - if (!$atomic instanceof Type\Atomic\TGenericObject) { + if (!$atomic instanceof Type\Atomic\TNamedObject) { continue; } @@ -762,9 +762,15 @@ private static function extractGeneratorReturnType(Codebase $codebase, Union $ac 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->type_params[3] ?? type::getMixed(), + $atomic_generator_return, $codebase, ); } From bbf3a9390f85480220a885828fb9070980ce4c7f Mon Sep 17 00:00:00 2001 From: azjezz Date: Fri, 30 May 2025 15:41:08 +0100 Subject: [PATCH 5/5] fix(tests): suppress mixed return issues in foreach test Signed-off-by: azjezz --- tests/Loop/ForeachTest.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/Loop/ForeachTest.php b/tests/Loop/ForeachTest.php index 81352475ec6..9a2e29a6457 100644 --- a/tests/Loop/ForeachTest.php +++ b/tests/Loop/ForeachTest.php @@ -1176,7 +1176,10 @@ function f(array $a): array { 'generatorWithUnspecifiedSend' => [ 'code' => <<<'PHP' */ + /** + * @return Generator + * @psalm-suppress MixedReturnStatement + */ function gen() : Generator { return yield 1; } @@ -1187,7 +1190,10 @@ function gen() : Generator { 'generatorWithMixedSend' => [ 'code' => <<<'PHP' */ + /** + * @return Generator + * @psalm-suppress MixedReturnStatement + */ function gen() : Generator { return yield 1; }