Auto merge of #9750 - kraktus:lazy_eval, r=xFrednet

Fix [`unnecessary_lazy_eval`] when type has significant drop

fix for #9427 (comment)

However current implementation gives too many false positive, rending the lint almost useless.

I don't know what's the best way to check if a type has a "significant" drop (in the common meaning, not the internal rustc one, for example Option<(u8, u8)> should not be considered significant)

changelog: Fix [`unnecessary_lazy_eval`] when type has significant drop

Author: bors

clippy_utils/src/eager_or_lazy.rs

@@ -91,6 +91,16 @@ fn fn_eagerness(cx: &LateContext<'_>, fn_id: DefId, name: Symbol, have_one_arg:
     }
 }
 
+fn res_has_significant_drop(res: Res, cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    if let Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) = res {
+        cx.typeck_results()
+            .expr_ty(e)
+            .has_significant_drop(cx.tcx, cx.param_env)
+    } else {
+        false
+    }
+}
+
 #[expect(clippy::too_many_lines)]
 fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessSuggestion {
     struct V<'cx, 'tcx> {
@@ -113,13 +123,8 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
                     },
                     args,
                 ) => match self.cx.qpath_res(path, hir_id) {
-                    Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) => {
-                        if self
-                            .cx
-                            .typeck_results()
-                            .expr_ty(e)
-                            .has_significant_drop(self.cx.tcx, self.cx.param_env)
-                        {
+                    res @ (Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_)) => {
+                        if res_has_significant_drop(res, self.cx, e) {
                             self.eagerness = ForceNoChange;
                             return;
                         }
@@ -147,6 +152,12 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
                     self.eagerness |= NoChange;
                     return;
                 },
+                ExprKind::Path(ref path) => {
+                    if res_has_significant_drop(self.cx.qpath_res(path, e.hir_id), self.cx, e) {
+                        self.eagerness = ForceNoChange;
+                        return;
+                    }
+                },
                 ExprKind::MethodCall(name, ..) => {
                     self.eagerness |= self
                         .cx
@@ -206,7 +217,6 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
                 | ExprKind::Match(..)
                 | ExprKind::Closure { .. }
                 | ExprKind::Field(..)
-                | ExprKind::Path(_)
                 | ExprKind::AddrOf(..)
                 | ExprKind::Struct(..)
                 | ExprKind::Repeat(..)

tests/ui/unnecessary_lazy_eval.fixed

@@ -33,6 +33,14 @@ impl Drop for Issue9427 {
     }
 }
 
+struct Issue9427FollowUp;
+
+impl Drop for Issue9427FollowUp {
+    fn drop(&mut self) {
+        panic!("side effect drop");
+    }
+}
+
 fn main() {
     let astronomers_pi = 10;
     let ext_arr: [usize; 1] = [2];
@@ -87,6 +95,7 @@ fn main() {
 
     // Should not lint - bool
     let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
+    let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
 
     // should not lint, bind_instead_of_map takes priority
     let _ = Some(10).and_then(|idx| Some(ext_arr[idx]));
@@ -133,13 +142,13 @@ fn main() {
     let _: Result<usize, usize> = res.or(Ok(astronomers_pi));
     let _: Result<usize, usize> = res.or(Ok(ext_str.some_field));
     let _: Result<usize, usize> = res.
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        or(Ok(ext_str.some_field));
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    or(Ok(ext_str.some_field));
 
     // neither bind_instead_of_map nor unnecessary_lazy_eval applies here
     let _: Result<usize, usize> = res.and_then(|x| Err(x));

tests/ui/unnecessary_lazy_eval.rs

@@ -33,6 +33,14 @@ impl Drop for Issue9427 {
     }
 }
 
+struct Issue9427FollowUp;
+
+impl Drop for Issue9427FollowUp {
+    fn drop(&mut self) {
+        panic!("side effect drop");
+    }
+}
+
 fn main() {
     let astronomers_pi = 10;
     let ext_arr: [usize; 1] = [2];
@@ -87,6 +95,7 @@ fn main() {
 
     // Should not lint - bool
     let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
+    let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
 
     // should not lint, bind_instead_of_map takes priority
     let _ = Some(10).and_then(|idx| Some(ext_arr[idx]));
@@ -133,13 +142,13 @@ fn main() {
     let _: Result<usize, usize> = res.or_else(|_| Ok(astronomers_pi));
     let _: Result<usize, usize> = res.or_else(|_| Ok(ext_str.some_field));
     let _: Result<usize, usize> = res.
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        // some lines
-        or_else(|_| Ok(ext_str.some_field));
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    // some lines
+    or_else(|_| Ok(ext_str.some_field));
 
     // neither bind_instead_of_map nor unnecessary_lazy_eval applies here
     let _: Result<usize, usize> = res.and_then(|x| Err(x));