Update unnecessary_operation and no_effect to not suggest removing

structs/enums wrappers when that type implements Drop as noted
in #2061.

clippy_lints/src/no_effect.rs

@@ -40,12 +40,22 @@ declare_lint! {
     "outer expressions with no effect"
 }
 
+/// Check whether this type implements Drop.
+fn has_drop(cx: &LateContext, expr: &Expr) -> bool {
+    let struct_ty = cx.tables.expr_ty(expr);
+    match struct_ty.ty_adt_def() {
+        Some(def) => def.has_dtor(cx.tcx),
+        _ => false,
+    }
+}
+
 fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
     if in_macro(expr.span) {
         return false;
     }
     match expr.node {
-        Expr_::ExprLit(..) | Expr_::ExprClosure(.., _) | Expr_::ExprPath(..) => true,
+        Expr_::ExprLit(..) | Expr_::ExprClosure(.., _) => true,
+        Expr_::ExprPath(..) => !has_drop(cx, expr),
         Expr_::ExprIndex(ref a, ref b) | Expr_::ExprBinary(_, ref a, ref b) => {
             has_no_effect(cx, a) && has_no_effect(cx, b)
         },
@@ -59,7 +69,7 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
         Expr_::ExprAddrOf(_, ref inner) |
         Expr_::ExprBox(ref inner) => has_no_effect(cx, inner),
         Expr_::ExprStruct(_, ref fields, ref base) => {
-            fields.iter().all(|field| has_no_effect(cx, &field.expr)) && match *base {
+            !has_drop(cx, expr) && fields.iter().all(|field| has_no_effect(cx, &field.expr)) && match *base {
                 Some(ref base) => has_no_effect(cx, base),
                 None => true,
             }
@@ -68,7 +78,7 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
             let def = cx.tables.qpath_def(qpath, callee.hir_id);
             match def {
                 Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => {
-                    args.iter().all(|arg| has_no_effect(cx, arg))
+                    !has_drop(cx, expr) && args.iter().all(|arg| has_no_effect(cx, arg))
                 },
                 _ => false,
             }
@@ -145,18 +155,23 @@ fn reduce_expression<'a>(cx: &LateContext, expr: &'a Expr) -> Option<Vec<&'a Exp
         Expr_::ExprTupField(ref inner, _) |
         Expr_::ExprAddrOf(_, ref inner) |
         Expr_::ExprBox(ref inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])),
-        Expr_::ExprStruct(_, ref fields, ref base) => Some(
+        Expr_::ExprStruct(_, ref fields, ref base) => {
-            fields
+            if has_drop(cx, expr) {
-                .iter()
+                None
-                .map(|f| &f.expr)
+            } else {
-                .chain(base)
+                Some(
-                .map(Deref::deref)
+                    fields
-                .collect(),
+                        .iter()
-        ),
+                        .map(|f| &f.expr)
+                        .chain(base)
+                        .map(Deref::deref)
+                        .collect())
+            }
+        },
         Expr_::ExprCall(ref callee, ref args) => if let Expr_::ExprPath(ref qpath) = callee.node {
             let def = cx.tables.qpath_def(qpath, callee.hir_id);
             match def {
-                Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => {
+                Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) if !has_drop(cx, expr) => {
                     Some(args.iter().collect())
                 },
                 _ => None,

tests/ui/no_effect.rs

@@ -16,6 +16,27 @@ enum Enum {
     Tuple(i32),
     Struct { field: i32 },
 }
+struct DropUnit;
+impl Drop for DropUnit {
+    fn drop(&mut self) {}
+}
+struct DropStruct {
+    field: i32
+}
+impl Drop for DropStruct {
+    fn drop(&mut self) {}
+}
+struct DropTuple(i32);
+impl Drop for DropTuple {
+    fn drop(&mut self) {}
+}
+enum DropEnum {
+    Tuple(i32),
+    Struct { field: i32 },
+}
+impl Drop for DropEnum {
+    fn drop(&mut self) {}
+}
 
 union Union {
     a: u8,
@@ -24,6 +45,7 @@ union Union {
 
 fn get_number() -> i32 { 0 }
 fn get_struct() -> Struct { Struct { field: 0 } }
+fn get_drop_struct() -> DropStruct { DropStruct { field: 0 } }
 
 unsafe fn unsafe_fn() -> i32 { 0 }
 
@@ -61,6 +83,11 @@ fn main() {
     // Do not warn
     get_number();
     unsafe { unsafe_fn() };
+    DropUnit;
+    DropStruct { field: 0 };
+    DropTuple(0);
+    DropEnum::Tuple(0);
+    DropEnum::Struct { field: 0 };
 
     Tuple(get_number());
     Struct { field: get_number() };
@@ -81,4 +108,12 @@ fn main() {
     [get_number(); 55];
     [42; 55][get_number() as usize];
     {get_number()};
+
+    // Do not warn
+    DropTuple(get_number());
+    DropStruct { field: get_number() };
+    DropStruct { field: get_number() };
+    DropStruct { ..get_drop_struct() };
+    DropEnum::Tuple(get_number());
+    DropEnum::Struct { field: get_number() };
 }

tests/ui/no_effect.stderr

@@ -1,270 +1,270 @@
 error: statement with no effect
-  --> $DIR/no_effect.rs:34:5
+  --> $DIR/no_effect.rs:56:5
    |
-34 |     0;
+56 |     0;
    |     ^^
    |
    = note: `-D no-effect` implied by `-D warnings`
 
-error: statement with no effect
-  --> $DIR/no_effect.rs:35:5
-   |
-35 |     s2;
-   |     ^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:36:5
-   |
-36 |     Unit;
-   |     ^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:37:5
-   |
-37 |     Tuple(0);
-   |     ^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:38:5
-   |
-38 |     Struct { field: 0 };
-   |     ^^^^^^^^^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:39:5
-   |
-39 |     Struct { ..s };
-   |     ^^^^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:40:5
-   |
-40 |     Union { a: 0 };
-   |     ^^^^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:41:5
-   |
-41 |     Enum::Tuple(0);
-   |     ^^^^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:42:5
-   |
-42 |     Enum::Struct { field: 0 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:43:5
-   |
-43 |     5 + 6;
-   |     ^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:44:5
-   |
-44 |     *&42;
-   |     ^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:45:5
-   |
-45 |     &6;
-   |     ^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:46:5
-   |
-46 |     (5, 6, 7);
-   |     ^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:47:5
-   |
-47 |     box 42;
-   |     ^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:48:5
-   |
-48 |     ..;
-   |     ^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:49:5
-   |
-49 |     5..;
-   |     ^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:50:5
-   |
-50 |     ..5;
-   |     ^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:51:5
-   |
-51 |     5..6;
-   |     ^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:52:5
-   |
-52 |     5...6;
-   |     ^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:53:5
-   |
-53 |     [42, 55];
-   |     ^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:54:5
-   |
-54 |     [42, 55][1];
-   |     ^^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:55:5
-   |
-55 |     (42, 55).1;
-   |     ^^^^^^^^^^^
-
-error: statement with no effect
-  --> $DIR/no_effect.rs:56:5
-   |
-56 |     [42; 55];
-   |     ^^^^^^^^^
-
 error: statement with no effect
   --> $DIR/no_effect.rs:57:5
    |
-57 |     [42; 55][13];
+57 |     s2;
-   |     ^^^^^^^^^^^^^
+   |     ^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:58:5
+   |
+58 |     Unit;
+   |     ^^^^^
 
 error: statement with no effect
   --> $DIR/no_effect.rs:59:5
    |
-59 |     || x += 5;
+59 |     Tuple(0);
+   |     ^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:60:5
+   |
+60 |     Struct { field: 0 };
+   |     ^^^^^^^^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:61:5
+   |
+61 |     Struct { ..s };
+   |     ^^^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:62:5
+   |
+62 |     Union { a: 0 };
+   |     ^^^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:63:5
+   |
+63 |     Enum::Tuple(0);
+   |     ^^^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:64:5
+   |
+64 |     Enum::Struct { field: 0 };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:65:5
+   |
+65 |     5 + 6;
+   |     ^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:66:5
+   |
+66 |     *&42;
+   |     ^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:67:5
+   |
+67 |     &6;
+   |     ^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:68:5
+   |
+68 |     (5, 6, 7);
+   |     ^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:69:5
+   |
+69 |     box 42;
+   |     ^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:70:5
+   |
+70 |     ..;
+   |     ^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:71:5
+   |
+71 |     5..;
+   |     ^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:72:5
+   |
+72 |     ..5;
+   |     ^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:73:5
+   |
+73 |     5..6;
+   |     ^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:74:5
+   |
+74 |     5...6;
+   |     ^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:75:5
+   |
+75 |     [42, 55];
+   |     ^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:76:5
+   |
+76 |     [42, 55][1];
+   |     ^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:77:5
+   |
+77 |     (42, 55).1;
+   |     ^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:78:5
+   |
+78 |     [42; 55];
+   |     ^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:79:5
+   |
+79 |     [42; 55][13];
+   |     ^^^^^^^^^^^^^
+
+error: statement with no effect
+  --> $DIR/no_effect.rs:81:5
+   |
+81 |     || x += 5;
    |     ^^^^^^^^^^
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:65:5
+  --> $DIR/no_effect.rs:92:5
    |
-65 |     Tuple(get_number());
+92 |     Tuple(get_number());
    |     ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
    |
    = note: `-D unnecessary-operation` implied by `-D warnings`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:66:5
+  --> $DIR/no_effect.rs:93:5
    |
-66 |     Struct { field: get_number() };
+93 |     Struct { field: get_number() };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:67:5
+  --> $DIR/no_effect.rs:94:5
    |
-67 |     Struct { ..get_struct() };
+94 |     Struct { ..get_struct() };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_struct();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:68:5
+  --> $DIR/no_effect.rs:95:5
    |
-68 |     Enum::Tuple(get_number());
+95 |     Enum::Tuple(get_number());
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:69:5
+  --> $DIR/no_effect.rs:96:5
    |
-69 |     Enum::Struct { field: get_number() };
+96 |     Enum::Struct { field: get_number() };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:70:5
+  --> $DIR/no_effect.rs:97:5
    |
-70 |     5 + get_number();
+97 |     5 + get_number();
    |     ^^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:71:5
+  --> $DIR/no_effect.rs:98:5
    |
-71 |     *&get_number();
+98 |     *&get_number();
    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:72:5
+  --> $DIR/no_effect.rs:99:5
    |
-72 |     &get_number();
+99 |     &get_number();
    |     ^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:73:5
+   --> $DIR/no_effect.rs:100:5
-   |
+    |
-73 |     (5, 6, get_number());
+100 |     (5, 6, get_number());
-   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();`
+    |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:74:5
+   --> $DIR/no_effect.rs:101:5
-   |
+    |
-74 |     box get_number();
+101 |     box get_number();
-   |     ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+    |     ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:75:5
+   --> $DIR/no_effect.rs:102:5
-   |
+    |
-75 |     get_number()..;
+102 |     get_number()..;
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:76:5
+   --> $DIR/no_effect.rs:103:5
-   |
+    |
-76 |     ..get_number();
+103 |     ..get_number();
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:77:5
+   --> $DIR/no_effect.rs:104:5
-   |
+    |
-77 |     5..get_number();
+104 |     5..get_number();
-   |     ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
+    |     ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:78:5
+   --> $DIR/no_effect.rs:105:5
-   |
+    |
-78 |     [42, get_number()];
+105 |     [42, get_number()];
-   |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
+    |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:79:5
+   --> $DIR/no_effect.rs:106:5
-   |
+    |
-79 |     [42, 55][get_number() as usize];
+106 |     [42, 55][get_number() as usize];
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_number() as usize;`
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_number() as usize;`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:80:5
+   --> $DIR/no_effect.rs:107:5
-   |
+    |
-80 |     (42, get_number()).1;
+107 |     (42, get_number()).1;
-   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
+    |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:81:5
+   --> $DIR/no_effect.rs:108:5
-   |
+    |
-81 |     [get_number(); 55];
+108 |     [get_number(); 55];
-   |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+    |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:82:5
+   --> $DIR/no_effect.rs:109:5
-   |
+    |
-82 |     [42; 55][get_number() as usize];
+109 |     [42; 55][get_number() as usize];
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_number() as usize;`
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_number() as usize;`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:83:5
+   --> $DIR/no_effect.rs:110:5
-   |
+    |
-83 |     {get_number()};
+110 |     {get_number()};
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: aborting due to 44 previous errors