fix while...else return not recognized as exhaustive #2868

[asukaminato0721]

Summary

Fixes #2868

teaching implicit-return analysis to treat while ... else like a normal-termination branch instead of an unconditional fallthrough.

Test Plan

add test

[Copilot]

Pull request overview

This PR fixes pyrefly’s implicit-return exhaustiveness analysis so that while ... else: is treated as a normal-termination branch (like for ... else:), avoiding false positives when the else branch returns.

Changes:

• Update function_last_expressions to analyze while loop orelse blocks when the loop can terminate normally.
• Add a regression test covering while ... else: return exhaustiveness (Issue #2868).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pyrefly/lib/binding/function.rs	Adjusts implicit-return control-flow analysis for Stmt::While to account for else termination.
pyrefly/lib/test/returns.rs	Adds a regression test ensuring while ... else: return is recognized as exhaustive.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In Stmt::While, has_break is computed unconditionally and then used to short-circuit the else-clause analysis for all non-while True cases. If the condition is statically false (e.g. while False:), the body is unreachable so any break inside it is unreachable too; treating it as a possible break will incorrectly return None here and can reintroduce a false-positive “missing return” even when the else branch is exhaustive. Consider branching on sys_info.evaluate_bool(&x.test) first (handle Some(false) by ignoring has_break and analyzing orelse if present), or only computing/applying has_break when the body is reachable (test != Some(false)).

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

mitmproxy (https://github.com/mitmproxy/mitmproxy)
- ERROR test/mitmproxy/proxy/layers/quic/test__stream_layers.py:428:38-42: Function declared to return `bool`, but one or more paths are missing an explicit `return` [bad-return]

setuptools (https://github.com/pypa/setuptools)
- ERROR setuptools/_distutils/command/build_py.py:279:39-73: No matching overload found for function `posixpath.join` called with arguments: (Literal[''] | Unknown | None, Unknown) [no-matching-overload]

werkzeug (https://github.com/pallets/werkzeug)
- ERROR src/werkzeug/test.py:1068:10-22: Function declared to return `TestResponse`, but one or more paths are missing an explicit `return` [bad-return]

prefect (https://github.com/PrefectHQ/prefect)
- ERROR src/integrations/prefect-dbt/prefect_dbt/cloud/jobs.py:381:6-10: Function declared to return `dict[Unknown, Unknown]`, but one or more paths are missing an explicit `return` [bad-return]

[github-actions]

Primer Diff Classification

✅ 4 improvement(s) | 4 project(s) total | -4 errors

4 improvement(s) across mitmproxy, setuptools, werkzeug, prefect.

Project	Verdict	Changes	Error Kinds	Root Cause
mitmproxy	✅ Improvement	-1	bad-return	pyrefly/lib/binding/function.rs
setuptools	✅ Improvement	-1	no-matching-overload	function_last_expressions()
werkzeug	✅ Improvement	-1	bad-return	pyrefly/lib/binding/function.rs
prefect	✅ Improvement	-1	bad-return	pyrefly/lib/binding/function.rs

Detailed analysis

✅ Improvement (4)

mitmproxy (-1)

This is a clear improvement. The handshake_completed method uses Python's while...else construct, which guarantees that either the loop body executes a return True or the else block executes return False. Pyrefly previously failed to recognize this pattern as exhaustive, producing a false positive bad-return error. The PR fix correctly teaches pyrefly's implicit-return analysis to treat the else clause of a while loop as a normal-termination branch.
Attribution: The change in pyrefly/lib/binding/function.rs in the function_last_expressions function, specifically the Stmt::While(x) branch, now correctly handles while...else blocks. Previously, when evaluate_bool(&x.test) != Some(true) (i.e., the loop is not an infinite loop), pyrefly returned None immediately, meaning it didn't recognize the else block as a valid termination path. The new code checks: if the loop is not infinite AND has no break AND has an else block, it recursively analyzes the else block via f(sys_info, &x.orelse, res)?. This correctly recognizes that while...else with returns in both branches is exhaustive.

setuptools (-1)

This is a clear improvement. The PR fixes pyrefly's while...else analysis so that methods like get_package_dir — which return str on every path through a while...else construct — are no longer incorrectly inferred as potentially returning None. The removed no-matching-overload error was a cascade false positive from that inference bug.
Attribution: The change to function_last_expressions() in pyrefly/lib/binding/function.rs now correctly handles the Stmt::While case by analyzing the else branch (x.orelse) when the loop condition isn't always true. Previously, non-infinite while loops with no breaks returned None (meaning 'cannot determine exhaustiveness'), causing implicit-return false positives. Now the else block is properly analyzed as a normal-termination branch, so get_package_dir's while...else is recognized as exhaustive.

werkzeug (-1)

This is a clear improvement. The Client.open() method in werkzeug has a while...else pattern where the else block contains a return statement. Since the while loop body has no break, the else block is guaranteed to execute when the loop terminates normally. Pyrefly previously couldn't recognize this pattern and incorrectly reported a missing return. The PR fix teaches pyrefly to analyze while...else blocks properly, removing this false positive.
Attribution: The change in pyrefly/lib/binding/function.rs in the function_last_expressions function modified the handling of Stmt::While. Previously, when evaluate_bool(&x.test) != Some(true) (i.e., the loop is not an infinite loop), it returned None immediately, meaning it couldn't see any return paths. The new code checks: if the loop has a break OR the orelse (else clause) is empty, return None (can't guarantee a return). Otherwise, it recursively analyzes the else clause via f(sys_info, &x.orelse, res)?. This correctly recognizes that a while...else with no break and a returning else block is exhaustive.

prefect (-1)

This is a clear improvement. The PR fixes pyrefly's control flow analysis to correctly handle while...else constructs. In Python, the else clause of a while loop executes when the loop condition becomes false (i.e., normal termination without break). The function trigger_dbt_cloud_job_run_and_wait_for_completion uses this pattern: the while loop either returns inside the loop body or falls through to the else clause which raises an exception. Every code path either returns or raises, so the bad-return error was a false positive that has been correctly removed.
Attribution: The change in pyrefly/lib/binding/function.rs in the function_last_expressions function modified the handling of Stmt::While. Previously, when evaluate_bool(&x.test) was not Some(true) (i.e., a non-infinite loop), it returned None immediately, meaning it couldn't analyze the else clause. The new code adds an else branch that calls f(sys_info, &x.orelse, res)? to analyze the else clause of the while loop, recognizing it as a valid termination path. This directly fixes the false positive where while...else with a return/raise in the else clause was not recognized as exhaustive.

---

Was this helpful? React with 👍 or 👎

_{Classification by primer-classifier (4 LLM)}