fix

Author: asukaminato0721

crates/pyrefly_config/src/error_kind.rs

@@ -242,6 +242,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.
@@ -616,6 +618,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

@@ -2894,6 +2894,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);
@@ -2903,6 +2904,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
                     *is_async,
                     *is_generator,
                     *has_explicit_return,
+                    *has_infinite_loop,
                     range,
                     errors,
                 );
@@ -6524,9 +6526,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

@@ -1253,6 +1253,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

@@ -484,8 +484,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.exprs
                 .into_map(|(last, x)| {
                     (
@@ -495,9 +497,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,
         )
     }
 
@@ -509,7 +514,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,
     ) {
@@ -521,6 +526,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
@@ -570,15 +579,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 {
@@ -597,7 +611,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,
@@ -985,6 +999,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,
 }
 
 /// Given the body of a function, what are the potential expressions that
@@ -1002,6 +1018,7 @@ pub(crate) fn function_last_expressions<'a>(
         x: &'a [Stmt],
         res: &mut Vec<(LastStmt, &'a Expr)>,
         environment: &mut bool,
+        has_infinite_loop: &mut bool,
     ) -> Option<()> {
         fn loop_body_has_break_statement(statement: &Stmt, has_break: &mut bool) {
             match statement {
@@ -1025,7 +1042,7 @@ pub(crate) fn function_last_expressions<'a>(
                 for y in &x.items {
                     res.push((LastStmt::With(kind), &y.context_expr));
                 }
-                f(sys_info, &x.body, res, environment)?;
+                f(sys_info, &x.body, res, environment, has_infinite_loop)?;
             }
             Stmt::While(x) => {
                 let test_value = sys_info.evaluate_bool(&x.test);
@@ -1038,10 +1055,11 @@ pub(crate) fn function_last_expressions<'a>(
                 }
                 if test_value == Some(true) && !has_break {
                     // Infinite loop with no break never falls through.
+                    *has_infinite_loop = true;
                 } else if has_break || x.orelse.is_empty() {
                     return None;
                 } else {
-                    f(sys_info, &x.orelse, res, environment)?;
+                    f(sys_info, &x.orelse, res, environment, has_infinite_loop)?;
                 }
             }
             Stmt::For(x) => {
@@ -1051,7 +1069,7 @@ pub(crate) fn function_last_expressions<'a>(
                 if has_break || x.orelse.is_empty() {
                     return None;
                 }
-                f(sys_info, &x.orelse, res, environment)?;
+                f(sys_info, &x.orelse, res, environment, has_infinite_loop)?;
             }
             Stmt::If(x) => {
                 *environment |= iter::once(Some(&*x.test))
@@ -1062,7 +1080,7 @@ pub(crate) fn function_last_expressions<'a>(
                 for (test, body) in sys_info.pruned_if_branches(x) {
                     any_branch_processed = true;
                     last_test = test;
-                    f(sys_info, body, res, environment)?;
+                    f(sys_info, body, res, environment, has_infinite_loop)?;
                 }
                 if !any_branch_processed {
                     // All branches were pruned, so the code falls through
@@ -1087,17 +1105,17 @@ pub(crate) fn function_last_expressions<'a>(
                         .iter()
                         .any(|stmt| matches!(stmt, Stmt::Return(_)))
                 {
-                    f(sys_info, &x.finalbody, res, environment)?;
+                    f(sys_info, &x.finalbody, res, environment, has_infinite_loop)?;
                 } else {
                     if x.orelse.is_empty() {
-                        f(sys_info, &x.body, res, environment)?;
+                        f(sys_info, &x.body, res, environment, has_infinite_loop)?;
                     } else {
-                        f(sys_info, &x.orelse, res, environment)?;
+                        f(sys_info, &x.orelse, res, environment, has_infinite_loop)?;
                     }
                     for handler in &x.handlers {
                         match handler {
                             ExceptHandler::ExceptHandler(x) => {
-                                f(sys_info, &x.body, res, environment)?
+                                f(sys_info, &x.body, res, environment, has_infinite_loop)?
                             }
                         }
                     }
@@ -1107,7 +1125,7 @@ pub(crate) fn function_last_expressions<'a>(
             Stmt::Match(x) => {
                 let mut syntactically_exhaustive = false;
                 for case in x.cases.iter() {
-                    f(sys_info, &case.body, res, environment)?;
+                    f(sys_info, &case.body, res, environment, has_infinite_loop)?;
                     // Must match the binding step's exhaustiveness judgment in
                     // `stmt_match`; otherwise the `Key::Exhaustive(Match, ...)` promised
                     // below is never inserted and solve time panics.
@@ -1134,10 +1152,18 @@ pub(crate) fn function_last_expressions<'a>(
 
     let mut res = Vec::new();
     let mut environment = false;
-    f(sys_info, x, &mut res, &mut environment)?;
+    let mut has_infinite_loop = false;
+    f(
+        sys_info,
+        x,
+        &mut res,
+        &mut environment,
+        &mut has_infinite_loop,
+    )?;
     Some(LastExpressions {
         exprs: res,
         decided_by_environment: environment,
+        has_infinite_loop,
     })
 }
 

pyrefly/lib/test/returns.rs

@@ -282,6 +282,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

@@ -60,6 +60,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

@@ -976,6 +976,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).