Auto merge of #9697 - Alexendoo:let-underscore-uplift, r=flip1995

Mark `let_underscore_lock` and `let_underscore_drop` as uplifted

Here I've renamed both the uplifted lints, however rustc's `let_underscore_lock` is slightly less capable than the clippy lint as it doesn't catch `parking_lot` types or `Result<Guard, ..>`, should we still remove it? The `Result` change looks like it was unintentional to me so that could probably be fixed upstream

changelog: Uplift [`let_underscore_drop`] to rustc rust-lang/rust#97739
changelog: Remove overlap between rustc's `let_underscore_lock` and Clippy's [`let_underscore_lock`]

r? `@flip1995`

clippy_lints/src/let_underscore.rs

@@ -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
@@ -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
@@ -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,
@@ -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",
+                );
             }
         }
     }

clippy_lints/src/lib.register_lints.rs

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

clippy_lints/src/lib.register_pedantic.rs

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

clippy_lints/src/renamed_lints.rs

@@ -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"),
@@ -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"),

lintcheck/src/main.rs

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

src/docs.rs

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

src/docs/let_underscore_lock.txt

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

tests/ui/let_underscore_lock.rs

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