Auto merge of #8937 - Jarcho:merge_match_passes, r=llogiq

Merge various passes into `Matches`

changelog: None
This commit is contained in:
bors 2022-06-04 07:41:27 +00:00
commit b1a3e7e9c8
24 changed files with 1050 additions and 1032 deletions

View file

@ -38,7 +38,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(casts::UNNECESSARY_CAST),
LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),
LintId::of(collapsible_if::COLLAPSIBLE_IF),
LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
LintId::of(comparison_chain::COMPARISON_CHAIN),
LintId::of(copies::IFS_SAME_COND),
LintId::of(copies::IF_SAME_THEN_ELSE),
@ -134,23 +133,25 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(main_recursion::MAIN_RECURSION),
LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
LintId::of(manual_bits::MANUAL_BITS),
LintId::of(manual_map::MANUAL_MAP),
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(manual_strip::MANUAL_STRIP),
LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR),
LintId::of(map_clone::MAP_CLONE),
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
LintId::of(match_result_ok::MATCH_RESULT_OK),
LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH),
LintId::of(matches::COLLAPSIBLE_MATCH),
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(matches::MANUAL_MAP),
LintId::of(matches::MANUAL_UNWRAP_OR),
LintId::of(matches::MATCH_AS_REF),
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
LintId::of(matches::MATCH_OVERLAPPING_ARM),
LintId::of(matches::MATCH_REF_PATS),
LintId::of(matches::MATCH_SINGLE_BINDING),
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),

View file

@ -25,9 +25,9 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(loops::SINGLE_ELEMENT_LOOP),
LintId::of(loops::WHILE_LET_LOOP),
LintId::of(manual_strip::MANUAL_STRIP),
LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR),
LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN),
LintId::of(matches::MANUAL_UNWRAP_OR),
LintId::of(matches::MATCH_AS_REF),
LintId::of(matches::MATCH_SINGLE_BINDING),
LintId::of(matches::NEEDLESS_MATCH),

View file

@ -39,7 +39,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
LintId::of(loops::ITER_NEXT_LOOP),
LintId::of(loops::NEVER_LOOP),
LintId::of(loops::WHILE_IMMUTABLE_CONDITION),
LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH),
LintId::of(matches::MATCH_STR_CASE_MISMATCH),
LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),
LintId::of(methods::CLONE_DOUBLE_REF),
LintId::of(methods::ITERATOR_STEP_BY_ZERO),

View file

@ -94,7 +94,6 @@ store.register_lints(&[
cognitive_complexity::COGNITIVE_COMPLEXITY,
collapsible_if::COLLAPSIBLE_ELSE_IF,
collapsible_if::COLLAPSIBLE_IF,
collapsible_match::COLLAPSIBLE_MATCH,
comparison_chain::COMPARISON_CHAIN,
copies::BRANCHES_SHARING_CODE,
copies::IFS_SAME_COND,
@ -252,33 +251,36 @@ store.register_lints(&[
manual_assert::MANUAL_ASSERT,
manual_async_fn::MANUAL_ASYNC_FN,
manual_bits::MANUAL_BITS,
manual_map::MANUAL_MAP,
manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
manual_ok_or::MANUAL_OK_OR,
manual_strip::MANUAL_STRIP,
manual_unwrap_or::MANUAL_UNWRAP_OR,
map_clone::MAP_CLONE,
map_err_ignore::MAP_ERR_IGNORE,
map_unit_fn::OPTION_MAP_UNIT_FN,
map_unit_fn::RESULT_MAP_UNIT_FN,
match_on_vec_items::MATCH_ON_VEC_ITEMS,
match_result_ok::MATCH_RESULT_OK,
match_str_case_mismatch::MATCH_STR_CASE_MISMATCH,
matches::COLLAPSIBLE_MATCH,
matches::INFALLIBLE_DESTRUCTURING_MATCH,
matches::MANUAL_MAP,
matches::MANUAL_UNWRAP_OR,
matches::MATCH_AS_REF,
matches::MATCH_BOOL,
matches::MATCH_LIKE_MATCHES_MACRO,
matches::MATCH_ON_VEC_ITEMS,
matches::MATCH_OVERLAPPING_ARM,
matches::MATCH_REF_PATS,
matches::MATCH_SAME_ARMS,
matches::MATCH_SINGLE_BINDING,
matches::MATCH_STR_CASE_MISMATCH,
matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
matches::MATCH_WILD_ERR_ARM,
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::TRY_ERR,
matches::WILDCARD_ENUM_MATCH_ARM,
matches::WILDCARD_IN_OR_PATTERNS,
mem_forget::MEM_FORGET,
@ -480,7 +482,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,
@ -522,7 +523,6 @@ store.register_lints(&[
transmute::USELESS_TRANSMUTE,
transmute::WRONG_TRANSMUTE,
transmuting_null::TRANSMUTING_NULL,
try_err::TRY_ERR,
types::BORROWED_BOX,
types::BOX_COLLECTION,
types::LINKEDLIST,

View file

@ -51,8 +51,8 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(macro_use::MACRO_USE_IMPORTS),
LintId::of(manual_assert::MANUAL_ASSERT),
LintId::of(manual_ok_or::MANUAL_OK_OR),
LintId::of(match_on_vec_items::MATCH_ON_VEC_ITEMS),
LintId::of(matches::MATCH_BOOL),
LintId::of(matches::MATCH_ON_VEC_ITEMS),
LintId::of(matches::MATCH_SAME_ARMS),
LintId::of(matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS),
LintId::of(matches::MATCH_WILD_ERR_ARM),

View file

@ -31,6 +31,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(literal_representation::DECIMAL_LITERAL_REPRESENTATION),
LintId::of(map_err_ignore::MAP_ERR_IGNORE),
LintId::of(matches::REST_PAT_IN_FULLY_BOUND_STRUCTS),
LintId::of(matches::TRY_ERR),
LintId::of(matches::WILDCARD_ENUM_MATCH_ARM),
LintId::of(mem_forget::MEM_FORGET),
LintId::of(methods::CLONE_ON_REF_PTR),
@ -68,7 +69,6 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(strings::STRING_SLICE),
LintId::of(strings::STRING_TO_STRING),
LintId::of(strings::STR_TO_STRING),
LintId::of(try_err::TRY_ERR),
LintId::of(types::RC_BUFFER),
LintId::of(types::RC_MUTEX),
LintId::of(undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS),

View file

@ -12,7 +12,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(collapsible_if::COLLAPSIBLE_ELSE_IF),
LintId::of(collapsible_if::COLLAPSIBLE_IF),
LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
LintId::of(comparison_chain::COMPARISON_CHAIN),
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
LintId::of(dereference::NEEDLESS_BORROW),
@ -46,11 +45,12 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(main_recursion::MAIN_RECURSION),
LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
LintId::of(manual_bits::MANUAL_BITS),
LintId::of(manual_map::MANUAL_MAP),
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
LintId::of(map_clone::MAP_CLONE),
LintId::of(match_result_ok::MATCH_RESULT_OK),
LintId::of(matches::COLLAPSIBLE_MATCH),
LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(matches::MANUAL_MAP),
LintId::of(matches::MATCH_LIKE_MATCHES_MACRO),
LintId::of(matches::MATCH_OVERLAPPING_ARM),
LintId::of(matches::MATCH_REF_PATS),

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

View file

@ -193,7 +193,6 @@ mod casts;
mod checked_conversions;
mod cognitive_complexity;
mod collapsible_if;
mod collapsible_match;
mod comparison_chain;
mod copies;
mod copy_iterator;
@ -281,17 +280,13 @@ mod main_recursion;
mod manual_assert;
mod manual_async_fn;
mod manual_bits;
mod manual_map;
mod manual_non_exhaustive;
mod manual_ok_or;
mod manual_strip;
mod manual_unwrap_or;
mod map_clone;
mod map_err_ignore;
mod map_unit_fn;
mod match_on_vec_items;
mod match_result_ok;
mod match_str_case_mismatch;
mod matches;
mod mem_forget;
mod mem_replace;
@ -372,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;
@ -391,7 +385,6 @@ mod trailing_empty_array;
mod trait_bounds;
mod transmute;
mod transmuting_null;
mod try_err;
mod types;
mod undocumented_unsafe_blocks;
mod unicode;
@ -569,7 +562,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(len_zero::LenZero));
store.register_late_pass(|| Box::new(attrs::Attributes));
store.register_late_pass(|| Box::new(blocks_in_if_conditions::BlocksInIfConditions));
store.register_late_pass(|| Box::new(collapsible_match::CollapsibleMatch));
store.register_late_pass(|| Box::new(unicode::Unicode));
store.register_late_pass(|| Box::new(uninit_vec::UninitVec));
store.register_late_pass(|| Box::new(unit_hash::UnitHash));
@ -707,7 +699,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
);
store.register_late_pass(move || Box::new(pass_by_ref_or_value));
store.register_late_pass(|| Box::new(ref_option_ref::RefOptionRef));
store.register_late_pass(|| Box::new(try_err::TryErr));
store.register_late_pass(|| Box::new(bytecount::ByteCount));
store.register_late_pass(|| Box::new(infinite_iter::InfiniteIter));
store.register_late_pass(|| Box::new(inline_fn_without_body::InlineFnWithoutBody));
@ -819,7 +810,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(if_not_else::IfNotElse));
store.register_late_pass(|| Box::new(equatable_if_let::PatternEquality));
store.register_late_pass(|| Box::new(mut_mutex_lock::MutMutexLock));
store.register_late_pass(|| Box::new(match_on_vec_items::MatchOnVecItems));
store.register_late_pass(|| Box::new(manual_async_fn::ManualAsyncFn));
store.register_late_pass(|| Box::new(vec_resize_to_zero::VecResizeToZero));
store.register_late_pass(|| Box::new(panic_in_result_fn::PanicInResultFn));
@ -837,7 +827,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(repeat_once::RepeatOnce));
store.register_late_pass(|| Box::new(unwrap_in_result::UnwrapInResult));
store.register_late_pass(|| Box::new(self_assignment::SelfAssignment));
store.register_late_pass(|| Box::new(manual_unwrap_or::ManualUnwrapOr));
store.register_late_pass(|| Box::new(manual_ok_or::ManualOkOr));
store.register_late_pass(|| Box::new(float_equality_without_abs::FloatEqualityWithoutAbs));
store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned));
@ -856,7 +845,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});
store.register_late_pass(|| Box::new(redundant_slicing::RedundantSlicing));
store.register_late_pass(|| Box::new(from_str_radix_10::FromStrRadix10));
store.register_late_pass(|| Box::new(manual_map::ManualMap));
store.register_late_pass(move || Box::new(if_then_some_else_none::IfThenSomeElseNone::new(msrv)));
store.register_late_pass(|| Box::new(bool_assert_comparison::BoolAssertComparison));
store.register_early_pass(move || Box::new(module_style::ModStyle));
@ -882,7 +870,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch));
store.register_late_pass(move || Box::new(format_args::FormatArgs));
store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray));
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
@ -895,7 +882,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;

View file

@ -1,316 +0,0 @@
use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
use clippy_utils::{
can_move_expr_to_closure, in_constant, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id,
peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
};
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{
def::Res, Arm, BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Path,
QPath, UnsafeSource,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, SyntaxContext};
declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `match` which could be implemented using `map`
///
/// ### Why is this bad?
/// Using the `map` method is clearer and more concise.
///
/// ### Example
/// ```rust
/// match Some(0) {
/// Some(x) => Some(x + 1),
/// None => None,
/// };
/// ```
/// Use instead:
/// ```rust
/// Some(0).map(|x| x + 1);
/// ```
#[clippy::version = "1.52.0"]
pub MANUAL_MAP,
style,
"reimplementation of `map`"
}
declare_lint_pass!(ManualMap => [MANUAL_MAP]);
impl<'tcx> LateLintPass<'tcx> for ManualMap {
#[expect(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let (scrutinee, then_pat, then_body, else_pat, else_body) = match IfLetOrMatch::parse(cx, expr) {
Some(IfLetOrMatch::IfLet(scrutinee, pat, body, Some(r#else))) => (scrutinee, pat, body, None, r#else),
Some(IfLetOrMatch::Match(
scrutinee,
[arm1 @ Arm { guard: None, .. }, arm2 @ Arm { guard: None, .. }],
_,
)) => (scrutinee, arm1.pat, arm1.body, Some(arm2.pat), arm2.body),
_ => return,
};
if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) {
return;
}
let (scrutinee_ty, ty_ref_count, ty_mutability) =
peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::Option)
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option))
{
return;
}
let expr_ctxt = expr.span.ctxt();
let (some_expr, some_pat, pat_ref_count, is_wild_none) = match (
try_parse_pattern(cx, then_pat, expr_ctxt),
else_pat.map_or(Some(OptionPat::Wild), |p| try_parse_pattern(cx, p, expr_ctxt)),
) {
(Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => {
(else_body, pattern, ref_count, true)
},
(Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => {
(else_body, pattern, ref_count, false)
},
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) if is_none_expr(cx, else_body) => {
(then_body, pattern, ref_count, true)
},
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_body) => {
(then_body, pattern, ref_count, false)
},
_ => return,
};
// Top level or patterns aren't allowed in closures.
if matches!(some_pat.kind, PatKind::Or(_)) {
return;
}
let some_expr = match get_some_expr(cx, some_expr, false, expr_ctxt) {
Some(expr) => expr,
None => return,
};
// These two lints will go back and forth with each other.
if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit
&& !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id)
{
return;
}
// `map` won't perform any adjustments.
if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() {
return;
}
// Determine which binding mode to use.
let explicit_ref = some_pat.contains_explicit_ref_binding();
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
let as_ref_str = match binding_ref {
Some(Mutability::Mut) => ".as_mut()",
Some(Mutability::Not) => ".as_ref()",
None => "",
};
match can_move_expr_to_closure(cx, some_expr.expr) {
Some(captures) => {
// Check if captures the closure will need conflict with borrows made in the scrutinee.
// TODO: check all the references made in the scrutinee expression. This will require interacting
// with the borrow checker. Currently only `<local>[.<field>]*` is checked for.
if let Some(binding_ref_mutability) = binding_ref {
let e = peel_hir_expr_while(scrutinee, |e| match e.kind {
ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e),
_ => None,
});
if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind {
match captures.get(l) {
Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return,
Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => {
return;
},
Some(CaptureKind::Ref(Mutability::Not)) | None => (),
}
}
}
},
None => return,
};
let mut app = Applicability::MachineApplicable;
// Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or
// it's being passed by value.
let scrutinee = peel_hir_expr_refs(scrutinee).0;
let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
let scrutinee_str =
if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX {
format!("({})", scrutinee_str)
} else {
scrutinee_str.into()
};
let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind {
if_chain! {
if !some_expr.needs_unsafe_block;
if let Some(func) = can_pass_as_func(cx, id, some_expr.expr);
if func.span.ctxt() == some_expr.expr.span.ctxt();
then {
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
} else {
if path_to_local_id(some_expr.expr, id)
&& !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id)
&& binding_ref.is_some()
{
return;
}
// `ref` and `ref mut` annotations were handled earlier.
let annotation = if matches!(annotation, BindingAnnotation::Mutable) {
"mut "
} else {
""
};
let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
if some_expr.needs_unsafe_block {
format!("|{}{}| unsafe {{ {} }}", annotation, some_binding, expr_snip)
} else {
format!("|{}{}| {}", annotation, some_binding, expr_snip)
}
}
}
} else if !is_wild_none && explicit_ref.is_none() {
// TODO: handle explicit reference annotations.
let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0;
let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
if some_expr.needs_unsafe_block {
format!("|{}| unsafe {{ {} }}", pat_snip, expr_snip)
} else {
format!("|{}| {}", pat_snip, expr_snip)
}
} else {
// Refutable bindings and mixed reference annotations can't be handled by `map`.
return;
};
span_lint_and_sugg(
cx,
MANUAL_MAP,
expr.span,
"manual implementation of `Option::map`",
"try this",
if else_pat.is_none() && is_else_clause(cx.tcx, expr) {
format!("{{ {}{}.map({}) }}", scrutinee_str, as_ref_str, body_str)
} else {
format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str)
},
app,
);
}
}
// Checks whether the expression could be passed as a function, or whether a closure is needed.
// Returns the function to be passed to `map` if it exists.
fn can_pass_as_func<'tcx>(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
match expr.kind {
ExprKind::Call(func, [arg])
if path_to_local_id(arg, binding)
&& cx.typeck_results().expr_adjustments(arg).is_empty()
&& !type_is_unsafe_function(cx, cx.typeck_results().expr_ty(func).peel_refs()) =>
{
Some(func)
},
_ => None,
}
}
enum OptionPat<'a> {
Wild,
None,
Some {
// The pattern contained in the `Some` tuple.
pattern: &'a Pat<'a>,
// The number of references before the `Some` tuple.
// e.g. `&&Some(_)` has a ref count of 2.
ref_count: usize,
},
}
struct SomeExpr<'tcx> {
expr: &'tcx Expr<'tcx>,
needs_unsafe_block: bool,
}
// Try to parse into a recognized `Option` pattern.
// i.e. `_`, `None`, `Some(..)`, or a reference to any of those.
fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> {
fn f<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
ref_count: usize,
ctxt: SyntaxContext,
) -> Option<OptionPat<'tcx>> {
match pat.kind {
PatKind::Wild => Some(OptionPat::Wild),
PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt),
PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone) => Some(OptionPat::None),
PatKind::TupleStruct(ref qpath, [pattern], _)
if is_lang_ctor(cx, qpath, OptionSome) && pat.span.ctxt() == ctxt =>
{
Some(OptionPat::Some { pattern, ref_count })
},
_ => None,
}
}
f(cx, pat, 0, ctxt)
}
// Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.
fn get_some_expr<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
needs_unsafe_block: bool,
ctxt: SyntaxContext,
) -> Option<SomeExpr<'tcx>> {
// TODO: Allow more complex expressions.
match expr.kind {
ExprKind::Call(
Expr {
kind: ExprKind::Path(ref qpath),
..
},
[arg],
) if ctxt == expr.span.ctxt() && is_lang_ctor(cx, qpath, OptionSome) => Some(SomeExpr {
expr: arg,
needs_unsafe_block,
}),
ExprKind::Block(
Block {
stmts: [],
expr: Some(expr),
rules,
..
},
_,
) => get_some_expr(
cx,
expr,
needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
ctxt,
),
_ => None,
}
}
// Checks for the `None` value.
fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
}

View file

@ -1,123 +0,0 @@
use clippy_utils::consts::constant_simple;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::contains_return_break_continue_macro;
use clippy_utils::{in_constant, is_lang_ctor, path_to_local_id, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
use rustc_hir::{Arm, Expr, ExprKind, PatKind};
use rustc_lint::LintContext;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Finds patterns that reimplement `Option::unwrap_or` or `Result::unwrap_or`.
///
/// ### Why is this bad?
/// Concise code helps focusing on behavior instead of boilerplate.
///
/// ### Example
/// ```rust
/// let foo: Option<i32> = None;
/// match foo {
/// Some(v) => v,
/// None => 1,
/// };
/// ```
///
/// Use instead:
/// ```rust
/// let foo: Option<i32> = None;
/// foo.unwrap_or(1);
/// ```
#[clippy::version = "1.49.0"]
pub MANUAL_UNWRAP_OR,
complexity,
"finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`"
}
declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]);
impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOr {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) {
return;
}
lint_manual_unwrap_or(cx, expr);
}
}
fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> {
if_chain! {
if arms.len() == 2;
if arms.iter().all(|arm| arm.guard.is_none());
if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| {
match arm.pat.kind {
PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
PatKind::TupleStruct(ref qpath, [pat], _) =>
matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr),
_ => false,
}
});
let unwrap_arm = &arms[1 - idx];
if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind;
if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk);
if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind;
if path_to_local_id(unwrap_arm.body, binding_hir_id);
if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty();
if !contains_return_break_continue_macro(or_arm.body);
then {
Some(or_arm)
} else {
None
}
}
}
if_chain! {
if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind;
let ty = cx.typeck_results().expr_ty(scrutinee);
if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) {
Some("Option")
} else if is_type_diagnostic_item(cx, ty, sym::Result) {
Some("Result")
} else {
None
};
if let Some(or_arm) = applicable_or_arm(cx, match_arms);
if let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span);
if let Some(indent) = indent_of(cx, expr.span);
if constant_simple(cx, cx.typeck_results(), or_arm.body).is_some();
then {
let reindented_or_body =
reindent_multiline(or_body_snippet.into(), true, Some(indent));
let suggestion = if scrutinee.span.from_expansion() {
// we don't want parentheses around macro, e.g. `(some_macro!()).unwrap_or(0)`
sugg::Sugg::hir_with_macro_callsite(cx, scrutinee, "..")
}
else {
sugg::Sugg::hir(cx, scrutinee, "..").maybe_par()
};
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR, expr.span,
&format!("this pattern reimplements `{}::unwrap_or`", ty_name),
"replace with",
format!(
"{}.unwrap_or({})",
suggestion,
reindented_or_body,
),
Applicability::MachineApplicable,
);
}
}
}

View file

@ -1,104 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, LangItem, MatchSource};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Checks for `match vec[idx]` or `match vec[n..m]`.
///
/// ### Why is this bad?
/// This can panic at runtime.
///
/// ### Example
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Bad
/// match arr[idx] {
/// 0 => println!("{}", 0),
/// 1 => println!("{}", 3),
/// _ => {},
/// }
/// ```
/// Use instead:
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Good
/// match arr.get(idx) {
/// Some(0) => println!("{}", 0),
/// Some(1) => println!("{}", 3),
/// _ => {},
/// }
/// ```
#[clippy::version = "1.45.0"]
pub MATCH_ON_VEC_ITEMS,
pedantic,
"matching on vector elements can panic"
}
declare_lint_pass!(MatchOnVecItems => [MATCH_ON_VEC_ITEMS]);
impl<'tcx> LateLintPass<'tcx> for MatchOnVecItems {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! {
if !in_external_macro(cx.sess(), expr.span);
if let ExprKind::Match(match_expr, _, MatchSource::Normal) = expr.kind;
if let Some(idx_expr) = is_vec_indexing(cx, match_expr);
if let ExprKind::Index(vec, idx) = idx_expr.kind;
then {
// FIXME: could be improved to suggest surrounding every pattern with Some(_),
// but only when `or_patterns` are stabilized.
span_lint_and_sugg(
cx,
MATCH_ON_VEC_ITEMS,
match_expr.span,
"indexing into a vector may panic",
"try this",
format!(
"{}.get({})",
snippet(cx, vec.span, ".."),
snippet(cx, idx.span, "..")
),
Applicability::MaybeIncorrect
);
}
}
}
}
fn is_vec_indexing<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! {
if let ExprKind::Index(array, index) = expr.kind;
if is_vector(cx, array);
if !is_full_range(cx, index);
then {
return Some(expr);
}
}
None
}
fn is_vector(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
let ty = ty.peel_refs();
is_type_diagnostic_item(cx, ty, sym::Vec)
}
fn is_full_range(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
let ty = ty.peel_refs();
is_type_lang_item(cx, ty, LangItem::RangeFull)
}

View file

@ -6,68 +6,28 @@ use if_chain::if_chain;
use rustc_errors::MultiSpan;
use rustc_hir::LangItem::OptionNone;
use rustc_hir::{Arm, Expr, Guard, HirId, Let, Pat, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_lint::LateContext;
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
/// Finds nested `match` or `if let` expressions where the patterns may be "collapsed" together
/// without adding any branches.
///
/// Note that this lint is not intended to find _all_ cases where nested match patterns can be merged, but only
/// cases where merging would most likely make the code more readable.
///
/// ### Why is this bad?
/// It is unnecessarily verbose and complex.
///
/// ### Example
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(n) => match n {
/// Ok(n) => n,
/// _ => return,
/// }
/// None => return,
/// };
/// }
/// ```
/// Use instead:
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(Ok(n)) => n,
/// _ => return,
/// };
/// }
/// ```
#[clippy::version = "1.50.0"]
pub COLLAPSIBLE_MATCH,
style,
"Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
}
use super::COLLAPSIBLE_MATCH;
declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]);
impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
match IfLetOrMatch::parse(cx, expr) {
Some(IfLetOrMatch::Match(_, arms, _)) => {
if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
for arm in arms {
check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body));
}
}
},
Some(IfLetOrMatch::IfLet(_, pat, body, els)) => {
check_arm(cx, false, pat, body, None, els);
},
None => {},
pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) {
for arm in arms {
check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body));
}
}
}
pub(super) fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
body: &'tcx Expr<'_>,
else_expr: Option<&'tcx Expr<'_>>,
) {
check_arm(cx, false, pat, body, None, else_expr);
}
fn check_arm<'tcx>(
cx: &LateContext<'tcx>,
outer_is_match: bool,

View file

@ -0,0 +1,306 @@
use crate::{map_unit_fn::OPTION_MAP_UNIT_FN, matches::MATCH_AS_REF};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function};
use clippy_utils::{
can_move_expr_to_closure, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id, peel_blocks,
peel_hir_expr_refs, peel_hir_expr_while, CaptureKind,
};
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{
def::Res, Arm, BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Path,
QPath, UnsafeSource,
};
use rustc_lint::LateContext;
use rustc_span::{sym, SyntaxContext};
use super::MANUAL_MAP;
pub(super) fn check_match<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
scrutinee: &'tcx Expr<'_>,
arms: &'tcx [Arm<'_>],
) {
if let [arm1, arm2] = arms
&& arm1.guard.is_none()
&& arm2.guard.is_none()
{
check(cx, expr, scrutinee, arm1.pat, arm1.body, Some(arm2.pat), arm2.body);
}
}
pub(super) fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
let_pat: &'tcx Pat<'_>,
let_expr: &'tcx Expr<'_>,
then_expr: &'tcx Expr<'_>,
else_expr: &'tcx Expr<'_>,
) {
check(cx, expr, let_expr, let_pat, then_expr, None, else_expr);
}
#[expect(clippy::too_many_lines)]
fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
scrutinee: &'tcx Expr<'_>,
then_pat: &'tcx Pat<'_>,
then_body: &'tcx Expr<'_>,
else_pat: Option<&'tcx Pat<'_>>,
else_body: &'tcx Expr<'_>,
) {
let (scrutinee_ty, ty_ref_count, ty_mutability) =
peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::Option)
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option))
{
return;
}
let expr_ctxt = expr.span.ctxt();
let (some_expr, some_pat, pat_ref_count, is_wild_none) = match (
try_parse_pattern(cx, then_pat, expr_ctxt),
else_pat.map_or(Some(OptionPat::Wild), |p| try_parse_pattern(cx, p, expr_ctxt)),
) {
(Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => {
(else_body, pattern, ref_count, true)
},
(Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count })) if is_none_expr(cx, then_body) => {
(else_body, pattern, ref_count, false)
},
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild)) if is_none_expr(cx, else_body) => {
(then_body, pattern, ref_count, true)
},
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None)) if is_none_expr(cx, else_body) => {
(then_body, pattern, ref_count, false)
},
_ => return,
};
// Top level or patterns aren't allowed in closures.
if matches!(some_pat.kind, PatKind::Or(_)) {
return;
}
let some_expr = match get_some_expr(cx, some_expr, false, expr_ctxt) {
Some(expr) => expr,
None => return,
};
// These two lints will go back and forth with each other.
if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit
&& !is_lint_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id)
{
return;
}
// `map` won't perform any adjustments.
if !cx.typeck_results().expr_adjustments(some_expr.expr).is_empty() {
return;
}
// Determine which binding mode to use.
let explicit_ref = some_pat.contains_explicit_ref_binding();
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
let as_ref_str = match binding_ref {
Some(Mutability::Mut) => ".as_mut()",
Some(Mutability::Not) => ".as_ref()",
None => "",
};
match can_move_expr_to_closure(cx, some_expr.expr) {
Some(captures) => {
// Check if captures the closure will need conflict with borrows made in the scrutinee.
// TODO: check all the references made in the scrutinee expression. This will require interacting
// with the borrow checker. Currently only `<local>[.<field>]*` is checked for.
if let Some(binding_ref_mutability) = binding_ref {
let e = peel_hir_expr_while(scrutinee, |e| match e.kind {
ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) => Some(e),
_ => None,
});
if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(l), .. })) = e.kind {
match captures.get(l) {
Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return,
Some(CaptureKind::Ref(Mutability::Not)) if binding_ref_mutability == Mutability::Mut => {
return;
},
Some(CaptureKind::Ref(Mutability::Not)) | None => (),
}
}
}
},
None => return,
};
let mut app = Applicability::MachineApplicable;
// Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or
// it's being passed by value.
let scrutinee = peel_hir_expr_refs(scrutinee).0;
let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
let scrutinee_str = if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX {
format!("({})", scrutinee_str)
} else {
scrutinee_str.into()
};
let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind {
if_chain! {
if !some_expr.needs_unsafe_block;
if let Some(func) = can_pass_as_func(cx, id, some_expr.expr);
if func.span.ctxt() == some_expr.expr.span.ctxt();
then {
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
} else {
if path_to_local_id(some_expr.expr, id)
&& !is_lint_allowed(cx, MATCH_AS_REF, expr.hir_id)
&& binding_ref.is_some()
{
return;
}
// `ref` and `ref mut` annotations were handled earlier.
let annotation = if matches!(annotation, BindingAnnotation::Mutable) {
"mut "
} else {
""
};
let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
if some_expr.needs_unsafe_block {
format!("|{}{}| unsafe {{ {} }}", annotation, some_binding, expr_snip)
} else {
format!("|{}{}| {}", annotation, some_binding, expr_snip)
}
}
}
} else if !is_wild_none && explicit_ref.is_none() {
// TODO: handle explicit reference annotations.
let pat_snip = snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app).0;
let expr_snip = snippet_with_context(cx, some_expr.expr.span, expr_ctxt, "..", &mut app).0;
if some_expr.needs_unsafe_block {
format!("|{}| unsafe {{ {} }}", pat_snip, expr_snip)
} else {
format!("|{}| {}", pat_snip, expr_snip)
}
} else {
// Refutable bindings and mixed reference annotations can't be handled by `map`.
return;
};
span_lint_and_sugg(
cx,
MANUAL_MAP,
expr.span,
"manual implementation of `Option::map`",
"try this",
if else_pat.is_none() && is_else_clause(cx.tcx, expr) {
format!("{{ {}{}.map({}) }}", scrutinee_str, as_ref_str, body_str)
} else {
format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str)
},
app,
);
}
// Checks whether the expression could be passed as a function, or whether a closure is needed.
// Returns the function to be passed to `map` if it exists.
fn can_pass_as_func<'tcx>(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
match expr.kind {
ExprKind::Call(func, [arg])
if path_to_local_id(arg, binding)
&& cx.typeck_results().expr_adjustments(arg).is_empty()
&& !type_is_unsafe_function(cx, cx.typeck_results().expr_ty(func).peel_refs()) =>
{
Some(func)
},
_ => None,
}
}
enum OptionPat<'a> {
Wild,
None,
Some {
// The pattern contained in the `Some` tuple.
pattern: &'a Pat<'a>,
// The number of references before the `Some` tuple.
// e.g. `&&Some(_)` has a ref count of 2.
ref_count: usize,
},
}
struct SomeExpr<'tcx> {
expr: &'tcx Expr<'tcx>,
needs_unsafe_block: bool,
}
// Try to parse into a recognized `Option` pattern.
// i.e. `_`, `None`, `Some(..)`, or a reference to any of those.
fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> {
fn f<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
ref_count: usize,
ctxt: SyntaxContext,
) -> Option<OptionPat<'tcx>> {
match pat.kind {
PatKind::Wild => Some(OptionPat::Wild),
PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt),
PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone) => Some(OptionPat::None),
PatKind::TupleStruct(ref qpath, [pattern], _)
if is_lang_ctor(cx, qpath, OptionSome) && pat.span.ctxt() == ctxt =>
{
Some(OptionPat::Some { pattern, ref_count })
},
_ => None,
}
}
f(cx, pat, 0, ctxt)
}
// Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.
fn get_some_expr<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
needs_unsafe_block: bool,
ctxt: SyntaxContext,
) -> Option<SomeExpr<'tcx>> {
// TODO: Allow more complex expressions.
match expr.kind {
ExprKind::Call(
Expr {
kind: ExprKind::Path(ref qpath),
..
},
[arg],
) if ctxt == expr.span.ctxt() && is_lang_ctor(cx, qpath, OptionSome) => Some(SomeExpr {
expr: arg,
needs_unsafe_block,
}),
ExprKind::Block(
Block {
stmts: [],
expr: Some(expr),
rules,
..
},
_,
) => get_some_expr(
cx,
expr,
needs_unsafe_block || *rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
ctxt,
),
_ => None,
}
}
// Checks for the `None` value.
fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone))
}

View file

@ -0,0 +1,83 @@
use clippy_utils::consts::constant_simple;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::contains_return_break_continue_macro;
use clippy_utils::{is_lang_ctor, path_to_local_id, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
use rustc_hir::{Arm, Expr, PatKind};
use rustc_lint::LateContext;
use rustc_span::sym;
use super::MANUAL_UNWRAP_OR;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
let ty = cx.typeck_results().expr_ty(scrutinee);
if_chain! {
if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) {
Some("Option")
} else if is_type_diagnostic_item(cx, ty, sym::Result) {
Some("Result")
} else {
None
};
if let Some(or_arm) = applicable_or_arm(cx, arms);
if let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span);
if let Some(indent) = indent_of(cx, expr.span);
if constant_simple(cx, cx.typeck_results(), or_arm.body).is_some();
then {
let reindented_or_body =
reindent_multiline(or_body_snippet.into(), true, Some(indent));
let suggestion = if scrutinee.span.from_expansion() {
// we don't want parentheses around macro, e.g. `(some_macro!()).unwrap_or(0)`
sugg::Sugg::hir_with_macro_callsite(cx, scrutinee, "..")
}
else {
sugg::Sugg::hir(cx, scrutinee, "..").maybe_par()
};
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR, expr.span,
&format!("this pattern reimplements `{}::unwrap_or`", ty_name),
"replace with",
format!(
"{}.unwrap_or({})",
suggestion,
reindented_or_body,
),
Applicability::MachineApplicable,
);
}
}
}
fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> {
if_chain! {
if arms.len() == 2;
if arms.iter().all(|arm| arm.guard.is_none());
if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| {
match arm.pat.kind {
PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone),
PatKind::TupleStruct(ref qpath, [pat], _) =>
matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr),
_ => false,
}
});
let unwrap_arm = &arms[1 - idx];
if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind;
if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk);
if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind;
if path_to_local_id(unwrap_arm.body, binding_hir_id);
if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty();
if !contains_return_break_continue_macro(or_arm.body);
then {
Some(or_arm)
} else {
None
}
}
}

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_wild;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{higher, is_wild};
use rustc_ast::{Attribute, LitKind};
use rustc_errors::Applicability;
use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat};
@ -11,22 +11,24 @@ use rustc_span::source_map::Spanned;
use super::MATCH_LIKE_MATCHES_MACRO;
/// Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!`
pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::IfLet {
let_pat,
pub(crate) fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
let_pat: &'tcx Pat<'_>,
let_expr: &'tcx Expr<'_>,
then_expr: &'tcx Expr<'_>,
else_expr: &'tcx Expr<'_>,
) {
find_matches_sugg(
cx,
let_expr,
if_then,
if_else: Some(if_else),
}) = higher::IfLet::hir(cx, expr)
{
find_matches_sugg(
cx,
let_expr,
IntoIterator::into_iter([(&[][..], Some(let_pat), if_then, None), (&[][..], None, if_else, None)]),
expr,
true,
);
}
IntoIterator::into_iter([
(&[][..], Some(let_pat), then_expr, None),
(&[][..], None, else_expr, None),
]),
expr,
true,
);
}
pub(super) fn check_match<'tcx>(

View file

@ -0,0 +1,61 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, LangItem};
use rustc_lint::LateContext;
use rustc_span::sym;
use super::MATCH_ON_VEC_ITEMS;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>) {
if_chain! {
if let Some(idx_expr) = is_vec_indexing(cx, scrutinee);
if let ExprKind::Index(vec, idx) = idx_expr.kind;
then {
// FIXME: could be improved to suggest surrounding every pattern with Some(_),
// but only when `or_patterns` are stabilized.
span_lint_and_sugg(
cx,
MATCH_ON_VEC_ITEMS,
scrutinee.span,
"indexing into a vector may panic",
"try this",
format!(
"{}.get({})",
snippet(cx, vec.span, ".."),
snippet(cx, idx.span, "..")
),
Applicability::MaybeIncorrect
);
}
}
}
fn is_vec_indexing<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! {
if let ExprKind::Index(array, index) = expr.kind;
if is_vector(cx, array);
if !is_full_range(cx, index);
then {
return Some(expr);
}
}
None
}
fn is_vector(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
let ty = ty.peel_refs();
is_type_diagnostic_item(cx, ty, sym::Vec)
}
fn is_full_range(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
let ty = ty.peel_refs();
is_type_lang_item(cx, ty, LangItem::RangeFull)
}

View file

@ -3,46 +3,13 @@ use clippy_utils::ty::is_type_diagnostic_item;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, Visitor};
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_hir::{Arm, Expr, ExprKind, PatKind};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Symbol;
use rustc_span::{sym, Span};
declare_clippy_lint! {
/// ### What it does
/// Checks for `match` expressions modifying the case of a string with non-compliant arms
///
/// ### Why is this bad?
/// The arm is unreachable, which is likely a mistake
///
/// ### Example
/// ```rust
/// # let text = "Foo";
/// match &*text.to_ascii_lowercase() {
/// "foo" => {},
/// "Bar" => {},
/// _ => {},
/// }
/// ```
/// Use instead:
/// ```rust
/// # let text = "Foo";
/// match &*text.to_ascii_lowercase() {
/// "foo" => {},
/// "bar" => {},
/// _ => {},
/// }
/// ```
#[clippy::version = "1.58.0"]
pub MATCH_STR_CASE_MISMATCH,
correctness,
"creation of a case altering match expression with non-compliant arms"
}
declare_lint_pass!(MatchStrCaseMismatch => [MATCH_STR_CASE_MISMATCH]);
use super::MATCH_STR_CASE_MISMATCH;
#[derive(Debug)]
enum CaseMethod {
@ -52,25 +19,21 @@ enum CaseMethod {
AsciiUppercase,
}
impl<'tcx> LateLintPass<'tcx> for MatchStrCaseMismatch {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if !in_external_macro(cx.tcx.sess, expr.span);
if let ExprKind::Match(match_expr, arms, MatchSource::Normal) = expr.kind;
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(match_expr).kind();
if let ty::Str = ty.kind();
then {
let mut visitor = MatchExprVisitor {
cx,
case_method: None,
};
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) {
if_chain! {
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(scrutinee).kind();
if let ty::Str = ty.kind();
then {
let mut visitor = MatchExprVisitor {
cx,
case_method: None,
};
visitor.visit_expr(match_expr);
visitor.visit_expr(scrutinee);
if let Some(case_method) = visitor.case_method {
if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) {
lint(cx, &case_method, bad_case_span, bad_case_sym.as_str());
}
if let Some(case_method) = visitor.case_method {
if let Some((bad_case_span, bad_case_sym)) = verify_case(&case_method, arms) {
lint(cx, &case_method, bad_case_span, bad_case_sym.as_str());
}
}
}

View file

@ -1,26 +1,34 @@
use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context};
use clippy_utils::{meets_msrv, msrvs};
use clippy_utils::{higher, in_constant, meets_msrv, msrvs};
use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat};
use rustc_lexer::{tokenize, TokenKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{Span, SpanData, SyntaxContext};
mod collapsible_match;
mod infallible_destructuring_match;
mod manual_map;
mod manual_unwrap_or;
mod match_as_ref;
mod match_bool;
mod match_like_matches;
mod match_on_vec_items;
mod match_ref_pats;
mod match_same_arms;
mod match_single_binding;
mod match_str_case_mismatch;
mod match_wild_enum;
mod match_wild_err_arm;
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 try_err;
mod wild_in_or_pats;
declare_clippy_lint! {
@ -610,6 +618,274 @@ declare_clippy_lint! {
"`match` or match-like `if let` that are unnecessary"
}
declare_clippy_lint! {
/// ### What it does
/// Finds nested `match` or `if let` expressions where the patterns may be "collapsed" together
/// without adding any branches.
///
/// Note that this lint is not intended to find _all_ cases where nested match patterns can be merged, but only
/// cases where merging would most likely make the code more readable.
///
/// ### Why is this bad?
/// It is unnecessarily verbose and complex.
///
/// ### Example
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(n) => match n {
/// Ok(n) => n,
/// _ => return,
/// }
/// None => return,
/// };
/// }
/// ```
/// Use instead:
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(Ok(n)) => n,
/// _ => return,
/// };
/// }
/// ```
#[clippy::version = "1.50.0"]
pub COLLAPSIBLE_MATCH,
style,
"Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
}
declare_clippy_lint! {
/// ### What it does
/// Finds patterns that reimplement `Option::unwrap_or` or `Result::unwrap_or`.
///
/// ### Why is this bad?
/// Concise code helps focusing on behavior instead of boilerplate.
///
/// ### Example
/// ```rust
/// let foo: Option<i32> = None;
/// match foo {
/// Some(v) => v,
/// None => 1,
/// };
/// ```
///
/// Use instead:
/// ```rust
/// let foo: Option<i32> = None;
/// foo.unwrap_or(1);
/// ```
#[clippy::version = "1.49.0"]
pub MANUAL_UNWRAP_OR,
complexity,
"finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `match vec[idx]` or `match vec[n..m]`.
///
/// ### Why is this bad?
/// This can panic at runtime.
///
/// ### Example
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Bad
/// match arr[idx] {
/// 0 => println!("{}", 0),
/// 1 => println!("{}", 3),
/// _ => {},
/// }
/// ```
/// Use instead:
/// ```rust, no_run
/// let arr = vec![0, 1, 2, 3];
/// let idx = 1;
///
/// // Good
/// match arr.get(idx) {
/// Some(0) => println!("{}", 0),
/// Some(1) => println!("{}", 3),
/// _ => {},
/// }
/// ```
#[clippy::version = "1.45.0"]
pub MATCH_ON_VEC_ITEMS,
pedantic,
"matching on vector elements can panic"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `match` expressions modifying the case of a string with non-compliant arms
///
/// ### Why is this bad?
/// The arm is unreachable, which is likely a mistake
///
/// ### Example
/// ```rust
/// # let text = "Foo";
/// match &*text.to_ascii_lowercase() {
/// "foo" => {},
/// "Bar" => {},
/// _ => {},
/// }
/// ```
/// Use instead:
/// ```rust
/// # let text = "Foo";
/// match &*text.to_ascii_lowercase() {
/// "foo" => {},
/// "bar" => {},
/// _ => {},
/// }
/// ```
#[clippy::version = "1.58.0"]
pub MATCH_STR_CASE_MISMATCH,
correctness,
"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"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `Err(x)?`.
///
/// ### Why is this bad?
/// The `?` operator is designed to allow calls that
/// can fail to be easily chained. For example, `foo()?.bar()` or
/// `foo(bar()?)`. Because `Err(x)?` can't be used that way (it will
/// always return), it is more clear to write `return Err(x)`.
///
/// ### Example
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// Err("failed")?;
/// }
/// Ok(0)
/// }
/// ```
/// Could be written:
///
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// return Err("failed".into());
/// }
/// Ok(0)
/// }
/// ```
#[clippy::version = "1.38.0"]
pub TRY_ERR,
restriction,
"return errors explicitly rather than hiding them behind a `?`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `match` which could be implemented using `map`
///
/// ### Why is this bad?
/// Using the `map` method is clearer and more concise.
///
/// ### Example
/// ```rust
/// match Some(0) {
/// Some(x) => Some(x + 1),
/// None => None,
/// };
/// ```
/// Use instead:
/// ```rust
/// Some(0).map(|x| x + 1);
/// ```
#[clippy::version = "1.52.0"]
pub MANUAL_MAP,
style,
"reimplementation of `map`"
}
#[derive(Default)]
pub struct Matches {
msrv: Option<RustcVersion>,
@ -644,19 +920,42 @@ impl_lint_pass!(Matches => [
MATCH_LIKE_MATCHES_MACRO,
MATCH_SAME_ARMS,
NEEDLESS_MATCH,
COLLAPSIBLE_MATCH,
MANUAL_UNWRAP_OR,
MATCH_ON_VEC_ITEMS,
MATCH_STR_CASE_MISMATCH,
SIGNIFICANT_DROP_IN_SCRUTINEE,
TRY_ERR,
MANUAL_MAP,
]);
impl<'tcx> LateLintPass<'tcx> for Matches {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
if in_external_macro(cx.sess(), expr.span) {
return;
}
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 !contains_cfg_arm(cx, expr, ex, arms) {
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 {
// These don't depend on a relationship between multiple arms
match_wild_err_arm::check(cx, ex, arms);
wild_in_or_pats::check(cx, arms);
}
if source == MatchSource::TryDesugar {
try_err::check(cx, expr, ex);
}
if !from_expansion && !contains_cfg_arm(cx, expr, ex, arms) {
if source == MatchSource::Normal {
if !(meets_msrv(self.msrv, msrvs::MATCHES_MACRO)
&& match_like_matches::check_match(cx, expr, ex, arms))
@ -671,6 +970,13 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
match_wild_enum::check(cx, ex, arms);
match_as_ref::check(cx, ex, arms, expr);
needless_match::check_match(cx, ex, arms, expr);
match_on_vec_items::check(cx, ex);
match_str_case_mismatch::check(cx, ex, arms);
if !in_constant(cx, expr.hir_id) {
manual_unwrap_or::check(cx, expr, ex, arms);
manual_map::check_match(cx, expr, ex, arms);
}
if self.infallible_destructuring_match_linted {
self.infallible_destructuring_match_linted = false;
@ -680,16 +986,35 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
}
match_ref_pats::check(cx, ex, arms.iter().map(|el| el.pat), expr);
}
// These don't depend on a relationship between multiple arms
match_wild_err_arm::check(cx, ex, arms);
wild_in_or_pats::check(cx, arms);
} else {
if meets_msrv(self.msrv, msrvs::MATCHES_MACRO) {
match_like_matches::check(cx, expr);
} else if let Some(if_let) = higher::IfLet::hir(cx, expr) {
collapsible_match::check_if_let(cx, if_let.let_pat, if_let.if_then, if_let.if_else);
if !from_expansion {
if let Some(else_expr) = if_let.if_else {
if meets_msrv(self.msrv, msrvs::MATCHES_MACRO) {
match_like_matches::check_if_let(
cx,
expr,
if_let.let_pat,
if_let.let_expr,
if_let.if_then,
else_expr,
);
}
if !in_constant(cx, expr.hir_id) {
manual_map::check_if_let(cx, expr, if_let.let_pat, if_let.let_expr, if_let.if_then, else_expr);
}
}
redundant_pattern_match::check_if_let(
cx,
expr,
if_let.let_pat,
if_let.let_expr,
if_let.if_else.is_some(),
);
needless_match::check_if_let(cx, expr, &if_let);
}
} else if !from_expansion {
redundant_pattern_match::check(cx, expr);
needless_match::check(cx, expr);
}
}

View file

@ -47,20 +47,18 @@ pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>],
/// some_enum
/// }
/// ```
pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) {
if let Some(ref if_let) = higher::IfLet::hir(cx, ex) {
if !is_else_clause(cx.tcx, ex) && expr_ty_matches_p_ty(cx, if_let.let_expr, ex) && check_if_let(cx, if_let) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
NEEDLESS_MATCH,
ex.span,
"this if-let expression is unnecessary",
"replace it with",
snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
applicability,
);
}
pub(crate) fn check_if_let<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'_>, if_let: &higher::IfLet<'tcx>) {
if !is_else_clause(cx.tcx, ex) && expr_ty_matches_p_ty(cx, if_let.let_expr, ex) && check_if_let_inner(cx, if_let) {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
NEEDLESS_MATCH,
ex.span,
"this if-let expression is unnecessary",
"replace it with",
snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(),
applicability,
);
}
}
@ -77,7 +75,7 @@ fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>])
true
}
fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
fn check_if_let_inner(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
if let Some(if_else) = if_let.if_else {
if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) {
return false;
@ -85,7 +83,7 @@ fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool {
// Recursively check for each `else if let` phrase,
if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) {
return check_if_let(cx, nested_if_let);
return check_if_let_inner(cx, nested_if_let);
}
if matches!(if_else.kind, ExprKind::Block(..)) {

View file

@ -18,19 +18,21 @@ use rustc_middle::ty::{self, subst::GenericArgKind, DefIdTree, Ty};
use rustc_span::sym;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::IfLet {
if_else,
let_pat,
let_expr,
..
}) = higher::IfLet::hir(cx, expr)
{
find_sugg_for_if_let(cx, expr, let_pat, let_expr, "if", if_else.is_some());
} else if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
find_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false);
}
}
pub(super) fn check_if_let<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'_>,
pat: &'tcx Pat<'_>,
scrutinee: &'tcx Expr<'_>,
has_else: bool,
) {
find_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else);
}
// Extract the generic arguments out of a type
fn try_get_generic_ty(ty: Ty<'_>, index: usize) -> Option<Ty<'_>> {
if_chain! {

View file

@ -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"
}
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) {
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> {

View file

@ -0,0 +1,145 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{get_parent_expr, is_lang_ctor, match_def_path, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::LangItem::ResultErr;
use rustc_hir::{Expr, ExprKind, LangItem, MatchSource, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::{hygiene, sym};
use super::TRY_ERR;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutinee: &'tcx Expr<'_>) {
// Looks for a structure like this:
// match ::std::ops::Try::into_result(Err(5)) {
// ::std::result::Result::Err(err) =>
// #[allow(unreachable_code)]
// return ::std::ops::Try::from_error(::std::convert::From::from(err)),
// ::std::result::Result::Ok(val) =>
// #[allow(unreachable_code)]
// val,
// };
if_chain! {
if let ExprKind::Call(match_fun, try_args) = scrutinee.kind;
if let ExprKind::Path(ref match_fun_path) = match_fun.kind;
if matches!(match_fun_path, QPath::LangItem(LangItem::TryTraitBranch, ..));
if let Some(try_arg) = try_args.get(0);
if let ExprKind::Call(err_fun, err_args) = try_arg.kind;
if let Some(err_arg) = err_args.get(0);
if let ExprKind::Path(ref err_fun_path) = err_fun.kind;
if is_lang_ctor(cx, err_fun_path, ResultErr);
if let Some(return_ty) = find_return_type(cx, &expr.kind);
then {
let prefix;
let suffix;
let err_ty;
if let Some(ty) = result_error_type(cx, return_ty) {
prefix = "Err(";
suffix = ")";
err_ty = ty;
} else if let Some(ty) = poll_result_error_type(cx, return_ty) {
prefix = "Poll::Ready(Err(";
suffix = "))";
err_ty = ty;
} else if let Some(ty) = poll_option_result_error_type(cx, return_ty) {
prefix = "Poll::Ready(Some(Err(";
suffix = ")))";
err_ty = ty;
} else {
return;
};
let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
let span = hygiene::walk_chain(err_arg.span, try_arg.span.ctxt());
let mut applicability = Applicability::MachineApplicable;
let origin_snippet = snippet_with_applicability(cx, span, "_", &mut applicability);
let ret_prefix = if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::Ret(_))) {
"" // already returns
} else {
"return "
};
let suggestion = if err_ty == expr_err_ty {
format!("{}{}{}{}", ret_prefix, prefix, origin_snippet, suffix)
} else {
format!("{}{}{}.into(){}", ret_prefix, prefix, origin_snippet, suffix)
};
span_lint_and_sugg(
cx,
TRY_ERR,
expr.span,
"returning an `Err(_)` with the `?` operator",
"try this",
suggestion,
applicability,
);
}
}
}
/// Finds function return type by examining return expressions in match arms.
fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
for arm in arms.iter() {
if let ExprKind::Ret(Some(ret)) = arm.body.kind {
return Some(cx.typeck_results().expr_ty(ret));
}
}
}
None
}
/// Extracts the error type from Result<T, E>.
fn result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(_, subst) = ty.kind();
if is_type_diagnostic_item(cx, ty, sym::Result);
then {
Some(subst.type_at(1))
} else {
None
}
}
}
/// Extracts the error type from Poll<Result<T, E>>.
fn poll_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(def, subst) = ty.kind();
if match_def_path(cx, def.did(), &paths::POLL);
let ready_ty = subst.type_at(0);
if let ty::Adt(ready_def, ready_subst) = ready_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Result, ready_def.did());
then {
Some(ready_subst.type_at(1))
} else {
None
}
}
}
/// Extracts the error type from Poll<Option<Result<T, E>>>.
fn poll_option_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(def, subst) = ty.kind();
if match_def_path(cx, def.did(), &paths::POLL);
let ready_ty = subst.type_at(0);
if let ty::Adt(ready_def, ready_subst) = ready_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Option, ready_def.did());
let some_ty = ready_subst.type_at(0);
if let ty::Adt(some_def, some_subst) = some_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Result, some_def.did());
then {
Some(some_subst.type_at(1))
} else {
None
}
}
}

View file

@ -1,186 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{get_parent_expr, is_lang_ctor, match_def_path, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::LangItem::ResultErr;
use rustc_hir::{Expr, ExprKind, LangItem, MatchSource, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{hygiene, sym};
declare_clippy_lint! {
/// ### What it does
/// Checks for usages of `Err(x)?`.
///
/// ### Why is this bad?
/// The `?` operator is designed to allow calls that
/// can fail to be easily chained. For example, `foo()?.bar()` or
/// `foo(bar()?)`. Because `Err(x)?` can't be used that way (it will
/// always return), it is more clear to write `return Err(x)`.
///
/// ### Example
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// Err("failed")?;
/// }
/// Ok(0)
/// }
/// ```
/// Could be written:
///
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// return Err("failed".into());
/// }
/// Ok(0)
/// }
/// ```
#[clippy::version = "1.38.0"]
pub TRY_ERR,
restriction,
"return errors explicitly rather than hiding them behind a `?`"
}
declare_lint_pass!(TryErr => [TRY_ERR]);
impl<'tcx> LateLintPass<'tcx> for TryErr {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// Looks for a structure like this:
// match ::std::ops::Try::into_result(Err(5)) {
// ::std::result::Result::Err(err) =>
// #[allow(unreachable_code)]
// return ::std::ops::Try::from_error(::std::convert::From::from(err)),
// ::std::result::Result::Ok(val) =>
// #[allow(unreachable_code)]
// val,
// };
if_chain! {
if !in_external_macro(cx.tcx.sess, expr.span);
if let ExprKind::Match(match_arg, _, MatchSource::TryDesugar) = expr.kind;
if let ExprKind::Call(match_fun, try_args) = match_arg.kind;
if let ExprKind::Path(ref match_fun_path) = match_fun.kind;
if matches!(match_fun_path, QPath::LangItem(LangItem::TryTraitBranch, ..));
if let Some(try_arg) = try_args.get(0);
if let ExprKind::Call(err_fun, err_args) = try_arg.kind;
if let Some(err_arg) = err_args.get(0);
if let ExprKind::Path(ref err_fun_path) = err_fun.kind;
if is_lang_ctor(cx, err_fun_path, ResultErr);
if let Some(return_ty) = find_return_type(cx, &expr.kind);
then {
let prefix;
let suffix;
let err_ty;
if let Some(ty) = result_error_type(cx, return_ty) {
prefix = "Err(";
suffix = ")";
err_ty = ty;
} else if let Some(ty) = poll_result_error_type(cx, return_ty) {
prefix = "Poll::Ready(Err(";
suffix = "))";
err_ty = ty;
} else if let Some(ty) = poll_option_result_error_type(cx, return_ty) {
prefix = "Poll::Ready(Some(Err(";
suffix = ")))";
err_ty = ty;
} else {
return;
};
let expr_err_ty = cx.typeck_results().expr_ty(err_arg);
let span = hygiene::walk_chain(err_arg.span, try_arg.span.ctxt());
let mut applicability = Applicability::MachineApplicable;
let origin_snippet = snippet_with_applicability(cx, span, "_", &mut applicability);
let ret_prefix = if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::Ret(_))) {
"" // already returns
} else {
"return "
};
let suggestion = if err_ty == expr_err_ty {
format!("{}{}{}{}", ret_prefix, prefix, origin_snippet, suffix)
} else {
format!("{}{}{}.into(){}", ret_prefix, prefix, origin_snippet, suffix)
};
span_lint_and_sugg(
cx,
TRY_ERR,
expr.span,
"returning an `Err(_)` with the `?` operator",
"try this",
suggestion,
applicability,
);
}
}
}
}
/// Finds function return type by examining return expressions in match arms.
fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option<Ty<'tcx>> {
if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr {
for arm in arms.iter() {
if let ExprKind::Ret(Some(ret)) = arm.body.kind {
return Some(cx.typeck_results().expr_ty(ret));
}
}
}
None
}
/// Extracts the error type from Result<T, E>.
fn result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(_, subst) = ty.kind();
if is_type_diagnostic_item(cx, ty, sym::Result);
then {
Some(subst.type_at(1))
} else {
None
}
}
}
/// Extracts the error type from Poll<Result<T, E>>.
fn poll_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(def, subst) = ty.kind();
if match_def_path(cx, def.did(), &paths::POLL);
let ready_ty = subst.type_at(0);
if let ty::Adt(ready_def, ready_subst) = ready_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Result, ready_def.did());
then {
Some(ready_subst.type_at(1))
} else {
None
}
}
}
/// Extracts the error type from Poll<Option<Result<T, E>>>.
fn poll_option_result_error_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
if_chain! {
if let ty::Adt(def, subst) = ty.kind();
if match_def_path(cx, def.did(), &paths::POLL);
let ready_ty = subst.type_at(0);
if let ty::Adt(ready_def, ready_subst) = ready_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Option, ready_def.did());
let some_ty = ready_subst.type_at(0);
if let ty::Adt(some_def, some_subst) = some_ty.kind();
if cx.tcx.is_diagnostic_item(sym::Result, some_def.did());
then {
Some(some_subst.type_at(1))
} else {
None
}
}
}