fix __bool__ check should look at Never return #1021

pyrefly/lib/alt/attr.rs

@@ -66,6 +66,7 @@ use crate::types::typed_dict::TypedDict;
 use crate::types::types::AnyStyle;
 use crate::types::types::BoundMethodType;
 use crate::types::types::Overload;
+use crate::types::types::OverloadType;
 use crate::types::types::SuperObj;
 use crate::types::types::Type;
 
@@ -2562,24 +2563,87 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                 false,
             );
 
-            if let Some(dunder_bool_ty) = dunder_bool_ty
-                && !dunder_bool_ty.is_never()
-                && self.as_call_target(dunder_bool_ty.clone()).is_error()
-            {
-                self.error(
-                    errors,
-                    range,
-                    ErrorInfo::Kind(ErrorKind::InvalidArgument),
-                    format!(
-                        "The `__bool__` attribute of `{}` has type `{}`, which is not callable",
-                        self.for_display(union_member_ty.clone()),
-                        self.for_display(dunder_bool_ty.clone()),
-                    ),
-                );
+            if let Some(dunder_bool_ty) = dunder_bool_ty {
+                if dunder_bool_ty.is_never() {
+                    return;
+                }
+
+                if self.as_call_target(dunder_bool_ty.clone()).is_error() {
+                    self.error(
+                        errors,
+                        range,
+                        ErrorInfo::Kind(ErrorKind::InvalidArgument),
+                        format!(
+                            "The `__bool__` attribute of `{}` has type `{}`, which is not callable",
+                            self.for_display(union_member_ty.clone()),
+                            self.for_display(dunder_bool_ty.clone()),
+                        ),
+                    );
+                    return;
+                }
+
+                if dunder_bool_ty
+                    .callable_return_type(self.heap)
+                    .is_some_and(|ret| {
+                        ret.is_never()
+                            && self.callable_has_explicit_return_annotation(&dunder_bool_ty)
+                    })
+                {
+                    self.error(
+                        errors,
+                        range,
+                        ErrorInfo::Kind(ErrorKind::InvalidArgument),
+                        format!(
+                            "The `__bool__` method of `{}` returns `Never`, so it cannot be used as a boolean",
+                            self.for_display(union_member_ty.clone()),
+                        ),
+                    );
+                }
             }
         };
         self.map_over_union(type_of_term_used_as_bool, f)
     }
+
+    fn callable_has_explicit_return_annotation(&self, ty: &Type) -> bool {
+        let function_has_explicit_return_annotation = |func: &Function| match &func.metadata.kind {
+            FunctionKind::Def(func_id) => func_id.def_index.is_some_and(|def_index| {
+                self.bindings()
+                    .function_def_has_return_annotation(def_index)
+            }),
+            _ => true,
+        };
+
+        match ty {
+            Type::Callable(_) => true,
+            Type::Function(func) => function_has_explicit_return_annotation(func),
+            Type::Overload(overload) => overload.signatures.iter().all(|sig| match sig {
+                OverloadType::Function(func) => function_has_explicit_return_annotation(func),
+                OverloadType::Forall(forall) => {
+                    function_has_explicit_return_annotation(&forall.body)
+                }
+            }),
+            Type::BoundMethod(method) => match &method.func {
+                BoundMethodType::Function(func) => function_has_explicit_return_annotation(func),
+                BoundMethodType::Forall(forall) => {
+                    function_has_explicit_return_annotation(&forall.body)
+                }
+                BoundMethodType::Overload(overload) => {
+                    overload.signatures.iter().all(|sig| match sig {
+                        OverloadType::Function(func) => {
+                            function_has_explicit_return_annotation(func)
+                        }
+                        OverloadType::Forall(forall) => {
+                            function_has_explicit_return_annotation(&forall.body)
+                        }
+                    })
+                }
+            },
+            _ => unreachable!(
+                "callable_has_explicit_return_annotation expects a callable type, got `{}`",
+                self.for_display(ty.clone())
+            ),
+        }
+    }
 }
 
 #[derive(Debug, Clone)]

pyrefly/lib/binding/bindings.rs

@@ -66,6 +66,7 @@ use crate::binding::binding::KeyExport;
 use crate::binding::binding::KeyLegacyTypeParam;
 use crate::binding::binding::KeyTypeAlias;
 use crate::binding::binding::KeyUndecoratedFunction;
+use crate::binding::binding::KeyUndecoratedFunctionRange;
 use crate::binding::binding::KeyYield;
 use crate::binding::binding::KeyYieldFrom;
 use crate::binding::binding::Keyed;
@@ -496,8 +497,8 @@ impl Bindings {
         }
     }
 
-    pub fn function_has_return_annotation(&self, name: &Identifier) -> bool {
-        let b = self.get(self.key_to_idx(&Key::ReturnType(ShortIdentifier::new(name))));
+    fn function_has_return_annotation_at_short_identifier(&self, name: ShortIdentifier) -> bool {
+        let b = self.get(self.key_to_idx(&Key::ReturnType(name)));
         if let Binding::ReturnType(box r) = b {
             r.kind.has_return_annotation()
         } else if matches!(b, Binding::Any(_)) {
@@ -506,15 +507,29 @@ impl Bindings {
         } else {
             panic!(
                 "Internal error: unexpected binding for return type `{}` @  {:?}: {}, module={}, path={}",
-                &name.id,
-                name.range,
+                self.module().display(&name),
+                name.range(),
                 b.display_with(self),
                 self.module().name(),
                 self.module().path(),
             )
         }
     }
 
+    pub fn function_has_return_annotation(&self, name: &Identifier) -> bool {
+        self.function_has_return_annotation_at_short_identifier(ShortIdentifier::new(name))
+    }
+
+    pub fn function_def_has_return_annotation(&self, def_index: FuncDefIndex) -> bool {
+        let Some(idx) =
+            self.key_to_idx_hashed_opt(Hashed::new(&KeyUndecoratedFunctionRange(def_index)))
+        else {
+            return false;
+        };
+        let short_identifier = self.get(idx).0;
+        self.function_has_return_annotation_at_short_identifier(short_identifier)
+    }
+
     pub fn new(
         x: ModModule,
         module_info: ModuleInfo,

pyrefly/lib/test/simple.rs

@@ -1693,6 +1693,35 @@ def test(o: C):
 "#,
 );
 
+testcase!(
+    test_dunder_bool_returning_never,
+    r#"
+from typing import Never
+
+class Foo:
+    def __bool__(self) -> Never:
+        raise TypeError
+
+if Foo(): ...  # E: The `__bool__` method of `Foo` returns `Never`, so it cannot be used as a boolean
+"#,
+);
+
+testcase!(
+    test_dunder_bool_returning_never_subclass_override,
+    r#"
+class Foo:
+    def __bool__(self):
+        raise TypeError
+
+class Bar(Foo):
+    def __bool__(self) -> bool:  # pyrefly: ignore[bad-override]
+        return True
+
+bar: Foo = Bar()
+if bar: ...
+"#,
+);
+
 testcase!(
     test_union_dunder_bool,
     r#"