From 6d6c63ee23c627aa9299cffa738f40189126c08a Mon Sep 17 00:00:00 2001 From: Micha White Date: Thu, 9 Jun 2022 16:25:48 -0400 Subject: [PATCH] Lint single_match with Options, Results, and Cows --- clippy_lints/src/matches/single_match.rs | 53 ++++++++++++++---------- tests/ui/single_match.stderr | 11 ++++- tests/ui/single_match_else.stderr | 42 ++++++++++++++++++- 3 files changed, 82 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/matches/single_match.rs b/clippy_lints/src/matches/single_match.rs index 0c4cb45d147..b54d4e8a0ee 100644 --- a/clippy_lints/src/matches/single_match.rs +++ b/clippy_lints/src/matches/single_match.rs @@ -140,6 +140,23 @@ fn check_opt_like<'a>( ty: Ty<'a>, els: Option<&Expr<'_>>, ) { + // We want to suggest to exclude an arm that contains only wildcards or forms the exhaustive + // match with the second branch, without enum variants in matches. + if !contains_only_wilds(arms[1].pat) && !form_exhaustive_matches(cx, ty, arms[0].pat, arms[1].pat) { + return; + } + + let mut paths_and_types = Vec::new(); + if !collect_pat_paths(&mut paths_and_types, cx, arms[1].pat, ty) { + return; + } + + if paths_and_types.iter().all(|info| in_candidate_enum(cx, info)) { + report_single_pattern(cx, ex, arms, expr, els); + } +} + +fn in_candidate_enum<'a>(cx: &LateContext<'a>, path_info: &(String, Ty<'_>)) -> bool { // list of candidate `Enum`s we know will never get any more members let candidates = &[ (&paths::COW, "Borrowed"), @@ -151,29 +168,13 @@ fn check_opt_like<'a>( (&paths::RESULT, "Ok"), ]; - // We want to suggest to exclude an arm that contains only wildcards or forms the exhaustive - // match with the second branch, without enum variants in matches. - if !contains_only_wilds(arms[1].pat) && !form_exhaustive_matches(arms[0].pat, arms[1].pat) { - return; - } - - let mut paths_and_types = Vec::new(); - if !collect_pat_paths(&mut paths_and_types, cx, arms[1].pat, ty) { - return; - } - - let in_candidate_enum = |path_info: &(String, Ty<'_>)| -> bool { - let (path, ty) = path_info; - for &(ty_path, pat_path) in candidates { - if path == pat_path && match_type(cx, *ty, ty_path) { - return true; - } + let (path, ty) = path_info; + for &(ty_path, pat_path) in candidates { + if path == pat_path && match_type(cx, *ty, ty_path) { + return true; } - false - }; - if paths_and_types.iter().all(in_candidate_enum) { - report_single_pattern(cx, ex, arms, expr, els); } + false } /// Collects paths and their types from the given patterns. Returns true if the given pattern could @@ -218,7 +219,7 @@ fn contains_only_wilds(pat: &Pat<'_>) -> bool { /// Returns true if the given patterns forms only exhaustive matches that don't contain enum /// patterns without a wildcard. -fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool { +fn form_exhaustive_matches<'a>(cx: &LateContext<'a>, ty: Ty<'a>, left: &Pat<'_>, right: &Pat<'_>) -> bool { match (&left.kind, &right.kind) { (PatKind::Wild, _) | (_, PatKind::Wild) => true, (PatKind::Tuple(left_in, left_pos), PatKind::Tuple(right_in, right_pos)) => { @@ -264,6 +265,14 @@ fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool { } true }, + (PatKind::TupleStruct(..), PatKind::Path(_) | PatKind::TupleStruct(..)) => { + let mut paths_and_types = Vec::new(); + if !collect_pat_paths(&mut paths_and_types, cx, right, ty) { + return false; + } + + paths_and_types.iter().all(|info| in_candidate_enum(cx, info)) + }, _ => false, } } diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index 318faf25717..4d2b9ec5f90 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -38,6 +38,15 @@ LL | | _ => {}, LL | | }; | |_____^ help: try this: `if let (2..=3, 7..=9) = z { dummy() }` +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:54:5 + | +LL | / match x { +LL | | Some(y) => dummy(), +LL | | None => (), +LL | | }; + | |_____^ help: try this: `if let Some(y) = x { dummy() }` + error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` --> $DIR/single_match.rs:59:5 | @@ -146,5 +155,5 @@ LL | | (..) => {}, LL | | } | |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}` -error: aborting due to 15 previous errors +error: aborting due to 16 previous errors diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 7756c6f204e..dc603578fde 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -20,5 +20,45 @@ LL + None LL ~ }; | -error: aborting due to previous error +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match_else.rs:84:5 + | +LL | / match Some(1) { +LL | | Some(a) => println!("${:?}", a), +LL | | None => { +LL | | println!("else block"); +LL | | return +LL | | }, +LL | | } + | |_____^ + | +help: try this + | +LL ~ if let Some(a) = Some(1) { println!("${:?}", a) } else { +LL + println!("else block"); +LL + return +LL + } + | + +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match_else.rs:93:5 + | +LL | / match Some(1) { +LL | | Some(a) => println!("${:?}", a), +LL | | None => { +LL | | println!("else block"); +LL | | return; +LL | | }, +LL | | } + | |_____^ + | +help: try this + | +LL ~ if let Some(a) = Some(1) { println!("${:?}", a) } else { +LL + println!("else block"); +LL + return; +LL + } + | + +error: aborting due to 3 previous errors