diff --git a/CHANGELOG.md b/CHANGELOG.md index 962f9067a4e4..e485c2c979e6 100644 --- 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 index 280b25d7c53a..46fbc9dc9a46 100644 --- 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 index 6191c41bb449..de14f2327db7 100644 --- 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, ¶m.pat, &mut tys)) diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs new file mode 100644 index 000000000000..2b59b7c6b4aa --- /dev/null +++ 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 _ = ` + /// 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 { + /// 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 index d4e86cbfe57a..2e5ce50bf34a 100644 --- 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 index 552a78d2861e..ba8ef2bb0f33 100644 --- 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 index f3012fba2da0..913e941660e3 100644 --- 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 new file mode 100644 index 000000000000..1f0dbcee42ad --- /dev/null +++ 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 { + Ok(0) +} + +#[must_use] +fn l(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 { + Ok(0) + } + + fn k(&self) -> u32 { + 0 + } + + #[must_use] + fn h() -> u32 { + 0 + } + + fn p() -> Result { + 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 new file mode 100644 index 000000000000..da007d3b0835 --- /dev/null +++ b/tests/ui/let_underscore.stderr @@ -0,0 +1,99 @@ +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:59:5 + | +LL | let _ = f(); + | ^^^^^^^^^^^^ + | + = note: `-D clippy::let-underscore-must-use` implied by `-D warnings` + = help: consider explicitly using function result + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:60:5 + | +LL | let _ = g(); + | ^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:62:5 + | +LL | let _ = l(0_u32); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider explicitly using function result + +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:66:5 + | +LL | let _ = s.f(); + | ^^^^^^^^^^^^^^ + | + = help: consider explicitly using function result + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:67:5 + | +LL | let _ = s.g(); + | ^^^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:70:5 + | +LL | let _ = S::h(); + | ^^^^^^^^^^^^^^^ + | + = help: consider explicitly using function result + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:71:5 + | +LL | let _ = S::p(); + | ^^^^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:73:5 + | +LL | let _ = S::a(); + | ^^^^^^^^^^^^^^^ + | + = help: consider explicitly using function result + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:75:5 + | +LL | let _ = if true { Ok(()) } else { Err(()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:79:5 + | +LL | let _ = a.is_ok(); + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider explicitly using function result + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:81:5 + | +LL | let _ = a.map(|_| ()); + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: non-binding let on an expression with #[must_use] type + --> $DIR/let_underscore.rs:83:5 + | +LL | let _ = a; + | ^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: aborting due to 12 previous errors +