Auto merge of rust-lang#9491 - kraktus:drop_copy, r=Jarcho

[`drop_copy`]: Do not lint idiomatic in match arm

close rust-lang/rust-clippy#9482

changelog: [`drop_copy`]: Do not lint idiomatic in match arm

Author: bors

clippy_lints/src/drop_forget_ref.rs

@@ -1,7 +1,8 @@
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
+use clippy_utils::get_parent_node;
 use clippy_utils::is_must_use_func_call;
 use clippy_utils::ty::{is_copy, is_must_use_ty, is_type_lang_item};
-use rustc_hir::{Expr, ExprKind, LangItem};
+use rustc_hir::{Arm, Expr, ExprKind, LangItem, Node};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::sym;
@@ -202,11 +203,13 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
             && let Some(fn_name) = cx.tcx.get_diagnostic_name(def_id)
         {
             let arg_ty = cx.typeck_results().expr_ty(arg);
+            let is_copy = is_copy(cx, arg_ty);
+            let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr);
             let (lint, msg) = match fn_name {
                 sym::mem_drop if arg_ty.is_ref() => (DROP_REF, DROP_REF_SUMMARY),
                 sym::mem_forget if arg_ty.is_ref() => (FORGET_REF, FORGET_REF_SUMMARY),
-                sym::mem_drop if is_copy(cx, arg_ty) => (DROP_COPY, DROP_COPY_SUMMARY),
-                sym::mem_forget if is_copy(cx, arg_ty) => (FORGET_COPY, FORGET_COPY_SUMMARY),
+                sym::mem_drop if is_copy && !drop_is_single_call_in_arm => (DROP_COPY, DROP_COPY_SUMMARY),
+                sym::mem_forget if is_copy => (FORGET_COPY, FORGET_COPY_SUMMARY),
                 sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => {
                     span_lint_and_help(
                         cx,
@@ -221,7 +224,9 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
                 sym::mem_drop
                     if !(arg_ty.needs_drop(cx.tcx, cx.param_env)
                         || is_must_use_func_call(cx, arg)
-                        || is_must_use_ty(cx, arg_ty)) =>
+                        || is_must_use_ty(cx, arg_ty)
+                        || drop_is_single_call_in_arm
+                        ) =>
                 {
                     (DROP_NON_DROP, DROP_NON_DROP_SUMMARY)
                 },
@@ -241,3 +246,18 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
         }
     }
 }
+
+// dropping returned value of a function like in the following snippet is considered idiomatic, see
+// #9482 for examples match <var> {
+//     <pat> => drop(fn_with_side_effect_and_returning_some_value()),
+//     ..
+// }
+fn is_single_call_in_arm<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'_>, drop_expr: &'tcx Expr<'_>) -> bool {
+    if matches!(arg.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)) {
+        let parent_node = get_parent_node(cx.tcx, drop_expr.hir_id);
+        if let Some(Node::Arm(Arm { body, .. })) = &parent_node {
+            return body.hir_id == drop_expr.hir_id;
+        }
+    }
+    false
+}

tests/ui/drop_forget_copy.rs

@@ -64,3 +64,23 @@ fn main() {
     let a5 = a1.clone();
     forget(a5);
 }
+
+#[allow(unused)]
+#[allow(clippy::unit_cmp)]
+fn issue9482(x: u8) {
+    fn println_and<T>(t: T) -> T {
+        println!("foo");
+        t
+    }
+
+    match x {
+        0 => drop(println_and(12)), // Don't lint (copy type), we only care about side-effects
+        1 => drop(println_and(String::new())), // Don't lint (no copy type), we only care about side-effects
+        2 => {
+            drop(println_and(13)); // Lint, even if we only care about the side-effect, it's already in a block
+        },
+        3 if drop(println_and(14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
+        4 => drop(2),                           // Lint, not a fn/method call
+        _ => (),
+    }
+}

tests/ui/drop_forget_copy.stderr

@@ -72,5 +72,41 @@ note: argument has type `SomeStruct`
 LL |     forget(s4);
    |            ^^
 
-error: aborting due to 6 previous errors
+error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact
+  --> $DIR/drop_forget_copy.rs:80:13
+   |
+LL |             drop(println_and(13)); // Lint, even if we only care about the side-effect, it's already in a block
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: argument has type `i32`
+  --> $DIR/drop_forget_copy.rs:80:18
+   |
+LL |             drop(println_and(13)); // Lint, even if we only care about the side-effect, it's already in a block
+   |                  ^^^^^^^^^^^^^^^
+
+error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact
+  --> $DIR/drop_forget_copy.rs:82:14
+   |
+LL |         3 if drop(println_and(14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
+   |              ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: argument has type `i32`
+  --> $DIR/drop_forget_copy.rs:82:19
+   |
+LL |         3 if drop(println_and(14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
+   |                   ^^^^^^^^^^^^^^^
+
+error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact
+  --> $DIR/drop_forget_copy.rs:83:14
+   |
+LL |         4 => drop(2),                           // Lint, not a fn/method call
+   |              ^^^^^^^
+   |
+note: argument has type `i32`
+  --> $DIR/drop_forget_copy.rs:83:19
+   |
+LL |         4 => drop(2),                           // Lint, not a fn/method call
+   |                   ^
+
+error: aborting due to 9 previous errors