about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-11-09 19:13:26 +0000
committerbors <bors@rust-lang.org>2020-11-09 19:13:26 +0000
commitdd826b4626c00da53f76f00f02f03556803e9cdb (patch)
treefcaba54948cd5788ba2547c8daf4bde005ee8f05
parentd212c382c3d00b8a2bb701313c7bdd605ea7e128 (diff)
parent4852cca61bb989adf77b1d202eae6b40067fa9ab (diff)
downloadrust-dd826b4626c00da53f76f00f02f03556803e9cdb.tar.gz
rust-dd826b4626c00da53f76f00f02f03556803e9cdb.zip
Auto merge of #6305 - smoelius:master, r=flip1995
Add `let_underscore_drop`

This line generalizes `let_underscore_lock` (#5101) to warn about any initializer expression that implements `Drop`.

So, for example, the following would generate a warning:
```rust
struct Droppable;
impl Drop for Droppable {
    fn drop(&mut self) {}
}
let _ = Droppable;
```

I tried to preserve the original `let_underscore_lock` functionality in the sense that the warning generated for
```rust
let _ = mutex.lock();
```
should be unchanged.

*Please keep the line below*
changelog: Add lint [`let_underscore_drop`]
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/let_underscore.rs64
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--src/lintlist/mod.rs7
-rw-r--r--tests/ui/filter_methods.rs1
-rw-r--r--tests/ui/filter_methods.stderr8
-rw-r--r--tests/ui/let_underscore_drop.rs19
-rw-r--r--tests/ui/let_underscore_drop.stderr27
-rw-r--r--tests/ui/map_clone.fixed1
-rw-r--r--tests/ui/map_clone.rs1
-rw-r--r--tests/ui/map_clone.stderr12
-rw-r--r--tests/ui/map_flatten.fixed1
-rw-r--r--tests/ui/map_flatten.rs1
-rw-r--r--tests/ui/map_flatten.stderr12
14 files changed, 139 insertions, 18 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e0770a45c53..816d25bcd93 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs
index ae2f6131b5b..6a5a77f8690 100644
--- a/clippy_lints/src/let_underscore.rs
+++ b/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,48 @@ 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;
+    ///     //               ^ dropped here
+    ///     /* more code */
+    /// }
+    /// ```
+    ///
+    /// Good:
+    /// ```rust,ignore
+    /// {
+    ///     let _droppable = Droppable;
+    ///     /* more code */
+    ///     // dropped at end of scope
+    /// }
+    /// ```
+    pub LET_UNDERSCORE_DROP,
+    pedantic,
+    "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 +125,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 +144,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,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 1f4bed92e69..20b38cbb6d0 100644
--- a/clippy_lints/src/lib.rs
+++ b/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,
@@ -1240,6 +1241,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&infinite_iter::MAYBE_INFINITE_ITER),
         LintId::of(&items_after_statements::ITEMS_AFTER_STATEMENTS),
         LintId::of(&large_stack_arrays::LARGE_STACK_ARRAYS),
+        LintId::of(&let_underscore::LET_UNDERSCORE_DROP),
         LintId::of(&literal_representation::LARGE_DIGIT_GROUPS),
         LintId::of(&literal_representation::UNREADABLE_LITERAL),
         LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index bc0a0ad2b17..4f1b56ed9be 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1118,6 +1118,13 @@ vec![
         module: "returns",
     },
     Lint {
+        name: "let_underscore_drop",
+        group: "pedantic",
+        desc: "non-binding let on a type that implements `Drop`",
+        deprecation: None,
+        module: "let_underscore",
+    },
+    Lint {
         name: "let_underscore_lock",
         group: "correctness",
         desc: "non-binding let on a synchronization lock",
diff --git a/tests/ui/filter_methods.rs b/tests/ui/filter_methods.rs
index ef434245fd7..51450241619 100644
--- a/tests/ui/filter_methods.rs
+++ b/tests/ui/filter_methods.rs
@@ -1,4 +1,5 @@
 #![warn(clippy::all, clippy::pedantic)]
+#![allow(clippy::clippy::let_underscore_drop)]
 #![allow(clippy::missing_docs_in_private_items)]
 
 fn main() {
diff --git a/tests/ui/filter_methods.stderr b/tests/ui/filter_methods.stderr
index 91718dd1175..11922681379 100644
--- a/tests/ui/filter_methods.stderr
+++ b/tests/ui/filter_methods.stderr
@@ -1,5 +1,5 @@
 error: called `filter(..).map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:5:21
+  --> $DIR/filter_methods.rs:6:21
    |
 LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,7 +8,7 @@ LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x *
    = help: this is more succinctly expressed by calling `.filter_map(..)` instead
 
 error: called `filter(..).flat_map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:7:21
+  --> $DIR/filter_methods.rs:8:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
    |  _____________________^
@@ -20,7 +20,7 @@ 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: called `filter_map(..).flat_map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:13:21
+  --> $DIR/filter_methods.rs:14:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
    |  _____________________^
@@ -32,7 +32,7 @@ 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: called `filter_map(..).map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:19:21
+  --> $DIR/filter_methods.rs:20:21
    |
 LL |       let _: Vec<_> = vec![5_i8; 6]
    |  _____________________^
diff --git a/tests/ui/let_underscore_drop.rs b/tests/ui/let_underscore_drop.rs
new file mode 100644
index 00000000000..98593edb9c5
--- /dev/null
+++ b/tests/ui/let_underscore_drop.rs
@@ -0,0 +1,19 @@
+#![warn(clippy::let_underscore_drop)]
+
+struct Droppable;
+
+impl Drop for Droppable {
+    fn drop(&mut self) {}
+}
+
+fn main() {
+    let unit = ();
+    let boxed = Box::new(());
+    let droppable = Droppable;
+    let optional = Some(Droppable);
+
+    let _ = ();
+    let _ = Box::new(());
+    let _ = Droppable;
+    let _ = Some(Droppable);
+}
diff --git a/tests/ui/let_underscore_drop.stderr b/tests/ui/let_underscore_drop.stderr
new file mode 100644
index 00000000000..66069e0c5e1
--- /dev/null
+++ b/tests/ui/let_underscore_drop.stderr
@@ -0,0 +1,27 @@
+error: non-binding `let` on a type that implements `Drop`
+  --> $DIR/let_underscore_drop.rs:16:5
+   |
+LL |     let _ = Box::new(());
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = 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: non-binding `let` on a type that implements `Drop`
+  --> $DIR/let_underscore_drop.rs:17:5
+   |
+LL |     let _ = Droppable;
+   |     ^^^^^^^^^^^^^^^^^^
+   |
+   = 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/let_underscore_drop.rs:18:5
+   |
+LL |     let _ = Some(Droppable);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui/map_clone.fixed b/tests/ui/map_clone.fixed
index 6e3a8e67e81..ce92b3c0c30 100644
--- a/tests/ui/map_clone.fixed
+++ b/tests/ui/map_clone.fixed
@@ -2,6 +2,7 @@
 #![warn(clippy::all, clippy::pedantic)]
 #![allow(clippy::iter_cloned_collect)]
 #![allow(clippy::clone_on_copy, clippy::redundant_clone)]
+#![allow(clippy::let_underscore_drop)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::redundant_closure_for_method_calls)]
 #![allow(clippy::many_single_char_names)]
diff --git a/tests/ui/map_clone.rs b/tests/ui/map_clone.rs
index 6fd395710d4..324c776c3c9 100644
--- a/tests/ui/map_clone.rs
+++ b/tests/ui/map_clone.rs
@@ -2,6 +2,7 @@
 #![warn(clippy::all, clippy::pedantic)]
 #![allow(clippy::iter_cloned_collect)]
 #![allow(clippy::clone_on_copy, clippy::redundant_clone)]
+#![allow(clippy::let_underscore_drop)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::redundant_closure_for_method_calls)]
 #![allow(clippy::many_single_char_names)]
diff --git a/tests/ui/map_clone.stderr b/tests/ui/map_clone.stderr
index 4f43cff5024..d84a5bf8d4d 100644
--- a/tests/ui/map_clone.stderr
+++ b/tests/ui/map_clone.stderr
@@ -1,5 +1,5 @@
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:10:22
+  --> $DIR/map_clone.rs:11:22
    |
 LL |     let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![5_i8; 6].iter().copied()`
@@ -7,31 +7,31 @@ LL |     let _: Vec<i8> = vec![5_i8; 6].iter().map(|x| *x).collect();
    = note: `-D clippy::map-clone` implied by `-D warnings`
 
 error: you are using an explicit closure for cloning elements
-  --> $DIR/map_clone.rs:11:26
+  --> $DIR/map_clone.rs:12:26
    |
 LL |     let _: Vec<String> = vec![String::new()].iter().map(|x| x.clone()).collect();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `vec![String::new()].iter().cloned()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:12:23
+  --> $DIR/map_clone.rs:13:23
    |
 LL |     let _: Vec<u32> = vec![42, 43].iter().map(|&x| x).collect();
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `vec![42, 43].iter().copied()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:14:26
+  --> $DIR/map_clone.rs:15:26
    |
 LL |     let _: Option<u64> = Some(&16).map(|b| *b);
    |                          ^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&16).copied()`
 
 error: you are using an explicit closure for copying elements
-  --> $DIR/map_clone.rs:15:25
+  --> $DIR/map_clone.rs:16:25
    |
 LL |     let _: Option<u8> = Some(&1).map(|x| x.clone());
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `Some(&1).copied()`
 
 error: you are needlessly cloning iterator elements
-  --> $DIR/map_clone.rs:26:29
+  --> $DIR/map_clone.rs:27:29
    |
 LL |     let _ = std::env::args().map(|v| v.clone());
    |                             ^^^^^^^^^^^^^^^^^^^ help: remove the `map` call
diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed
index a5fdf7df613..a7ab5a12cb7 100644
--- a/tests/ui/map_flatten.fixed
+++ b/tests/ui/map_flatten.fixed
@@ -1,6 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::all, clippy::pedantic)]
+#![allow(clippy::let_underscore_drop)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::map_identity)]
 
diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs
index abbc4e16e56..e364a05f376 100644
--- a/tests/ui/map_flatten.rs
+++ b/tests/ui/map_flatten.rs
@@ -1,6 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::all, clippy::pedantic)]
+#![allow(clippy::let_underscore_drop)]
 #![allow(clippy::missing_docs_in_private_items)]
 #![allow(clippy::map_identity)]
 
diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr
index b6479cd69ea..d4e27f9aa07 100644
--- a/tests/ui/map_flatten.stderr
+++ b/tests/ui/map_flatten.stderr
@@ -1,5 +1,5 @@
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:14:46
+  --> $DIR/map_flatten.rs:15:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)`
@@ -7,31 +7,31 @@ LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll
    = note: `-D clippy::map-flatten` implied by `-D warnings`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:15:46
+  --> $DIR/map_flatten.rs:16:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:16:46
+  --> $DIR/map_flatten.rs:17:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:17:46
+  --> $DIR/map_flatten.rs:18:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))`
 
 error: called `map(..).flatten()` on an `Iterator`
-  --> $DIR/map_flatten.rs:20:46
+  --> $DIR/map_flatten.rs:21:46
    |
 LL |     let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
    |                                              ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)`
 
 error: called `map(..).flatten()` on an `Option`
-  --> $DIR/map_flatten.rs:23:39
+  --> $DIR/map_flatten.rs:24:39
    |
 LL |     let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
    |                                       ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`