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 while event := ...: if ...: return True / else: return False, which is exhaustive. Pyrefly was previously unable to recognize while...else as providing complete return coverage, producing a false positive bad-return error. The PR correctly fixes this by analyzing the else branch of while loops.
Attribution: The change in pyrefly/lib/binding/function.rs in the function_last_expressions function modified the Stmt::While handling. Previously, when evaluate_bool(&x.test) != Some(true) (i.e., the while condition isn't always true), it returned None immediately, meaning pyrefly couldn't see through the while...else pattern. The new code checks: if the loop isn't infinite AND has an else clause AND has no break, it recurses into the else clause via f(sys_info, &x.orelse, res)?, allowing pyrefly to recognize that the else clause provides a return path.

setuptools (-1)

This is a clear improvement. The PR fixes pyrefly's while...else return analysis, which removes a false positive cascade: incorrect None in get_package_dir's return type → no-matching-overload on os.path.join. The get_package_dir method genuinely always returns str.
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) as a normal-termination path. Previously, non-infinite while loops would return None (meaning 'cannot determine last expression'), causing the function's return type to include None. Now, when there's an else block and no break, pyrefly processes the else block's expressions, correctly recognizing that the function returns from the else branch.

werkzeug (-1)

This is a clear improvement. The Client.open method has two return paths: (1) return response on line 1124 when follow_redirects is False, and (2) return response on line 1151 in the while...else block. Since the while loop has no break statement, the else block always executes on normal loop termination. Pyrefly was incorrectly reporting a missing return because it didn't understand while...else control flow. The PR fix in pyrefly/lib/binding/function.rs teaches pyrefly to treat the else branch of a while loop as a normal-termination path, 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., not an infinite loop), it returned None immediately, meaning it couldn't analyze the else branch. The new code now properly handles the non-infinite-loop case: when there's no break and the else block is non-empty, it recurses into x.orelse to check for returns there. This correctly recognizes that while...else with a return in the else block is exhaustive.

prefect (-1)

This is a clear improvement. The removed error was a false positive caused by pyrefly not recognizing while...else as providing exhaustive control flow coverage. The function at line 381 does return or raise on all paths — the while...else pattern at lines 523-541 ensures that either a return happens inside the loop or an exception is raised in the else clause. The PR fix correctly teaches pyrefly to analyze the else branch of while loops.
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 didn't analyze the else clause. The new code properly handles the case where the while loop has an else clause: f(sys_info, &x.orelse, res)? is now called, allowing the else block's return/raise to be recognized as a valid termination path.

---

Was this helpful? React with 👍 or 👎

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

[meta-codesync]

@grievejia has imported this pull request. If you are a Meta employee, you can view this in D98066544.