Auto merge of rust-lang#5329 - matthiaskrgr:int_arith_on_ref_5328, r=flip1995

integer_arithmetic: detect integer arithmetic on references.

changelog: integer_arithmetic fix false negatives with references on integers

Fixes rust-lang#5328

Author: bors

clippy_dev/src/stderr_length_check.rs

@@ -4,6 +4,8 @@ use std::path::{Path, PathBuf};
 
 use walkdir::WalkDir;
 
+use clippy_dev::clippy_project_root;
+
 // The maximum length allowed for stderr files.
 //
 // We limit this because small files are easier to deal with than bigger files.
@@ -14,22 +16,24 @@ pub fn check() {
 
     if !exceeding_files.is_empty() {
         eprintln!("Error: stderr files exceeding limit of {} lines:", LENGTH_LIMIT);
-        for path in exceeding_files {
-            println!("{}", path.display());
+        for (path, count) in exceeding_files {
+            println!("{}: {}", path.display(), count);
         }
         std::process::exit(1);
     }
 }
 
-fn exceeding_stderr_files() -> Vec<PathBuf> {
+fn exceeding_stderr_files() -> Vec<(PathBuf, usize)> {
     // We use `WalkDir` instead of `fs::read_dir` here in order to recurse into subdirectories.
-    WalkDir::new("../tests/ui")
+    WalkDir::new(clippy_project_root().join("tests/ui"))
         .into_iter()
         .filter_map(Result::ok)
+        .filter(|f| !f.file_type().is_dir())
         .filter_map(|e| {
             let p = e.into_path();
-            if p.extension() == Some(OsStr::new("stderr")) && count_linenumbers(&p) > LENGTH_LIMIT {
-                Some(p)
+            let count = count_linenumbers(&p);
+            if p.extension() == Some(OsStr::new("stderr")) && count > LENGTH_LIMIT {
+                Some((p, count))
             } else {
                 None
             }

clippy_lints/src/arithmetic.rs

@@ -81,11 +81,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
                     | hir::BinOpKind::Gt => return,
                     _ => (),
                 }
+
                 let (l_ty, r_ty) = (cx.tables.expr_ty(l), cx.tables.expr_ty(r));
-                if l_ty.is_integral() && r_ty.is_integral() {
+                if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
                     span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
                     self.expr_span = Some(expr.span);
-                } else if l_ty.is_floating_point() && r_ty.is_floating_point() {
+                } else if l_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {
                     span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected");
                     self.expr_span = Some(expr.span);
                 }

tests/ui/arithmetic.stderr

@@ -51,15 +51,4 @@ fn test_complex_conditions() {
     }
 }
 
-fn test_nested() {
-    fn nested() {
-        let x = Some(());
-        if x.is_some() {
-            x.unwrap(); // unnecessary
-        } else {
-            x.unwrap(); // will panic
-        }
-    }
-}
-
 fn main() {}

tests/ui/checked_unwrap/complex_conditionals.rs

@@ -188,22 +188,5 @@ LL |     if x.is_ok() || !(y.is_ok() && z.is_err()) {
 LL |         z.unwrap_err(); // unnecessary
    |         ^^^^^^^^^^^^^^
 
-error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
-  --> $DIR/complex_conditionals.rs:58:13
-   |
-LL |         if x.is_some() {
-   |            ----------- the check is happening here
-LL |             x.unwrap(); // unnecessary
-   |             ^^^^^^^^^^
-
-error: This call to `unwrap()` will always panic.
-  --> $DIR/complex_conditionals.rs:60:13
-   |
-LL |         if x.is_some() {
-   |            ----------- because of this check
-...
-LL |             x.unwrap(); // will panic
-   |             ^^^^^^^^^^
-
-error: aborting due to 22 previous errors
+error: aborting due to 20 previous errors
 

tests/ui/checked_unwrap/complex_conditionals.stderr

@@ -0,0 +1,15 @@
+#![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
+#![allow(clippy::if_same_then_else)]
+
+fn test_nested() {
+    fn nested() {
+        let x = Some(());
+        if x.is_some() {
+            x.unwrap(); // unnecessary
+        } else {
+            x.unwrap(); // will panic
+        }
+    }
+}
+
+fn main() {}

tests/ui/checked_unwrap/complex_conditionals_nested.rs

@@ -0,0 +1,31 @@
+error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
+  --> $DIR/complex_conditionals_nested.rs:8:13
+   |
+LL |         if x.is_some() {
+   |            ----------- the check is happening here
+LL |             x.unwrap(); // unnecessary
+   |             ^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/complex_conditionals_nested.rs:1:35
+   |
+LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
+   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: This call to `unwrap()` will always panic.
+  --> $DIR/complex_conditionals_nested.rs:10:13
+   |
+LL |         if x.is_some() {
+   |            ----------- because of this check
+...
+LL |             x.unwrap(); // will panic
+   |             ^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/complex_conditionals_nested.rs:1:9
+   |
+LL | #![deny(clippy::panicking_unwrap, clippy::unnecessary_unwrap)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/checked_unwrap/complex_conditionals_nested.stderr

@@ -0,0 +1,53 @@
+#![warn(clippy::integer_arithmetic, clippy::float_arithmetic)]
+#![allow(
+    unused,
+    clippy::shadow_reuse,
+    clippy::shadow_unrelated,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::op_ref,
+    clippy::trivially_copy_pass_by_ref
+)]
+
+#[rustfmt::skip]
+fn main() {
+    let mut f = 1.0f32;
+
+    f * 2.0;
+
+    1.0 + f;
+    f * 2.0;
+    f / 2.0;
+    f - 2.0 * 4.2;
+    -f;
+
+    f += 1.0;
+    f -= 1.0;
+    f *= 2.0;
+    f /= 2.0;
+}
+
+// also warn about floating point arith with references involved
+
+pub fn float_arith_ref() {
+    3.1_f32 + &1.2_f32;
+    &3.4_f32 + 1.5_f32;
+    &3.5_f32 + &1.3_f32;
+}
+
+pub fn float_foo(f: &f32) -> f32 {
+    let a = 5.1;
+    a + f
+}
+
+pub fn float_bar(f1: &f32, f2: &f32) -> f32 {
+    f1 + f2
+}
+
+pub fn float_baz(f1: f32, f2: &f32) -> f32 {
+    f1 + f2
+}
+
+pub fn float_qux(f1: f32, f2: f32) -> f32 {
+    (&f1 + &f2)
+}