Skip to content

return.type with unresolved(*NEVER*) template arguments in an if branch whose condition is always false (unresolvedTemplateArguments) #15476

Open
phpstan/phpstan-src
#6827

Description

@SanderMuller

Bug report

With bleedingEdge on 2.3.1, a generic call in an if branch whose condition is always false reports return.type. Its template arguments stay unresolved(*NEVER*), because the declared return type does not reach them.

In the snippet, Factory::box() takes its template types only from an optional parameter:

  • liveBranch() returns $this->factory->box() when $item is null: no error. The template arguments resolve from the declared @return Box<int, Item>.
  • deadBranch() has the same body, but $item cannot be null. PHPStan reports identical.alwaysFalse for the condition on line 37, which is correct. It also reports return.type on line 38: should return App\Box<int, App\Item> but returns App\Box<unresolved(*NEVER*), unresolved(*NEVER*)>.

The return.type goes away with featureToggles.unresolvedTemplateArguments: false, and 2.2.17 does not report it. It still happens on 2.3.x at 62ed2007866083dd5a3bf943a37b874b65a1dd84. An elseif with an always-false condition, a ternary with an always-false condition, and an unreachable statement after a return do not report it.

I found this with Laravel's collect() in a trait method that starts with if (! $video) { return collect(); }. One class that uses the trait returns a non-nullable video, so in that class the guard is dead, and the error is reported only "in context of" that class. Larastan 3.13.0 leaves an empty collect() to this toggle.

Code snippet that reproduces the problem

https://phpstan.org/r/076b7a73-f94a-47ac-971d-d74420c480c8

Expected output

Only identical.alwaysFalse on line 37, as on 2.2.17.

Root cause

  • ReturnHandler adds the constraint from the declared return type to the scope of the return (src/Analyser/StmtHandler/ReturnHandler.php:63, through NodeScopeResolver::collectReturnSend()).
  • That constraint reaches the resolver through the exit points of the enclosing statement result (src/Analyser/InternalStatementResult.php:37).
  • IfHandler keeps the branch's exit points only when the condition is not always false (src/Analyser/StmtHandler/IfHandler.php:72). For an always-false condition, the branch result is dropped, so the marker in the branch has no constraint and resolves to its initial never.
  • The branch statements are still walked, so the rule for the return in the branch reports the unresolved type.

As an experiment, I added the branch's template argument constraints to the falsey scope when the condition is always false, right after $scope = $condResult->getFalseyScope();. With that, the snippet reports only line 37. I did not run the test suite with it, and I am not sure whether a dead branch should feed constraints at all, or whether its rules should not see the unresolved type instead.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions