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

crates/pyrefly_config/src/error_kind.rs

@@ -464,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`.
@@ -661,6 +663,7 @@ impl ErrorKind {
             ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore,
             ErrorKind::IncompatibleComparison => Severity::Ignore,
             ErrorKind::InstanceVarAssign => Severity::Ignore,
+            ErrorKind::UnexpectedNever => Severity::Warn,
             ErrorKind::InvalidAbstractMethod => Severity::Ignore,
             ErrorKind::InvalidCast => Severity::Ignore,
             ErrorKind::InvalidDecorator => Severity::Warn,

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,7 +3153,9 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
                 implicit_return,
                 is_async,
                 is_generator,
-                has_explicit_return,
+                returns,
+                function,
+                check_never_return,
             } => {
                 let annotation = self.get_idx(*annotation).annotation.get_type().clone();
                 let implicit_return = self.get_idx(*implicit_return);
@@ -3161,10 +3164,50 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
                     &annotation,
                     *is_async,
                     *is_generator,
-                    *has_explicit_return,
+                    !returns.is_empty(),
                     range,
                     errors,
                 );
+                if *check_never_return
+                    && !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,

pyrefly/lib/binding/binding.rs

@@ -1300,14 +1300,16 @@ pub enum BindingExpect {
         existing: Idx<KeyAnnotation>,
         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<KeyAnnotation>,
         implicit_return: Idx<Key>,
         is_async: bool,
         is_generator: bool,
-        has_explicit_return: bool,
+        returns: Box<[Idx<Key>]>,
+        function: Idx<KeyUndecoratedFunction>,
+        check_never_return: bool,
     },
     /// Expression used in a boolean context (`bool()`, `if`, or `while`)
     Bool(Expr),

pyrefly/lib/binding/function.rs

@@ -633,6 +633,7 @@ impl<'a> BindingsBuilder<'a> {
         implicit_return: Option<Idx<Key>>,
         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);
@@ -642,7 +643,6 @@ impl<'a> BindingsBuilder<'a> {
             } else {
                 None
             };
-
         // Collect the keys of explicit returns.
         let return_keys = yields_and_returns
             .returns
@@ -692,14 +692,19 @@ 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) => {
+                    let function = self.idx_for_promise(KeyUndecoratedFunction(
+                        ShortIdentifier::new(func_name),
+                    ));
                     self.insert_binding(
                         KeyExpect::ValidateImplicitReturn(range),
                         BindingExpect::ValidateImplicitReturn {
                             annotation,
                             implicit_return,
                             is_async,
                             is_generator,
-                            has_explicit_return: !return_keys.is_empty(),
+                            returns: return_keys,
+                            function,
+                            check_never_return,
                         },
                     );
                     ReturnTypeKind::ShouldTrustAnnotation {
@@ -861,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 {
@@ -959,6 +969,7 @@ impl<'a> BindingsBuilder<'a> {
                 implicit_return,
                 false,
                 facts.is_stub(),
+                check_never_return,
             );
             (false, self_assignments)
         } else {
@@ -996,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.

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

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

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