fix show warning when function returning Not[Never] contains a Never loop #1132

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,

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)

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),

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<Key>` (which could lead
     /// to a panic).
-    fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> Idx<Key> {
-        let last_exprs = function_last_expressions(body, self.sys_info).map(|x| {
+    fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> (Idx<Key>, 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<KeyAnnotation>)>,
-        implicit_return: Option<Idx<Key>>,
+        implicit_return: Option<(Idx<Key>, 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,15 +577,20 @@ 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 {
                             annotation,
                             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<TextRange>,
         environment: &mut bool,
+        has_infinite_loop: &mut bool,
     ) -> Option<Vec<Vec<LastPathExpr<'a>>>> {
         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,15 +1187,21 @@ 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)),
                     }
                     if body_acc.is_some() {
                         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,15 +1217,21 @@ 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)
                 }
             }
             Stmt::Match(x) => {
                 let mut syntactically_exhaustive = false;
                 let mut acc: Option<Vec<Vec<LastPathExpr<'a>>>> = 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,
     })
 }
 

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#"

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"],

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).