Auto merge of #4823 - Areredify:must_use_res, r=flip1995

bors · bors · commit b38b026a989a · 2019-12-23T06:16:37.000Z
Add `let_underscore_must_use` lint changelog: closes #4812 , added a new `let_underscore_must_use` lint, moved `is_must_use_ty` to utils, added `is_must_use_fn` util function
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1064,6 +1064,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_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
 [`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
 [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
diff --git a/README.md b/README.md
@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 339 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 340 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 
diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs
@@ -1,6 +1,7 @@
 use crate::utils::{
-    attrs::is_proc_macro, iter_input_pats, match_def_path, qpath_res, return_ty, snippet, snippet_opt,
-    span_help_and_lint, span_lint, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function,
+    attrs::is_proc_macro, is_must_use_ty, iter_input_pats, match_def_path, must_use_attr, qpath_res, return_ty,
+    snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_then, trait_ref_of_method,
+    type_is_unsafe_function,
 };
 use matches::matches;
 use rustc::hir::{self, def::Res, def_id::DefId, intravisit};
@@ -466,15 +467,6 @@ fn check_must_use_candidate<'a, 'tcx>(
     });
 }
 
-fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
-    attrs.iter().find(|attr| {
-        attr.ident().map_or(false, |ident| {
-            let ident: &str = &ident.as_str();
-            "must_use" == ident
-        })
-    })
-}
-
 fn returns_unit(decl: &hir::FnDecl) -> bool {
     match decl.output {
         hir::FunctionRetTy::DefaultReturn(_) => true,
@@ -486,41 +478,6 @@ fn returns_unit(decl: &hir::FnDecl) -> bool {
     }
 }
 
-fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
-    use ty::TyKind::*;
-    match ty.kind {
-        Adt(ref adt, _) => must_use_attr(&cx.tcx.get_attrs(adt.did)).is_some(),
-        Foreign(ref did) => must_use_attr(&cx.tcx.get_attrs(*did)).is_some(),
-        Slice(ref ty) | Array(ref ty, _) | RawPtr(ty::TypeAndMut { ref ty, .. }) | Ref(_, 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)
-        },
-        Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
-        Opaque(ref def_id, _) => {
-            for (predicate, _) in cx.tcx.predicates_of(*def_id).predicates {
-                if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
-                    if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() {
-                        return true;
-                    }
-                }
-            }
-            false
-        },
-        Dynamic(binder, _) => {
-            for predicate in binder.skip_binder().iter() {
-                if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
-                    if must_use_attr(&cx.tcx.get_attrs(trait_ref.def_id)).is_some() {
-                        return true;
-                    }
-                }
-            }
-            false
-        },
-        _ => false,
-    }
-}
-
 fn has_mutable_arg(cx: &LateContext<'_, '_>, body: &hir::Body<'_>) -> bool {
     let mut tys = FxHashSet::default();
     body.params.iter().any(|param| is_mutable_pat(cx, &param.pat, &mut tys))
diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs
@@ -0,0 +1,62 @@
+use if_chain::if_chain;
+use rustc::declare_lint_pass;
+use rustc::hir::*;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc_session::declare_tool_lint;
+
+use crate::utils::{is_must_use_func_call, is_must_use_ty, span_help_and_lint};
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for `let _ = <expr>`
+    /// where expr is #[must_use]
+    ///
+    /// **Why is this bad?** It's better to explicitly
+    /// handle the value of a #[must_use] expr
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// fn f() -> Result<u32, u32> {
+    ///     Ok(0)
+    /// }
+    ///
+    /// let _ = f();
+    /// // is_ok() is marked #[must_use]
+    /// let _ = f().is_ok();
+    /// ```
+    pub LET_UNDERSCORE_MUST_USE,
+    restriction,
+    "non-binding let on a #[must_use] expression"
+}
+
+declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore {
+    fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt) {
+        if_chain! {
+            if let StmtKind::Local(ref local) = stmt.kind;
+            if let PatKind::Wild = local.pat.kind;
+            if let Some(ref init) = local.init;
+            then {
+                if is_must_use_ty(cx, cx.tables.expr_ty(init)) {
+                   span_help_and_lint(
+                        cx,
+                        LET_UNDERSCORE_MUST_USE,
+                        stmt.span,
+                        "non-binding let on an expression with #[must_use] type",
+                        "consider explicitly using expression value"
+                    )
+                } else if is_must_use_func_call(cx, init) {
+                    span_help_and_lint(
+                        cx,
+                        LET_UNDERSCORE_MUST_USE,
+                        stmt.span,
+                        "non-binding let on a result of a #[must_use] function",
+                        "consider explicitly using function result"
+                    )
+                }
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -220,6 +220,7 @@ pub mod large_enum_variant;
 pub mod large_stack_arrays;
 pub mod len_zero;
 pub mod let_if_seq;
+pub mod let_underscore;
 pub mod lifetimes;
 pub mod literal_representation;
 pub mod loops;
@@ -555,6 +556,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         &len_zero::LEN_WITHOUT_IS_EMPTY,
         &len_zero::LEN_ZERO,
         &let_if_seq::USELESS_LET_IF_SEQ,
+        &let_underscore::LET_UNDERSCORE_MUST_USE,
         &lifetimes::EXTRA_UNUSED_LIFETIMES,
         &lifetimes::NEEDLESS_LIFETIMES,
         &literal_representation::DECIMAL_LITERAL_REPRESENTATION,
@@ -970,6 +972,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
     store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
     store.register_early_pass(|| box as_conversions::AsConversions);
     store.register_early_pass(|| box utils::internal_lints::ProduceIce);
+    store.register_late_pass(|| box let_underscore::LetUnderscore);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -982,6 +985,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
         LintId::of(&indexing_slicing::INDEXING_SLICING),
         LintId::of(&inherent_impl::MULTIPLE_INHERENT_IMPL),
         LintId::of(&integer_division::INTEGER_DIVISION),
+        LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE),
         LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION),
         LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
         LintId::of(&mem_forget::MEM_FORGET),
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
@@ -41,7 +41,7 @@ use rustc::ty::{
 };
 use rustc_errors::Applicability;
 use smallvec::SmallVec;
-use syntax::ast::{self, LitKind};
+use syntax::ast::{self, Attribute, LitKind};
 use syntax::attr;
 use syntax::source_map::{Span, DUMMY_SP};
 use syntax::symbol::{kw, Symbol};
@@ -1233,3 +1233,71 @@ pub fn parent_node_is_if_expr<'a, 'b>(expr: &Expr, cx: &LateContext<'a, 'b>) ->
         _ => false,
     }
 }
+
+pub fn must_use_attr(attrs: &[Attribute]) -> Option<&Attribute> {
+    attrs.iter().find(|attr| {
+        attr.ident().map_or(false, |ident| {
+            let ident: &str = &ident.as_str();
+            "must_use" == ident
+        })
+    })
+}
+
+// Returns whether the type has #[must_use] attribute
+pub fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
+    use ty::TyKind::*;
+    match ty.kind {
+        Adt(ref adt, _) => must_use_attr(&cx.tcx.get_attrs(adt.did)).is_some(),
+        Foreign(ref did) => must_use_attr(&cx.tcx.get_attrs(*did)).is_some(),
+        Slice(ref ty) | Array(ref ty, _) | RawPtr(ty::TypeAndMut { ref ty, .. }) | Ref(_, 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)
+        },
+        Tuple(ref substs) => substs.types().any(|ty| is_must_use_ty(cx, ty)),
+        Opaque(ref def_id, _) => {
+            for (predicate, _) in cx.tcx.predicates_of(*def_id).predicates {
+                if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
+                    if must_use_attr(&cx.tcx.get_attrs(poly_trait_predicate.skip_binder().trait_ref.def_id)).is_some() {
+                        return true;
+                    }
+                }
+            }
+            false
+        },
+        Dynamic(binder, _) => {
+            for predicate in binder.skip_binder().iter() {
+                if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
+                    if must_use_attr(&cx.tcx.get_attrs(trait_ref.def_id)).is_some() {
+                        return true;
+                    }
+                }
+            }
+            false
+        },
+        _ => false,
+    }
+}
+
+// check if expr is calling method or function with #[must_use] attribyte
+pub fn is_must_use_func_call(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
+    let did = match expr.kind {
+        ExprKind::Call(ref path, _) => if_chain! {
+            if let ExprKind::Path(ref qpath) = path.kind;
+            if let def::Res::Def(_, did) = cx.tables.qpath_res(qpath, path.hir_id);
+            then {
+                Some(did)
+            } else {
+                None
+            }
+        },
+        ExprKind::MethodCall(_, _, _) => cx.tables.type_dependent_def_id(expr.hir_id),
+        _ => None,
+    };
+
+    if let Some(did) = did {
+        must_use_attr(&cx.tcx.get_attrs(did)).is_some()
+    } else {
+        false
+    }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 339] = [
+pub const ALL_LINTS: [Lint; 340] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -938,6 +938,13 @@ pub const ALL_LINTS: [Lint; 339] = [
         deprecation: None,
         module: "returns",
     },
+    Lint {
+        name: "let_underscore_must_use",
+        group: "restriction",
+        desc: "non-binding let on a #[must_use] expression",
+        deprecation: None,
+        module: "let_underscore",
+    },
     Lint {
         name: "let_unit_value",
         group: "style",
diff --git a/tests/ui/let_underscore.rs b/tests/ui/let_underscore.rs
@@ -0,0 +1,84 @@
+#![warn(clippy::let_underscore_must_use)]
+
+#[must_use]
+fn f() -> u32 {
+    0
+}
+
+fn g() -> Result<u32, u32> {
+    Ok(0)
+}
+
+#[must_use]
+fn l<T>(x: T) -> T {
+    x
+}
+
+fn h() -> u32 {
+    0
+}
+
+struct S {}
+
+impl S {
+    #[must_use]
+    pub fn f(&self) -> u32 {
+        0
+    }
+
+    pub fn g(&self) -> Result<u32, u32> {
+        Ok(0)
+    }
+
+    fn k(&self) -> u32 {
+        0
+    }
+
+    #[must_use]
+    fn h() -> u32 {
+        0
+    }
+
+    fn p() -> Result<u32, u32> {
+        Ok(0)
+    }
+}
+
+trait Trait {
+    #[must_use]
+    fn a() -> u32;
+}
+
+impl Trait for S {
+    fn a() -> u32 {
+        0
+    }
+}
+
+fn main() {
+    let _ = f();
+    let _ = g();
+    let _ = h();
+    let _ = l(0_u32);
+
+    let s = S {};
+
+    let _ = s.f();
+    let _ = s.g();
+    let _ = s.k();
+
+    let _ = S::h();
+    let _ = S::p();
+
+    let _ = S::a();
+
+    let _ = if true { Ok(()) } else { Err(()) };
+
+    let a = Result::<(), ()>::Ok(());
+
+    let _ = a.is_ok();
+
+    let _ = a.map(|_| ());
+
+    let _ = a;
+}
diff --git a/tests/ui/let_underscore.stderr b/tests/ui/let_underscore.stderr
