From fe06d5b69552cafa80d9d302e31fea45b1d92eca Mon Sep 17 00:00:00 2001 From: Mikhail Babenko Date: Sat, 16 Nov 2019 17:55:00 +0300 Subject: [PATCH] implemented `let_underscore` lint actually add files update lints change to pedantic --- CHANGELOG.md | 1 + README.md | 2 +- clippy_lints/src/functions.rs | 6 +- clippy_lints/src/let_underscore.rs | 62 ++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ clippy_lints/src/utils/mod.rs | 24 +++++++- src/lintlist/mod.rs | 9 ++- tests/ui/let_underscore.rs | 84 +++++++++++++++++++++++++++ tests/ui/let_underscore.stderr | 91 ++++++++++++++++++++++++++++++ 9 files changed, 277 insertions(+), 6 deletions(-) create mode 100644 clippy_lints/src/let_underscore.rs create mode 100644 tests/ui/let_underscore.rs create mode 100644 tests/ui/let_underscore.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index a9448a57f7fd..2afc9d1e3e75 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 97a7c97b49a2..6133fa4c3a57 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 07a4333df0dd..41c0e1a1d13b 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,7 +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, - must_use_attr, is_must_use_ty, + 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}; 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 d14f946c8eb8..f2c3b810b784 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -221,6 +221,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; @@ -553,6 +554,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, @@ -969,6 +971,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), @@ -981,6 +984,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 eeb9696a34ab..5a4f14be5627 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, Attribute}; +use syntax::ast::{self, Attribute, LitKind}; use syntax::attr; use syntax::source_map::{Span, DUMMY_SP}; use syntax::symbol::{kw, Symbol}; @@ -1283,3 +1283,25 @@ pub fn is_must_use_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> boo } } +// 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 1f1c24b2c303..6afe4da8e6ac 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..e8f541ef511d --- /dev/null +++ b/tests/ui/let_underscore.stderr @@ -0,0 +1,91 @@ +error: non-binding let on a result of a #[must_use] function + --> $DIR/let_underscore.rs:46: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:47: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:51: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:52: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:54: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:55: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:57: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:59: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:63: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:65: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:67:5 + | +LL | let _ = a; + | ^^^^^^^^^^ + | + = help: consider explicitly using expression value + +error: aborting due to 11 previous errors +