Auto merge of rust-lang#10864 - y21:issue10437, r=blyxyas,xFrednet

[`unnecessary_lazy_eval`]: don't lint on types with deref impl

Fixes rust-lang#10437.
This PR changes clippy's util module `eager_or_lazy` to also consider deref expressions whose type has a non-builtin deref impl and not suggest replacing it as that might have observable side effects.
A prominent example might be the `lazy_static` macro, which creates a newtype with a `Deref` impl that you need to go through to get access to the inner value. Going from lazy to eager can make a difference there.

changelog: [`unnecessary_lazy_eval`]: don't lint on types with non-builtin deref impl

Author: bors

clippy_utils/src/eager_or_lazy.rs

@@ -173,11 +173,15 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
                         self.eagerness = Lazy;
                     }
                 },
-
+                // Custom `Deref` impl might have side effects
+                ExprKind::Unary(UnOp::Deref, e)
+                    if self.cx.typeck_results().expr_ty(e).builtin_deref(true).is_none() =>
+                {
+                    self.eagerness |= NoChange;
+                },
                 // Dereferences should be cheap, but dereferencing a raw pointer earlier may not be safe.
                 ExprKind::Unary(UnOp::Deref, e) if !self.cx.typeck_results().expr_ty(e).is_unsafe_ptr() => (),
                 ExprKind::Unary(UnOp::Deref, _) => self.eagerness |= NoChange,
-
                 ExprKind::Unary(_, e)
                     if matches!(
                         self.cx.typeck_results().expr_ty(e).kind(),

tests/ui/unnecessary_lazy_eval.fixed

@@ -5,6 +5,8 @@
 #![allow(clippy::bind_instead_of_map)]
 #![allow(clippy::map_identity)]
 
+use std::ops::Deref;
+
 extern crate proc_macros;
 use proc_macros::with_span;
 
@@ -41,6 +43,15 @@ impl Drop for Issue9427FollowUp {
     }
 }
 
+struct Issue10437;
+impl Deref for Issue10437 {
+    type Target = u32;
+    fn deref(&self) -> &Self::Target {
+        println!("side effect deref");
+        &0
+    }
+}
+
 fn main() {
     let astronomers_pi = 10;
     let ext_arr: [usize; 1] = [2];
@@ -65,6 +76,12 @@ fn main() {
     let _ = nested_tuple_opt.unwrap_or(Some((1, 2)));
     let _ = cond.then_some(astronomers_pi);
 
+    // Should lint - Builtin deref
+    let r = &1;
+    let _ = Some(1).unwrap_or(*r);
+    let b = Box::new(1);
+    let _ = Some(1).unwrap_or(*b);
+
     // Cases when unwrap is not called on a simple variable
     let _ = Some(10).unwrap_or(2);
     let _ = Some(10).and(ext_opt);
@@ -93,6 +110,9 @@ fn main() {
     let _ = deep.0.or_else(|| some_call());
     let _ = opt.ok_or_else(|| ext_arr[0]);
 
+    let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
+    let _ = Some(1).unwrap_or(*Issue10437);
+
     // Should not lint - bool
     let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
     let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop

tests/ui/unnecessary_lazy_eval.rs

@@ -5,6 +5,8 @@
 #![allow(clippy::bind_instead_of_map)]
 #![allow(clippy::map_identity)]
 
+use std::ops::Deref;
+
 extern crate proc_macros;
 use proc_macros::with_span;
 
@@ -41,6 +43,15 @@ impl Drop for Issue9427FollowUp {
     }
 }
 
+struct Issue10437;
+impl Deref for Issue10437 {
+    type Target = u32;
+    fn deref(&self) -> &Self::Target {
+        println!("side effect deref");
+        &0
+    }
+}
+
 fn main() {
     let astronomers_pi = 10;
     let ext_arr: [usize; 1] = [2];
@@ -65,6 +76,12 @@ fn main() {
     let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
     let _ = cond.then(|| astronomers_pi);
 
+    // Should lint - Builtin deref
+    let r = &1;
+    let _ = Some(1).unwrap_or_else(|| *r);
+    let b = Box::new(1);
+    let _ = Some(1).unwrap_or_else(|| *b);
+
     // Cases when unwrap is not called on a simple variable
     let _ = Some(10).unwrap_or_else(|| 2);
     let _ = Some(10).and_then(|_| ext_opt);
@@ -93,6 +110,9 @@ fn main() {
     let _ = deep.0.or_else(|| some_call());
     let _ = opt.ok_or_else(|| ext_arr[0]);
 
+    let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
+    let _ = Some(1).unwrap_or(*Issue10437);
+
     // Should not lint - bool
     let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
     let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop