Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add let_underscore_lock lint #5101

Merged
merged 3 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
51 changes: 47 additions & 4 deletions clippy_lints/src/let_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_type, paths, span_lint_and_help};

declare_clippy_lint! {
/// **What it does:** Checks for `let _ = <expr>`
Expand All @@ -30,7 +30,40 @@ 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_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.
///
/// **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 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<'_>) {
Expand All @@ -43,8 +76,18 @@ 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(
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,
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_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"];
Expand Down Expand Up @@ -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_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"];
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/let_underscore_lock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![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();
let _ = m.try_lock();
let _ = rw.try_read();
let _ = rw.try_write();
}
51 changes: 51 additions & 0 deletions tests/ui/let_underscore_lock.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
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 or dropping explicitly with `std::mem::drop`

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 or dropping explicitly with `std::mem::drop`

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 or dropping explicitly with `std::mem::drop`

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

File renamed without changes.
Original file line number Diff line number Diff line change
@@ -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();
| ^^^^^^^^^^^^
Expand All @@ -8,87 +8,87 @@ 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();
| ^^^^^^^^^^^^
|
= 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);
| ^^^^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^^
|
= 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(()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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();
| ^^^^^^^^^^^^^^^^^^
|
= 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(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^
|
= 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;
| ^^^^^^^^^^
Expand Down