Add `let_underscore_lock` lint.

Similar to `let_underscore_drop`, this lint checks for statements similar
to `let _ = foo`, where `foo` is a lock guard. These types of let
statements are especially problematic because the lock gets released
immediately, instead of at the end of the scope. This behavior is almost
always the wrong thing.

4 files changed, 91 insertions, 3 deletions

diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs
index 44242173c00..81906a24d90 100644
--- a/compiler/rustc_lint/src/let_underscore.rs
+++ b/compiler/rustc_lint/src/let_underscore.rs
@@ -1,5 +1,7 @@
 use crate::{LateContext, LateLintPass, LintContext};
 use rustc_hir as hir;
+use rustc_middle::ty::{self, subst::GenericArgKind};
+use rustc_span::Symbol;
 
 declare_lint! {
     /// The `let_underscore_drop` lint checks for statements which don't bind
@@ -43,7 +45,53 @@ declare_lint! {
     "non-binding let on a type that implements `Drop`"
 }
 
-declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP]);
+declare_lint! {
+    /// The `let_underscore_lock` lint checks for statements which don't bind
+    /// a mutex to anything, causing the lock to be released immediately instead
+    /// of at end of scope, which is typically incorrect.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::sync::{Arc, Mutex};
+    /// use std::thread;
+    /// let data = Arc::new(Mutex::new(0));
+    ///
+    /// thread::spawn(move || {
+    ///     // The lock is immediately released instead of at the end of the
+    ///     // scope, which is probably not intended.
+    ///     let _ = data.lock().unwrap();
+    ///     println!("doing some work");
+    ///     let mut lock = data.lock().unwrap();
+    ///     *lock += 1;
+    /// });
+    /// ```
+    /// ### Explanation
+    ///
+    /// Statements which assign an expression to an underscore causes the
+    /// expression to immediately drop instead of extending the expression's
+    /// lifetime to the end of the scope. This is usually unintended,
+    /// especially for types like `MutexGuard`, which are typically used to
+    /// lock a mutex for the duration of an entire scope.
+    ///
+    /// If you want to extend the expression's lifetime to the end of the scope,
+    /// assign an underscore-prefixed name (such as `_foo`) to the expression.
+    /// If you do actually want to drop the expression immediately, then
+    /// calling `std::mem::drop` on the expression is clearer and helps convey
+    /// intent.
+    pub LET_UNDERSCORE_LOCK,
+    Warn,
+    "non-binding let on a synchronization lock"
+}
+
+declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]);
+
+const SYNC_GUARD_PATHS: [&[&str]; 5] = [
+    &["std", "sync", "mutex", "MutexGuard"],
+    &["std", "sync", "rwlock", "RwLockReadGuard"],
+    &["std", "sync", "rwlock", "RwLockWriteGuard"],
+    &["parking_lot", "raw_mutex", "RawMutex"],
+    &["parking_lot", "raw_rwlock", "RawRwLock"],
+];
 
 impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
     fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) {
@@ -53,7 +101,27 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
         if let Some(init) = local.init {
             let init_ty = cx.typeck_results().expr_ty(init);
             let needs_drop = init_ty.needs_drop(cx.tcx, cx.param_env);
-            if needs_drop {
+            let is_sync_lock = init_ty.walk().any(|inner| match inner.unpack() {
+                GenericArgKind::Type(inner_ty) => {
+                    SYNC_GUARD_PATHS.iter().any(|guard_path| match inner_ty.kind() {
+                        ty::Adt(adt, _) => {
+                            let ty_path = cx.get_def_path(adt.did());
+                            guard_path.iter().map(|x| Symbol::intern(x)).eq(ty_path.iter().copied())
+                        }
+                        _ => false,
+                    })
+                }
+
+                GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
+            });
+            if is_sync_lock {
+                cx.struct_span_lint(LET_UNDERSCORE_LOCK, local.span, |lint| {
+                    lint.build("non-binding let on a synchronization lock")
+                        .help("consider binding to an unused variable")
+                        .help("consider explicitly droping with `std::mem::drop`")
+                        .emit();
+                })
+            } else if needs_drop {
                 cx.struct_span_lint(LET_UNDERSCORE_DROP, local.span, |lint| {
                     lint.build("non-binding let on a type that implements `Drop`")
                         .help("consider binding to an unused variable")
diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
index 55396b36dbc..79661c0fefe 100644
--- a/compiler/rustc_lint/src/lib.rs
+++ b/compiler/rustc_lint/src/lib.rs
@@ -317,7 +317,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
         REDUNDANT_SEMICOLONS
     );
 
-    add_lint_group!("let_underscore", LET_UNDERSCORE_DROP);
+    add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK);
 
     add_lint_group!(
         "rust_2018_idioms",
diff --git a/src/test/ui/let_underscore_lock.rs b/src/test/ui/let_underscore_lock.rs
new file mode 100644
index 00000000000..774b610db2f
--- /dev/null
+++ b/src/test/ui/let_underscore_lock.rs
@@ -0,0 +1,8 @@
+// run-pass
+
+use std::sync::{Arc, Mutex};
+
+fn main() {
+    let data = Arc::new(Mutex::new(0));
+    let _ = data.lock().unwrap(); //~WARNING non-binding let on a synchronization lock
+}
diff --git a/src/test/ui/let_underscore_lock.stderr b/src/test/ui/let_underscore_lock.stderr
new file mode 100644
index 00000000000..77379d8c3db
--- /dev/null
+++ b/src/test/ui/let_underscore_lock.stderr
@@ -0,0 +1,12 @@
+warning: non-binding let on a synchronization lock
+  --> $DIR/let_underscore_lock.rs:7:5
+   |
+LL |     let _ = data.lock().unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(let_underscore_lock)]` on by default
+   = help: consider binding to an unused variable
+   = help: consider explicitly droping with `std::mem::drop`
+
+warning: 1 warning emitted
+