Merge pull request #2112 from topecongiro/issue-2109

Add a suggestion to replace `map(f).unwrap_or(None)` with `and_then(f)`.

diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs
index 1d5e51187bbd430f4aadbe7f74755f803c2d9858..f5543821949a4de453dc435da1d36b78ef7e3438 100644 (file)
--- a/clippy_lints/src/no_effect.rs
+++ b/clippy_lints/src/no_effect.rs
@@ -1,7 +1,7 @@
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::hir::def::Def;
 use rustc::hir::{BiAnd, BiOr, BlockCheckMode, Expr, Expr_, Stmt, StmtSemi, UnsafeSource};
-use utils::{in_macro, snippet_opt, span_lint, span_lint_and_sugg};
+use utils::{in_macro, snippet_opt, span_lint, span_lint_and_sugg, has_drop};
 use std::ops::Deref;
 
 /// **What it does:** Checks for statements which have no effect.
@@ -45,7 +45,8 @@ fn has_no_effect(cx: &LateContext, expr: &Expr) -> bool {
         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 +60,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 +69,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 +146,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(
-            fields
-                .iter()
-                .map(|f| &f.expr)
-                .chain(base)
-                .map(Deref::deref)
-                .collect(),
-        ),
+        Expr_::ExprStruct(_, ref fields, ref base) => {
+            if has_drop(cx, expr) {
+                None
+            } else {
+                Some(
+                    fields
+                        .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,

diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index ec0521ce4f2df88c3b95cc241a741815ce13f8f9..cf0aaf6dbaa2703d30fb7d4e11eb0f50a15ad7c9 100644 (file)
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -358,6 +358,15 @@ pub fn implements_trait<'a, 'tcx>(
     })
 }
 
+/// Check whether this type implements Drop.
+pub 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,
+    }
+}
+
 /// Resolve the definition of a node from its `HirId`.
 pub fn resolve_node(cx: &LateContext, qpath: &QPath, id: HirId) -> def::Def {
     cx.tables.qpath_def(qpath, id)

diff --git a/tests/ui/no_effect.rs b/tests/ui/no_effect.rs
index ba7826d653edd33150858e1f9d471afbfcee0b12..a037ac3cf0e11cfe2d8c02654b97f5858aeba97f 100644 (file)
--- a/tests/ui/no_effect.rs
+++ b/tests/ui/no_effect.rs
@@ -16,7 +16,30 @@ 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) {}
+}
+struct FooString {
+    s: String,
+}
 union Union {
     a: u8,
     b: f64,
@@ -24,6 +47,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 }
 
@@ -57,10 +81,17 @@ fn main() {
     [42; 55][13];
     let mut x = 0;
     || x += 5;
+    let s: String = "foo".into();
+    FooString { s: s };
 
     // 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 +112,13 @@ fn main() {
     [get_number(); 55];
     [42; 55][get_number() as usize];
     {get_number()};
+    FooString { s: String::from("blah"), };
+
+    // 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() };
 }

diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr
index 43e7fbb360988244ff4358b2a46e3598e0e8edf9..0d8d6624a833d22ffebceff7643dea1b7d2b71d4 100644 (file)
--- a/tests/ui/no_effect.stderr
+++ b/tests/ui/no_effect.stderr
@@ -1,268 +1,280 @@
 error: statement with no effect
-  --> $DIR/no_effect.rs:34:5
+  --> $DIR/no_effect.rs:58:5
    |
-34 |     0;
+58 |     0;
    |     ^^
    |
    = note: `-D no-effect` implied by `-D warnings`
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:35:5
+  --> $DIR/no_effect.rs:59:5
    |
-35 |     s2;
+59 |     s2;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:36:5
+  --> $DIR/no_effect.rs:60:5
    |
-36 |     Unit;
+60 |     Unit;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:37:5
+  --> $DIR/no_effect.rs:61:5
    |
-37 |     Tuple(0);
+61 |     Tuple(0);
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:38:5
+  --> $DIR/no_effect.rs:62:5
    |
-38 |     Struct { field: 0 };
+62 |     Struct { field: 0 };
    |     ^^^^^^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:39:5
+  --> $DIR/no_effect.rs:63:5
    |
-39 |     Struct { ..s };
+63 |     Struct { ..s };
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:40:5
+  --> $DIR/no_effect.rs:64:5
    |
-40 |     Union { a: 0 };
+64 |     Union { a: 0 };
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:41:5
+  --> $DIR/no_effect.rs:65:5
    |
-41 |     Enum::Tuple(0);
+65 |     Enum::Tuple(0);
    |     ^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:42:5
+  --> $DIR/no_effect.rs:66:5
    |
-42 |     Enum::Struct { field: 0 };
+66 |     Enum::Struct { field: 0 };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:43:5
+  --> $DIR/no_effect.rs:67:5
    |
-43 |     5 + 6;
+67 |     5 + 6;
    |     ^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:44:5
+  --> $DIR/no_effect.rs:68:5
    |
-44 |     *&42;
+68 |     *&42;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:45:5
+  --> $DIR/no_effect.rs:69:5
    |
-45 |     &6;
+69 |     &6;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:46:5
+  --> $DIR/no_effect.rs:70:5
    |
-46 |     (5, 6, 7);
+70 |     (5, 6, 7);
    |     ^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:47:5
+  --> $DIR/no_effect.rs:71:5
    |
-47 |     box 42;
+71 |     box 42;
    |     ^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:48:5
+  --> $DIR/no_effect.rs:72:5
    |
-48 |     ..;
+72 |     ..;
    |     ^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:49:5
+  --> $DIR/no_effect.rs:73:5
    |
-49 |     5..;
+73 |     5..;
    |     ^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:50:5
+  --> $DIR/no_effect.rs:74:5
    |
-50 |     ..5;
+74 |     ..5;
    |     ^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:51:5
+  --> $DIR/no_effect.rs:75:5
    |
-51 |     5..6;
+75 |     5..6;
    |     ^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:52:5
+  --> $DIR/no_effect.rs:76:5
    |
-52 |     5..=6;
+76 |     5..=6;
    |     ^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:53:5
+  --> $DIR/no_effect.rs:77:5
    |
-53 |     [42, 55];
+77 |     [42, 55];
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:54:5
+  --> $DIR/no_effect.rs:78:5
    |
-54 |     [42, 55][1];
+78 |     [42, 55][1];
    |     ^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:55:5
+  --> $DIR/no_effect.rs:79:5
    |
-55 |     (42, 55).1;
+79 |     (42, 55).1;
    |     ^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:56:5
+  --> $DIR/no_effect.rs:80:5
    |
-56 |     [42; 55];
+80 |     [42; 55];
    |     ^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:57:5
+  --> $DIR/no_effect.rs:81:5
    |
-57 |     [42; 55][13];
+81 |     [42; 55][13];
    |     ^^^^^^^^^^^^^
 
 error: statement with no effect
-  --> $DIR/no_effect.rs:59:5
+  --> $DIR/no_effect.rs:83:5
    |
-59 |     || x += 5;
+83 |     || x += 5;
    |     ^^^^^^^^^^
 
+error: statement with no effect
+  --> $DIR/no_effect.rs:85:5
+   |
+85 |     FooString { s: s };
+   |     ^^^^^^^^^^^^^^^^^^^
+
 error: statement can be reduced
-  --> $DIR/no_effect.rs:65:5
+  --> $DIR/no_effect.rs:96:5
    |
-65 |     Tuple(get_number());
+96 |     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:97:5
    |
-66 |     Struct { field: get_number() };
+97 |     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:98:5
    |
-67 |     Struct { ..get_struct() };
+98 |     Struct { ..get_struct() };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_struct();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:68:5
+  --> $DIR/no_effect.rs:99:5
    |
-68 |     Enum::Tuple(get_number());
+99 |     Enum::Tuple(get_number());
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:69:5
-   |
-69 |     Enum::Struct { field: get_number() };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:100:5
+    |
+100 |     Enum::Struct { field: get_number() };
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:70:5
-   |
-70 |     5 + get_number();
-   |     ^^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
+   --> $DIR/no_effect.rs:101:5
+    |
+101 |     5 + get_number();
+    |     ^^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:71:5
-   |
-71 |     *&get_number();
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:102:5
+    |
+102 |     *&get_number();
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:72:5
-   |
-72 |     &get_number();
-   |     ^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:103:5
+    |
+103 |     &get_number();
+    |     ^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:73:5
-   |
-73 |     (5, 6, get_number());
-   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();`
+   --> $DIR/no_effect.rs:104:5
+    |
+104 |     (5, 6, get_number());
+    |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:74:5
-   |
-74 |     box get_number();
-   |     ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:105:5
+    |
+105 |     box get_number();
+    |     ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:75:5
-   |
-75 |     get_number()..;
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:106:5
+    |
+106 |     get_number()..;
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:76:5
-   |
-76 |     ..get_number();
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:107:5
+    |
+107 |     ..get_number();
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:77:5
-   |
-77 |     5..get_number();
-   |     ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
+   --> $DIR/no_effect.rs:108:5
+    |
+108 |     5..get_number();
+    |     ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:78:5
-   |
-78 |     [42, get_number()];
-   |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
+   --> $DIR/no_effect.rs:109:5
+    |
+109 |     [42, get_number()];
+    |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:79:5
-   |
-79 |     [42, 55][get_number() as usize];
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_number() as usize;`
+   --> $DIR/no_effect.rs:110:5
+    |
+110 |     [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
-   |
-80 |     (42, get_number()).1;
-   |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
+   --> $DIR/no_effect.rs:111:5
+    |
+111 |     (42, get_number()).1;
+    |     ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:81:5
-   |
-81 |     [get_number(); 55];
-   |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:112:5
+    |
+112 |     [get_number(); 55];
+    |     ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
 
 error: statement can be reduced
-  --> $DIR/no_effect.rs:82:5
-   |
-82 |     [42; 55][get_number() as usize];
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_number() as usize;`
+   --> $DIR/no_effect.rs:113:5
+    |
+113 |     [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
-   |
-83 |     {get_number()};
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+   --> $DIR/no_effect.rs:114:5
+    |
+114 |     {get_number()};
+    |     ^^^^^^^^^^^^^^^ help: replace it with: `get_number();`
+
+error: statement can be reduced
+   --> $DIR/no_effect.rs:115:5
+    |
+115 |     FooString { s: String::from("blah"), };
+    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `String::from("blah");`