From a5a07e503f524a0e8b92938301857b3007579115 Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Thu, 20 Jan 2022 14:50:14 +0300 Subject: [PATCH 1/7] single_match: Don't lint non-exhaustive matches; support tuples This commit changes the behavior of `single_match` lint. After that, we won't lint non-exhaustive matches like this: ```rust match Some(v) { Some(a) => println!("${:?}", a), None => {}, } ``` The rationale is that, because the type of `a` could be changed, so the user can get non-exhaustive match after applying the suggested lint (see https://github.com/rust-lang/rust-clippy/issues/8282#issuecomment-1013566068 for context). We also will lint `match` constructions with tuples. When we see the tuples on the both arms, we will check them both at the same time, and if they form exhaustive match, we could display the warning. Closes #8282 --- clippy_lints/src/matches.rs | 145 ++++++++++++++++++++++++++---- tests/ui/single_match.rs | 50 +++++++++++ tests/ui/single_match.stderr | 38 +++++--- tests/ui/single_match_else.stderr | 42 +-------- 4 files changed, 206 insertions(+), 69 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 60dd957db01f..9da052450694 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -5,7 +5,7 @@ use clippy_utils::diagnostics::{ use clippy_utils::macros::{is_panic, root_macro_call}; use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; +use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, peel_mid_ty_refs}; use clippy_utils::visitors::is_local_used; use clippy_utils::{ get_parent_expr, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs, @@ -31,7 +31,7 @@ use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::{Span, Spanned}; use rustc_span::sym; -use std::cmp::Ordering; +use std::cmp::{max, Ordering}; use std::collections::hash_map::Entry; declare_clippy_lint! { @@ -741,7 +741,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp let ty = cx.typeck_results().expr_ty(ex); if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) { check_single_match_single_pattern(cx, ex, arms, expr, els); - check_single_match_opt_like(cx, ex, arms, expr, ty, els); + check_single_match_opt_like(cx, ex, arms, expr, els); } } } @@ -835,7 +835,6 @@ fn check_single_match_opt_like( ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>, - ty: Ty<'_>, els: Option<&Expr<'_>>, ) { // list of candidate `Enum`s we know will never get any more members @@ -849,25 +848,135 @@ fn check_single_match_opt_like( (&paths::RESULT, "Ok"), ]; - let path = match arms[1].pat.kind { - PatKind::TupleStruct(ref path, inner, _) => { - // Contains any non wildcard patterns (e.g., `Err(err)`)? - if !inner.iter().all(is_wild) { - return; + // We want to suggest to exclude an arm that contains only wildcards or forms the ehaustive + // match with the second branch. + if !contains_only_wilds(arms[1].pat) && !form_exhaustive_tuples(arms[0].pat, arms[1].pat) { + return; + } + + let mut paths = Vec::new(); + if !collect_pat_paths(&mut paths, arms[1].pat) { + return; + } + + let in_candidate_enum = |path: &String| -> bool { + for &(_, pat_path) in candidates { + if path == pat_path { + return true; + } + } + false + }; + if paths.iter().all(in_candidate_enum) { + report_single_match_single_pattern(cx, ex, arms, expr, els); + } +} + +/// Collects paths from the given paths. Returns true if the given pattern could be simplified, +/// false otherwise. +fn collect_pat_paths(acc: &mut Vec, pat: &Pat<'_>) -> bool { + match pat.kind { + PatKind::Wild => true, + PatKind::Tuple(inner, _) => { + for p in inner { + if !collect_pat_paths(acc, p) { + return false; + } } - rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false)) + true + }, + PatKind::TupleStruct(ref path, ..) => { + acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { + s.print_qpath(path, false); + })); + true + }, + PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => { + acc.push(ident.to_string()); + true }, - PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => ident.to_string(), PatKind::Path(ref path) => { - rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false)) + acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { + s.print_qpath(path, false); + })); + true }, - _ => return, - }; + _ => false, + } +} - for &(ty_path, pat_path) in candidates { - if path == *pat_path && match_type(cx, ty, ty_path) { - report_single_match_single_pattern(cx, ex, arms, expr, els); - } +/// Returns true if the given arm of pattern matching contains wildcard patterns. +fn contains_only_wilds(pat: &Pat<'_>) -> bool { + match pat.kind { + PatKind::Wild => true, + PatKind::Tuple(inner, _) | PatKind::TupleStruct(_, inner, ..) => inner.iter().all(contains_only_wilds), + _ => false, + } +} + +/// Returns true if the given patterns form the tuples that exhaustively matches all possible +/// parameters. +/// +/// Here are some examples: +/// +/// ``` +/// // Doesn't form exhaustive match, because the first arm may contain not only E::V. +/// match x { +/// (Some(E::V), _) => todo!(), +/// (None, _) => {} +/// } +/// +/// // Forms exhaustive match, because the patterns cover all possible cases at the same positions. +/// match x { +/// (Some(_), _) => todo!(), +/// (None, _) => {} +/// } +/// ``` +fn form_exhaustive_tuples(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)) => { + // We don't actually know the position and presence of the `..` (dotdot) operator in + // the arms, so we need to evaluate the correct offsets here in order to iterate in + // both arms at the same time. + let len = max( + left_in.len() + { + if left_pos.is_some() { 1 } else { 0 } + }, + right_in.len() + { + if right_pos.is_some() { 1 } else { 0 } + }, + ); + let mut left_pos = left_pos.unwrap_or(usize::MAX); + let mut right_pos = right_pos.unwrap_or(usize::MAX); + let mut left_span = 0; + let mut right_span = 0; + for i in 0..len { + let mut found_dotdot = false; + if i == left_pos { + left_span += 1; + if left_span < len - left_in.len() { + left_pos += 1; + } + found_dotdot = true; + } + if i == right_pos { + right_span += 1; + if right_span < len - right_in.len() { + right_pos += 1; + } + found_dotdot = true; + } + if found_dotdot { + continue; + } + if !contains_only_wilds(&left_in[i - left_span]) && !contains_only_wilds(&right_in[i - right_span]) { + return false; + } + } + true + }, + _ => false, } } diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index b1819e08d53b..aa93f4d8ca56 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -145,6 +145,56 @@ fn if_suggestion() { }; } +// See: issue #8282 +fn ranges() { + enum E { + V, + } + let x = (Some(E::V), Some(42)); + + // don't lint + match x { + (Some(E::V), _) => {}, + (None, _) => {}, + } + + // lint + match x { + (Some(_), _) => {}, + (None, _) => {}, + } + + // lint + match x { + (Some(E::V), _) => todo!(), + (_, _) => {}, + } + + // lint + match (Some(42), Some(E::V), Some(42)) { + (.., Some(E::V), _) => {}, + (..) => {}, + } + + // don't lint + match (Some(E::V), Some(E::V), Some(E::V)) { + (.., Some(E::V), _) => {}, + (.., None, _) => {}, + } + + // don't lint + match (Some(E::V), Some(E::V), Some(E::V)) { + (Some(E::V), ..) => {}, + (None, ..) => {}, + } + + // don't lint + match (Some(E::V), Some(E::V), Some(E::V)) { + (_, Some(E::V), ..) => {}, + (_, None, ..) => {}, + } +} + macro_rules! single_match { ($num:literal) => { match $num { diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index c261b5111c8b..f72ad853fed8 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -38,15 +38,6 @@ 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 | @@ -128,5 +119,32 @@ LL | | _ => (), LL | | }; | |_____^ help: try this: `if let None = x { println!() }` -error: aborting due to 13 previous errors +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:162:5 + | +LL | / match x { +LL | | (Some(_), _) => {}, +LL | | (None, _) => {}, +LL | | } + | |_____^ help: try this: `if let (Some(_), _) = x {}` + +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:168:5 + | +LL | / match x { +LL | | (Some(E::V), _) => todo!(), +LL | | (_, _) => {}, +LL | | } + | |_____^ help: try this: `if let (Some(E::V), _) = x { todo!() }` + +error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` + --> $DIR/single_match.rs:174:5 + | +LL | / match (Some(42), Some(E::V), Some(42)) { +LL | | (.., Some(E::V), _) => {}, +LL | | (..) => {}, +LL | | } + | |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}` + +error: aborting due to 15 previous errors diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index c61d80a905c9..21ea704b62ab 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -19,45 +19,5 @@ LL + None LL + } | -error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match_else.rs:70: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:79: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 +error: aborting due to previous error From a0c50875202e8d13b70d4cf8e4d347ddb04b876c Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Fri, 21 Jan 2022 07:24:07 +0300 Subject: [PATCH 2/7] single_match: Clarify the `don't lint` test case --- tests/ui/single_match.rs | 10 ++++++---- tests/ui/single_match.stderr | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index aa93f4d8ca56..0e50b3e4a6e2 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -152,7 +152,9 @@ fn ranges() { } let x = (Some(E::V), Some(42)); - // don't lint + // Don't lint, because the `E` enum can be extended with additional fields later. Thus, the + // proposed replacement to `if let Some(E::V)` may hide non-exhaustive warnings that appeared + // because of `match` construction. match x { (Some(E::V), _) => {}, (None, _) => {}, @@ -176,19 +178,19 @@ fn ranges() { (..) => {}, } - // don't lint + // Don't lint, see above. match (Some(E::V), Some(E::V), Some(E::V)) { (.., Some(E::V), _) => {}, (.., None, _) => {}, } - // don't lint + // Don't lint, see above. match (Some(E::V), Some(E::V), Some(E::V)) { (Some(E::V), ..) => {}, (None, ..) => {}, } - // don't lint + // Don't lint, see above. match (Some(E::V), Some(E::V), Some(E::V)) { (_, Some(E::V), ..) => {}, (_, None, ..) => {}, diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index f72ad853fed8..318faf257175 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -120,7 +120,7 @@ LL | | }; | |_____^ help: try this: `if let None = x { println!() }` error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:162:5 + --> $DIR/single_match.rs:164:5 | LL | / match x { LL | | (Some(_), _) => {}, @@ -129,7 +129,7 @@ LL | | } | |_____^ help: try this: `if let (Some(_), _) = x {}` error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:168:5 + --> $DIR/single_match.rs:170:5 | LL | / match x { LL | | (Some(E::V), _) => todo!(), @@ -138,7 +138,7 @@ LL | | } | |_____^ help: try this: `if let (Some(E::V), _) = x { todo!() }` error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let` - --> $DIR/single_match.rs:174:5 + --> $DIR/single_match.rs:176:5 | LL | / match (Some(42), Some(E::V), Some(42)) { LL | | (.., Some(E::V), _) => {}, From 49ae73b450b61f0240e629ced2024a9474f35adc Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Fri, 21 Jan 2022 07:28:40 +0300 Subject: [PATCH 3/7] matches: Simplify code --- clippy_lints/src/matches.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 9da052450694..be9eff4237ba 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -877,14 +877,7 @@ fn check_single_match_opt_like( fn collect_pat_paths(acc: &mut Vec, pat: &Pat<'_>) -> bool { match pat.kind { PatKind::Wild => true, - PatKind::Tuple(inner, _) => { - for p in inner { - if !collect_pat_paths(acc, p) { - return false; - } - } - true - }, + PatKind::Tuple(inner, _) => inner.iter().all(|p| collect_pat_paths(acc, p)), PatKind::TupleStruct(ref path, ..) => { acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { s.print_qpath(path, false); From 4aee3b1f1e977136b401cc6633da5f1234501347 Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Wed, 26 Jan 2022 14:46:30 +0300 Subject: [PATCH 4/7] matches: Clarify the behavior of exhaustive check --- clippy_lints/src/matches.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index dc6e4f96969d..71842ded69d6 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -848,9 +848,9 @@ fn check_single_match_opt_like( (&paths::RESULT, "Ok"), ]; - // We want to suggest to exclude an arm that contains only wildcards or forms the ehaustive - // match with the second branch. - if !contains_only_wilds(arms[1].pat) && !form_exhaustive_tuples(arms[0].pat, arms[1].pat) { + // 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; } @@ -907,30 +907,30 @@ fn contains_only_wilds(pat: &Pat<'_>) -> bool { } } -/// Returns true if the given patterns form the tuples that exhaustively matches all possible -/// parameters. +/// Returns true if the given patterns forms only exhaustive matches that don't contain enum +/// patterns without a wildcard. /// -/// Here are some examples: +/// For example: /// /// ``` -/// // Doesn't form exhaustive match, because the first arm may contain not only E::V. +/// // Returns false, because the first arm contain enum without a wildcard. /// match x { /// (Some(E::V), _) => todo!(), /// (None, _) => {} /// } /// -/// // Forms exhaustive match, because the patterns cover all possible cases at the same positions. +/// // Returns true, because the both arms form exhaustive matches and without enum variants. /// match x { /// (Some(_), _) => todo!(), /// (None, _) => {} /// } /// ``` -fn form_exhaustive_tuples(left: &Pat<'_>, right: &Pat<'_>) -> bool { +fn form_exhaustive_matches(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)) => { - // We don't actually know the position and presence of the `..` (dotdot) operator in - // the arms, so we need to evaluate the correct offsets here in order to iterate in + // We don't actually know the position and the presence of the `..` (dotdot) operator + // in the arms, so we need to evaluate the correct offsets here in order to iterate in // both arms at the same time. let len = max( left_in.len() + { From 81015870df891084b24497718af086f0575a121b Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Wed, 26 Jan 2022 18:02:32 +0300 Subject: [PATCH 5/7] matches: Improve naming. NFC. --- clippy_lints/src/matches.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 71842ded69d6..e8789a92f133 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -942,20 +942,20 @@ fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool { ); let mut left_pos = left_pos.unwrap_or(usize::MAX); let mut right_pos = right_pos.unwrap_or(usize::MAX); - let mut left_span = 0; - let mut right_span = 0; + let mut left_dot_space = 0; + let mut right_dot_space = 0; for i in 0..len { let mut found_dotdot = false; if i == left_pos { - left_span += 1; - if left_span < len - left_in.len() { + left_dot_space += 1; + if left_dot_space < len - left_in.len() { left_pos += 1; } found_dotdot = true; } if i == right_pos { - right_span += 1; - if right_span < len - right_in.len() { + right_dot_space += 1; + if right_dot_space < len - right_in.len() { right_pos += 1; } found_dotdot = true; @@ -963,7 +963,9 @@ fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool { if found_dotdot { continue; } - if !contains_only_wilds(&left_in[i - left_span]) && !contains_only_wilds(&right_in[i - right_span]) { + if !contains_only_wilds(&left_in[i - left_dot_space]) + && !contains_only_wilds(&right_in[i - right_dot_space]) + { return false; } } From 467a0bfdea3ba03de05b90a3de85d4467a28762e Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Wed, 26 Jan 2022 18:20:35 +0300 Subject: [PATCH 6/7] matches: Restore `match_type` logic; add tests for these cases --- clippy_lints/src/matches.rs | 45 +++++++++++++++++++++---------------- tests/ui/single_match.rs | 26 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index e8789a92f133..47c02e228215 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -5,7 +5,7 @@ use clippy_utils::diagnostics::{ use clippy_utils::macros::{is_panic, root_macro_call}; use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, peel_mid_ty_refs}; +use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; use clippy_utils::visitors::is_local_used; use clippy_utils::{ get_parent_expr, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs, @@ -741,7 +741,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp let ty = cx.typeck_results().expr_ty(ex); if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) { check_single_match_single_pattern(cx, ex, arms, expr, els); - check_single_match_opt_like(cx, ex, arms, expr, els); + check_single_match_opt_like(cx, ex, arms, expr, ty, els); } } } @@ -830,11 +830,12 @@ fn report_single_match_single_pattern( ); } -fn check_single_match_opt_like( - cx: &LateContext<'_>, +fn check_single_match_opt_like<'a>( + cx: &LateContext<'a>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>, + ty: Ty<'a>, els: Option<&Expr<'_>>, ) { // list of candidate `Enum`s we know will never get any more members @@ -854,44 +855,50 @@ fn check_single_match_opt_like( return; } - let mut paths = Vec::new(); - if !collect_pat_paths(&mut paths, arms[1].pat) { + 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: &String| -> bool { - for &(_, pat_path) in candidates { - if path == pat_path { + let in_candidate_enum = |path_info: &(String, &TyS<'_>)| -> 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; } } false }; - if paths.iter().all(in_candidate_enum) { + if paths_and_types.iter().all(in_candidate_enum) { report_single_match_single_pattern(cx, ex, arms, expr, els); } } -/// Collects paths from the given paths. Returns true if the given pattern could be simplified, -/// false otherwise. -fn collect_pat_paths(acc: &mut Vec, pat: &Pat<'_>) -> bool { +/// Collects paths and their types from the given patterns. Returns true if the given pattern could +/// be simplified, false otherwise. +fn collect_pat_paths<'a>(acc: &mut Vec<(String, Ty<'a>)>, cx: &LateContext<'a>, pat: &Pat<'_>, ty: Ty<'a>) -> bool { match pat.kind { PatKind::Wild => true, - PatKind::Tuple(inner, _) => inner.iter().all(|p| collect_pat_paths(acc, p)), + PatKind::Tuple(inner, _) => inner.iter().all(|p| { + let p_ty = cx.typeck_results().pat_ty(p); + collect_pat_paths(acc, cx, p, p_ty) + }), PatKind::TupleStruct(ref path, ..) => { - acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { + let path = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { s.print_qpath(path, false); - })); + }); + acc.push((path, ty)); true }, PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => { - acc.push(ident.to_string()); + acc.push((ident.to_string(), ty)); true }, PatKind::Path(ref path) => { - acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { + let path = rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| { s.print_qpath(path, false); - })); + }); + acc.push((path, ty)); true }, _ => false, diff --git a/tests/ui/single_match.rs b/tests/ui/single_match.rs index 0e50b3e4a6e2..bd3718880463 100644 --- a/tests/ui/single_match.rs +++ b/tests/ui/single_match.rs @@ -197,6 +197,32 @@ fn ranges() { } } +fn skip_type_aliases() { + enum OptionEx { + Some(i32), + None, + } + enum ResultEx { + Err(i32), + Ok(i32), + } + + use OptionEx::{None, Some}; + use ResultEx::{Err, Ok}; + + // don't lint + match Err(42) { + Ok(_) => dummy(), + Err(_) => (), + }; + + // don't lint + match Some(1i32) { + Some(_) => dummy(), + None => (), + }; +} + macro_rules! single_match { ($num:literal) => { match $num { From a8fdf5ca8aab08ba56678a4bce36f9da5ae8e52d Mon Sep 17 00:00:00 2001 From: Georgy Komarov Date: Wed, 26 Jan 2022 19:09:31 +0300 Subject: [PATCH 7/7] matches: Remove extra comment --- clippy_lints/src/matches.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 47c02e228215..ab030ffb4063 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -916,22 +916,6 @@ 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. -/// -/// For example: -/// -/// ``` -/// // Returns false, because the first arm contain enum without a wildcard. -/// match x { -/// (Some(E::V), _) => todo!(), -/// (None, _) => {} -/// } -/// -/// // Returns true, because the both arms form exhaustive matches and without enum variants. -/// match x { -/// (Some(_), _) => todo!(), -/// (None, _) => {} -/// } -/// ``` fn form_exhaustive_matches(left: &Pat<'_>, right: &Pat<'_>) -> bool { match (&left.kind, &right.kind) { (PatKind::Wild, _) | (_, PatKind::Wild) => true,