Merge pull request PyO3#555 from macisamuele/maci-run-clippy-on-all-t…

…he-codebase

Run clippy on all the codebase

.travis.yml

@@ -18,10 +18,10 @@ matrix:
       python: "3.8-dev"
     - name: Minimum nightly
       python: "3.7"
-      # Keep this synced up with build.rs
-      env: TRAVIS_RUST_VERSION=nightly-2019-07-19
-      # Tested via anaconda PyPy (since travis's PyPy version is too old)
-    - name: PyPy3.5 7.0
+      # Keep this synced up with build.rs and ensure that the nightly version does have clippy available
+      # https://static.rust-lang.org/dist/YYYY-MM-DD/clippy-nightly-x86_64-unknown-linux-gnu.tar.gz exists
+      env: TRAVIS_RUST_VERSION=nightly-2019-07-19  
+    - name: PyPy3.5 7.0 # Tested via anaconda PyPy (since travis's PyPy version is too old)
       python: "3.7"
       env: FEATURES="pypy" PATH="$PATH:/opt/anaconda/envs/pypy3/bin"
   allow_failures:

Makefile

@@ -1,15 +1,31 @@
-.PHONY: test test_py3 publish
+.PHONY: test test_py3 publish clippy lint fmt
+
+# Constants used in clippy target
+CLIPPY_LINTS_TO_DENY := warnings
+CLIPPY_LINTS_TO_ALLOW := clippy::new_ret_no_self
 
 test:
 	cargo test
-	cargo clippy
+	${MAKE} clippy
 	tox
 	for example in examples/*; do tox -e py -c $$example/tox.ini; done
 
 test_py3:
 	tox -e py3
 	for example in examples/*; do tox -e py3 -c $$example/tox.ini; done
 
+fmt:
+	cargo fmt --all -- --check
+
+clippy:
+	@touch src/lib.rs  # Touching file to ensure that cargo clippy will re-check the project
+	cargo clippy --all-features --all-targets -- \
+		$(addprefix -D ,${CLIPPY_LINTS_TO_DENY}) \
+		$(addprefix -A ,${CLIPPY_LINTS_TO_ALLOW})
+
+lint: fmt clippy
+	@true
+
 publish: test
 	cargo publish --manifest-path pyo3-derive-backend/Cargo.toml
 	cargo publish --manifest-path pyo3cls/Cargo.toml

benches/bench_dict.rs

@@ -9,7 +9,7 @@ use test::Bencher;
 fn iter_dict(b: &mut Bencher) {
     let gil = Python::acquire_gil();
     let py = gil.python();
-    const LEN: usize = 1_000_00;
+    const LEN: usize = 100_000;
     let dict = (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict(py);
     let mut sum = 0;
     b.iter(|| {

ci/travis/test.sh

@@ -11,8 +11,7 @@ else
 fi
 
 if [ "$TRAVIS_JOB_NAME" = "Minimum nightly" ]; then
-    cargo fmt --all -- --check
-    cargo clippy --features "$FEATURES num-complex" -- -D warnings
+    make lint
 fi
 
 for example_dir in examples/*; do

src/buffer.rs

@@ -706,6 +706,7 @@ mod test {
         assert_eq!(buffer.to_vec::<u8>(py).unwrap(), b"abcde");
     }
 
+    #[allow(clippy::float_cmp)] // The test wants to ensure that no precision was lost on the Python round-trip
     #[test]
     fn test_array_buffer() {
         let gil = Python::acquire_gil();
@@ -715,7 +716,7 @@ mod test {
             .unwrap()
             .call_method("array", ("f", (1.0, 1.5, 2.0, 2.5)), None)
             .unwrap();
-        let buffer = PyBuffer::get(py, array.into()).unwrap();
+        let buffer = PyBuffer::get(py, array).unwrap();
         assert_eq!(buffer.dimensions(), 1);
         assert_eq!(buffer.item_count(), 4);
         assert_eq!(buffer.format().to_str().unwrap(), "f");

src/types/complex.rs

@@ -196,6 +196,7 @@ mod complex_conversion {
     complex_conversion!(f32);
     complex_conversion!(f64);
 
+    #[allow(clippy::float_cmp)] // The test wants to ensure that no precision was lost on the Python round-trip
     #[test]
     fn from_complex() {
         let gil = Python::acquire_gil();
@@ -230,11 +231,13 @@ mod test {
 
     #[test]
     fn test_from_double() {
+        use assert_approx_eq::assert_approx_eq;
+
         let gil = Python::acquire_gil();
         let py = gil.python();
         let complex = PyComplex::from_doubles(py, 3.0, 1.2);
-        assert_eq!(complex.real(), 3.0);
-        assert_eq!(complex.imag(), 1.2);
+        assert_approx_eq!(complex.real(), 3.0);
+        assert_approx_eq!(complex.imag(), 1.2);
     }
 
     #[cfg(not(Py_LIMITED_API))]
@@ -281,8 +284,8 @@ mod test {
         let l = PyComplex::from_doubles(py, 3.0, 1.2);
         let r = PyComplex::from_doubles(py, 1.0, 2.6);
         let res = l / r;
-        assert_approx_eq!(res.real(), 0.7886597938144329);
-        assert_approx_eq!(res.imag(), -0.8505154639175257);
+        assert_approx_eq!(res.real(), 0.788_659_793_814_432_9);
+        assert_approx_eq!(res.imag(), -0.850_515_463_917_525_7);
     }
 
     #[cfg(not(Py_LIMITED_API))]
@@ -302,7 +305,7 @@ mod test {
         let gil = Python::acquire_gil();
         let py = gil.python();
         let val = PyComplex::from_doubles(py, 3.0, 1.2);
-        assert_approx_eq!(val.abs(), 3.2310988842807022);
+        assert_approx_eq!(val.abs(), 3.231_098_884_280_702_2);
     }
 
     #[cfg(not(Py_LIMITED_API))]
@@ -313,7 +316,7 @@ mod test {
         let l = PyComplex::from_doubles(py, 3.0, 1.2);
         let r = PyComplex::from_doubles(py, 1.2, 2.6);
         let val = l.pow(r);
-        assert_approx_eq!(val.real(), -1.4193099970166037);
-        assert_approx_eq!(val.imag(), -0.5412974660335446);
+        assert_approx_eq!(val.real(), -1.419_309_997_016_603_7);
+        assert_approx_eq!(val.imag(), -0.541_297_466_033_544_6);
     }
 }

src/types/floatob.rs

@@ -91,11 +91,13 @@ mod test {
         ($func_name:ident, $t1:ty, $t2:ty) => (
             #[test]
             fn $func_name() {
+                use assert_approx_eq::assert_approx_eq;
+
                 let gil = Python::acquire_gil();
                 let py = gil.python();
                 let val = 123 as $t1;
                 let obj = val.to_object(py);
-                assert_eq!(obj.extract::<$t2>(py).unwrap(), val as $t2);
+                assert_approx_eq!(obj.extract::<$t2>(py).unwrap(), val as $t2);
             }
         )
     );
@@ -106,10 +108,12 @@ mod test {
 
     #[test]
     fn test_as_double_macro() {
+        use assert_approx_eq::assert_approx_eq;
+
         let gil = Python::acquire_gil();
         let py = gil.python();
         let v = 1.23f64;
         let obj = v.to_object(py);
-        assert_eq!(v, unsafe { PyFloat_AS_DOUBLE(obj.as_ptr()) });
+        assert_approx_eq!(v, unsafe { PyFloat_AS_DOUBLE(obj.as_ptr()) });
     }
 }

src/types/tuple.rs

@@ -154,7 +154,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         fn to_object(&self, py: Python) -> PyObject {
             unsafe {
                 let ptr = ffi::PyTuple_New($length);
-                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.to_object(py).into_ptr());)+;
+                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.to_object(py).into_ptr());)+
                 PyObject::from_owned_ptr_or_panic(py, ptr)
             }
         }
@@ -163,7 +163,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         fn into_object(self, py: Python) -> PyObject {
             unsafe {
                 let ptr = ffi::PyTuple_New($length);
-                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_object(py).into_ptr());)+;
+                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_object(py).into_ptr());)+
                 PyObject::from_owned_ptr_or_panic(py, ptr)
             }
         }
@@ -173,7 +173,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         fn into_py(self, py: Python) -> Py<PyTuple> {
             unsafe {
                 let ptr = ffi::PyTuple_New($length);
-                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_object(py).into_ptr());)+;
+                $(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_object(py).into_ptr());)+
                 Py::from_owned_ptr_or_panic(ptr)
             }
         }

tests/test_datetime.rs

@@ -4,6 +4,7 @@ use pyo3::ffi::*;
 use pyo3::prelude::*;
 use pyo3::types::{IntoPyDict, PyAny};
 
+#[allow(clippy::trivially_copy_pass_by_ref)]
 fn _get_subclasses<'p>(
     py: &'p Python,
     py_type: &str,
@@ -101,6 +102,7 @@ fn test_delta_check() {
 
 #[test]
 fn test_datetime_utc() {
+    use assert_approx_eq::assert_approx_eq;
     use pyo3::types::PyDateTime;
 
     let gil = Python::acquire_gil();
@@ -119,10 +121,11 @@ fn test_datetime_utc() {
         .unwrap()
         .extract()
         .unwrap();
-    assert_eq!(offset, 0f32);
+    assert_approx_eq!(offset, 0f32);
 }
 
-static INVALID_DATES: &'static [(i32, u8, u8)] = &[
+#[cfg(Py_3_6)]
+static INVALID_DATES: &[(i32, u8, u8)] = &[
     (-1, 1, 1),
     (0, 1, 1),
     (10000, 1, 1),
@@ -134,7 +137,8 @@ static INVALID_DATES: &'static [(i32, u8, u8)] = &[
     (2018, 1, 32),
 ];
 
-static INVALID_TIMES: &'static [(u8, u8, u8, u32)] =
+#[cfg(Py_3_6)]
+static INVALID_TIMES: &[(u8, u8, u8, u32)] =
     &[(25, 0, 0, 0), (255, 0, 0, 0), (0, 60, 0, 0), (0, 0, 61, 0)];
 
 #[cfg(Py_3_6)]
@@ -145,7 +149,7 @@ fn test_pydate_out_of_bounds() {
     // This test is an XFAIL on Python < 3.6 until bounds checking is implemented
     let gil = Python::acquire_gil();
     let py = gil.python();
-    for val in INVALID_DATES.into_iter() {
+    for val in INVALID_DATES {
         let (year, month, day) = val;
         let dt = PyDate::new(py, *year, *month, *day);
         dt.unwrap_err();

tests/test_dict_iter.rs

@@ -12,5 +12,5 @@ fn iter_dict_nosegv() {
         let i: u64 = k.extract().unwrap();
         sum += i;
     }
-    assert_eq!(sum, 49999995000000);
+    assert_eq!(sum, 49_999_995_000_000);
 }

tests/test_sequence.rs

@@ -40,8 +40,8 @@ impl PySequenceProtocol for ByteSequence {
     fn __getitem__(&self, idx: isize) -> PyResult<u8> {
         self.elements
             .get(idx as usize)
-            .map(|&byte| byte)
-            .ok_or(IndexError::py_err("list index out of range"))
+            .copied()
+            .ok_or_else(|| IndexError::py_err("list index out of range"))
     }
 
     fn __setitem__(&mut self, idx: isize, value: u8) -> PyResult<()> {

tests/test_various.rs

@@ -111,7 +111,7 @@ fn pytuple_pyclass_iter() {
             PyRef::new(py, SimplePyClass {}).unwrap(),
             PyRef::new(py, SimplePyClass {}).unwrap(),
         ]
-        .into_iter(),
+        .iter(),
     );
     py_assert!(py, tup, "type(tup[0]).__name__ == 'SimplePyClass'");
     py_assert!(py, tup, "type(tup[0]).__name__ == type(tup[0]).__name__");