Skip to content

Commit 63ab7a5

Browse files
basil-cowbasil-cow
committed
lint all guard types, not just lock functions
1 parent 9b88a2b commit 63ab7a5

File tree

4 files changed

+72
-45
lines changed

4 files changed

+72
-45
lines changed

‎clippy_lints/src/let_underscore.rs

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use rustc_hir::*;
44
use rustc_lint::{LateContext, LateLintPass};
55
use rustc_session::{declare_lint_pass, declare_tool_lint};
66

7-
use crate::utils::{is_must_use_func_call, is_must_use_ty, match_def_path, paths, span_lint_and_help};
7+
use crate::utils::{is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help};
88

99
declare_clippy_lint! {
1010
/// **What it does:** Checks for `let _ = <expr>`
@@ -31,12 +31,13 @@ declare_clippy_lint! {
3131
}
3232

3333
declare_clippy_lint! {
34-
/// **What it does:** Checks for `let _ = sync_primitive.lock()`
34+
/// **What it does:** Checks for `let _ = sync_lock`
3535
///
36-
/// **Why is this bad?** This statement locks the synchronization
37-
/// primitive and immediately drops the lock, which is probably
38-
/// not intended. To extend lock lifetime to the end of the scope,
39-
/// use an underscore-prefixed name instead (i.e. _lock).
36+
/// **Why is this bad?** This statement immediately drops the lock instead of
37+
/// extending it's lifetime to the end of the scope, which is often not intended.
38+
/// To extend lock lifetime to the end of the scope, use an underscore-prefixed
39+
/// name instead (i.e. _lock). If you want to explicitly drop the lock,
40+
/// `std::mem::drop` conveys your intention better and is less error-prone.
4041
///
4142
/// **Known problems:** None.
4243
///
@@ -58,7 +59,11 @@ declare_clippy_lint! {
5859

5960
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK]);
6061

61-
const LOCK_METHODS_PATHS: [&[&str]; 3] = [&paths::MUTEX_LOCK, &paths::RWLOCK_READ, &paths::RWLOCK_WRITE];
62+
const SYNC_GUARD_PATHS: [&[&str]; 3] = [
63+
&paths::MUTEX_GUARD,
64+
&paths::RWLOCK_READ_GUARD,
65+
&paths::RWLOCK_WRITE_GUARD,
66+
];
6267

6368
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore {
6469
fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &Stmt<'_>) {
@@ -71,37 +76,32 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore {
7176
if let PatKind::Wild = local.pat.kind;
7277
if let Some(ref init) = local.init;
7378
then {
74-
if_chain! {
75-
if let ExprKind::MethodCall(_, _, _) = init.kind;
76-
let method_did = cx.tables.type_dependent_def_id(init.hir_id).unwrap();
77-
if LOCK_METHODS_PATHS.iter().any(|path| match_def_path(cx, method_did, path));
78-
then {
79-
span_lint_and_help(
80-
cx,
81-
LET_UNDERSCORE_LOCK,
82-
stmt.span,
83-
"non-binding let on a synchronization lock",
84-
"consider using an underscore-prefixed named binding"
85-
)
86-
} else {
87-
if is_must_use_ty(cx, cx.tables.expr_ty(init)) {
88-
span_lint_and_help(
89-
cx,
90-
LET_UNDERSCORE_MUST_USE,
91-
stmt.span,
92-
"non-binding let on an expression with `#[must_use]` type",
93-
"consider explicitly using expression value"
94-
)
95-
} else if is_must_use_func_call(cx, init) {
96-
span_lint_and_help(
97-
cx,
98-
LET_UNDERSCORE_MUST_USE,
99-
stmt.span,
100-
"non-binding let on a result of a `#[must_use]` function",
101-
"consider explicitly using function result"
102-
)
103-
}
104-
}
79+
let check_ty = |ty| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, ty, path));
80+
if cx.tables.expr_ty(init).walk().any(check_ty) {
81+
span_lint_and_help(
82+
cx,
83+
LET_UNDERSCORE_LOCK,
84+
stmt.span,
85+
"non-binding let on a synchronization lock",
86+
"consider using an underscore-prefixed named \
87+
binding or dropping explicitly with `std::mem::drop`"
88+
)
89+
} else if is_must_use_ty(cx, cx.tables.expr_ty(init)) {
90+
span_lint_and_help(
91+
cx,
92+
LET_UNDERSCORE_MUST_USE,
93+
stmt.span,
94+
"non-binding let on an expression with `#[must_use]` type",
95+
"consider explicitly using expression value"
96+
)
97+
} else if is_must_use_func_call(cx, init) {
98+
span_lint_and_help(
99+
cx,
100+
LET_UNDERSCORE_MUST_USE,
101+
stmt.span,
102+
"non-binding let on a result of a `#[must_use]` function",
103+
"consider explicitly using function result"
104+
)
105105
}
106106
}
107107
}

‎clippy_lints/src/utils/paths.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"];
5858
pub const MEM_UNINITIALIZED: [&str; 3] = ["core", "mem", "uninitialized"];
5959
pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"];
6060
pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"];
61-
pub const MUTEX_LOCK: [&str; 5] = ["std", "sync", "mutex", "Mutex", "lock"];
61+
pub const MUTEX_GUARD: [&str; 4] = ["std", "sync", "mutex", "MutexGuard"];
6262
pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
6363
pub const OPS_MODULE: [&str; 2] = ["core", "ops"];
6464
pub const OPTION: [&str; 3] = ["core", "option", "Option"];
@@ -101,8 +101,8 @@ pub const REPEAT: [&str; 3] = ["core", "iter", "repeat"];
101101
pub const RESULT: [&str; 3] = ["core", "result", "Result"];
102102
pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"];
103103
pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"];
104-
pub const RWLOCK_READ: [&str; 5] = ["std", "sync", "rwlock", "RwLock", "read"];
105-
pub const RWLOCK_WRITE: [&str; 5] = ["std", "sync", "rwlock", "RwLock", "write"];
104+
pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"];
105+
pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"];
106106
pub const SERDE_DE_VISITOR: [&str; 3] = ["serde", "de", "Visitor"];
107107
pub const SLICE_INTO_VEC: [&str; 4] = ["alloc", "slice", "<impl [T]>", "into_vec"];
108108
pub const SLICE_ITER: [&str; 3] = ["core", "slice", "Iter"];

‎tests/ui/let_underscore_lock.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,7 @@ fn main() {
77
let _ = m.lock();
88
let _ = rw.read();
99
let _ = rw.write();
10+
let _ = m.try_lock();
11+
let _ = rw.try_read();
12+
let _ = rw.try_write();
1013
}

‎tests/ui/let_underscore_lock.stderr

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,47 @@ LL | let _ = m.lock();
55
| ^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::let-underscore-lock` implied by `-D warnings`
8-
= help: consider using an underscore-prefixed named binding
8+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
99

1010
error: non-binding let on a synchronization lock
1111
--> $DIR/let_underscore_lock.rs:8:5
1212
|
1313
LL | let _ = rw.read();
1414
| ^^^^^^^^^^^^^^^^^^
1515
|
16-
= help: consider using an underscore-prefixed named binding
16+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
1717

1818
error: non-binding let on a synchronization lock
1919
--> $DIR/let_underscore_lock.rs:9:5
2020
|
2121
LL | let _ = rw.write();
2222
| ^^^^^^^^^^^^^^^^^^^
2323
|
24-
= help: consider using an underscore-prefixed named binding
24+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
2525

26-
error: aborting due to 3 previous errors
26+
error: non-binding let on a synchronization lock
27+
--> $DIR/let_underscore_lock.rs:10:5
28+
|
29+
LL | let _ = m.try_lock();
30+
| ^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
33+
34+
error: non-binding let on a synchronization lock
35+
--> $DIR/let_underscore_lock.rs:11:5
36+
|
37+
LL | let _ = rw.try_read();
38+
| ^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
41+
42+
error: non-binding let on a synchronization lock
43+
--> $DIR/let_underscore_lock.rs:12:5
44+
|
45+
LL | let _ = rw.try_write();
46+
| ^^^^^^^^^^^^^^^^^^^^^^^
47+
|
48+
= help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
49+
50+
error: aborting due to 6 previous errors
2751

0 commit comments

Comments
 (0)