Merge SignificantDropInScrutinee into Matches lint pass

Author: Jarcho

clippy_lints/src/lib.register_all.rs

@@ -151,6 +151,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(matches::MATCH_STR_CASE_MISMATCH),
     LintId::of(matches::NEEDLESS_MATCH),
     LintId::of(matches::REDUNDANT_PATTERN_MATCHING),
+    LintId::of(matches::SIGNIFICANT_DROP_IN_SCRUTINEE),
     LintId::of(matches::SINGLE_MATCH),
     LintId::of(matches::WILDCARD_IN_OR_PATTERNS),
     LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE),
@@ -282,7 +283,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(self_assignment::SELF_ASSIGNMENT),
     LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
     LintId::of(serde_api::SERDE_API_MISUSE),
-    LintId::of(significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE),
     LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
     LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
     LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),

clippy_lints/src/lib.register_lints.rs

@@ -276,6 +276,7 @@ store.register_lints(&[
     matches::NEEDLESS_MATCH,
     matches::REDUNDANT_PATTERN_MATCHING,
     matches::REST_PAT_IN_FULLY_BOUND_STRUCTS,
+    matches::SIGNIFICANT_DROP_IN_SCRUTINEE,
     matches::SINGLE_MATCH,
     matches::SINGLE_MATCH_ELSE,
     matches::WILDCARD_ENUM_MATCH_ARM,
@@ -479,7 +480,6 @@ store.register_lints(&[
     shadow::SHADOW_REUSE,
     shadow::SHADOW_SAME,
     shadow::SHADOW_UNRELATED,
-    significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE,
     single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES,
     single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
     size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,

clippy_lints/src/lib.register_suspicious.rs

@@ -24,12 +24,12 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
     LintId::of(loops::EMPTY_LOOP),
     LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
     LintId::of(loops::MUT_RANGE_BOUND),
+    LintId::of(matches::SIGNIFICANT_DROP_IN_SCRUTINEE),
     LintId::of(methods::NO_EFFECT_REPLACE),
     LintId::of(methods::SUSPICIOUS_MAP),
     LintId::of(mut_key::MUTABLE_KEY_TYPE),
     LintId::of(octal_escapes::OCTAL_ESCAPES),
     LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
-    LintId::of(significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
     LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF),

clippy_lints/src/lib.rs

@@ -367,7 +367,6 @@ mod self_named_constructors;
 mod semicolon_if_nothing_returned;
 mod serde_api;
 mod shadow;
-mod significant_drop_in_scrutinee;
 mod single_char_lifetime_names;
 mod single_component_path_imports;
 mod size_of_in_element_count;
@@ -886,7 +885,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
     store.register_early_pass(|| Box::new(doc_link_with_quotes::DocLinkWithQuotes));
     store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion));
-    store.register_late_pass(|| Box::new(significant_drop_in_scrutinee::SignificantDropInScrutinee));
     let allow_dbg_in_tests = conf.allow_dbg_in_tests;
     store.register_late_pass(move || Box::new(dbg_macro::DbgMacro::new(allow_dbg_in_tests)));
     let cargo_ignore_publish = conf.cargo_ignore_publish;

clippy_lints/src/matches/mod.rs

@@ -25,6 +25,7 @@ mod needless_match;
 mod overlapping_arms;
 mod redundant_pattern_match;
 mod rest_pat_in_fully_bound_struct;
+mod significant_drop_in_scrutinee;
 mod single_match;
 mod wild_in_or_pats;
 
@@ -748,6 +749,82 @@ declare_clippy_lint! {
     "creation of a case altering match expression with non-compliant arms"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Check for temporaries returned from function calls in a match scrutinee that have the
+    /// `clippy::has_significant_drop` attribute.
+    ///
+    /// ### Why is this bad?
+    /// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
+    /// an important side-effect, such as unlocking a mutex, making it important for users to be
+    /// able to accurately understand their lifetimes. When a temporary is returned in a function
+    /// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
+    /// be surprising.
+    ///
+    /// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
+    /// function call that returns a `MutexGuard` and then tries to lock again in one of the match
+    /// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
+    /// the match block and thus will not unlock.
+    ///
+    /// ### Example
+    /// ```rust.ignore
+    /// # use std::sync::Mutex;
+    ///
+    /// # struct State {}
+    ///
+    /// # impl State {
+    /// #     fn foo(&self) -> bool {
+    /// #         true
+    /// #     }
+    ///
+    /// #     fn bar(&self) {}
+    /// # }
+    ///
+    ///
+    /// let mutex = Mutex::new(State {});
+    ///
+    /// match mutex.lock().unwrap().foo() {
+    ///     true => {
+    ///         mutex.lock().unwrap().bar(); // Deadlock!
+    ///     }
+    ///     false => {}
+    /// };
+    ///
+    /// println!("All done!");
+    ///
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::sync::Mutex;
+    ///
+    /// # struct State {}
+    ///
+    /// # impl State {
+    /// #     fn foo(&self) -> bool {
+    /// #         true
+    /// #     }
+    ///
+    /// #     fn bar(&self) {}
+    /// # }
+    ///
+    /// let mutex = Mutex::new(State {});
+    ///
+    /// let is_foo = mutex.lock().unwrap().foo();
+    /// match is_foo {
+    ///     true => {
+    ///         mutex.lock().unwrap().bar();
+    ///     }
+    ///     false => {}
+    /// };
+    ///
+    /// println!("All done!");
+    /// ```
+    #[clippy::version = "1.60.0"]
+    pub SIGNIFICANT_DROP_IN_SCRUTINEE,
+    suspicious,
+    "warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
+}
+
 #[derive(Default)]
 pub struct Matches {
     msrv: Option<RustcVersion>,
@@ -786,6 +863,7 @@ impl_lint_pass!(Matches => [
     MANUAL_UNWRAP_OR,
     MATCH_ON_VEC_ITEMS,
     MATCH_STR_CASE_MISMATCH,
+    SIGNIFICANT_DROP_IN_SCRUTINEE,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -796,9 +874,12 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
         let from_expansion = expr.span.from_expansion();
 
         if let ExprKind::Match(ex, arms, source) = expr.kind {
-            if !span_starts_with(cx, expr.span, "match") {
+            if source == MatchSource::Normal && !span_starts_with(cx, expr.span, "match") {
                 return;
             }
+            if matches!(source, MatchSource::Normal | MatchSource::ForLoopDesugar) {
+                significant_drop_in_scrutinee::check(cx, expr, ex, source);
+            }
 
             collapsible_match::check_match(cx, arms);
             if !from_expansion {

clippy_lints/src/significant_drop_in_scrutinee.rs renamed to clippy_lints/src/matches/significant_drop_in_scrutinee.rs

@@ -5,98 +5,24 @@ use clippy_utils::source::{indent_of, snippet};
 use rustc_errors::{Applicability, Diagnostic};
 use rustc_hir::intravisit::{walk_expr, Visitor};
 use rustc_hir::{Expr, ExprKind, MatchSource};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_lint::{LateContext, LintContext};
 use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::{Ty, TypeAndMut};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::Span;
 
-declare_clippy_lint! {
-    /// ### What it does
-    /// Check for temporaries returned from function calls in a match scrutinee that have the
-    /// `clippy::has_significant_drop` attribute.
-    ///
-    /// ### Why is this bad?
-    /// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
-    /// an important side-effect, such as unlocking a mutex, making it important for users to be
-    /// able to accurately understand their lifetimes. When a temporary is returned in a function
-    /// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
-    /// be surprising.
-    ///
-    /// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
-    /// function call that returns a `MutexGuard` and then tries to lock again in one of the match
-    /// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
-    /// the match block and thus will not unlock.
-    ///
-    /// ### Example
-    /// ```rust.ignore
-    /// # use std::sync::Mutex;
-    ///
-    /// # struct State {}
-    ///
-    /// # impl State {
-    /// #     fn foo(&self) -> bool {
-    /// #         true
-    /// #     }
-    ///
-    /// #     fn bar(&self) {}
-    /// # }
-    ///
-    ///
-    /// let mutex = Mutex::new(State {});
-    ///
-    /// match mutex.lock().unwrap().foo() {
-    ///     true => {
-    ///         mutex.lock().unwrap().bar(); // Deadlock!
-    ///     }
-    ///     false => {}
-    /// };
-    ///
-    /// println!("All done!");
-    ///
-    /// ```
-    /// Use instead:
-    /// ```rust
-    /// # use std::sync::Mutex;
-    ///
-    /// # struct State {}
-    ///
-    /// # impl State {
-    /// #     fn foo(&self) -> bool {
-    /// #         true
-    /// #     }
-    ///
-    /// #     fn bar(&self) {}
-    /// # }
-    ///
-    /// let mutex = Mutex::new(State {});
-    ///
-    /// let is_foo = mutex.lock().unwrap().foo();
-    /// match is_foo {
-    ///     true => {
-    ///         mutex.lock().unwrap().bar();
-    ///     }
-    ///     false => {}
-    /// };
-    ///
-    /// println!("All done!");
-    /// ```
-    #[clippy::version = "1.60.0"]
-    pub SIGNIFICANT_DROP_IN_SCRUTINEE,
-    suspicious,
-    "warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
-}
-
-declare_lint_pass!(SignificantDropInScrutinee => [SIGNIFICANT_DROP_IN_SCRUTINEE]);
+use super::SIGNIFICANT_DROP_IN_SCRUTINEE;
 
-impl<'tcx> LateLintPass<'tcx> for SignificantDropInScrutinee {
-    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
-        if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, expr) {
-            for found in suggestions {
-                span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| {
-                    set_diagnostic(diag, cx, expr, found);
-                });
-            }
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    scrutinee: &'tcx Expr<'_>,
+    source: MatchSource,
+) {
+    if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, scrutinee, source) {
+        for found in suggestions {
+            span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| {
+                set_diagnostic(diag, cx, expr, found);
+            });
         }
     }
 }
@@ -148,28 +74,18 @@ fn set_diagnostic<'tcx>(diag: &mut Diagnostic, cx: &LateContext<'tcx>, expr: &'t
 /// may have a surprising lifetime.
 fn has_significant_drop_in_scrutinee<'tcx, 'a>(
     cx: &'a LateContext<'tcx>,
-    expr: &'tcx Expr<'tcx>,
+    scrutinee: &'tcx Expr<'tcx>,
+    source: MatchSource,
 ) -> Option<(Vec<FoundSigDrop>, &'static str)> {
-    match expr.kind {
-        ExprKind::Match(match_expr, _, source) => {
-            match source {
-                MatchSource::Normal | MatchSource::ForLoopDesugar => {
-                    let mut helper = SigDropHelper::new(cx);
-                    helper.find_sig_drop(match_expr).map(|drops| {
-                        let message = if source == MatchSource::Normal {
-                            "temporary with significant drop in match scrutinee"
-                        } else {
-                            "temporary with significant drop in for loop"
-                        };
-                        (drops, message)
-                    })
-                },
-                // MatchSource of TryDesugar or AwaitDesugar is out of scope for this lint
-                MatchSource::TryDesugar | MatchSource::AwaitDesugar => None,
-            }
-        },
-        _ => None,
-    }
+    let mut helper = SigDropHelper::new(cx);
+    helper.find_sig_drop(scrutinee).map(|drops| {
+        let message = if source == MatchSource::Normal {
+            "temporary with significant drop in match scrutinee"
+        } else {
+            "temporary with significant drop in for loop"
+        };
+        (drops, message)
+    })
 }
 
 struct SigDropHelper<'a, 'tcx> {