lint all guard types, not just lock functions

This commit is contained in:
Areredify 2020-01-30 18:10:19 +03:00
parent 9b88a2b295
commit 63ab7a5e8c
4 changed files with 72 additions and 45 deletions

View File

@ -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 _ = <expr>`
@ -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"
)
}
}
}

View File

@ -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", "<impl [T]>", "into_vec"];
pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"];

View File

@ -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();
}

View File

@ -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