From 2b2bd23b3e16485d537fbcb32aa7180523eddfde Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 31 Jul 2026 12:41:50 +0900 Subject: [PATCH] fix --- crates/pyrefly_config/src/error_kind.rs | 3 + pyrefly/lib/alt/solve.rs | 20 +++++++ pyrefly/lib/binding/binding.rs | 1 + pyrefly/lib/binding/function.rs | 76 ++++++++++++++++++------- pyrefly/lib/test/returns.rs | 36 ++++++++++++ scripts/error_presets.json | 1 + website/docs/error-kinds.mdx | 14 +++++ 7 files changed, 132 insertions(+), 19 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 9f48799a62..dc254b4278 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -250,6 +250,8 @@ pub enum ErrorKind { /// An inconsistency between a function parameter's type in an overload signature and its /// default value in the implementation. InconsistentOverloadDefault, + /// A function declares a non-None, non-Never return type but can only loop forever. + InfiniteLoop, /// Internal Pyrefly error. InternalError, /// An `@abstractmethod` is defined in a class that is not abstract. @@ -636,6 +638,7 @@ impl ErrorKind { ErrorKind::ImplicitReexport => Severity::Ignore, ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore, ErrorKind::IncompatibleComparison => Severity::Ignore, + ErrorKind::InfiniteLoop => Severity::Warn, ErrorKind::InvalidAbstractMethod => Severity::Ignore, ErrorKind::InvalidCast => Severity::Ignore, ErrorKind::InvalidDecorator => Severity::Warn, diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 32a82afc6f..861d82616f 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3069,6 +3069,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { is_async, is_generator, has_explicit_return, + has_infinite_loop, } => { let annotation = self.get_idx(*annotation).annotation.get_type().clone(); let implicit_return = self.get_idx(*implicit_return); @@ -3078,6 +3079,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { *is_async, *is_generator, *has_explicit_return, + *has_infinite_loop, range, errors, ); @@ -6854,9 +6856,27 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { is_async: bool, is_generator: bool, has_explicit_returns: bool, + has_infinite_loop: bool, range: TextRange, errors: &ErrorCollector, ) { + if has_infinite_loop + && !has_explicit_returns + && !is_generator + && !annotation.is_never() + && !annotation.is_none() + && implicit_return.ty().is_never() + { + self.error( + errors, + range, + ErrorKind::InfiniteLoop, + format!( + "Function declared to return `{}` but can never return", + self.for_display(annotation.clone()) + ), + ); + } if is_async && is_generator { let hints = self.decompose_hint(HintRef::soft(annotation), |hint| { self.decompose_async_generator(hint) diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 33c1e032cb..9b223e47bb 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1300,6 +1300,7 @@ pub enum BindingExpect { is_async: bool, is_generator: bool, has_explicit_return: bool, + has_infinite_loop: bool, }, /// Expression used in a boolean context (`bool()`, `if`, or `while`) Bool(Expr), diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index d8d5c05e4e..c064805733 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -483,8 +483,10 @@ impl<'a> BindingsBuilder<'a> { /// This function must not be called unless the function body statements will be bound; /// it relies on that binding to ensure we don't have a dangling `Idx` (which could lead /// to a panic). - fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> Idx { - let last_exprs = function_last_expressions(body, self.sys_info).map(|x| { + fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> (Idx, bool) { + let last_exprs = function_last_expressions(body, self.sys_info); + let has_infinite_loop = last_exprs.as_ref().is_some_and(|x| x.has_infinite_loop); + let last_exprs = last_exprs.map(|x| { x.clauses .into_map(|clause| { clause @@ -493,9 +495,12 @@ impl<'a> BindingsBuilder<'a> { }) .into_boxed_slice() }); - self.insert_binding( - Key::ReturnImplicit(ShortIdentifier::new(func_name)), - Binding::ReturnImplicit(ReturnImplicit { last_exprs }), + ( + self.insert_binding( + Key::ReturnImplicit(ShortIdentifier::new(func_name)), + Binding::ReturnImplicit(ReturnImplicit { last_exprs }), + ), + has_infinite_loop, ) } @@ -507,7 +512,7 @@ impl<'a> BindingsBuilder<'a> { is_async: bool, yields_and_returns: YieldsAndReturns, return_ann_with_range: Option<(TextRange, Idx)>, - implicit_return: Option>, + implicit_return: Option<(Idx, bool)>, should_infer_return_type: bool, is_stub: bool, ) { @@ -519,6 +524,10 @@ impl<'a> BindingsBuilder<'a> { } else { None }; + let has_reachable_return = yields_and_returns + .returns + .iter() + .any(|(_, _, is_unreachable)| !is_unreachable); // Collect the keys of explicit returns. let return_keys = yields_and_returns @@ -568,7 +577,11 @@ impl<'a> BindingsBuilder<'a> { let return_type_binding = { let kind = match (return_ann_with_range, implicit_return, is_stub) { - (Some((range, annotation)), Some(implicit_return), false) => { + ( + Some((range, annotation)), + Some((implicit_return, has_infinite_loop)), + false, + ) => { self.insert_binding( KeyExpect::ValidateImplicitReturn(range), BindingExpect::ValidateImplicitReturn { @@ -576,7 +589,8 @@ impl<'a> BindingsBuilder<'a> { implicit_return, is_async, is_generator, - has_explicit_return: !return_keys.is_empty(), + has_explicit_return: has_reachable_return, + has_infinite_loop, }, ); ReturnTypeKind::ShouldTrustAnnotation { @@ -595,7 +609,7 @@ impl<'a> BindingsBuilder<'a> { is_generator, } } - (None, Some(implicit_return), _) if should_infer_return_type => { + (None, Some((implicit_return, _)), _) if should_infer_return_type => { // We don't have an explicit return annotation, but we want to infer it. ReturnTypeKind::ShouldInferType { returns: return_keys, @@ -994,6 +1008,8 @@ pub(crate) struct LastExpressions<'a> { /// settling an assertion. The answer then holds for that configuration only, which suits /// inferring a return type but not reporting code as dead. pub decided_by_environment: bool, + /// Set when an unconditional loop is one of the terminal paths. + pub has_infinite_loop: bool, } fn or_dnf<'a>( @@ -1045,6 +1061,7 @@ pub(crate) fn function_last_expressions<'a>( x: &'a [Stmt], guards: &mut Vec, environment: &mut bool, + has_infinite_loop: &mut bool, ) -> Option>>> { fn loop_body_has_break_statement(statement: &Stmt, has_break: &mut bool) { match statement { @@ -1078,7 +1095,7 @@ pub(crate) fn function_last_expressions<'a>( guards: guards.as_slice().into(), }); } - let body = f(sys_info, &x.body, guards, environment)?; + let body = f(sys_info, &x.body, guards, environment, has_infinite_loop)?; and_dnf(Some(vec![with_clause]), Some(body)) } Stmt::While(x) => { @@ -1090,11 +1107,12 @@ pub(crate) fn function_last_expressions<'a>( .visit(&mut |stmt| loop_body_has_break_statement(stmt, &mut has_break)); } if test_value == Some(true) && !has_break { + *has_infinite_loop = true; Some(vec![vec![]]) } else if has_break || x.orelse.is_empty() { None } else { - f(sys_info, &x.orelse, guards, environment) + f(sys_info, &x.orelse, guards, environment, has_infinite_loop) } } Stmt::For(x) => { @@ -1104,7 +1122,7 @@ pub(crate) fn function_last_expressions<'a>( if has_break || x.orelse.is_empty() { None } else { - f(sys_info, &x.orelse, guards, environment) + f(sys_info, &x.orelse, guards, environment, has_infinite_loop) } } Stmt::If(x) => { @@ -1127,10 +1145,10 @@ pub(crate) fn function_last_expressions<'a>( // evaluates as false. // Neither can be dead by its test, so it needs no guard. let branch = if index == 0 && test.is_none() { - f(sys_info, body, guards, environment)? + f(sys_info, body, guards, environment, has_infinite_loop)? } else { guards.push(range); - let branch = f(sys_info, body, guards, environment); + let branch = f(sys_info, body, guards, environment, has_infinite_loop); guards.pop(); branch.unwrap_or_else(|| { vec![vec![LastPathExpr { @@ -1169,7 +1187,7 @@ pub(crate) fn function_last_expressions<'a>( } else { &x.orelse }; - match f(sys_info, body_suite, guards, environment) { + match f(sys_info, body_suite, guards, environment, has_infinite_loop) { None => body_acc = None, Some(p) => body_acc = and_dnf(body_acc, Some(p)), } @@ -1177,7 +1195,13 @@ pub(crate) fn function_last_expressions<'a>( for handler in &x.handlers { match handler { ExceptHandler::ExceptHandler(h) => { - match f(sys_info, &h.body, guards, environment) { + match f( + sys_info, + &h.body, + guards, + environment, + has_infinite_loop, + ) { None => { body_acc = None; break; @@ -1193,7 +1217,13 @@ pub(crate) fn function_last_expressions<'a>( if x.finalbody.is_empty() { body_paths } else { - let finally_paths = f(sys_info, &x.finalbody, guards, environment); + let finally_paths = f( + sys_info, + &x.finalbody, + guards, + environment, + has_infinite_loop, + ); or_dnf(finally_paths, body_paths) } } @@ -1201,7 +1231,7 @@ pub(crate) fn function_last_expressions<'a>( let mut syntactically_exhaustive = false; let mut acc: Option>>> = Some(vec![vec![]]); for case in x.cases.iter() { - let branch = f(sys_info, &case.body, guards, environment)?; + let branch = f(sys_info, &case.body, guards, environment, has_infinite_loop)?; acc = and_dnf(acc, Some(branch)); if match_case_always_matches(&case.pattern, &x.subject, case.guard.as_deref()) { syntactically_exhaustive = true; @@ -1223,10 +1253,18 @@ pub(crate) fn function_last_expressions<'a>( } let mut environment = false; - let clauses = f(sys_info, x, &mut Vec::new(), &mut environment)?; + let mut has_infinite_loop = false; + let clauses = f( + sys_info, + x, + &mut Vec::new(), + &mut environment, + &mut has_infinite_loop, + )?; Some(LastExpressions { clauses, decided_by_environment: environment, + has_infinite_loop, }) } diff --git a/pyrefly/lib/test/returns.rs b/pyrefly/lib/test/returns.rs index 58ddd9fb84..9af6a8ecf4 100644 --- a/pyrefly/lib/test/returns.rs +++ b/pyrefly/lib/test/returns.rs @@ -324,6 +324,42 @@ def f() -> int: "#, ); +// Regression test for https://github.com/facebook/pyrefly/issues/1132 +testcase!( + test_return_infinite_loop_without_return, + r#" +from typing import Never, TypeAlias + +NeverAlias: TypeAlias = Never + +def f() -> int: # E: Function declared to return `int` but can never return + while True: + pass + +def returns_never() -> Never: + while True: + pass + +def returns_never_alias() -> NeverAlias: + while True: + pass + +def event_loop() -> None: + while True: + pass + +def unreachable_return() -> int: # E: Function declared to return `int` but can never return + while True: + continue + return 1 # E: This code is unreachable + +def falls_through(b: bool) -> int: # E: Function declared to return `int` but is missing an explicit `return` + if b: + while True: + pass +"#, +); + testcase!( test_return_while_false_break_else, r#" diff --git a/scripts/error_presets.json b/scripts/error_presets.json index 96e04f2ea0..311cbe9e77 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -61,6 +61,7 @@ "inconsistent-inheritance": ["legacy", "default", "strict", "all"], "inconsistent-overload": ["legacy", "default", "strict", "all"], "inconsistent-overload-default": ["legacy", "default", "strict", "all"], + "infinite-loop": ["legacy", "default", "strict", "all"], "internal-error": ["legacy", "default", "strict", "all"], "invalid-abstract-method": ["all"], "invalid-annotation": ["basic", "legacy", "default", "strict", "all"], diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index e0d3ce5806..0681373ab9 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -999,6 +999,20 @@ def f(x: bool = False) -> int | None: return 0 if x else None ``` +## infinite-loop + +Default severity: `warn` + +This warning is emitted when a function declares a return type other than +`None`, `Never`, or `NoReturn`, but its body has no reachable return and ends +in an unconditional loop. + +```python +def f() -> int: # Function declared to return `int` but can never return [infinite-loop] + while True: + pass +``` + ## internal-error Ideally you'll never see this one. If you do, please consider [filing a bug](https://github.com/facebook/pyrefly/issues).