From 751a700c22d3635c2f3bdfb9b1b6cebd078251b2 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 31 Jul 2026 12:41:50 +0900 Subject: [PATCH 1/3] 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 | 13 +++++ 7 files changed, 131 insertions(+), 19 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 894363e776..cb4f1ef867 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -272,6 +272,8 @@ pub enum ErrorKind { InconsistentOverloadDefault, /// Cannot assign to a class-body-declared instance variable via the class (e.g. `C.instance_var = ...`). InstanceVarAssign, + /// 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. @@ -661,6 +663,7 @@ impl ErrorKind { ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore, ErrorKind::IncompatibleComparison => Severity::Ignore, ErrorKind::InstanceVarAssign => 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 db6ecdabd7..508feb1b1e 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3153,6 +3153,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); @@ -3162,6 +3163,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { *is_async, *is_generator, *has_explicit_return, + *has_infinite_loop, range, errors, ); @@ -7101,9 +7103,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 11d43233c5..d94e6f0c30 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1308,6 +1308,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 5d482d2d55..8540539c82 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -606,8 +606,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 @@ -616,9 +618,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, ) } @@ -630,7 +635,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, ) { @@ -642,6 +647,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 @@ -691,7 +700,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 { @@ -699,7 +712,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 { @@ -718,7 +732,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, @@ -1159,6 +1173,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>( @@ -1210,6 +1226,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 { @@ -1243,7 +1260,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) => { @@ -1255,11 +1272,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) => { @@ -1269,7 +1287,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) => { @@ -1292,10 +1310,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 { @@ -1334,7 +1352,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)), } @@ -1342,7 +1360,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; @@ -1358,7 +1382,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) } } @@ -1366,7 +1396,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; @@ -1388,10 +1418,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 ab0bae7c92..8a90dac7c7 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -62,6 +62,7 @@ "inconsistent-overload": ["legacy", "default", "strict", "all"], "inconsistent-overload-default": ["legacy", "default", "strict", "all"], "instance-var-assign": ["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 48cc306b71..b9247f3866 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1021,6 +1021,19 @@ assigned via the class; inherited fields are also covered. Standard class metada instances and is not flagged. Writes intercepted by metaclass data descriptors are also exempt from this diagnostic. Assignments via `setattr` and `del` are out of scope. +## 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 From 2ac184323e3c033d12649fc1dfb816c1a3eb1ad6 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 3 Nov 2026 03:01:20 +0900 Subject: [PATCH 2/3] comment --- crates/pyrefly_config/src/error_kind.rs | 6 +- pyrefly/lib/alt/solve.rs | 65 ++++++++----- pyrefly/lib/binding/binding.rs | 8 +- pyrefly/lib/binding/function.rs | 81 +++++----------- pyrefly/lib/test/assign.rs | 2 +- pyrefly/lib/test/flow_branching.rs | 2 +- pyrefly/lib/test/returns.rs | 123 +++++++++++++++++++++++- scripts/error_presets.json | 2 +- website/docs/error-kinds.mdx | 38 +++++--- 9 files changed, 222 insertions(+), 105 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index cb4f1ef867..47ad622a09 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -272,8 +272,6 @@ pub enum ErrorKind { InconsistentOverloadDefault, /// Cannot assign to a class-body-declared instance variable via the class (e.g. `C.instance_var = ...`). InstanceVarAssign, - /// 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. @@ -466,6 +464,8 @@ pub enum ErrorKind { UnboundName, /// An error caused by a keyword argument used in the wrong place. UnexpectedKeyword, + /// A function's inferred return type is `Never`, contrary to its annotation. + UnexpectedNever, /// An error caused by passing a positional argument for a keyword-only parameter. UnexpectedPositionalArgument, /// Attempting to use a type checker directive without importing it from `typing`. @@ -663,7 +663,7 @@ impl ErrorKind { ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore, ErrorKind::IncompatibleComparison => Severity::Ignore, ErrorKind::InstanceVarAssign => Severity::Ignore, - ErrorKind::InfiniteLoop => Severity::Warn, + ErrorKind::UnexpectedNever => 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 508feb1b1e..f50038ae3a 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -13,6 +13,7 @@ use pyrefly_graph::index::Idx; use pyrefly_python::ast::Ast; use pyrefly_python::dunder; use pyrefly_python::module_name::ModuleName; +use pyrefly_python::module_path::ModuleStyle; use pyrefly_python::short_identifier::ShortIdentifier; use pyrefly_python::sys_info::SysInfo; use pyrefly_types::dimension::Int; @@ -3152,8 +3153,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { implicit_return, is_async, is_generator, - has_explicit_return, - has_infinite_loop, + returns, + function, } => { let annotation = self.get_idx(*annotation).annotation.get_type().clone(); let implicit_return = self.get_idx(*implicit_return); @@ -3162,11 +3163,49 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { &annotation, *is_async, *is_generator, - *has_explicit_return, - *has_infinite_loop, + !returns.is_empty(), range, errors, ); + if !is_generator + && !annotation.is_never() + && !annotation.is_none() + && implicit_return.ty().is_never() + && returns.iter().all(|key| self.get_idx(*key).ty().is_never()) + { + let function = self.get_idx(*function); + let flags = &function.metadata.flags; + // Overrides keep the parent's return contract even when this body never returns. + let is_override = flags.is_override + || function.defining_cls.as_ref().is_some_and(|cls| { + self.get_mro_for_class(cls) + .ancestors(self.stdlib) + .any(|base| { + self.get_class_member( + base.class_object(), + &function.metadata.kind.function_name(), + ) + .is_some() + }) + }); + if !flags.facts().is_stub() + && !flags.body_kind.is_placeholder_or_trivial() + && !flags.is_overload + && !flags.is_abstract_method + && flags.module_style != ModuleStyle::Interface + && !is_override + { + self.error( + errors, + range, + ErrorKind::UnexpectedNever, + format!( + "Function declared to return `{}` but can never return", + self.for_display(annotation.clone()) + ), + ); + } + } } BindingExpect::MatchExhaustiveness { subject_idx, @@ -7103,27 +7142,9 @@ 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 d94e6f0c30..150e9d2b11 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1300,15 +1300,15 @@ pub enum BindingExpect { existing: Idx, name: Name, }, - /// Validate a function implementation's implicit return without making the - /// published return type depend on the function body. + /// Validate a function's return annotation without making the published + /// return type depend on the function body. ValidateImplicitReturn { annotation: Idx, implicit_return: Idx, is_async: bool, is_generator: bool, - has_explicit_return: bool, - has_infinite_loop: bool, + returns: Box<[Idx]>, + function: Idx, }, /// 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 8540539c82..1cc07c5f88 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -606,10 +606,8 @@ 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, 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| { + fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> Idx { + let last_exprs = function_last_expressions(body, self.sys_info).map(|x| { x.clauses .into_map(|clause| { clause @@ -618,12 +616,9 @@ impl<'a> BindingsBuilder<'a> { }) .into_boxed_slice() }); - ( - self.insert_binding( - Key::ReturnImplicit(ShortIdentifier::new(func_name)), - Binding::ReturnImplicit(ReturnImplicit { last_exprs }), - ), - has_infinite_loop, + self.insert_binding( + Key::ReturnImplicit(ShortIdentifier::new(func_name)), + Binding::ReturnImplicit(ReturnImplicit { last_exprs }), ) } @@ -635,7 +630,7 @@ impl<'a> BindingsBuilder<'a> { is_async: bool, yields_and_returns: YieldsAndReturns, return_ann_with_range: Option<(TextRange, Idx)>, - implicit_return: Option<(Idx, bool)>, + implicit_return: Option>, should_infer_return_type: bool, is_stub: bool, ) { @@ -647,11 +642,6 @@ 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 .returns @@ -700,11 +690,10 @@ 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, has_infinite_loop)), - false, - ) => { + (Some((range, annotation)), Some(implicit_return), false) => { + let function = self.idx_for_promise(KeyUndecoratedFunction( + ShortIdentifier::new(func_name), + )); self.insert_binding( KeyExpect::ValidateImplicitReturn(range), BindingExpect::ValidateImplicitReturn { @@ -712,8 +701,8 @@ impl<'a> BindingsBuilder<'a> { implicit_return, is_async, is_generator, - has_explicit_return: has_reachable_return, - has_infinite_loop, + returns: return_keys, + function, }, ); ReturnTypeKind::ShouldTrustAnnotation { @@ -732,7 +721,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, @@ -1173,8 +1162,6 @@ 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>( @@ -1226,7 +1213,6 @@ 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 { @@ -1260,7 +1246,7 @@ pub(crate) fn function_last_expressions<'a>( guards: guards.as_slice().into(), }); } - let body = f(sys_info, &x.body, guards, environment, has_infinite_loop)?; + let body = f(sys_info, &x.body, guards, environment)?; and_dnf(Some(vec![with_clause]), Some(body)) } Stmt::While(x) => { @@ -1272,12 +1258,11 @@ 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, has_infinite_loop) + f(sys_info, &x.orelse, guards, environment) } } Stmt::For(x) => { @@ -1287,7 +1272,7 @@ pub(crate) fn function_last_expressions<'a>( if has_break || x.orelse.is_empty() { None } else { - f(sys_info, &x.orelse, guards, environment, has_infinite_loop) + f(sys_info, &x.orelse, guards, environment) } } Stmt::If(x) => { @@ -1310,10 +1295,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, has_infinite_loop)? + f(sys_info, body, guards, environment)? } else { guards.push(range); - let branch = f(sys_info, body, guards, environment, has_infinite_loop); + let branch = f(sys_info, body, guards, environment); guards.pop(); branch.unwrap_or_else(|| { vec![vec![LastPathExpr { @@ -1352,7 +1337,7 @@ pub(crate) fn function_last_expressions<'a>( } else { &x.orelse }; - match f(sys_info, body_suite, guards, environment, has_infinite_loop) { + match f(sys_info, body_suite, guards, environment) { None => body_acc = None, Some(p) => body_acc = and_dnf(body_acc, Some(p)), } @@ -1360,13 +1345,7 @@ 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, - has_infinite_loop, - ) { + match f(sys_info, &h.body, guards, environment) { None => { body_acc = None; break; @@ -1382,13 +1361,7 @@ 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, - has_infinite_loop, - ); + let finally_paths = f(sys_info, &x.finalbody, guards, environment); or_dnf(finally_paths, body_paths) } } @@ -1396,7 +1369,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, has_infinite_loop)?; + let branch = f(sys_info, &case.body, guards, environment)?; acc = and_dnf(acc, Some(branch)); if match_case_always_matches(&case.pattern, &x.subject, case.guard.as_deref()) { syntactically_exhaustive = true; @@ -1418,18 +1391,10 @@ pub(crate) fn function_last_expressions<'a>( } let mut environment = false; - let mut has_infinite_loop = false; - let clauses = f( - sys_info, - x, - &mut Vec::new(), - &mut environment, - &mut has_infinite_loop, - )?; + let clauses = f(sys_info, x, &mut Vec::new(), &mut environment)?; Some(LastExpressions { clauses, decided_by_environment: environment, - has_infinite_loop, }) } diff --git a/pyrefly/lib/test/assign.rs b/pyrefly/lib/test/assign.rs index 810db0ef21..9bef4961d8 100644 --- a/pyrefly/lib/test/assign.rs +++ b/pyrefly/lib/test/assign.rs @@ -324,7 +324,7 @@ testcase!( test_never, r#" from typing import Any, Never, NoReturn -def foo(x: Never) -> Any: +def foo(x: Never) -> Any: # E: Function declared to return `Any` but can never return y: NoReturn = x z: int = x return x diff --git a/pyrefly/lib/test/flow_branching.rs b/pyrefly/lib/test/flow_branching.rs index d9f16a8b0f..968499204a 100644 --- a/pyrefly/lib/test/flow_branching.rs +++ b/pyrefly/lib/test/flow_branching.rs @@ -1912,7 +1912,7 @@ assert_type(vari, Literal["test"]) # E: `vari` is uninitialized testcase!( test_assert_false_terminates_flow, r#" -def test1() -> int: +def test1() -> int: # E: Function declared to return `int` but can never return assert False def test2() -> int: # E: Function declared to return `int` but is missing an explicit `return` assert True diff --git a/pyrefly/lib/test/returns.rs b/pyrefly/lib/test/returns.rs index 9af6a8ecf4..af6bbbfa25 100644 --- a/pyrefly/lib/test/returns.rs +++ b/pyrefly/lib/test/returns.rs @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +use crate::test::util::TestEnv; use crate::testcase; testcase!( @@ -188,7 +189,7 @@ from typing import NoReturn def fail() -> NoReturn: raise Exception() -def f() -> int: +def f() -> int: # E: Function declared to return `int` but can never return fail() "#, ); @@ -360,6 +361,124 @@ def falls_through(b: bool) -> int: # E: Function declared to return `int` but i "#, ); +testcase!( + test_unexpected_never_return, + r#" +from abc import ABC, abstractmethod +from typing import Any, AsyncIterator, Iterator, Never, NoReturn, Protocol, assert_type, overload, override + +def fatal() -> Never: + raise RuntimeError + +def calls_fatal() -> int: # E: Function declared to return `int` but can never return + fatal() + +def returns_fatal() -> str: # E: Function declared to return `str` but can never return + return fatal() + +def raises() -> int: # E: Function declared to return `int` but can never return + raise ValueError + +def raises_none() -> None: + raise ValueError + +async def async_fatal() -> int: # E: Function declared to return `int` but can never return + return fatal() + +def finally_fatal() -> int: # E: Function declared to return `int` but can never return + try: + pass + finally: + fatal() + +def branches(flag: bool) -> int: # E: Function declared to return `int` but can never return + if flag: + return fatal() + else: + raise ValueError + +def returns_normally(flag: bool) -> int: + if flag: + fatal() + return 1 + +def gradual() -> Any: # E: Function declared to return `Any` but can never return + fatal() + +def placeholder() -> int: + """Implementation belongs in a subclass.""" + raise NotImplementedError("not implemented") + +def stub() -> int: ... + +def intentional() -> NoReturn: + fatal() + +def generator() -> Iterator[int]: + yield 1 + fatal() + +async def async_generator() -> AsyncIterator[int]: + yield 1 + fatal() + +@overload +def overloaded(x: int) -> int: ... +@overload +def overloaded(x: str) -> str: ... +def overloaded(x: int | str) -> int | str: # E: Function declared to return `int | str` but can never return + fatal() + +class Base(ABC): + @abstractmethod + def abstract(self) -> int: + fatal() + + def implemented(self) -> int: + return 1 + +class Child(Base): + def abstract(self) -> int: + fatal() + + @override + def implemented(self) -> int: + fatal() + + def raising_none(self) -> None: + """This operation is unsupported.""" + raise ValueError + + def calls_fatal(self) -> int: # E: Function declared to return `int` but can never return + fatal() + +class Interface(Protocol): + def stub(self) -> int: ... + +# The published type must still follow the annotation. +assert_type(calls_fatal(), int) +"#, +); + +testcase!( + test_unexpected_never_in_stub_file, + { + let mut env = TestEnv::new(); + env.add_with_path( + "stub", + "stub.pyi", + "def f() -> int:\n raise RuntimeError\n", + ); + env + }, + r#" +from typing import assert_type +from stub import f + +assert_type(f(), int) +"#, +); + testcase!( test_return_while_false_break_else, r#" @@ -415,7 +534,7 @@ testcase!( r#" from typing import assert_type -def f() -> int: +def f() -> int: # E: Function declared to return `int` but can never return raise Exception() assert_type(f(), int) "#, diff --git a/scripts/error_presets.json b/scripts/error_presets.json index 8a90dac7c7..c53b664d58 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -62,7 +62,6 @@ "inconsistent-overload": ["legacy", "default", "strict", "all"], "inconsistent-overload-default": ["legacy", "default", "strict", "all"], "instance-var-assign": ["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"], @@ -135,6 +134,7 @@ "unannotated-return": ["all"], "unbound-name": ["default", "strict", "all"], "unexpected-keyword": ["basic", "legacy", "default", "strict", "all"], + "unexpected-never": ["legacy", "default", "strict", "all"], "unexpected-positional-argument": ["basic", "legacy", "default", "strict", "all"], "unimported-directive": ["legacy", "default", "strict", "all"], "uninitialized-instance-variable": ["all"], diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index b9247f3866..9304753d60 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1021,19 +1021,6 @@ assigned via the class; inherited fields are also covered. Standard class metada instances and is not flagged. Writes intercepted by metaclass data descriptors are also exempt from this diagnostic. Assignments via `setattr` and `del` are out of scope. -## 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 @@ -2229,6 +2216,31 @@ def two_args(a: int, b: int) -> int: two_args(a=1, b=2, c=3) ``` +## unexpected-never + +Default severity: `warn` + +Pyrefly emits this warning when the inferred return type is `Never`, but the +return annotation is neither `Never`, `NoReturn`, nor `None`. This includes infinite +loops, unconditional exceptions, and calls to functions that never return. +Pyrefly checks both explicit returns and paths that reach the end of the body. + +```python +from typing import Never + +def stop() -> Never: + raise RuntimeError + +def f() -> int: # Function declared to return `int` but can never return [unexpected-never] + return stop() +``` + +Pyrefly exempts placeholder bodies, stubs, overload declarations, abstract +methods, and overrides, including overrides without an `@override` decorator. +It also exempts generators and functions annotated `-> None`, including +long-running event loops and methods that always raise. The declared return +type remains the type visible to callers. + ## unexpected-positional-argument A positional argument was passed for a keyword-only parameter. From 8c952ecde9e3ea70a0adc9686f23ba63dd5f8361 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Wed, 4 Nov 2026 21:44:57 +0900 Subject: [PATCH 3/3] fix test --- pyrefly/lib/alt/solve.rs | 4 +++- pyrefly/lib/binding/binding.rs | 1 + pyrefly/lib/binding/function.rs | 23 ++++++++++++++++------- pyrefly/lib/test/narrow.rs | 6 +++--- pyrefly/lib/test/returns.rs | 16 ++++++++++++++-- website/docs/error-kinds.mdx | 3 +++ 6 files changed, 40 insertions(+), 13 deletions(-) diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index f50038ae3a..ac62f50155 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -3155,6 +3155,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { is_generator, returns, function, + check_never_return, } => { let annotation = self.get_idx(*annotation).annotation.get_type().clone(); let implicit_return = self.get_idx(*implicit_return); @@ -3167,7 +3168,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { range, errors, ); - if !is_generator + if *check_never_return + && !is_generator && !annotation.is_never() && !annotation.is_none() && implicit_return.ty().is_never() diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 150e9d2b11..3decdd728b 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -1309,6 +1309,7 @@ pub enum BindingExpect { is_generator: bool, returns: Box<[Idx]>, function: Idx, + check_never_return: 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 1cc07c5f88..eaad0328db 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -633,6 +633,7 @@ impl<'a> BindingsBuilder<'a> { implicit_return: Option>, should_infer_return_type: bool, is_stub: bool, + check_never_return: bool, ) { let is_generator = yields_and_returns.is_generator; let return_ann = return_ann_with_range.as_ref().map(|(_, key)| *key); @@ -703,6 +704,7 @@ impl<'a> BindingsBuilder<'a> { is_generator, returns: return_keys, function, + check_never_return, }, ); ReturnTypeKind::ShouldTrustAnnotation { @@ -864,16 +866,21 @@ impl<'a> BindingsBuilder<'a> { } else { body.as_slice() }; - let body_kind = match body_no_docstring { - // raise NotImplementedError(...) - [Stmt::Raise(StmtRaise { exc: Some(exc), .. })] - if self.as_special_export(match &**exc { + let raises_not_implemented = match body_no_docstring.last() { + Some(Stmt::Raise(StmtRaise { exc: Some(exc), .. })) => { + self.as_special_export(match &**exc { Expr::Call(ExprCall { func, .. }) => func, other => other, - }) == Some(SpecialExport::NotImplementedError) => - { - BodyKind::RaiseNotImplementedError + }) == Some(SpecialExport::NotImplementedError) } + _ => false, + }; + // A raise-only body or a terminal NotImplementedError can stand in for an implementation. + let check_never_return = + !raises_not_implemented && !matches!(body_no_docstring, [Stmt::Raise(_)]); + let body_kind = match body_no_docstring { + // raise NotImplementedError(...) + [Stmt::Raise(_)] if raises_not_implemented => BodyKind::RaiseNotImplementedError, // return NotImplemented [ Stmt::Return(StmtReturn { @@ -962,6 +969,7 @@ impl<'a> BindingsBuilder<'a> { implicit_return, false, facts.is_stub(), + check_never_return, ); (false, self_assignments) } else { @@ -999,6 +1007,7 @@ impl<'a> BindingsBuilder<'a> { implicit_return, should_infer, facts.is_stub(), + check_never_return, ); // Mirror the `ReturnTypeKind::ShouldInferType` arm in `analyze_return_type`: // we infer iff there's no return annotation and inference was requested. diff --git a/pyrefly/lib/test/narrow.rs b/pyrefly/lib/test/narrow.rs index 44a705ea70..1f208680fb 100644 --- a/pyrefly/lib/test/narrow.rs +++ b/pyrefly/lib/test/narrow.rs @@ -1003,7 +1003,7 @@ testcase!( from typing import assert_type def make_it() -> int | tuple[int, str] | tuple[bool] | tuple[str]: - assert False + ... def test(): c = make_it() @@ -1027,7 +1027,7 @@ testcase!( from typing import assert_type def make_it() -> int | list[int] | list[bool] | list[str]: - assert False + ... def test(): c = make_it() @@ -1087,7 +1087,7 @@ testcase!( from typing import assert_type, Iterable def make_it() -> int | tuple[int, str] | list[bool] | list[str]: - assert False + ... def test(): c = make_it() diff --git a/pyrefly/lib/test/returns.rs b/pyrefly/lib/test/returns.rs index af6bbbfa25..0c2fc1d68c 100644 --- a/pyrefly/lib/test/returns.rs +++ b/pyrefly/lib/test/returns.rs @@ -376,7 +376,15 @@ def calls_fatal() -> int: # E: Function declared to return `int` but can never def returns_fatal() -> str: # E: Function declared to return `str` but can never return return fatal() -def raises() -> int: # E: Function declared to return `int` but can never return +def raises() -> int: + raise ValueError + +def raises_with_docstring() -> int: + """This operation is unsupported.""" + raise ValueError + +def raises_after_work() -> int: # E: Function declared to return `int` but can never return + print("starting") raise ValueError def raises_none() -> None: @@ -409,6 +417,10 @@ def placeholder() -> int: """Implementation belongs in a subclass.""" raise NotImplementedError("not implemented") +def placeholder_with_checks(x: int) -> int: + assert_type(x, int) + raise NotImplementedError + def stub() -> int: ... def intentional() -> NoReturn: @@ -534,7 +546,7 @@ testcase!( r#" from typing import assert_type -def f() -> int: # E: Function declared to return `int` but can never return +def f() -> int: raise Exception() assert_type(f(), int) "#, diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 9304753d60..16194bef83 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -2237,6 +2237,9 @@ def f() -> int: # Function declared to return `int` but can never return [unexp Pyrefly exempts placeholder bodies, stubs, overload declarations, abstract methods, and overrides, including overrides without an `@override` decorator. +For this warning, a placeholder body can be a single `raise` statement or a +body that ends with `raise NotImplementedError`. An optional docstring does +not affect these exemptions. It also exempts generators and functions annotated `-> None`, including long-running event loops and methods that always raise. The declared return type remains the type visible to callers.