Add let_underscore_drop

CHANGELOG.md

@@ -1787,6 +1787,7 @@ Released 2018-09-13
 [`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty
 [`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
 [`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
+[`let_underscore_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_drop
 [`let_underscore_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_lock
 [`let_underscore_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_must_use
 [`let_unit_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value

clippy_lints/src/let_underscore.rs

@@ -5,7 +5,7 @@ use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
-use crate::utils::{is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help};
+use crate::utils::{implements_trait, is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for `let _ = <expr>`
@@ -58,7 +58,40 @@ declare_clippy_lint! {
     "non-binding let on a synchronization lock"
 }
 
-declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK]);
+declare_clippy_lint! {
+    /// **What it does:** Checks for `let _ = <expr>`
+    /// where expr has a type that implements `Drop`
+    ///
+    /// **Why is this bad?** This statement immediately drops the initializer
+    /// expression instead of extending its lifetime to the end of the scope, which
+    /// is often not intended. To extend the expression's lifetime to the end of the
+    /// scope, use an underscore-prefixed name instead (i.e. _var). If you want to
+    /// explicitly drop the expression, `std::mem::drop` conveys your intention
+    /// better and is less error-prone.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// Bad:
+    /// ```rust,ignore
+    /// struct Droppable;
+    /// impl Drop for Droppable {
+    ///     fn drop(&mut self) {}
+    /// }
+    /// let _ = Droppable;
+    /// ```
+    ///
+    /// Good:
+    /// ```rust,ignore
+    /// let _droppable = Droppable;
+    /// ```
+    pub LET_UNDERSCORE_DROP,
+    correctness,
+    "non-binding let on a type that implements `Drop`"
+}
+
+declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_DROP]);
 
 const SYNC_GUARD_PATHS: [&[&str]; 3] = [
     &paths::MUTEX_GUARD,
@@ -84,6 +117,15 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
 
                     GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
                 });
+                let implements_drop = cx.tcx.lang_items().drop_trait().map_or(false, |drop_trait|
+                    init_ty.walk().any(|inner| match inner.unpack() {
+                        GenericArgKind::Type(inner_ty) => {
+                            implements_trait(cx, inner_ty, drop_trait, &[])
+                        },
+
+                        GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
+                    })
+                );
                 if contains_sync_guard {
                     span_lint_and_help(
                         cx,
@@ -94,6 +136,16 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
                         "consider using an underscore-prefixed named \
                             binding or dropping explicitly with `std::mem::drop`"
                     )
+                } else if implements_drop {
+                    span_lint_and_help(
+                        cx,
+                        LET_UNDERSCORE_DROP,
+                        local.span,
+                        "non-binding let on a type that implements `Drop`",
+                        None,
+                        "consider using an underscore-prefixed named \
+                            binding or dropping explicitly with `std::mem::drop`"
+                    )
                 } else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
                     span_lint_and_help(
                         cx,

clippy_lints/src/lib.rs

@@ -622,6 +622,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &len_zero::LEN_WITHOUT_IS_EMPTY,
         &len_zero::LEN_ZERO,
         &let_if_seq::USELESS_LET_IF_SEQ,
+        &let_underscore::LET_UNDERSCORE_DROP,
         &let_underscore::LET_UNDERSCORE_LOCK,
         &let_underscore::LET_UNDERSCORE_MUST_USE,
         &lifetimes::EXTRA_UNUSED_LIFETIMES,
@@ -1383,6 +1384,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&len_zero::COMPARISON_TO_EMPTY),
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
+        LintId::of(&let_underscore::LET_UNDERSCORE_DROP),
         LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
         LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES),
         LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
@@ -1809,6 +1811,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&infinite_iter::INFINITE_ITER),
         LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
         LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
+        LintId::of(&let_underscore::LET_UNDERSCORE_DROP),
         LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
         LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES),
         LintId::of(&loops::FOR_LOOPS_OVER_FALLIBLES),

src/lintlist/mod.rs

@@ -1117,6 +1117,13 @@ vec![
         deprecation: None,
         module: "returns",
     },
+    Lint {
+        name: "let_underscore_drop",
+        group: "correctness",
+        desc: "non-binding let on a type that implements `Drop`",
+        deprecation: None,
+        module: "let_underscore",
+    },
     Lint {
         name: "let_underscore_lock",
         group: "correctness",

tests/ui/borrow_box.stderr

@@ -22,6 +22,15 @@ error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
 LL |     fn test4(a: &Box<bool>);
    |                 ^^^^^^^^^^ help: try: `&bool`
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/borrow_box.rs:63:5
+   |
+LL |     let _ = foo;
+   |     ^^^^^^^^^^^^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
   --> $DIR/borrow_box.rs:95:25
    |
@@ -64,5 +73,5 @@ error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
 LL | pub fn test20(_display: &Box<(dyn Display + Send)>) {}
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&(dyn Display + Send)`
 
-error: aborting due to 10 previous errors
+error: aborting due to 11 previous errors
 

tests/ui/borrow_interior_mutable_const/others.stderr

@@ -47,6 +47,15 @@ LL |     let _once_mut = &mut ONCE_INIT; //~ ERROR interior mutability
    |
    = help: assign this const to a local or static variable, and use the variable here
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/others.rs:72:5
+   |
+LL |     let _ = &ATOMIC_TUPLE; //~ ERROR interior mutability
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: a `const` item with interior mutability should not be borrowed
   --> $DIR/others.rs:72:14
    |
@@ -95,6 +104,22 @@ LL |     let _ = ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability
    |
    = help: assign this const to a local or static variable, and use the variable here
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/others.rs:83:5
+   |
+LL |     let _ = ATOMIC_TUPLE.1.into_iter();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/others.rs:85:5
+   |
+LL |     let _ = &{ ATOMIC_TUPLE };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: a `const` item with interior mutability should not be borrowed
   --> $DIR/others.rs:87:5
    |
@@ -111,5 +136,5 @@ LL |     assert_eq!(CELL.get(), 6); //~ ERROR interior mutability
    |
    = help: assign this const to a local or static variable, and use the variable here
 
-error: aborting due to 14 previous errors
+error: aborting due to 17 previous errors
 

tests/ui/box_vec.stderr

@@ -1,3 +1,16 @@
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/box_vec.rs:7:9
+   |
+LL |         let _: Box<$x> = Box::new($init);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     boxit!(Vec::new(), Vec<u8>);
+   |     ---------------------------- in this macro invocation
+   |
+   = note: `-D clippy::let-underscore-drop` implied by `-D warnings`
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error: you seem to be trying to use `Box<Vec<T>>`. Consider using just `Vec<T>`
   --> $DIR/box_vec.rs:14:18
    |
@@ -7,5 +20,5 @@ LL | pub fn test(foo: Box<Vec<bool>>) {
    = note: `-D clippy::box-vec` implied by `-D warnings`
    = help: `Vec<T>` is already on the heap, `Box<Vec<T>>` makes an extra allocation.
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 

tests/ui/crashes/ice-4968.stderr

@@ -0,0 +1,11 @@
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/ice-4968.rs:16:9
+   |
+LL |         let _: Vec<ManuallyDrop<T::Assoc>> = mem::transmute(slice);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: aborting due to previous error
+

tests/ui/crashes/ice-5223.stderr

@@ -0,0 +1,11 @@
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/ice-5223.rs:14:9
+   |
+LL |         let _ = self.arr.iter().cloned().collect::<Vec<_>>();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: aborting due to previous error
+

tests/ui/escape_analysis.stderr

@@ -12,5 +12,26 @@ error: local variable doesn't need to be boxed here
 LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
    |            ^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/escape_analysis.rs:166:9
+   |
+LL | /         let _ = move || {
+LL | |             consume(x);
+LL | |         };
+   | |__________^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/escape_analysis.rs:172:9
+   |
+LL | /         let _ = || {
+LL | |             borrow(&x);
+LL | |         };
+   | |__________^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: aborting due to 4 previous errors
 

tests/ui/eta.stderr

@@ -64,6 +64,15 @@ error: redundant closure found
 LL |     let e: std::vec::Vec<char> = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect();
    |                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase`
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/eta.rs:107:5
+   |
+LL |     let _: Vec<_> = arr.iter().map(|x| x.map_err(|e| some.take().unwrap()(e))).collect();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[deny(clippy::let_underscore_drop)]` on by default
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: redundant closure found
   --> $DIR/eta.rs:172:27
    |
@@ -76,5 +85,13 @@ error: redundant closure found
 LL |     let a = Some(1u8).map(|a| closure(a));
    |                           ^^^^^^^^^^^^^^ help: remove closure as shown: `closure`
 
-error: aborting due to 12 previous errors
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/eta.rs:203:5
+   |
+LL |     let _ = [Bar].iter().map(|s| s.to_string()).collect::<Vec<_>>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: aborting due to 14 previous errors
 

tests/ui/filter_methods.stderr

@@ -1,3 +1,12 @@
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/filter_methods.rs:5:5
+   |
+LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::let-underscore-drop` implied by `-D warnings`
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: called `filter(..).map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:5:21
    |
@@ -7,6 +16,18 @@ LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x *
    = note: `-D clippy::filter-map` implied by `-D warnings`
    = help: this is more succinctly expressed by calling `.filter_map(..)` instead
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/filter_methods.rs:7:5
+   |
+LL | /     let _: Vec<_> = vec![5_i8; 6]
+LL | |         .into_iter()
+LL | |         .filter(|&x| x == 0)
+LL | |         .flat_map(|x| x.checked_mul(2))
+LL | |         .collect();
+   | |___________________^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: called `filter(..).flat_map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:7:21
    |
@@ -19,6 +40,18 @@ LL | |         .flat_map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/filter_methods.rs:13:5
+   |
+LL | /     let _: Vec<_> = vec![5_i8; 6]
+LL | |         .into_iter()
+LL | |         .filter_map(|x| x.checked_mul(2))
+LL | |         .flat_map(|x| x.checked_mul(2))
+LL | |         .collect();
+   | |___________________^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: called `filter_map(..).flat_map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:13:21
    |
@@ -31,6 +64,18 @@ LL | |         .flat_map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
 
+error: non-binding let on a type that implements `Drop`
+  --> $DIR/filter_methods.rs:19:5
+   |
+LL | /     let _: Vec<_> = vec![5_i8; 6]
+LL | |         .into_iter()
+LL | |         .filter_map(|x| x.checked_mul(2))
+LL | |         .map(|x| x.checked_mul(2))
+LL | |         .collect();
+   | |___________________^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
 error: called `filter_map(..).map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:19:21
    |
@@ -43,5 +88,5 @@ LL | |         .map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by only calling `.filter_map(..)` instead
 
-error: aborting due to 4 previous errors
+error: aborting due to 8 previous errors