From: Oliver Schneider Date: Sun, 8 Oct 2017 09:51:15 +0000 (+0200) Subject: Merge pull request #2112 from topecongiro/issue-2109 X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=a54baad4fa177efac04eafd6d9ffe6236f7d3c45;hp=7f4b583c477850c937b5ad3012cc3f5216c2baeb;p=rust.git 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 1d5e51187bb..f5543821949 100644 --- 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 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 ec0521ce4f2..cf0aaf6dbaa 100644 --- 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 ba7826d653e..a037ac3cf0e 100644 --- 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 43e7fbb3609..0d8d6624a83 100644 --- 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");`