From d1f862171150f91c114eb126c559e90873c8dc00 Mon Sep 17 00:00:00 2001 From: Mikhail Babenko Date: Mon, 27 Jan 2020 17:34:30 +0300 Subject: [PATCH 1/3] 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 d5dbbabb3..324272257 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 2df3cccb8..4d746596c 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 7008c6d68..443a9c7e9 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 7980a02b3..ff8acb321 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 b762c8d13..1f7d450ab 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 7f481542f..cfe207251 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 e7d5f712b..bcd560fe4 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 From 9b88a2b295e51803a74ad8bb852549693815e0c0 Mon Sep 17 00:00:00 2001 From: Mikhail Babenko Date: Tue, 28 Jan 2020 12:28:11 +0300 Subject: [PATCH 2/3] decouple 'let_underscore' tests --- README.md | 2 +- clippy_lints/src/let_underscore.rs | 2 +- src/lintlist/mod.rs | 2 +- tests/ui/let_underscore_lock.rs | 10 ++++ tests/ui/let_underscore_lock.stderr | 27 ++++++++++ ...derscore.rs => let_underscore_must_use.rs} | 7 --- ....stderr => let_underscore_must_use.stderr} | 51 +++++-------------- 7 files changed, 53 insertions(+), 48 deletions(-) create mode 100644 tests/ui/let_underscore_lock.rs create mode 100644 tests/ui/let_underscore_lock.stderr rename tests/ui/{let_underscore.rs => let_underscore_must_use.rs} (88%) rename tests/ui/{let_underscore.stderr => let_underscore_must_use.stderr} (66%) diff --git a/README.md b/README.md index 0cb14eda6..b68eb3ed7 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 350 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 351 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/let_underscore.rs b/clippy_lints/src/let_underscore.rs index 4d746596c..a43674316 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -80,7 +80,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { cx, LET_UNDERSCORE_LOCK, stmt.span, - "non-binding let on an a synchronization lock", + "non-binding let on a synchronization lock", "consider using an underscore-prefixed named binding" ) } else { diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 1f7d450ab..0c5b8146b 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; 350] = [ +pub const ALL_LINTS: [Lint; 351] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", diff --git a/tests/ui/let_underscore_lock.rs b/tests/ui/let_underscore_lock.rs new file mode 100644 index 000000000..f4a33c360 --- /dev/null +++ b/tests/ui/let_underscore_lock.rs @@ -0,0 +1,10 @@ +#![warn(clippy::let_underscore_lock)] + +fn main() { + 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_lock.stderr b/tests/ui/let_underscore_lock.stderr new file mode 100644 index 000000000..bd297f602 --- /dev/null +++ b/tests/ui/let_underscore_lock.stderr @@ -0,0 +1,27 @@ +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:7:5 + | +LL | let _ = m.lock(); + | ^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::let-underscore-lock` implied by `-D warnings` + = help: consider using an underscore-prefixed named binding + +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:8:5 + | +LL | let _ = rw.read(); + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding + +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:9:5 + | +LL | let _ = rw.write(); + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding + +error: aborting due to 3 previous errors + diff --git a/tests/ui/let_underscore.rs b/tests/ui/let_underscore_must_use.rs similarity index 88% rename from tests/ui/let_underscore.rs rename to tests/ui/let_underscore_must_use.rs index cfe207251..7f481542f 100644 --- a/tests/ui/let_underscore.rs +++ b/tests/ui/let_underscore_must_use.rs @@ -88,11 +88,4 @@ 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_must_use.stderr similarity index 66% rename from tests/ui/let_underscore.stderr rename to tests/ui/let_underscore_must_use.stderr index bcd560fe4..447f2419e 100644 --- a/tests/ui/let_underscore.stderr +++ b/tests/ui/let_underscore_must_use.stderr @@ -1,5 +1,5 @@ error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore.rs:66:5 + --> $DIR/let_underscore_must_use.rs:66:5 | LL | let _ = f(); | ^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | let _ = f(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore.rs:67:5 + --> $DIR/let_underscore_must_use.rs:67:5 | LL | let _ = g(); | ^^^^^^^^^^^^ @@ -16,7 +16,7 @@ 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:69:5 + --> $DIR/let_underscore_must_use.rs:69:5 | LL | let _ = l(0_u32); | ^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ 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:73:5 + --> $DIR/let_underscore_must_use.rs:73:5 | LL | let _ = s.f(); | ^^^^^^^^^^^^^^ @@ -32,7 +32,7 @@ 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:74:5 + --> $DIR/let_underscore_must_use.rs:74:5 | LL | let _ = s.g(); | ^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ 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:77:5 + --> $DIR/let_underscore_must_use.rs:77:5 | LL | let _ = S::h(); | ^^^^^^^^^^^^^^^ @@ -48,7 +48,7 @@ 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:78:5 + --> $DIR/let_underscore_must_use.rs:78:5 | LL | let _ = S::p(); | ^^^^^^^^^^^^^^^ @@ -56,7 +56,7 @@ 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:80:5 + --> $DIR/let_underscore_must_use.rs:80:5 | LL | let _ = S::a(); | ^^^^^^^^^^^^^^^ @@ -64,7 +64,7 @@ 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:82:5 + --> $DIR/let_underscore_must_use.rs:82:5 | LL | let _ = if true { Ok(()) } else { Err(()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ 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:86:5 + --> $DIR/let_underscore_must_use.rs:86:5 | LL | let _ = a.is_ok(); | ^^^^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ 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:88:5 + --> $DIR/let_underscore_must_use.rs:88:5 | LL | let _ = a.map(|_| ()); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -88,37 +88,12 @@ 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:90:5 + --> $DIR/let_underscore_must_use.rs:90:5 | LL | let _ = a; | ^^^^^^^^^^ | = help: consider explicitly using expression value -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 +error: aborting due to 12 previous errors From 63ab7a5e8cf8f2d0b3b40fa611e07e15ae204990 Mon Sep 17 00:00:00 2001 From: Areredify Date: Thu, 30 Jan 2020 18:10:19 +0300 Subject: [PATCH 3/3] lint all guard types, not just lock functions --- clippy_lints/src/let_underscore.rs | 76 ++++++++++++++--------------- clippy_lints/src/utils/paths.rs | 6 +-- tests/ui/let_underscore_lock.rs | 3 ++ tests/ui/let_underscore_lock.stderr | 32 ++++++++++-- 4 files changed, 72 insertions(+), 45 deletions(-) diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index a43674316..c2a404ebe 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, match_def_path, paths, span_lint_and_help}; +use crate::utils::{is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help}; declare_clippy_lint! { /// **What it does:** Checks for `let _ = ` @@ -31,12 +31,13 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for `let _ = sync_primitive.lock()` + /// **What it does:** Checks for `let _ = sync_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). + /// **Why is this bad?** This statement immediately drops the lock instead of + /// extending it's lifetime to the end of the scope, which is often not intended. + /// To extend lock lifetime to the end of the scope, use an underscore-prefixed + /// name instead (i.e. _lock). If you want to explicitly drop the lock, + /// `std::mem::drop` conveys your intention better and is less error-prone. /// /// **Known problems:** None. /// @@ -58,7 +59,11 @@ declare_clippy_lint! { 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]; +const SYNC_GUARD_PATHS: [&[&str]; 3] = [ + &paths::MUTEX_GUARD, + &paths::RWLOCK_READ_GUARD, + &paths::RWLOCK_WRITE_GUARD, +]; impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt<'_>) { @@ -71,37 +76,32 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore { if let PatKind::Wild = local.pat.kind; if let Some(ref init) = local.init; then { - 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 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" - ) - } - } + let check_ty = |ty| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, ty, path)); + if cx.tables.expr_ty(init).walk().any(check_ty) { + span_lint_and_help( + cx, + LET_UNDERSCORE_LOCK, + stmt.span, + "non-binding let on a synchronization lock", + "consider using an underscore-prefixed named \ + binding or dropping explicitly with `std::mem::drop`" + ) + } 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/utils/paths.rs b/clippy_lints/src/utils/paths.rs index ff8acb321..0af7f946f 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -58,7 +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 MUTEX_GUARD: [&str; 4] = ["std", "sync", "mutex", "MutexGuard"]; 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"]; @@ -101,8 +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 RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"]; +pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"]; 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/tests/ui/let_underscore_lock.rs b/tests/ui/let_underscore_lock.rs index f4a33c360..88fb216a7 100644 --- a/tests/ui/let_underscore_lock.rs +++ b/tests/ui/let_underscore_lock.rs @@ -7,4 +7,7 @@ fn main() { let _ = m.lock(); let _ = rw.read(); let _ = rw.write(); + let _ = m.try_lock(); + let _ = rw.try_read(); + let _ = rw.try_write(); } diff --git a/tests/ui/let_underscore_lock.stderr b/tests/ui/let_underscore_lock.stderr index bd297f602..5d5f6059e 100644 --- a/tests/ui/let_underscore_lock.stderr +++ b/tests/ui/let_underscore_lock.stderr @@ -5,7 +5,7 @@ LL | let _ = m.lock(); | ^^^^^^^^^^^^^^^^^ | = note: `-D clippy::let-underscore-lock` implied by `-D warnings` - = help: consider using an underscore-prefixed named binding + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` error: non-binding let on a synchronization lock --> $DIR/let_underscore_lock.rs:8:5 @@ -13,7 +13,7 @@ error: non-binding let on a synchronization lock LL | let _ = rw.read(); | ^^^^^^^^^^^^^^^^^^ | - = help: consider using an underscore-prefixed named binding + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` error: non-binding let on a synchronization lock --> $DIR/let_underscore_lock.rs:9:5 @@ -21,7 +21,31 @@ error: non-binding let on a synchronization lock LL | let _ = rw.write(); | ^^^^^^^^^^^^^^^^^^^ | - = help: consider using an underscore-prefixed named binding + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` -error: aborting due to 3 previous errors +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:10:5 + | +LL | let _ = m.try_lock(); + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` + +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:11:5 + | +LL | let _ = rw.try_read(); + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` + +error: non-binding let on a synchronization lock + --> $DIR/let_underscore_lock.rs:12:5 + | +LL | let _ = rw.try_write(); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop` + +error: aborting due to 6 previous errors