Add `let_underscore_must_use` lint.

Similar to `let_underscore_drop`, this lint checks for statements similar
to `let _ = foo`, where `foo` is an expression marked `must_use`.

diff --git a/compiler/rustc_lint/src/let_underscore.rs b/compiler/rustc_lint/src/let_underscore.rs
index 81906a24d902957a79147e382e3a64c0f0c92c8a..40e6d12abf91066771f978d56ab647bfdf77ff28 100644 (file)
--- a/compiler/rustc_lint/src/let_underscore.rs
+++ b/compiler/rustc_lint/src/let_underscore.rs
@@ -1,6 +1,6 @@
 use crate::{LateContext, LateLintPass, LintContext};
 use rustc_hir as hir;
-use rustc_middle::ty::{self, subst::GenericArgKind};
+use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
 use rustc_span::Symbol;
 
 declare_lint! {
@@ -83,7 +83,32 @@
     "non-binding let on a synchronization lock"
 }
 
-declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK]);
+declare_lint! {
+    /// The `let_underscore_must_use` lint checks for statements which don't bind
+    /// a `must_use` expression to anything, causing the lock to be released
+    /// immediately instead of at end of scope, which is typically incorrect.
+    ///
+    /// ### Example
+    /// ```rust
+    /// #[must_use]
+    /// struct SomeStruct;
+    ///
+    /// fn main() {
+    ///     // SomeStuct is dropped immediately instead of at end of scope.
+    ///     let _ = SomeStruct;
+    /// }
+    /// ```
+    /// ### Explanation
+    ///
+    /// Statements which assign an expression to an underscore causes the
+    /// expression to immediately drop. Usually, it's better to explicitly handle
+    /// the `must_use` expression.
+    pub LET_UNDERSCORE_MUST_USE,
+    Warn,
+    "non-binding let on a expression marked `must_use`"
+}
+
+declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK, LET_UNDERSCORE_MUST_USE]);
 
 const SYNC_GUARD_PATHS: [&[&str]; 5] = [
     &["std", "sync", "mutex", "MutexGuard"],
@@ -114,6 +139,8 @@ fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) {
 
                 GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
             });
+            let is_must_use_ty = is_must_use_ty(cx, cx.typeck_results().expr_ty(init));
+            let is_must_use_func_call = is_must_use_func_call(cx, init);
             if is_sync_lock {
                 cx.struct_span_lint(LET_UNDERSCORE_LOCK, local.span, |lint| {
                     lint.build("non-binding let on a synchronization lock")
@@ -121,6 +148,13 @@ fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) {
                         .help("consider explicitly droping with `std::mem::drop`")
                         .emit();
                 })
+            } else if is_must_use_ty || is_must_use_func_call {
+                cx.struct_span_lint(LET_UNDERSCORE_MUST_USE, local.span, |lint| {
+                    lint.build("non-binding let on a expression marked `must_use`")
+                        .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`")
@@ -130,5 +164,73 @@ fn check_local(&mut self, cx: &LateContext<'_>, local: &hir::Local<'_>) {
                 })
             }
         }
+
+        fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+            match ty.kind() {
+                ty::Adt(adt, _) => has_must_use_attr(cx, adt.did()),
+                ty::Foreign(ref did) => has_must_use_attr(cx, *did),
+                ty::Slice(ty)
+                | ty::Array(ty, _)
+                | ty::RawPtr(ty::TypeAndMut { ty, .. })
+                | ty::Ref(_, ty, _) => {
+                    // for the Array case we don't need to care for the len == 0 case
+                    // because we don't want to lint functions returning empty arrays
+                    is_must_use_ty(cx, *ty)
+                }
+                ty::Tuple(substs) => substs.iter().any(|ty| is_must_use_ty(cx, ty)),
+                ty::Opaque(ref def_id, _) => {
+                    for (predicate, _) in cx.tcx.explicit_item_bounds(*def_id) {
+                        if let ty::PredicateKind::Trait(trait_predicate) =
+                            predicate.kind().skip_binder()
+                        {
+                            if has_must_use_attr(cx, trait_predicate.trait_ref.def_id) {
+                                return true;
+                            }
+                        }
+                    }
+                    false
+                }
+                ty::Dynamic(binder, _) => {
+                    for predicate in binder.iter() {
+                        if let ty::ExistentialPredicate::Trait(ref trait_ref) =
+                            predicate.skip_binder()
+                        {
+                            if has_must_use_attr(cx, trait_ref.def_id) {
+                                return true;
+                            }
+                        }
+                    }
+                    false
+                }
+                _ => false,
+            }
+        }
+
+        // check if expr is calling method or function with #[must_use] attribute
+        fn is_must_use_func_call(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
+            let did = match expr.kind {
+                hir::ExprKind::Call(path, _) if let hir::ExprKind::Path(ref qpath) = path.kind => {
+                    if let hir::def::Res::Def(_, did) = cx.qpath_res(qpath, path.hir_id) {
+                        Some(did)
+                    } else {
+                        None
+                    }
+                },
+                hir::ExprKind::MethodCall(..) => {
+                    cx.typeck_results().type_dependent_def_id(expr.hir_id)
+                }
+                _ => None,
+            };
+
+            did.map_or(false, |did| has_must_use_attr(cx, did))
+        }
+
+        // returns true if DefId contains a `#[must_use]` attribute
+        fn has_must_use_attr(cx: &LateContext<'_>, did: hir::def_id::DefId) -> bool {
+            cx.tcx
+                .get_attrs(did, rustc_span::sym::must_use)
+                .find(|a| a.has_name(rustc_span::sym::must_use))
+                .is_some()
+        }
     }
 }

diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
index 79661c0fefe8d4305a84da6c9f1664a9bbdc0650..4359a54b698dc9dffe6e494bbbe12800a2c76b99 100644 (file)
--- a/compiler/rustc_lint/src/lib.rs
+++ b/compiler/rustc_lint/src/lib.rs
@@ -317,7 +317,12 @@ macro_rules! register_passes {
         REDUNDANT_SEMICOLONS
     );
 
-    add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK);
+    add_lint_group!(
+        "let_underscore",
+        LET_UNDERSCORE_DROP,
+        LET_UNDERSCORE_LOCK,
+        LET_UNDERSCORE_MUST_USE
+    );
 
     add_lint_group!(
         "rust_2018_idioms",

diff --git a/src/test/ui/let_underscore_must_use.rs b/src/test/ui/let_underscore_must_use.rs
new file mode 100644 (file)
index 0000000..6a78e3f
--- /dev/null
+++ b/src/test/ui/let_underscore_must_use.rs
@@ -0,0 +1,12 @@
+// run-pass
+
+#[must_use]
+struct MustUseType;
+
+#[must_use]
+fn must_use_function() -> () {}
+
+fn main() {
+    let _ = MustUseType; //~WARNING non-binding let on a expression marked `must_use`
+    let _ = must_use_function(); //~WARNING non-binding let on a expression marked `must_use`
+}

diff --git a/src/test/ui/let_underscore_must_use.stderr b/src/test/ui/let_underscore_must_use.stderr
new file mode 100644 (file)
index 0000000..0b84038
--- /dev/null
+++ b/src/test/ui/let_underscore_must_use.stderr
@@ -0,0 +1,21 @@
+warning: non-binding let on a expression marked `must_use`
+  --> $DIR/let_underscore_must_use.rs:10:5
+   |
+LL |     let _ = MustUseType;
+   |     ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(let_underscore_must_use)]` on by default
+   = help: consider binding to an unused variable
+   = help: consider explicitly droping with `std::mem::drop`
+
+warning: non-binding let on a expression marked `must_use`
+  --> $DIR/let_underscore_must_use.rs:11:5
+   |
+LL |     let _ = must_use_function();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider binding to an unused variable
+   = help: consider explicitly droping with `std::mem::drop`
+
+warning: 2 warnings emitted
+