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

Mark let_underscore_lock and let_underscore_drop as uplifted #9697

Merged
merged 2 commits into from
Oct 23, 2022
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
144 changes: 41 additions & 103 deletions clippy_lints/src/let_underscore.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::{is_must_use_ty, is_type_diagnostic_item, match_type};
use clippy_utils::ty::{is_must_use_ty, match_type};
use clippy_utils::{is_must_use_func_call, paths};
use if_chain::if_chain;
use rustc_hir::{Local, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Symbol};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -35,8 +33,9 @@ declare_clippy_lint! {

declare_clippy_lint! {
/// ### What it does
/// Checks for `let _ = sync_lock`.
/// This supports `mutex` and `rwlock` in `std::sync` and `parking_lot`.
/// Checks for `let _ = sync_lock`. This supports `mutex` and `rwlock` in
/// `parking_lot`. For `std` locks see the `rustc` lint
/// [`let_underscore_lock`](https://doc.rust-lang.org/nightly/rustc/lints/listing/deny-by-default.html#let-underscore-lock)
///
/// ### Why is this bad?
/// This statement immediately drops the lock instead of
Expand All @@ -60,47 +59,7 @@ declare_clippy_lint! {
"non-binding let on a synchronization lock"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for `let _ = <expr>`
/// where expr has a type that implements `Drop`
///
/// ### Why is this bad?
/// This statement immediately drops the initializer
/// expression instead of extending its lifetime to the end of the scope, which
/// is often not intended. To extend the expression's lifetime to the end of the
/// scope, use an underscore-prefixed name instead (i.e. _var). If you want to
/// explicitly drop the expression, `std::mem::drop` conveys your intention
/// better and is less error-prone.
///
/// ### Example
/// ```rust
/// # struct DroppableItem;
/// {
/// let _ = DroppableItem;
/// // ^ dropped here
/// /* more code */
/// }
/// ```
///
/// Use instead:
/// ```rust
/// # struct DroppableItem;
/// {
/// let _droppable = DroppableItem;
/// /* more code */
/// // dropped at end of scope
/// }
/// ```
#[clippy::version = "1.50.0"]
pub LET_UNDERSCORE_DROP,
pedantic,
"non-binding let on a type that implements `Drop`"
}

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

const SYNC_GUARD_SYMS: [Symbol; 3] = [sym::MutexGuard, sym::RwLockReadGuard, sym::RwLockWriteGuard];
declare_lint_pass!(LetUnderscore => [LET_UNDERSCORE_MUST_USE, LET_UNDERSCORE_LOCK]);

const SYNC_GUARD_PATHS: [&[&str]; 3] = [
&paths::PARKING_LOT_MUTEX_GUARD,
Expand All @@ -110,64 +69,43 @@ const SYNC_GUARD_PATHS: [&[&str]; 3] = [

impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
if in_external_macro(cx.tcx.sess, local.span) {
return;
}

if_chain! {
if let PatKind::Wild = local.pat.kind;
if let Some(init) = local.init;
then {
let init_ty = cx.typeck_results().expr_ty(init);
let contains_sync_guard = init_ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => {
SYNC_GUARD_SYMS
.iter()
.any(|&sym| is_type_diagnostic_item(cx, inner_ty, sym))
|| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path))
},

GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
});
if contains_sync_guard {
span_lint_and_help(
cx,
LET_UNDERSCORE_LOCK,
local.span,
"non-binding let on a synchronization lock",
None,
"consider using an underscore-prefixed named \
binding or dropping explicitly with `std::mem::drop`",
);
} else if init_ty.needs_drop(cx.tcx, cx.param_env) {
span_lint_and_help(
cx,
LET_UNDERSCORE_DROP,
local.span,
"non-binding `let` on a type that implements `Drop`",
None,
"consider using an underscore-prefixed named \
if !in_external_macro(cx.tcx.sess, local.span)
&& let PatKind::Wild = local.pat.kind
&& let Some(init) = local.init
{
let init_ty = cx.typeck_results().expr_ty(init);
let contains_sync_guard = init_ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path)),
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
});
if contains_sync_guard {
span_lint_and_help(
cx,
LET_UNDERSCORE_LOCK,
local.span,
"non-binding let on a synchronization lock",
None,
"consider using an underscore-prefixed named \
binding or dropping explicitly with `std::mem::drop`",
);
} else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
span_lint_and_help(
cx,
LET_UNDERSCORE_MUST_USE,
local.span,
"non-binding let on an expression with `#[must_use]` type",
None,
"consider explicitly using expression value",
);
} else if is_must_use_func_call(cx, init) {
span_lint_and_help(
cx,
LET_UNDERSCORE_MUST_USE,
local.span,
"non-binding let on a result of a `#[must_use]` function",
None,
"consider explicitly using function result",
);
}
);
} else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
span_lint_and_help(
cx,
LET_UNDERSCORE_MUST_USE,
local.span,
"non-binding let on an expression with `#[must_use]` type",
None,
"consider explicitly using expression value",
);
} else if is_must_use_func_call(cx, init) {
span_lint_and_help(
cx,
LET_UNDERSCORE_MUST_USE,
local.span,
"non-binding let on a result of a `#[must_use]` function",
None,
"consider explicitly using function result",
);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ store.register_lints(&[
len_zero::LEN_WITHOUT_IS_EMPTY,
len_zero::LEN_ZERO,
let_if_seq::USELESS_LET_IF_SEQ,
let_underscore::LET_UNDERSCORE_DROP,
let_underscore::LET_UNDERSCORE_LOCK,
let_underscore::LET_UNDERSCORE_MUST_USE,
lifetimes::EXTRA_UNUSED_LIFETIMES,
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(items_after_statements::ITEMS_AFTER_STATEMENTS),
LintId::of(iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR),
LintId::of(large_stack_arrays::LARGE_STACK_ARRAYS),
LintId::of(let_underscore::LET_UNDERSCORE_DROP),
LintId::of(literal_representation::LARGE_DIGIT_GROUPS),
LintId::of(literal_representation::UNREADABLE_LITERAL),
LintId::of(loops::EXPLICIT_INTO_ITER_LOOP),
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/renamed_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::disallowed_method", "clippy::disallowed_methods"),
("clippy::disallowed_type", "clippy::disallowed_types"),
("clippy::eval_order_dependence", "clippy::mixed_read_write_in_expression"),
("clippy::for_loop_over_option", "for_loops_over_fallibles"),
("clippy::for_loop_over_result", "for_loops_over_fallibles"),
("clippy::identity_conversion", "clippy::useless_conversion"),
("clippy::if_let_some_result", "clippy::match_result_ok"),
("clippy::logic_bug", "clippy::overly_complex_bool_expr"),
Expand All @@ -31,10 +29,13 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::to_string_in_display", "clippy::recursive_format_impl"),
("clippy::zero_width_space", "clippy::invisible_characters"),
("clippy::drop_bounds", "drop_bounds"),
("clippy::for_loop_over_option", "for_loops_over_fallibles"),
("clippy::for_loop_over_result", "for_loops_over_fallibles"),
("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"),
("clippy::into_iter_on_array", "array_into_iter"),
("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"),
("clippy::invalid_ref", "invalid_value"),
("clippy::let_underscore_drop", "let_underscore_drop"),
("clippy::mem_discriminant_non_enum", "enum_intrinsics_non_enums"),
("clippy::panic_params", "non_fmt_panics"),
("clippy::positional_named_format_parameters", "named_arguments_used_positionally"),
Expand Down
2 changes: 1 addition & 1 deletion lintcheck/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ fn main() {
.unwrap();

let server = config.recursive.then(|| {
fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive").unwrap_or_default();
let _ = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive");

LintcheckServer::spawn(recursive_options)
});
Expand Down
1 change: 0 additions & 1 deletion src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ docs! {
"len_without_is_empty",
"len_zero",
"let_and_return",
"let_underscore_drop",
"let_underscore_lock",
"let_underscore_must_use",
"let_unit_value",
Expand Down
29 changes: 0 additions & 29 deletions src/docs/let_underscore_drop.txt

This file was deleted.

5 changes: 3 additions & 2 deletions src/docs/let_underscore_lock.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### What it does
Checks for `let _ = sync_lock`.
This supports `mutex` and `rwlock` in `std::sync` and `parking_lot`.
Checks for `let _ = sync_lock`. This supports `mutex` and `rwlock` in
`parking_lot`. For `std` locks see the `rustc` lint
[`let_underscore_lock`](https://doc.rust-lang.org/nightly/rustc/lints/listing/deny-by-default.html#let-underscore-lock)

### Why is this bad?
This statement immediately drops the lock instead of
Expand Down
28 changes: 0 additions & 28 deletions tests/ui/let_underscore_drop.rs

This file was deleted.

27 changes: 0 additions & 27 deletions tests/ui/let_underscore_drop.stderr

This file was deleted.

31 changes: 17 additions & 14 deletions tests/ui/let_underscore_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,6 @@
extern crate parking_lot;

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

// These shouldn't throw an error.
let _ = m;
let _ = rw;

use parking_lot::{lock_api::RawMutex, Mutex, RwLock};

let p_m: Mutex<()> = Mutex::const_new(RawMutex::INIT, ());
Expand All @@ -34,3 +20,20 @@ fn main() {
let _ = p_m1;
let _ = p_rw;
}

fn uplifted() {
// shouldn't lint std locks as they were uplifted as rustc's `let_underscore_lock`

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

let _ = m;
let _ = rw;
}
Loading