Introspection: use object as input argument type for method returning NotImplemented on conversion error (#5841)

* Introspection: use object as input argument type for method returning NotImplemented on conversion error

* Add changelog

Author: Tpt

newsfragments/5841.changed.md

@@ -0,0 +1 @@
+Stubs: use `object` as the input annotation type of magic methods returning `NonImplemented` if the input value is not of the correct type

pyo3-macros-backend/src/introspection.rs

@@ -106,6 +106,7 @@ pub fn function_introspection_code(
     returns: ReturnType,
     decorators: impl IntoIterator<Item = PyExpr>,
     is_async: bool,
+    is_returning_not_implemented_on_extraction_error: bool,
     doc: Option<&PythonDoc>,
     parent: Option<&Type>,
 ) -> TokenStream {
@@ -114,7 +115,12 @@ pub fn function_introspection_code(
         ("name", IntrospectionNode::String(name.into())),
         (
             "arguments",
-            arguments_introspection_data(signature, first_argument, parent),
+            arguments_introspection_data(
+                signature,
+                first_argument,
+                is_returning_not_implemented_on_extraction_error,
+                parent,
+            ),
         ),
         (
             "returns",
@@ -213,6 +219,7 @@ pub fn attribute_introspection_code(
 fn arguments_introspection_data<'a>(
     signature: &'a FunctionSignature<'a>,
     first_argument: Option<&'a str>,
+    is_returning_not_implemented_on_extraction_error: bool,
     class_type: Option<&Type>,
 ) -> IntrospectionNode<'a> {
     let mut argument_desc = signature.arguments.iter().filter(|arg| {
@@ -248,7 +255,12 @@ fn arguments_introspection_data<'a>(
         } else {
             panic!("Less arguments than in python signature");
         };
-        let arg = argument_introspection_data(param, arg_desc, class_type);
+        let arg = argument_introspection_data(
+            param,
+            arg_desc,
+            is_returning_not_implemented_on_extraction_error,
+            class_type,
+        );
         if i < signature.python_signature.positional_only_parameters {
             posonlyargs.push(arg);
         } else {
@@ -271,7 +283,12 @@ fn arguments_introspection_data<'a>(
         let Some(FnArg::Regular(arg_desc)) = argument_desc.next() else {
             panic!("Less arguments than in python signature");
         };
-        kwonlyargs.push(argument_introspection_data(param, arg_desc, class_type));
+        kwonlyargs.push(argument_introspection_data(
+            param,
+            arg_desc,
+            is_returning_not_implemented_on_extraction_error,
+            class_type,
+        ));
     }
 
     if let Some(param) = &signature.python_signature.kwargs {
@@ -307,14 +324,18 @@ fn arguments_introspection_data<'a>(
 fn argument_introspection_data<'a>(
     name: &'a str,
     desc: &'a RegularArg<'_>,
+    is_returning_not_implemented_on_extraction_error: bool,
     class_type: Option<&Type>,
 ) -> AttributedIntrospectionNode<'a> {
     let mut params: HashMap<_, _> = [("name", IntrospectionNode::String(name.into()))].into();
     if let Some(expr) = &desc.default_value {
         params.insert("default", PyExpr::constant_from_expression(expr).into());
     }
 
-    if let Some(annotation) = &desc.annotation {
+    if is_returning_not_implemented_on_extraction_error {
+        // all inputs are allowed, we use `object`
+        params.insert("annotation", PyExpr::builtin("object").into());
+    } else if let Some(annotation) = &desc.annotation {
         params.insert("annotation", annotation.clone().into());
     } else if desc.from_py_with.is_none() {
         // If from_py_with is set we don't know anything on the input type

pyo3-macros-backend/src/pyclass.rs

@@ -968,6 +968,7 @@ fn implement_py_formatting(
             names: &["__str__"],
             arguments: Vec::new(),
             returns: parse_quote! { ::std::string::String },
+            is_returning_not_implemented_on_extraction_error: false,
         },
         ctx,
     )
@@ -1057,6 +1058,7 @@ fn impl_simple_enum(
                 names: &["__repr__"],
                 arguments: Vec::new(),
                 returns: parse_quote! { &'static str },
+                is_returning_not_implemented_on_extraction_error: false,
             },
             ctx,
         )?;
@@ -1090,6 +1092,7 @@ fn impl_simple_enum(
                 names: &["__int__"],
                 arguments: Vec::new(),
                 returns: parse_quote!(#repr_type),
+                is_returning_not_implemented_on_extraction_error: false,
             },
             ctx,
         )?;
@@ -1609,6 +1612,7 @@ fn impl_complex_enum_tuple_variant_len(
             names: &["__len__"],
             arguments: Vec::new(),
             returns: parse_quote! { ::std::primitive::usize },
+            is_returning_not_implemented_on_extraction_error: false,
         },
         ctx,
     )?;
@@ -1658,6 +1662,7 @@ fn impl_complex_enum_tuple_variant_getitem(
                 annotation: None,
             })],
             returns: parse_quote! { #pyo3_path::Py<#pyo3_path::PyAny> }, // TODO: figure out correct type
+            is_returning_not_implemented_on_extraction_error: false,
         },
         ctx,
     )?;
@@ -1779,6 +1784,7 @@ fn impl_complex_enum_variant_qualname(
 struct FunctionIntrospectionData<'a> {
     names: &'a [&'a str],
     arguments: Vec<FnArg<'a>>,
+    is_returning_not_implemented_on_extraction_error: bool,
     returns: syn::Type,
 }
 
@@ -1799,6 +1805,7 @@ impl FunctionIntrospectionData<'_> {
                     parse_quote!(-> #returns),
                     [],
                     false,
+                    self.is_returning_not_implemented_on_extraction_error,
                     None,
                     Some(cls),
                 )
@@ -2043,6 +2050,7 @@ fn complex_enum_struct_variant_new<'a>(
         &spec,
         &[],
         &variant_cls_type,
+        false,
         ctx,
     ));
     Ok(def)
@@ -2107,6 +2115,7 @@ fn complex_enum_tuple_variant_new<'a>(
         &spec,
         &[],
         &variant_cls_type,
+        false,
         ctx,
     ));
     Ok(def)
@@ -2151,6 +2160,7 @@ fn complex_enum_variant_field_getter(
         &spec,
         field_attrs,
         variant_cls_type,
+        false,
         ctx,
     ));
     Ok(getter)
@@ -2201,6 +2211,7 @@ fn descriptors_to_items(
                     parse_quote!(-> #return_type),
                     vec![PyExpr::builtin("property")],
                     false,
+                    false,
                     utils::get_doc(&field.attrs, None).as_ref(),
                     Some(&parse_quote!(#cls)),
                 ));
@@ -2255,7 +2266,8 @@ fn descriptors_to_items(
                         "setter",
                     )],
                     false,
-                    utils::get_doc(&field.attrs, None).as_ref(),
+                    false,
+                    get_doc(&field.attrs, None).as_ref(),
                     Some(&parse_quote!(#cls)),
                 ));
             }
@@ -2411,27 +2423,20 @@ fn pyclass_richcmp_simple_enum(
         }
     };
     #[cfg(feature = "experimental-inspect")]
-    let never = parse_quote!(!); // we need to set a type, let's pick something small, it is overridden by annotation anyway
+    let any = parse_quote!(#pyo3_path::Py<#pyo3_path::PyAny>);
     #[cfg(feature = "experimental-inspect")]
     let introspection = FunctionIntrospectionData {
         names: &["__eq__", "__ne__"],
         arguments: vec![FnArg::Regular(RegularArg {
             name: Cow::Owned(format_ident!("other")),
-            ty: &never,
+            ty: &any,
             from_py_with: None,
             default_value: None,
             option_wrapped_type: None,
-            annotation: Some(
-                options
-                    .eq
-                    .map(|_| PyExpr::from_type(cls.clone(), None))
-                    .into_iter()
-                    .chain(options.eq_int.map(|_| PyExpr::builtin("int")))
-                    .reduce(PyExpr::union)
-                    .expect("At least one must be defined"),
-            ),
+            annotation: None,
         })],
-        returns: parse_quote! { ::std::primitive::bool },
+        returns: parse_quote!(::std::primitive::bool),
+        is_returning_not_implemented_on_extraction_error: true,
     };
     let richcmp_slot = if options.eq.is_some() {
         generate_protocol_slot(
@@ -2507,6 +2512,7 @@ fn pyclass_richcmp(
                     annotation: None,
                 })],
                 returns: parse_quote! { ::std::primitive::bool },
+                is_returning_not_implemented_on_extraction_error: true,
             },
             ctx,
         )?;
@@ -2546,6 +2552,7 @@ fn pyclass_hash(
                     names: &["__hash__"],
                     arguments: Vec::new(),
                     returns: parse_quote! { ::std::primitive::u64 },
+                    is_returning_not_implemented_on_extraction_error: false,
                 },
                 ctx,
             )?;
@@ -2632,6 +2639,7 @@ fn pyclass_new_impl<'a>(
                         })
                         .collect(),
                     returns: ty.clone(),
+                    is_returning_not_implemented_on_extraction_error: false,
                 },
                 ctx,
             )

pyo3-macros-backend/src/pyfunction.rs

@@ -411,6 +411,7 @@ pub fn impl_wrap_pyfunction(
         func.sig.output.clone(),
         empty(),
         func.sig.asyncness.is_some(),
+        false,
         get_doc(&func.attrs, None).as_ref(),
         None,
     );

pyo3-macros-backend/src/pyimpl.rs

@@ -144,6 +144,7 @@ pub fn impl_methods(
                         &method.spec,
                         &meth.attrs,
                         ty,
+                        method.is_returning_not_implemented_on_extraction_error(),
                         ctx,
                     ));
                     match pymethod::gen_py_method(ty, method, &meth.attrs, ctx)? {
@@ -386,6 +387,7 @@ pub fn method_introspection_code(
     spec: &FnSpec<'_>,
     attrs: &[syn::Attribute],
     parent: &syn::Type,
+    is_returning_not_implemented_on_extraction_error: bool,
     ctx: &Ctx,
 ) -> TokenStream {
     let Ctx { pyo3_path, .. } = ctx;
@@ -405,7 +407,13 @@ pub fn method_introspection_code(
                 // We cant to keep the first argument type, hence this hack
                 spec.signature.arguments.pop();
                 spec.signature.python_signature.positional_parameters.pop();
-                method_introspection_code(&spec, attrs, parent, ctx)
+                method_introspection_code(
+                    &spec,
+                    attrs,
+                    parent,
+                    is_returning_not_implemented_on_extraction_error,
+                    ctx,
+                )
             })
             .collect();
     }
@@ -493,6 +501,7 @@ pub fn method_introspection_code(
         return_type,
         decorators,
         spec.asyncness.is_some(),
+        is_returning_not_implemented_on_extraction_error,
         get_doc(attrs, None).as_ref(),
         Some(parent),
     )

pyo3-macros-backend/src/pymethod.rs

@@ -208,6 +208,24 @@ impl<'a> PyMethod<'a> {
             spec,
         })
     }
+
+    #[cfg(feature = "experimental-inspect")]
+    pub fn is_returning_not_implemented_on_extraction_error(&self) -> bool {
+        match &self.kind {
+            PyMethodKind::Fn => false,
+            PyMethodKind::Proto(proto) => match proto {
+                PyMethodProtoKind::Slot(slot) => {
+                    matches!(slot.extract_error_mode, ExtractErrorMode::NotImplemented)
+                }
+                PyMethodProtoKind::SlotFragment(slot) => {
+                    matches!(slot.extract_error_mode, ExtractErrorMode::NotImplemented)
+                }
+                PyMethodProtoKind::Call
+                | PyMethodProtoKind::Traverse
+                | PyMethodProtoKind::Clear => false,
+            },
+        }
+    }
 }
 
 pub fn is_proto_method(name: &str) -> bool {

pytests/noxfile.py

@@ -48,14 +48,11 @@ def mypy(session: nox.Session):
         (Path("pyo3_pytests") / "py.typed").touch()
         session.install(".[dev]")
 
-        # TODO: remove --disable-error-code", "override" when __eq__ and __ne__ will always take object for input
         session.run_always(
             "python",
             "-m",
             "mypy",
             "tests",
-            "--disable-error-code",
-            "override",
         )
         # TODO: enable stubtest when previously listed errors will be fixed session.run_always("python", "-m", "mypy.stubtest", "pyo3_pytests")
     finally: