Skip to content

Commit dbc7753

Browse files
JarchoJarcho
committed
Merge SignificantDropInScrutinee into Matches lint pass
1 parent 8c8a52e commit dbc7753

File tree

6 files changed

+109
-114
lines changed

6 files changed

+109
-114
lines changed

‎clippy_lints/src/lib.register_all.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
151151
LintId::of(matches::MATCH_STR_CASE_MISMATCH),
152152
LintId::of(matches::NEEDLESS_MATCH),
153153
LintId::of(matches::REDUNDANT_PATTERN_MATCHING),
154+
LintId::of(matches::SIGNIFICANT_DROP_IN_SCRUTINEE),
154155
LintId::of(matches::SINGLE_MATCH),
155156
LintId::of(matches::WILDCARD_IN_OR_PATTERNS),
156157
LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE),
@@ -282,7 +283,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
282283
LintId::of(self_assignment::SELF_ASSIGNMENT),
283284
LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
284285
LintId::of(serde_api::SERDE_API_MISUSE),
285-
LintId::of(significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE),
286286
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
287287
LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
288288
LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),

‎clippy_lints/src/lib.register_lints.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ store.register_lints(&[
276276
matches::NEEDLESS_MATCH,
277277
matches::REDUNDANT_PATTERN_MATCHING,
278278
matches::REST_PAT_IN_FULLY_BOUND_STRUCTS,
279+
matches::SIGNIFICANT_DROP_IN_SCRUTINEE,
279280
matches::SINGLE_MATCH,
280281
matches::SINGLE_MATCH_ELSE,
281282
matches::WILDCARD_ENUM_MATCH_ARM,
@@ -479,7 +480,6 @@ store.register_lints(&[
479480
shadow::SHADOW_REUSE,
480481
shadow::SHADOW_SAME,
481482
shadow::SHADOW_UNRELATED,
482-
significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE,
483483
single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES,
484484
single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
485485
size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,

‎clippy_lints/src/lib.register_suspicious.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
2424
LintId::of(loops::EMPTY_LOOP),
2525
LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES),
2626
LintId::of(loops::MUT_RANGE_BOUND),
27+
LintId::of(matches::SIGNIFICANT_DROP_IN_SCRUTINEE),
2728
LintId::of(methods::NO_EFFECT_REPLACE),
2829
LintId::of(methods::SUSPICIOUS_MAP),
2930
LintId::of(mut_key::MUTABLE_KEY_TYPE),
3031
LintId::of(octal_escapes::OCTAL_ESCAPES),
3132
LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
32-
LintId::of(significant_drop_in_scrutinee::SIGNIFICANT_DROP_IN_SCRUTINEE),
3333
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
3434
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
3535
LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF),

‎clippy_lints/src/lib.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,6 @@ mod self_named_constructors;
367367
mod semicolon_if_nothing_returned;
368368
mod serde_api;
369369
mod shadow;
370-
mod significant_drop_in_scrutinee;
371370
mod single_char_lifetime_names;
372371
mod single_component_path_imports;
373372
mod size_of_in_element_count;
@@ -886,7 +885,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
886885
store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation));
887886
store.register_early_pass(|| Box::new(doc_link_with_quotes::DocLinkWithQuotes));
888887
store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion));
889-
store.register_late_pass(|| Box::new(significant_drop_in_scrutinee::SignificantDropInScrutinee));
890888
let allow_dbg_in_tests = conf.allow_dbg_in_tests;
891889
store.register_late_pass(move || Box::new(dbg_macro::DbgMacro::new(allow_dbg_in_tests)));
892890
let cargo_ignore_publish = conf.cargo_ignore_publish;

‎clippy_lints/src/matches/mod.rs‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ mod needless_match;
2525
mod overlapping_arms;
2626
mod redundant_pattern_match;
2727
mod rest_pat_in_fully_bound_struct;
28+
mod significant_drop_in_scrutinee;
2829
mod single_match;
2930
mod wild_in_or_pats;
3031

@@ -748,6 +749,82 @@ declare_clippy_lint! {
748749
"creation of a case altering match expression with non-compliant arms"
749750
}
750751

752+
declare_clippy_lint! {
753+
/// ### What it does
754+
/// Check for temporaries returned from function calls in a match scrutinee that have the
755+
/// `clippy::has_significant_drop` attribute.
756+
///
757+
/// ### Why is this bad?
758+
/// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
759+
/// an important side-effect, such as unlocking a mutex, making it important for users to be
760+
/// able to accurately understand their lifetimes. When a temporary is returned in a function
761+
/// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
762+
/// be surprising.
763+
///
764+
/// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
765+
/// function call that returns a `MutexGuard` and then tries to lock again in one of the match
766+
/// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
767+
/// the match block and thus will not unlock.
768+
///
769+
/// ### Example
770+
/// ```rust.ignore
771+
/// # use std::sync::Mutex;
772+
///
773+
/// # struct State {}
774+
///
775+
/// # impl State {
776+
/// # fn foo(&self) -> bool {
777+
/// # true
778+
/// # }
779+
///
780+
/// # fn bar(&self) {}
781+
/// # }
782+
///
783+
///
784+
/// let mutex = Mutex::new(State {});
785+
///
786+
/// match mutex.lock().unwrap().foo() {
787+
/// true => {
788+
/// mutex.lock().unwrap().bar(); // Deadlock!
789+
/// }
790+
/// false => {}
791+
/// };
792+
///
793+
/// println!("All done!");
794+
///
795+
/// ```
796+
/// Use instead:
797+
/// ```rust
798+
/// # use std::sync::Mutex;
799+
///
800+
/// # struct State {}
801+
///
802+
/// # impl State {
803+
/// # fn foo(&self) -> bool {
804+
/// # true
805+
/// # }
806+
///
807+
/// # fn bar(&self) {}
808+
/// # }
809+
///
810+
/// let mutex = Mutex::new(State {});
811+
///
812+
/// let is_foo = mutex.lock().unwrap().foo();
813+
/// match is_foo {
814+
/// true => {
815+
/// mutex.lock().unwrap().bar();
816+
/// }
817+
/// false => {}
818+
/// };
819+
///
820+
/// println!("All done!");
821+
/// ```
822+
#[clippy::version = "1.60.0"]
823+
pub SIGNIFICANT_DROP_IN_SCRUTINEE,
824+
suspicious,
825+
"warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
826+
}
827+
751828
#[derive(Default)]
752829
pub struct Matches {
753830
msrv: Option<RustcVersion>,
@@ -786,6 +863,7 @@ impl_lint_pass!(Matches => [
786863
MANUAL_UNWRAP_OR,
787864
MATCH_ON_VEC_ITEMS,
788865
MATCH_STR_CASE_MISMATCH,
866+
SIGNIFICANT_DROP_IN_SCRUTINEE,
789867
]);
790868

791869
impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -796,9 +874,12 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
796874
let from_expansion = expr.span.from_expansion();
797875

798876
if let ExprKind::Match(ex, arms, source) = expr.kind {
799-
if !span_starts_with(cx, expr.span, "match") {
877+
if source == MatchSource::Normal && !span_starts_with(cx, expr.span, "match") {
800878
return;
801879
}
880+
if matches!(source, MatchSource::Normal | MatchSource::ForLoopDesugar) {
881+
significant_drop_in_scrutinee::check(cx, expr, ex, source);
882+
}
802883

803884
collapsible_match::check_match(cx, arms);
804885
if !from_expansion {

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

Lines changed: 24 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -5,98 +5,24 @@ use clippy_utils::source::{indent_of, snippet};
55
use rustc_errors::{Applicability, Diagnostic};
66
use rustc_hir::intravisit::{walk_expr, Visitor};
77
use rustc_hir::{Expr, ExprKind, MatchSource};
8-
use rustc_lint::{LateContext, LateLintPass, LintContext};
8+
use rustc_lint::{LateContext, LintContext};
99
use rustc_middle::ty::subst::GenericArgKind;
1010
use rustc_middle::ty::{Ty, TypeAndMut};
11-
use rustc_session::{declare_lint_pass, declare_tool_lint};
1211
use rustc_span::Span;
1312

14-
declare_clippy_lint! {
15-
/// ### What it does
16-
/// Check for temporaries returned from function calls in a match scrutinee that have the
17-
/// `clippy::has_significant_drop` attribute.
18-
///
19-
/// ### Why is this bad?
20-
/// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
21-
/// an important side-effect, such as unlocking a mutex, making it important for users to be
22-
/// able to accurately understand their lifetimes. When a temporary is returned in a function
23-
/// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
24-
/// be surprising.
25-
///
26-
/// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
27-
/// function call that returns a `MutexGuard` and then tries to lock again in one of the match
28-
/// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
29-
/// the match block and thus will not unlock.
30-
///
31-
/// ### Example
32-
/// ```rust.ignore
33-
/// # use std::sync::Mutex;
34-
///
35-
/// # struct State {}
36-
///
37-
/// # impl State {
38-
/// # fn foo(&self) -> bool {
39-
/// # true
40-
/// # }
41-
///
42-
/// # fn bar(&self) {}
43-
/// # }
44-
///
45-
///
46-
/// let mutex = Mutex::new(State {});
47-
///
48-
/// match mutex.lock().unwrap().foo() {
49-
/// true => {
50-
/// mutex.lock().unwrap().bar(); // Deadlock!
51-
/// }
52-
/// false => {}
53-
/// };
54-
///
55-
/// println!("All done!");
56-
///
57-
/// ```
58-
/// Use instead:
59-
/// ```rust
60-
/// # use std::sync::Mutex;
61-
///
62-
/// # struct State {}
63-
///
64-
/// # impl State {
65-
/// # fn foo(&self) -> bool {
66-
/// # true
67-
/// # }
68-
///
69-
/// # fn bar(&self) {}
70-
/// # }
71-
///
72-
/// let mutex = Mutex::new(State {});
73-
///
74-
/// let is_foo = mutex.lock().unwrap().foo();
75-
/// match is_foo {
76-
/// true => {
77-
/// mutex.lock().unwrap().bar();
78-
/// }
79-
/// false => {}
80-
/// };
81-
///
82-
/// println!("All done!");
83-
/// ```
84-
#[clippy::version = "1.60.0"]
85-
pub SIGNIFICANT_DROP_IN_SCRUTINEE,
86-
suspicious,
87-
"warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
88-
}
89-
90-
declare_lint_pass!(SignificantDropInScrutinee => [SIGNIFICANT_DROP_IN_SCRUTINEE]);
13+
use super::SIGNIFICANT_DROP_IN_SCRUTINEE;
9114

92-
impl<'tcx> LateLintPass<'tcx> for SignificantDropInScrutinee {
93-
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
94-
if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, expr) {
95-
for found in suggestions {
96-
span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| {
97-
set_diagnostic(diag, cx, expr, found);
98-
});
99-
}
15+
pub(super) fn check<'tcx>(
16+
cx: &LateContext<'tcx>,
17+
expr: &'tcx Expr<'tcx>,
18+
scrutinee: &'tcx Expr<'_>,
19+
source: MatchSource,
20+
) {
21+
if let Some((suggestions, message)) = has_significant_drop_in_scrutinee(cx, scrutinee, source) {
22+
for found in suggestions {
23+
span_lint_and_then(cx, SIGNIFICANT_DROP_IN_SCRUTINEE, found.found_span, message, |diag| {
24+
set_diagnostic(diag, cx, expr, found);
25+
});
10026
}
10127
}
10228
}
@@ -148,28 +74,18 @@ fn set_diagnostic<'tcx>(diag: &mut Diagnostic, cx: &LateContext<'tcx>, expr: &'t
14874
/// may have a surprising lifetime.
14975
fn has_significant_drop_in_scrutinee<'tcx, 'a>(
15076
cx: &'a LateContext<'tcx>,
151-
expr: &'tcx Expr<'tcx>,
77+
scrutinee: &'tcx Expr<'tcx>,
78+
source: MatchSource,
15279
) -> Option<(Vec<FoundSigDrop>, &'static str)> {
153-
match expr.kind {
154-
ExprKind::Match(match_expr, _, source) => {
155-
match source {
156-
MatchSource::Normal | MatchSource::ForLoopDesugar => {
157-
let mut helper = SigDropHelper::new(cx);
158-
helper.find_sig_drop(match_expr).map(|drops| {
159-
let message = if source == MatchSource::Normal {
160-
"temporary with significant drop in match scrutinee"
161-
} else {
162-
"temporary with significant drop in for loop"
163-
};
164-
(drops, message)
165-
})
166-
},
167-
// MatchSource of TryDesugar or AwaitDesugar is out of scope for this lint
168-
MatchSource::TryDesugar | MatchSource::AwaitDesugar => None,
169-
}
170-
},
171-
_ => None,
172-
}
80+
let mut helper = SigDropHelper::new(cx);
81+
helper.find_sig_drop(scrutinee).map(|drops| {
82+
let message = if source == MatchSource::Normal {
83+
"temporary with significant drop in match scrutinee"
84+
} else {
85+
"temporary with significant drop in for loop"
86+
};
87+
(drops, message)
88+
})
17389
}
17490

17591
struct SigDropHelper<'a, 'tcx> {

0 commit comments

Comments
 (0)