From: Areredify Date: Thu, 30 Jan 2020 15:10:19 +0000 (+0300) Subject: lint all guard types, not just lock functions X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;ds=sidebyside;h=63ab7a5e8cf8f2d0b3b40fa611e07e15ae204990;p=rust.git lint all guard types, not just lock functions --- diff --git a/clippy_lints/src/let_underscore.rs b/clippy_lints/src/let_underscore.rs index a43674316a1..c2a404ebee7 100644 --- a/clippy_lints/src/let_underscore.rs +++ b/clippy_lints/src/let_underscore.rs @@ -4,7 +4,7 @@ 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! { - /// **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_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 @@ fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt<'_>) { 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 ff8acb321a4..0af7f946fa9 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -58,7 +58,7 @@ 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 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 f4a33c36066..88fb216a743 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 bd297f6020c..5d5f6059ef1 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