🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

Propagate hasYield from yield in ternary branches - #5904

Merged
staabm merged 4 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-c2he1eh
Jun 21, 2026
Merged

Propagate hasYield from yield in ternary branches#5904
staabm merged 4 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-c2he1eh

Conversation

@phpstan-bot

@phpstan-bot phpstan-bot commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

A closure that only yields inside a ternary branch (e.g.
$x = $cond ? yield foo() : false;) was wrongly reported with
Anonymous function should return Generator<...> but return statement is missing.
even though the presence of yield anywhere in the body makes the function a
generator, so no explicit return is required.

The root cause was that the ternary expression handler did not propagate the
hasYield flag from its branches. While fixing it I found the same class of bug
in the throw expression handler.

Changes

  • src/Analyser/ExprHandler/TernaryHandler.phpprocessExpr() now ORs
    hasYield from the condition with the results of the if and else branches
    (and the default branch of the short ternary ?:), instead of only using the
    condition's hasYield.
  • tests/PHPStan/Rules/Missing/data/bug-5681.php and
    tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php — regression tests.

I audited every other ExprHandler on the same axis (sub-expressions that may
contain a yield): Coalesce, Match, BinaryOp, BooleanAnd, BooleanOr,
AssignOp, Assign, Array, InterpolatedString, Isset, New,
MethodCall, StaticCall, the various call handlers and the unary handlers all
already OR hasYield from all their sub-expressions correctly. Closure and
arrow-function handlers correctly return hasYield: false (an inner closure's
yield belongs to the inner closure, not the outer function).

Root cause

MissingReturnRule treats a function as a generator (and therefore not
requiring a return) when the statement result reports hasYield. This flag is
computed bottom-up through the expression handlers. Each handler must OR the
hasYield of every sub-expression that can execute a yield. TernaryHandler
reported only the condition's hasYield.

Test

  • tests/PHPStan/Rules/Missing/data/bug-5681.php reproduces the reported case
    (yield in the if branch of a ternary) plus three analogous cases: yield
    in the else branch, yield in the default branch of ?:. All three produced a false return.missing error
    before the fix and none do after. Wired up via
    MissingReturnRuleTest::testBug5681().

Fixes phpstan/phpstan#5681

Comment thread src/Analyser/ExprHandler/ThrowHandler.php Outdated
@staabm staabm changed the title Propagate hasYield from yield in ternary branches and throw expressions Propagate hasYield from yield in ternary branches Jun 21, 2026
Comment thread tests/PHPStan/Rules/Missing/MissingReturnRuleTest.php
@staabm
staabm requested a review from VincentLanglet June 21, 2026 12:53
staabm and others added 4 commits June 21, 2026 15:49
…ressions

- `TernaryHandler::processExpr()` only reported `hasYield` from the
  condition, ignoring `yield` placed in the `if`/`else` branches (and the
  default branch of the short `?:`). It now ORs the condition result with
  both branch results.
- `ThrowHandler::processExpr()` hardcoded `hasYield: false`, dropping a
  `yield` inside a `throw` expression (e.g. `throw yield $x`). It now
  propagates `hasYield` from the thrown expression.
- Without these, a closure/function whose only `yield` lives inside a
  ternary branch or a throw expression was not recognised as a generator
  by `MissingReturnRule`, producing a false "should return Generator but
  return statement is missing" error.
`yield` within a `throw` expression is too artificial to support for now,
so drop the `ThrowHandler` change and its test case, keeping only the
ternary-branch fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm
staabm force-pushed the create-pull-request/patch-c2he1eh branch from a65179a to d34c952 Compare June 21, 2026 13:49
@VincentLanglet

Copy link
Copy Markdown
Contributor

It is still reported without the throw Exception ?

@staabm

staabm commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

throw yield $e is still reported yes.

I can't think I have ever seen code like this

@staabm

staabm commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

I can't think I have ever seen code like this

1 search-hit across github.com: https://github.com/search?q=%22throw+yield+%24%22&type=code

@VincentLanglet

Copy link
Copy Markdown
Contributor

throw yield $e is still reported yes.

I can't think I have ever seen code like this

I mean https://phpstan.org/r/1cda3b84-18c7-4b3e-a3e1-e51fff27c908

@VincentLanglet

Copy link
Copy Markdown
Contributor

throw yield $e is still reported yes.
I can't think I have ever seen code like this

I mean phpstan.org/r/1cda3b84-18c7-4b3e-a3e1-e51fff27c908

But seems like it's already not reported with an if
https://phpstan.org/r/a57b620f-9c33-427a-af74-8d5f0c7cff78

@staabm
staabm merged commit bb545e9 into phpstan:2.2.x Jun 21, 2026
670 of 672 checks passed
@staabm
staabm deleted the create-pull-request/patch-c2he1eh branch June 21, 2026 15:23
@staabm

staabm commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Yes - the bug only happened on short-if

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive with conditional yield

3 participants