Merge pull request #2196 from davidhewitt/more-clippy

clippy: enable some more lints

.cargo/config

@@ -6,3 +6,19 @@ pyo3_doc_internal = "doc --lib --no-default-features --features=full --no-deps -
 
 [build]
 rustdocflags = ["--cfg", "docsrs"]
+
+[target.'cfg(feature = "cargo-clippy")']
+rustflags = [
+    "-Dclippy::checked_conversions",
+    "-Dclippy::dbg_macro",
+    "-Dclippy::explicit_into_iter_loop",
+    "-Dclippy::explicit_iter_loop",
+    "-Dclippy::filter_map_next",
+    "-Dclippy::flat_map_option",
+    "-Dclippy::let_unit_value",
+    "-Dclippy::manual_assert",
+    "-Dclippy::manual_ok_or",
+    "-Dclippy::todo",
+    "-Dclippy::unnecessary_wraps",
+    "-Dclippy::useless_transmute",
+]

pyo3-build-config/src/impl_.rs

@@ -755,7 +755,7 @@ impl BuildFlags {
         let mut script = String::from("import sysconfig\n");
         script.push_str("config = sysconfig.get_config_vars()\n");
 
-        for k in BuildFlags::ALL.iter() {
+        for k in &BuildFlags::ALL {
             script.push_str(&format!("print(config.get('{}', '0'))\n", k));
         }
 
@@ -793,10 +793,10 @@ impl Display for BuildFlags {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         let mut first = true;
         for flag in &self.0 {
-            if !first {
-                write!(f, ",")?;
-            } else {
+            if first {
                 first = false;
+            } else {
+                write!(f, ",")?;
             }
             write!(f, "{}", flag)?;
         }

pyo3-ffi/src/object.rs

@@ -63,9 +63,7 @@ pub unsafe fn Py_Is(x: *mut PyObject, y: *mut PyObject) -> c_int {
 
 #[inline]
 pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t {
-    if ob.is_null() {
-        panic!();
-    }
+    assert!(!ob.is_null());
     (*ob).ob_refcnt
 }
 

pyo3-macros-backend/src/deprecations.rs

@@ -35,7 +35,10 @@ impl ToTokens for Deprecations {
             let ident = deprecation.ident(*span);
             quote_spanned!(
                 *span =>
-                let _ = _pyo3::impl_::deprecations::#ident;
+                #[allow(clippy::let_unit_value)]
+                {
+                    let _ = _pyo3::impl_::deprecations::#ident;
+                }
             )
             .to_tokens(tokens)
         }

pyo3-macros-backend/src/method.rs

@@ -420,7 +420,7 @@ impl<'a> FnSpec<'a> {
     }
 
     pub fn default_value(&self, name: &syn::Ident) -> Option<TokenStream> {
-        for s in self.attrs.iter() {
+        for s in &self.attrs {
             match s {
                 Argument::Arg(path, opt) | Argument::Kwarg(path, opt) => {
                     if path.is_ident(name) {
@@ -437,7 +437,7 @@ impl<'a> FnSpec<'a> {
     }
 
     pub fn is_pos_only(&self, name: &syn::Ident) -> bool {
-        for s in self.attrs.iter() {
+        for s in &self.attrs {
             if let Argument::PosOnlyArg(path, _) = s {
                 if path.is_ident(name) {
                     return true;
@@ -448,7 +448,7 @@ impl<'a> FnSpec<'a> {
     }
 
     pub fn is_kw_only(&self, name: &syn::Ident) -> bool {
-        for s in self.attrs.iter() {
+        for s in &self.attrs {
             if let Argument::Kwarg(path, _) = s {
                 if path.is_ident(name) {
                     return true;

pyo3-macros-backend/src/params.rs

@@ -81,7 +81,7 @@ pub fn impl_arg_params(
     let mut required_positional_parameters = 0usize;
     let mut keyword_only_parameters = Vec::new();
 
-    for arg in spec.args.iter() {
+    for arg in &spec.args {
         if arg.py || is_args(&spec.attrs, arg.name) || is_kwargs(&spec.attrs, arg.name) {
             continue;
         }

pyo3-macros-backend/src/pyclass.rs

@@ -474,7 +474,7 @@ pub fn build_py_enum(
             .map(|attr| (get_class_python_name(&enum_.ident, args), attr)),
     );
     let enum_ = PyClassEnum::new(enum_)?;
-    impl_enum(enum_, args, doc, method_type, options)
+    Ok(impl_enum(enum_, args, doc, method_type, options))
 }
 
 fn impl_enum(
@@ -483,7 +483,7 @@ fn impl_enum(
     doc: PythonDoc,
     methods_type: PyClassMethodsType,
     options: PyClassPyO3Options,
-) -> syn::Result<TokenStream> {
+) -> TokenStream {
     let krate = get_pyo3_crate(&options.krate);
     impl_enum_class(enum_, args, doc, methods_type, krate)
 }
@@ -494,7 +494,7 @@ fn impl_enum_class(
     doc: PythonDoc,
     methods_type: PyClassMethodsType,
     krate: syn::Path,
-) -> syn::Result<TokenStream> {
+) -> TokenStream {
     let cls = enum_.ident;
     let variants = enum_.variants;
     let pytypeinfo = impl_pytypeinfo(cls, args, None);
@@ -589,7 +589,7 @@ fn impl_enum_class(
     .doc(doc)
     .impl_all();
 
-    Ok(quote! {
+    quote! {
         const _: () = {
             use #krate as _pyo3;
 
@@ -601,7 +601,7 @@ fn impl_enum_class(
                 #(#default_methods)*
             }
         };
-    })
+    }
 }
 
 fn enum_default_methods<'a>(

pyo3-macros-backend/src/pymethod.rs

@@ -193,7 +193,7 @@ pub fn gen_py_method(
                     GeneratedPyMethod::Proto(impl_call_slot(cls, method.spec)?)
                 }
                 PyMethodProtoKind::Traverse => {
-                    GeneratedPyMethod::Proto(impl_traverse_slot(cls, method.spec)?)
+                    GeneratedPyMethod::Proto(impl_traverse_slot(cls, method.spec))
                 }
                 PyMethodProtoKind::SlotFragment(slot_fragment_def) => {
                     let proto = slot_fragment_def.generate_pyproto_fragment(cls, spec)?;
@@ -307,9 +307,9 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec) -> Result<TokenStream> {
     }})
 }
 
-fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec) -> Result<TokenStream> {
+fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec) -> TokenStream {
     let ident = spec.name;
-    Ok(quote! {{
+    quote! {{
         pub unsafe extern "C" fn __wrap_(
             slf: *mut _pyo3::ffi::PyObject,
             visit: _pyo3::ffi::visitproc,
@@ -334,7 +334,7 @@ fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec) -> Result<TokenStream> {
             slot: _pyo3::ffi::Py_tp_traverse,
             pfunc: __wrap_ as _pyo3::ffi::traverseproc as _
         }
-    }})
+    }}
 }
 
 fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec) -> TokenStream {

pytests/src/buf_and_str.rs

@@ -26,9 +26,9 @@ impl BytesExtractor {
         Ok(rust_string.len())
     }
 
-    pub fn from_str_lossy(&mut self, string: &PyString) -> PyResult<usize> {
+    pub fn from_str_lossy(&mut self, string: &PyString) -> usize {
         let rust_string_lossy: String = string.to_string_lossy().to_string();
-        Ok(rust_string_lossy.len())
+        rust_string_lossy.len()
     }
 
     pub fn from_buffer(&mut self, buf: &PyAny) -> PyResult<usize> {

pytests/src/subclassing.rs

@@ -12,8 +12,8 @@ impl Subclassable {
         Subclassable {}
     }
 
-    fn __str__(&self) -> PyResult<&'static str> {
-        Ok("Subclassable")
+    fn __str__(&self) -> &'static str {
+        "Subclassable"
     }
 }
 

src/conversions/array.rs

@@ -97,6 +97,7 @@ mod min_const_generics {
             }
             let _ = catch_unwind_silent(move || {
                 let _: Result<[CountDrop; 4], ()> = super::array_try_from_fn(|idx| {
+                    #[allow(clippy::manual_assert)]
                     if idx == 2 {
                         panic!("peek a boo");
                     }

src/gil.rs

@@ -241,6 +241,7 @@ impl GILGuard {
 impl Drop for GILGuard {
     fn drop(&mut self) {
         // First up, try to detect if the order of destruction is correct.
+        #[allow(clippy::manual_assert)]
         let _ = GIL_COUNT.try_with(|c| {
             if self.gstate == ffi::PyGILState_STATE::PyGILState_UNLOCKED && c.get() != 1 {
                 // XXX: this panic commits to leaking all objects in the pool as well as

src/impl_/pyclass.rs

@@ -847,12 +847,12 @@ pub struct ThreadCheckerImpl<T>(thread::ThreadId, PhantomData<T>);
 
 impl<T> PyClassThreadChecker<T> for ThreadCheckerImpl<T> {
     fn ensure(&self) {
-        if thread::current().id() != self.0 {
-            panic!(
-                "{} is unsendable, but sent to another thread!",
-                std::any::type_name::<T>()
-            );
-        }
+        assert_eq!(
+            thread::current().id(),
+            self.0,
+            "{} is unsendable, but sent to another thread!",
+            std::any::type_name::<T>()
+        );
     }
     fn new() -> Self {
         ThreadCheckerImpl(thread::current().id(), PhantomData)

src/impl_/pymodule.rs

@@ -138,6 +138,7 @@ mod tests {
 
         static INIT_CALLED: AtomicBool = AtomicBool::new(false);
 
+        #[allow(clippy::unnecessary_wraps)]
         fn init(_: Python, _: &PyModule) -> PyResult<()> {
             INIT_CALLED.store(true, Ordering::SeqCst);
             Ok(())

src/test_hygiene/pyfunction.rs

@@ -1,5 +1,5 @@
 #![no_implicit_prelude]
-#![allow(unused_variables)]
+#![allow(unused_variables, clippy::unnecessary_wraps)]
 
 #[crate::pyfunction]
 #[pyo3(crate = "crate")]

src/test_hygiene/pymethods.rs

@@ -1,5 +1,5 @@
 #![no_implicit_prelude]
-#![allow(unused_variables)]
+#![allow(unused_variables, clippy::unnecessary_wraps)]
 
 #[crate::pyclass]
 #[pyo3(crate = "crate")]

src/test_hygiene/pymodule.rs

@@ -1,5 +1,5 @@
 #![no_implicit_prelude]
-#![allow(unused_variables)]
+#![allow(unused_variables, clippy::unnecessary_wraps)]
 
 #[crate::pyfunction]
 #[pyo3(crate = "crate")]

tests/test_anyhow.rs

@@ -5,6 +5,7 @@ fn test_anyhow_py_function_ok_result() {
     use pyo3::{py_run, pyfunction, wrap_pyfunction, Python};
 
     #[pyfunction]
+    #[allow(clippy::unnecessary_wraps)]
     fn produce_ok_result() -> anyhow::Result<String> {
         Ok(String::from("OK buddy"))
     }

tests/test_gc.rs

@@ -120,6 +120,8 @@ fn gc_integration() {
 
         let mut borrow = inst.borrow_mut();
         borrow.self_ref = inst.to_object(py);
+
+        py_run!(py, inst, "import gc; assert inst in gc.get_objects()");
     }
 
     let gil = Python::acquire_gil();
@@ -128,25 +130,6 @@ fn gc_integration() {
     assert!(drop_called.load(Ordering::Relaxed));
 }
 
-#[pyclass]
-struct GcIntegration2 {}
-
-#[pymethods]
-impl GcIntegration2 {
-    fn __traverse__(&self, _visit: PyVisit) -> Result<(), PyTraverseError> {
-        Ok(())
-    }
-    fn __clear__(&mut self) {}
-}
-
-#[test]
-fn gc_integration2() {
-    let gil = Python::acquire_gil();
-    let py = gil.python();
-    let inst = PyCell::new(py, GcIntegration2 {}).unwrap();
-    py_run!(py, inst, "import gc; assert inst in gc.get_objects()");
-}
-
 #[pyclass(subclass)]
 struct BaseClassWithDrop {
     data: Option<Arc<AtomicBool>>,
@@ -231,6 +214,8 @@ impl TraversableClass {
 #[pymethods]
 impl TraversableClass {
     fn __clear__(&mut self) {}
+
+    #[allow(clippy::unnecessary_wraps)]
     fn __traverse__(&self, _visit: PyVisit) -> Result<(), PyTraverseError> {
         self.traversed.store(true, Ordering::Relaxed);
         Ok(())

tests/test_methods.rs

@@ -842,12 +842,12 @@ impl r#RawIdents {
         r#type: PyObject,
         r#subtype: PyObject,
         r#subsubtype: PyObject,
-    ) -> PyResult<Self> {
-        Ok(Self {
+    ) -> Self {
+        Self {
             r#type,
             r#subtype,
             r#subsubtype,
-        })
+        }
     }
 
     #[getter(r#subtype)]

tests/test_module.rs

@@ -432,6 +432,7 @@ fn test_module_functions_with_module() {
 #[test]
 fn test_module_doc_hidden() {
     #[doc(hidden)]
+    #[allow(clippy::unnecessary_wraps)]
     #[pymodule]
     fn my_module(_py: Python, _m: &PyModule) -> PyResult<()> {
         Ok(())

tests/test_sequence.rs

@@ -19,7 +19,7 @@ impl ByteSequence {
     fn new(elements: Option<&PyList>) -> PyResult<Self> {
         if let Some(pylist) = elements {
             let mut elems = Vec::with_capacity(pylist.len());
-            for pyelem in pylist.into_iter() {
+            for pyelem in pylist {
                 let elem = u8::extract(pyelem)?;
                 elems.push(elem);
             }

tests/test_sequence_pyproto.rs

@@ -22,7 +22,7 @@ impl ByteSequence {
     fn new(elements: Option<&PyList>) -> PyResult<Self> {
         if let Some(pylist) = elements {
             let mut elems = Vec::with_capacity(pylist.len());
-            for pyelem in pylist.into_iter() {
+            for pyelem in pylist {
                 let elem = u8::extract(pyelem)?;
                 elems.push(elem);
             }