Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 13:43:17 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge SignificantDropInScrutinee into Matches lint pass

Browse source
This commit is contained in:
Jason Newcomb 2022-06-03 13:35:10 -04:00
parent 8c8a52eeeb
commit dbc7753fb2
6 changed files with 109 additions and 114 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/lib.register_all.rs
View file

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

2
clippy_lints/src/lib.register_lints.rs
View file

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

2
clippy_lints/src/lib.register_suspicious.rs
View file

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

2
clippy_lints/src/lib.rs
View file

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

83
clippy_lints/src/matches/mod.rs
View file

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

108
clippy_lints/src/significant_drop_in_scrutinee.rs → clippy_lints/src/matches/significant_drop_in_scrutinee.rs
View file

@ -5,93 +5,20 @@ 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"
}
use super::SIGNIFICANT_DROP_IN_SCRUTINEE;
declare_lint_pass!(SignificantDropInScrutinee => [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) {
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);
@ -99,7 +26,6 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropInScrutinee {
}
}
}
}
fn set_diagnostic<'tcx>(diag: &mut Diagnostic, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, found: FoundSigDrop) {
if found.lint_suggestion == LintSuggestion::MoveAndClone {
@ -148,14 +74,11 @@ 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| {
helper.find_sig_drop(scrutinee).map(|drops| {
let message = if source == MatchSource::Normal {
"temporary with significant drop in match scrutinee"
} else {
@ -163,13 +86,6 @@ fn has_significant_drop_in_scrutinee<'tcx, 'a>(
};
(drops, message)
})
},
// MatchSource of TryDesugar or AwaitDesugar is out of scope for this lint
MatchSource::TryDesugar | MatchSource::AwaitDesugar => None,
}
},
_ => None,
}
}
struct SigDropHelper<'a, 'tcx> {