From d1f862171150f91c114eb126c559e90873c8dc00 Mon Sep 17 00:00:00 2001 From: Mikhail Babenko Date: Mon, 27 Jan 2020 17:34:30 +0300 Subject: [PATCH] add lint --- CHANGELOG.md | 1 + clippy_lints/src/let_underscore.rs | 79 +++++++++++++++++++++++------- clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/utils/paths.rs | 3 ++ src/lintlist/mod.rs | 7 +++ tests/ui/let_underscore.rs | 7 +++ tests/ui/let_underscore.stderr | 27 +++++++++- 7 files changed, 108 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5dbbabb3c5..32427225769 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1153,6 +1153,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_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_lock [`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 diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index 2df3cccb83b..4d746596c37 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -4,7 +4,7 @@ use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use crate::utils::{is_must_use_func_call, is_must_use_ty, span_lint_and_help}; +use crate::utils::{is_must_use_func_call, is_must_use_ty, match_def_path, paths, span_lint_and_help}; declare_clippy_lint! { /// **What it does:** Checks for `let _ = ` @@ -30,7 +30,35 @@ declare_clippy_lint! { "non-binding let on a `#[must_use]` expression" } -declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE]); +declare_clippy_lint! { + /// **What it does:** Checks for `let _ = sync_primitive.lock()` + /// + /// **Why is this bad?** This statement locks the synchronization + /// primitive and immediately drops the lock, which is probably + /// not intended. To extend lock lifetime to the end of the scope, + /// use an underscore-prefixed name instead (i.e. _lock). + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// Bad: + /// ```rust,ignore + /// let _ = mutex.lock(); + /// ``` + /// + /// Good: + /// ```rust,ignore + /// let _lock = mutex.lock(); + /// ``` + pub LET_UNDERSCORE_LOCK, + correctness, + "non-binding let on a synchronization lock" +} + +declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK]); + +const LOCK_METHODS_PATHS: [&[&str]; 3] = [&paths::MUTEX_LOCK, &paths::RWLOCK_READ, &paths::RWLOCK_WRITE]; impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt<'_>) { @@ -43,22 +71,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { 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_lint_and_help( - 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_lint_and_help( - cx, - LET_UNDERSCORE_MUST_USE, - stmt.span, - "non-binding let on a result of a `#[must_use]` function", - "consider explicitly using function result" - ) + if_chain! { + if let ExprKind::MethodCall(_, _, _) = init.kind; + let method_did = cx.tables.type_dependent_def_id(init.hir_id).unwrap(); + if LOCK_METHODS_PATHS.iter().any(|path| match_def_path(cx, method_did, path)); + then { + span_lint_and_help( + cx, + LET_UNDERSCORE_LOCK, + stmt.span, + "non-binding let on an a synchronization lock", + "consider using an underscore-prefixed named binding" + ) + } else { + if is_must_use_ty(cx, cx.tables.expr_ty(init)) { + span_lint_and_help( + 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_lint_and_help( + 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 7008c6d6839..443a9c7e9d9 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -566,6 +566,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &len_zero::LEN_WITHOUT_IS_EMPTY, &len_zero::LEN_ZERO, &let_if_seq::USELESS_LET_IF_SEQ, + &let_underscore::LET_UNDERSCORE_LOCK, &let_underscore::LET_UNDERSCORE_MUST_USE, &lifetimes::EXTRA_UNUSED_LIFETIMES, &lifetimes::NEEDLESS_LIFETIMES, @@ -1171,6 +1172,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_ZERO), LintId::of(&let_if_seq::USELESS_LET_IF_SEQ), + LintId::of(&let_underscore::LET_UNDERSCORE_LOCK), LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES), LintId::of(&lifetimes::NEEDLESS_LIFETIMES), LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING), @@ -1556,6 +1558,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&infinite_iter::INFINITE_ITER), LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY), LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY), + LintId::of(&let_underscore::LET_UNDERSCORE_LOCK), LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES), LintId::of(&loops::FOR_LOOP_OVER_OPTION), LintId::of(&loops::FOR_LOOP_OVER_RESULT), diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 7980a02b3ba..ff8acb321a4 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -58,6 +58,7 @@ pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"]; pub const MEM_UNINITIALIZED: [&str; 3] = ["core", "mem", "uninitialized"]; pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"]; pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"]; +pub const MUTEX_LOCK: [&str; 5] = ["std", "sync", "mutex", "Mutex", "lock"]; pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"]; pub const OPS_MODULE: [&str; 2] = ["core", "ops"]; pub const OPTION: [&str; 3] = ["core", "option", "Option"]; @@ -100,6 +101,8 @@ pub const REPEAT: [&str; 3] = ["core", "iter", "repeat"]; pub const RESULT: [&str; 3] = ["core", "result", "Result"]; pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"]; pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"]; +pub const RWLOCK_READ: [&str; 5] = ["std", "sync", "rwlock", "RwLock", "read"]; +pub const RWLOCK_WRITE: [&str; 5] = ["std", "sync", "rwlock", "RwLock", "write"]; pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"]; pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "", "into_vec"]; pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b762c8d1395..1f7d450ab66 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -959,6 +959,13 @@ pub const ALL_LINTS: [Lint; 350] = [ deprecation: None, module: "returns", }, + Lint { + name: "let_underscore_lock", + group: "correctness", + desc: "non-binding let on a synchronization lock", + deprecation: None, + module: "let_underscore", + }, Lint { name: "let_underscore_must_use", group: "restriction", diff --git a/tests/ui/let_underscore.rs b/tests/ui/let_underscore.rs index 7f481542fa7..cfe207251f4 100644 --- a/tests/ui/let_underscore.rs +++ b/tests/ui/let_underscore.rs @@ -88,4 +88,11 @@ fn main() { let _ = a.map(|_| ()); let _ = a; + + let m = std::sync::Mutex::new(()); + let rw = std::sync::RwLock::new(()); + + let _ = m.lock(); + let _ = rw.read(); + let _ = rw.write(); } diff --git a/tests/ui/let_underscore.stderr b/tests/ui/let_underscore.stderr index e7d5f712bec..bcd560fe493 100644 --- a/tests/ui/let_underscore.stderr +++ b/tests/ui/let_underscore.stderr @@ -95,5 +95,30 @@ LL | let _ = a; | = help: consider explicitly using expression value -error: aborting due to 12 previous errors +error: non-binding let on an a synchronization lock + --> $DIR/let_underscore.rs:95:5 + | +LL | let _ = m.lock(); + | ^^^^^^^^^^^^^^^^^ + | + = note: `#[deny(clippy::let_underscore_lock)]` on by default + = help: consider using an underscore-prefixed named binding + +error: non-binding let on an a synchronization lock + --> $DIR/let_underscore.rs:96:5 + | +LL | let _ = rw.read(); + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding + +error: non-binding let on an a synchronization lock + --> $DIR/let_underscore.rs:97:5 + | +LL | let _ = rw.write(); + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding + +error: aborting due to 15 previous errors