diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 16173580fd4..703aa458f44 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -637,12 +637,6 @@ fn has_needless_main(code: String, edition: Edition) -> bool { loop { match parser.parse_item(ForceCollect::No) { Ok(Some(item)) => match &item.kind { - // Tests with one of these items are ignored - ItemKind::Static(..) - | ItemKind::Const(..) - | ItemKind::ExternCrate(..) - | ItemKind::ForeignMod(..) => return false, - // We found a main function ... ItemKind::Fn(box Fn { sig, body: Some(block), .. }) if item.ident.name == sym::main => { @@ -661,8 +655,13 @@ fn has_needless_main(code: String, edition: Edition) -> bool { return false; } }, - // Another function was found; this case is ignored too - ItemKind::Fn(..) => return false, + // Tests with one of these items are ignored + ItemKind::Static(..) + | ItemKind::Const(..) + | ItemKind::ExternCrate(..) + | ItemKind::ForeignMod(..) + // Another function was found; this case is ignored + | ItemKind::Fn(..) => return false, _ => {}, }, Ok(None) => break, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 504235d0d1e..f2a07999144 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -23,6 +23,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) +extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index d11dda57e6f..b8591fe0db0 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -1,19 +1,66 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::{path_to_local, search_same, SpanlessEq, SpanlessHash}; -use rustc_hir::{Arm, Expr, HirId, HirIdMap, HirIdSet, Pat, PatKind}; +use core::cmp::Ordering; +use core::iter; +use core::slice; +use rustc_arena::DroplessArena; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::def_id::DefId; +use rustc_hir::{Arm, Expr, ExprKind, HirId, HirIdMap, HirIdSet, Pat, PatKind, RangeEnd}; use rustc_lint::LateContext; +use rustc_middle::ty; +use rustc_span::Symbol; use std::collections::hash_map::Entry; use super::MATCH_SAME_ARMS; -pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { +#[allow(clippy::too_many_lines)] +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 { let mut h = SpanlessHash::new(cx); h.hash_expr(arm.body); h.finish() }; + let arena = DroplessArena::default(); + let normalized_pats: Vec<_> = arms + .iter() + .map(|a| NormalizedPat::from_pat(cx, &arena, a.pat)) + .collect(); + + // The furthast forwards a pattern can move without semantic changes + let forwards_blocking_idxs: Vec<_> = normalized_pats + .iter() + .enumerate() + .map(|(i, pat)| { + normalized_pats[i + 1..] + .iter() + .enumerate() + .find_map(|(j, other)| pat.has_overlapping_values(other).then(|| i + 1 + j)) + .unwrap_or(normalized_pats.len()) + }) + .collect(); + + // The furthast backwards a pattern can move without semantic changes + let backwards_blocking_idxs: Vec<_> = normalized_pats + .iter() + .enumerate() + .map(|(i, pat)| { + normalized_pats[..i] + .iter() + .enumerate() + .rev() + .zip(forwards_blocking_idxs[..i].iter().copied().rev()) + .skip_while(|&(_, forward_block)| forward_block > i) + .find_map(|((j, other), forward_block)| { + (forward_block == i || pat.has_overlapping_values(other)).then(|| j) + }) + .unwrap_or(0) + }) + .collect(); + let eq = |&(lindex, lhs): &(usize, &Arm<'_>), &(rindex, rhs): &(usize, &Arm<'_>)| -> bool { let min_index = usize::min(lindex, rindex); let max_index = usize::max(lindex, rindex); @@ -42,53 +89,316 @@ pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { } }; // Arms with a guard are ignored, those can’t always be merged together - // This is also the case for arms in-between each there is an arm with a guard - (min_index..=max_index).all(|index| arms[index].guard.is_none()) - && SpanlessEq::new(cx) - .expr_fallback(eq_fallback) - .eq_expr(lhs.body, rhs.body) - // these checks could be removed to allow unused bindings - && bindings_eq(lhs.pat, local_map.keys().copied().collect()) - && bindings_eq(rhs.pat, local_map.values().copied().collect()) + // If both arms overlap with an arm in between then these can't be merged either. + !(backwards_blocking_idxs[max_index] > min_index && forwards_blocking_idxs[min_index] < max_index) + && lhs.guard.is_none() + && rhs.guard.is_none() + && SpanlessEq::new(cx) + .expr_fallback(eq_fallback) + .eq_expr(lhs.body, rhs.body) + // these checks could be removed to allow unused bindings + && bindings_eq(lhs.pat, local_map.keys().copied().collect()) + && bindings_eq(rhs.pat, local_map.values().copied().collect()) }; let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect(); - for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) { - span_lint_and_then( - cx, - MATCH_SAME_ARMS, - j.body.span, - "this `match` has identical arm bodies", - |diag| { - diag.span_note(i.body.span, "same as this"); + for (&(i, arm1), &(j, arm2)) in search_same(&indexed_arms, hash, eq) { + if matches!(arm2.pat.kind, PatKind::Wild) { + span_lint_and_then( + cx, + MATCH_SAME_ARMS, + arm1.span, + "this match arm has an identical body to the `_` wildcard arm", + |diag| { + diag.span_suggestion( + arm1.span, + "try removing the arm", + String::new(), + Applicability::MaybeIncorrect, + ) + .help("or try changing either arm body") + .span_note(arm2.span, "`_` wildcard arm here"); + }, + ); + } else { + let back_block = backwards_blocking_idxs[j]; + let (keep_arm, move_arm) = if back_block < i || (back_block == 0 && forwards_blocking_idxs[i] <= j) { + (arm1, arm2) + } else { + (arm2, arm1) + }; - // Note: this does not use `span_suggestion` on purpose: - // there is no clean way - // to remove the other arm. Building a span and suggest to replace it to "" - // makes an even more confusing error message. Also in order not to make up a - // span for the whole pattern, the suggestion is only shown when there is only - // one pattern. The user should know about `|` if they are already using it… + span_lint_and_then( + cx, + MATCH_SAME_ARMS, + keep_arm.span, + "this match arm has an identical body to another arm", + |diag| { + let move_pat_snip = snippet(cx, move_arm.pat.span, ""); + let keep_pat_snip = snippet(cx, keep_arm.pat.span, ""); - let lhs = snippet(cx, i.pat.span, ""); - let rhs = snippet(cx, j.pat.span, ""); + diag.span_suggestion( + keep_arm.pat.span, + "try merging the arm patterns", + format!("{} | {}", keep_pat_snip, move_pat_snip), + Applicability::MaybeIncorrect, + ) + .help("or try changing either arm body") + .span_note(move_arm.span, "other arm here"); + }, + ); + } + } +} - if let PatKind::Wild = j.pat.kind { - // if the last arm is _, then i could be integrated into _ - // note that i.pat cannot be _, because that would mean that we're - // hiding all the subsequent arms, and rust won't compile - diag.span_note( - i.body.span, - &format!( - "`{}` has the same arm body as the `_` wildcard, consider removing it", - lhs - ), - ); - } else { - diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,)) - .help("...or consider changing the match arm bodies"); +#[derive(Clone, Copy)] +enum NormalizedPat<'a> { + Wild, + Struct(Option, &'a [(Symbol, Self)]), + Tuple(Option, &'a [Self]), + Or(&'a [Self]), + Path(Option), + LitStr(Symbol), + LitBytes(&'a [u8]), + LitInt(u128), + LitBool(bool), + Range(PatRange), + /// A slice pattern. If the second value is `None`, then this matches an exact size. Otherwise + /// the first value contains everything before the `..` wildcard pattern, and the second value + /// contains everything afterwards. Note that either side, or both sides, may contain zero + /// patterns. + Slice(&'a [Self], Option<&'a [Self]>), +} + +#[derive(Clone, Copy)] +struct PatRange { + start: u128, + end: u128, + bounds: RangeEnd, +} +impl PatRange { + fn contains(&self, x: u128) -> bool { + x >= self.start + && match self.bounds { + RangeEnd::Included => x <= self.end, + RangeEnd::Excluded => x < self.end, + } + } + + fn overlaps(&self, other: &Self) -> bool { + // Note: Empty ranges are impossible, so this is correct even though it would return true if an + // empty exclusive range were to reside within an inclusive range. + (match self.bounds { + RangeEnd::Included => self.end >= other.start, + RangeEnd::Excluded => self.end > other.start, + } && match other.bounds { + RangeEnd::Included => self.start <= other.end, + RangeEnd::Excluded => self.start < other.end, + }) + } +} + +/// Iterates over the pairs of fields with matching names. +fn iter_matching_struct_fields<'a>( + left: &'a [(Symbol, NormalizedPat<'a>)], + right: &'a [(Symbol, NormalizedPat<'a>)], +) -> impl Iterator, &'a NormalizedPat<'a>)> + 'a { + struct Iter<'a>( + slice::Iter<'a, (Symbol, NormalizedPat<'a>)>, + slice::Iter<'a, (Symbol, NormalizedPat<'a>)>, + ); + impl<'a> Iterator for Iter<'a> { + type Item = (&'a NormalizedPat<'a>, &'a NormalizedPat<'a>); + fn next(&mut self) -> Option { + // Note: all the fields in each slice are sorted by symbol value. + let mut left = self.0.next()?; + let mut right = self.1.next()?; + loop { + match left.0.cmp(&right.0) { + Ordering::Equal => return Some((&left.1, &right.1)), + Ordering::Less => left = self.0.next()?, + Ordering::Greater => right = self.1.next()?, } + } + } + } + Iter(left.iter(), right.iter()) +} + +#[allow(clippy::similar_names)] +impl<'a> NormalizedPat<'a> { + #[allow(clippy::too_many_lines)] + fn from_pat(cx: &LateContext<'_>, arena: &'a DroplessArena, pat: &'a Pat<'_>) -> Self { + match pat.kind { + PatKind::Wild | PatKind::Binding(.., None) => Self::Wild, + PatKind::Binding(.., Some(pat)) | PatKind::Box(pat) | PatKind::Ref(pat, _) => { + Self::from_pat(cx, arena, pat) }, - ); + PatKind::Struct(ref path, fields, _) => { + let fields = + arena.alloc_from_iter(fields.iter().map(|f| (f.ident.name, Self::from_pat(cx, arena, f.pat)))); + fields.sort_by_key(|&(name, _)| name); + Self::Struct(cx.qpath_res(path, pat.hir_id).opt_def_id(), fields) + }, + PatKind::TupleStruct(ref path, pats, wild_idx) => { + let adt = match cx.typeck_results().pat_ty(pat).ty_adt_def() { + Some(x) => x, + None => return Self::Wild, + }; + let (var_id, variant) = if adt.is_enum() { + match cx.qpath_res(path, pat.hir_id).opt_def_id() { + Some(x) => (Some(x), adt.variant_with_ctor_id(x)), + None => return Self::Wild, + } + } else { + (None, adt.non_enum_variant()) + }; + let (front, back) = match wild_idx { + Some(i) => pats.split_at(i), + None => (pats, [].as_slice()), + }; + let pats = arena.alloc_from_iter( + front + .iter() + .map(|pat| Self::from_pat(cx, arena, pat)) + .chain(iter::repeat_with(|| Self::Wild).take(variant.fields.len() - pats.len())) + .chain(back.iter().map(|pat| Self::from_pat(cx, arena, pat))), + ); + Self::Tuple(var_id, pats) + }, + PatKind::Or(pats) => Self::Or(arena.alloc_from_iter(pats.iter().map(|pat| Self::from_pat(cx, arena, pat)))), + PatKind::Path(ref path) => Self::Path(cx.qpath_res(path, pat.hir_id).opt_def_id()), + PatKind::Tuple(pats, wild_idx) => { + let field_count = match cx.typeck_results().pat_ty(pat).kind() { + ty::Tuple(subs) => subs.len(), + _ => return Self::Wild, + }; + let (front, back) = match wild_idx { + Some(i) => pats.split_at(i), + None => (pats, [].as_slice()), + }; + let pats = arena.alloc_from_iter( + front + .iter() + .map(|pat| Self::from_pat(cx, arena, pat)) + .chain(iter::repeat_with(|| Self::Wild).take(field_count - pats.len())) + .chain(back.iter().map(|pat| Self::from_pat(cx, arena, pat))), + ); + Self::Tuple(None, pats) + }, + PatKind::Lit(e) => match &e.kind { + // TODO: Handle negative integers. They're currently treated as a wild match. + ExprKind::Lit(lit) => match lit.node { + LitKind::Str(sym, _) => Self::LitStr(sym), + LitKind::ByteStr(ref bytes) => Self::LitBytes(&**bytes), + LitKind::Byte(val) => Self::LitInt(val.into()), + LitKind::Char(val) => Self::LitInt(val.into()), + LitKind::Int(val, _) => Self::LitInt(val), + LitKind::Bool(val) => Self::LitBool(val), + LitKind::Float(..) | LitKind::Err(_) => Self::Wild, + }, + _ => Self::Wild, + }, + PatKind::Range(start, end, bounds) => { + // TODO: Handle negative integers. They're currently treated as a wild match. + let start = match start { + None => 0, + Some(e) => match &e.kind { + ExprKind::Lit(lit) => match lit.node { + LitKind::Int(val, _) => val, + LitKind::Char(val) => val.into(), + LitKind::Byte(val) => val.into(), + _ => return Self::Wild, + }, + _ => return Self::Wild, + }, + }; + let (end, bounds) = match end { + None => (u128::MAX, RangeEnd::Included), + Some(e) => match &e.kind { + ExprKind::Lit(lit) => match lit.node { + LitKind::Int(val, _) => (val, bounds), + LitKind::Char(val) => (val.into(), bounds), + LitKind::Byte(val) => (val.into(), bounds), + _ => return Self::Wild, + }, + _ => return Self::Wild, + }, + }; + Self::Range(PatRange { start, end, bounds }) + }, + PatKind::Slice(front, wild_pat, back) => Self::Slice( + arena.alloc_from_iter(front.iter().map(|pat| Self::from_pat(cx, arena, pat))), + wild_pat.map(|_| &*arena.alloc_from_iter(back.iter().map(|pat| Self::from_pat(cx, arena, pat)))), + ), + } + } + + /// Checks if two patterns overlap in the values they can match assuming they are for the same + /// type. + fn has_overlapping_values(&self, other: &Self) -> bool { + match (*self, *other) { + (Self::Wild, _) | (_, Self::Wild) => true, + (Self::Or(pats), ref other) | (ref other, Self::Or(pats)) => { + pats.iter().any(|pat| pat.has_overlapping_values(other)) + }, + (Self::Struct(lpath, lfields), Self::Struct(rpath, rfields)) => { + if lpath != rpath { + return false; + } + iter_matching_struct_fields(lfields, rfields).all(|(lpat, rpat)| lpat.has_overlapping_values(rpat)) + }, + (Self::Tuple(lpath, lpats), Self::Tuple(rpath, rpats)) => { + if lpath != rpath { + return false; + } + lpats + .iter() + .zip(rpats.iter()) + .all(|(lpat, rpat)| lpat.has_overlapping_values(rpat)) + }, + (Self::Path(x), Self::Path(y)) => x == y, + (Self::LitStr(x), Self::LitStr(y)) => x == y, + (Self::LitBytes(x), Self::LitBytes(y)) => x == y, + (Self::LitInt(x), Self::LitInt(y)) => x == y, + (Self::LitBool(x), Self::LitBool(y)) => x == y, + (Self::Range(ref x), Self::Range(ref y)) => x.overlaps(y), + (Self::Range(ref range), Self::LitInt(x)) | (Self::LitInt(x), Self::Range(ref range)) => range.contains(x), + (Self::Slice(lpats, None), Self::Slice(rpats, None)) => { + lpats.len() == rpats.len() && lpats.iter().zip(rpats.iter()).all(|(x, y)| x.has_overlapping_values(y)) + }, + (Self::Slice(pats, None), Self::Slice(front, Some(back))) + | (Self::Slice(front, Some(back)), Self::Slice(pats, None)) => { + // Here `pats` is an exact size match. If the combined lengths of `front` and `back` are greater + // then the minium length required will be greater than the length of `pats`. + if pats.len() < front.len() + back.len() { + return false; + } + pats[..front.len()] + .iter() + .zip(front.iter()) + .chain(pats[pats.len() - back.len()..].iter().zip(back.iter())) + .all(|(x, y)| x.has_overlapping_values(y)) + }, + (Self::Slice(lfront, Some(lback)), Self::Slice(rfront, Some(rback))) => lfront + .iter() + .zip(rfront.iter()) + .chain(lback.iter().rev().zip(rback.iter().rev())) + .all(|(x, y)| x.has_overlapping_values(y)), + + // Enums can mix unit variants with tuple/struct variants. These can never overlap. + (Self::Path(_), Self::Tuple(..) | Self::Struct(..)) + | (Self::Tuple(..) | Self::Struct(..), Self::Path(_)) => false, + + // Tuples can be matched like a struct. + (Self::Tuple(x, _), Self::Struct(y, _)) | (Self::Struct(x, _), Self::Tuple(y, _)) => { + // TODO: check fields here. + x == y + }, + + // TODO: Lit* with Path, Range with Path, LitBytes with Slice + _ => true, + } } } diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 532bd810a2e..f3d818cc348 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -581,14 +581,19 @@ fn check_tts<'a>(&self, cx: &EarlyContext<'a>, tts: TokenStream, is_write: bool) }; let replacement: String = match lit.token.kind { - LitKind::Integer | LitKind::Float | LitKind::Err => continue, LitKind::StrRaw(_) | LitKind::ByteStrRaw(_) if matches!(fmtstr.style, StrStyle::Raw(_)) => { lit.token.symbol.as_str().replace('{', "{{").replace('}', "}}") }, LitKind::Str | LitKind::ByteStr if matches!(fmtstr.style, StrStyle::Cooked) => { lit.token.symbol.as_str().replace('{', "{{").replace('}', "}}") }, - LitKind::StrRaw(_) | LitKind::Str | LitKind::ByteStrRaw(_) | LitKind::ByteStr => continue, + LitKind::StrRaw(_) + | LitKind::Str + | LitKind::ByteStrRaw(_) + | LitKind::ByteStr + | LitKind::Integer + | LitKind::Float + | LitKind::Err => continue, LitKind::Byte | LitKind::Char => match lit.token.symbol.as_str() { "\"" if matches!(fmtstr.style, StrStyle::Cooked) => "\\\"", "\"" if matches!(fmtstr.style, StrStyle::Raw(0)) => continue, diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr index 7752a8a6ff2..b6d04263b37 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -1,128 +1,121 @@ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:13:14 +error: this match arm has an identical body to the `_` wildcard arm + --> $DIR/match_same_arms.rs:11:9 | -LL | _ => 0, //~ ERROR match arms have same body - | ^ +LL | Abc::A => 0, + | ^^^^^^^^^^^ help: try removing the arm | = note: `-D clippy::match-same-arms` implied by `-D warnings` -note: same as this - --> $DIR/match_same_arms.rs:11:19 + = help: or try changing either arm body +note: `_` wildcard arm here + --> $DIR/match_same_arms.rs:13:9 | -LL | Abc::A => 0, - | ^ -note: `Abc::A` has the same arm body as the `_` wildcard, consider removing it - --> $DIR/match_same_arms.rs:11:19 - | -LL | Abc::A => 0, - | ^ +LL | _ => 0, //~ ERROR match arms have same body + | ^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:18:20 - | -LL | (.., 3) => 42, //~ ERROR match arms have same body - | ^^ - | -note: same as this - --> $DIR/match_same_arms.rs:17:23 - | -LL | (1, .., 3) => 42, - | ^^ -help: consider refactoring into `(1, .., 3) | (.., 3)` +error: this match arm has an identical body to another arm --> $DIR/match_same_arms.rs:17:9 | LL | (1, .., 3) => 42, - | ^^^^^^^^^^ - = help: ...or consider changing the match arm bodies + | ----------^^^^^^ + | | + | help: try merging the arm patterns: `(1, .., 3) | (.., 3)` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms.rs:18:9 + | +LL | (.., 3) => 42, //~ ERROR match arms have same body + | ^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:24:15 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms.rs:24:9 | LL | 51 => 1, //~ ERROR match arms have same body - | ^ + | --^^^^^ + | | + | help: try merging the arm patterns: `51 | 42` | -note: same as this - --> $DIR/match_same_arms.rs:23:15 - | -LL | 42 => 1, - | ^ -help: consider refactoring into `42 | 51` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms.rs:23:9 | LL | 42 => 1, - | ^^ - = help: ...or consider changing the match arm bodies + | ^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:26:15 - | -LL | 52 => 2, //~ ERROR match arms have same body - | ^ - | -note: same as this - --> $DIR/match_same_arms.rs:25:15 - | -LL | 41 => 2, - | ^ -help: consider refactoring into `41 | 52` +error: this match arm has an identical body to another arm --> $DIR/match_same_arms.rs:25:9 | LL | 41 => 2, - | ^^ - = help: ...or consider changing the match arm bodies + | --^^^^^ + | | + | help: try merging the arm patterns: `41 | 52` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms.rs:26:9 + | +LL | 52 => 2, //~ ERROR match arms have same body + | ^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:32:14 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms.rs:32:9 | LL | 2 => 2, //~ ERROR 2nd matched arms have same body - | ^ + | -^^^^^ + | | + | help: try merging the arm patterns: `2 | 1` | -note: same as this - --> $DIR/match_same_arms.rs:31:14 - | -LL | 1 => 2, - | ^ -help: consider refactoring into `1 | 2` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms.rs:31:9 | LL | 1 => 2, - | ^ - = help: ...or consider changing the match arm bodies + | ^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:33:14 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms.rs:33:9 | LL | 3 => 2, //~ ERROR 3rd matched arms have same body - | ^ + | -^^^^^ + | | + | help: try merging the arm patterns: `3 | 1` | -note: same as this - --> $DIR/match_same_arms.rs:31:14 - | -LL | 1 => 2, - | ^ -help: consider refactoring into `1 | 3` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms.rs:31:9 | LL | 1 => 2, - | ^ - = help: ...or consider changing the match arm bodies + | ^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:50:55 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms.rs:32:9 + | +LL | 2 => 2, //~ ERROR 2nd matched arms have same body + | -^^^^^ + | | + | help: try merging the arm patterns: `2 | 3` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms.rs:33:9 + | +LL | 3 => 2, //~ ERROR 3rd matched arms have same body + | ^^^^^^ + +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms.rs:50:17 | LL | CommandInfo::External { name, .. } => name.to_string(), - | ^^^^^^^^^^^^^^^^ + | ----------------------------------^^^^^^^^^^^^^^^^^^^^ + | | + | help: try merging the arm patterns: `CommandInfo::External { name, .. } | CommandInfo::BuiltIn { name, .. }` | -note: same as this - --> $DIR/match_same_arms.rs:49:54 - | -LL | CommandInfo::BuiltIn { name, .. } => name.to_string(), - | ^^^^^^^^^^^^^^^^ -help: consider refactoring into `CommandInfo::BuiltIn { name, .. } | CommandInfo::External { name, .. }` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms.rs:49:17 | LL | CommandInfo::BuiltIn { name, .. } => name.to_string(), - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: ...or consider changing the match arm bodies + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: aborting due to 8 previous errors diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index 67e1d518483..dbfeb4379d5 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -174,4 +174,57 @@ fn main() { Some(2) => 2, _ => 1, }; + + enum Foo { + X(u32), + Y(u32), + Z(u32), + } + + // Don't lint. `Foo::X(0)` and `Foo::Z(_)` overlap with the arm in between. + let _ = match Foo::X(0) { + Foo::X(0) => 1, + Foo::X(_) | Foo::Y(_) | Foo::Z(0) => 2, + Foo::Z(_) => 1, + _ => 0, + }; + + // Suggest moving `Foo::Z(_)` up. + let _ = match Foo::X(0) { + Foo::X(0) => 1, + Foo::X(_) | Foo::Y(_) => 2, + Foo::Z(_) => 1, + _ => 0, + }; + + // Suggest moving `Foo::X(0)` down. + let _ = match Foo::X(0) { + Foo::X(0) => 1, + Foo::Y(_) | Foo::Z(0) => 2, + Foo::Z(_) => 1, + _ => 0, + }; + + // Don't lint. + let _ = match 0 { + -2 => 1, + -5..=50 => 2, + -150..=88 => 1, + _ => 3, + }; + + struct Bar { + x: u32, + y: u32, + z: u32, + } + + // Lint. + let _ = match None { + Some(Bar { x: 0, y: 5, .. }) => 1, + Some(Bar { y: 10, z: 0, .. }) => 2, + None => 50, + Some(Bar { y: 0, x: 5, .. }) => 1, + _ => 200, + }; } diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index e1ed12f9370..14a672ba2fe 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -1,175 +1,138 @@ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:20:14 +error: this match arm has an identical body to the `_` wildcard arm + --> $DIR/match_same_arms2.rs:11:9 | -LL | _ => { - | ______________^ +LL | / 42 => { +LL | | foo(); +LL | | let mut a = 42 + [23].len() as i32; +LL | | if true { +... | +LL | | a +LL | | }, + | |_________^ help: try removing the arm + | + = note: `-D clippy::match-same-arms` implied by `-D warnings` + = help: or try changing either arm body +note: `_` wildcard arm here + --> $DIR/match_same_arms2.rs:20:9 + | +LL | / _ => { LL | | //~ ERROR match arms have same body LL | | foo(); LL | | let mut a = 42 + [23].len() as i32; ... | LL | | a -LL | | }, - | |_________^ - | - = note: `-D clippy::match-same-arms` implied by `-D warnings` -note: same as this - --> $DIR/match_same_arms2.rs:11:15 - | -LL | 42 => { - | _______________^ -LL | | foo(); -LL | | let mut a = 42 + [23].len() as i32; -LL | | if true { -... | -LL | | a -LL | | }, - | |_________^ -note: `42` has the same arm body as the `_` wildcard, consider removing it - --> $DIR/match_same_arms2.rs:11:15 - | -LL | 42 => { - | _______________^ -LL | | foo(); -LL | | let mut a = 42 + [23].len() as i32; -LL | | if true { -... | -LL | | a LL | | }, | |_________^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:34:15 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:34:9 | LL | 51 => foo(), //~ ERROR match arms have same body - | ^^^^^ + | --^^^^^^^^^ + | | + | help: try merging the arm patterns: `51 | 42` | -note: same as this - --> $DIR/match_same_arms2.rs:33:15 - | -LL | 42 => foo(), - | ^^^^^ -help: consider refactoring into `42 | 51` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms2.rs:33:9 | LL | 42 => foo(), - | ^^ - = help: ...or consider changing the match arm bodies + | ^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:40:17 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:40:9 | LL | None => 24, //~ ERROR match arms have same body - | ^^ + | ----^^^^^^ + | | + | help: try merging the arm patterns: `None | Some(_)` | -note: same as this - --> $DIR/match_same_arms2.rs:39:20 - | -LL | Some(_) => 24, - | ^^ -help: consider refactoring into `Some(_) | None` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms2.rs:39:9 | LL | Some(_) => 24, - | ^^^^^^^ - = help: ...or consider changing the match arm bodies + | ^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:62:28 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:62:9 | LL | (None, Some(a)) => bar(a), //~ ERROR match arms have same body - | ^^^^^^ + | ---------------^^^^^^^^^^ + | | + | help: try merging the arm patterns: `(None, Some(a)) | (Some(a), None)` | -note: same as this - --> $DIR/match_same_arms2.rs:61:28 - | -LL | (Some(a), None) => bar(a), - | ^^^^^^ -help: consider refactoring into `(Some(a), None) | (None, Some(a))` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms2.rs:61:9 | LL | (Some(a), None) => bar(a), - | ^^^^^^^^^^^^^^^ - = help: ...or consider changing the match arm bodies + | ^^^^^^^^^^^^^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:68:26 - | -LL | (.., Some(a)) => bar(a), //~ ERROR match arms have same body - | ^^^^^^ - | -note: same as this - --> $DIR/match_same_arms2.rs:67:26 - | -LL | (Some(a), ..) => bar(a), - | ^^^^^^ -help: consider refactoring into `(Some(a), ..) | (.., Some(a))` +error: this match arm has an identical body to another arm --> $DIR/match_same_arms2.rs:67:9 | LL | (Some(a), ..) => bar(a), - | ^^^^^^^^^^^^^ - = help: ...or consider changing the match arm bodies + | -------------^^^^^^^^^^ + | | + | help: try merging the arm patterns: `(Some(a), ..) | (.., Some(a))` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:68:9 + | +LL | (.., Some(a)) => bar(a), //~ ERROR match arms have same body + | ^^^^^^^^^^^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:102:29 - | -LL | (Ok(_), Some(x)) => println!("ok {}", x), - | ^^^^^^^^^^^^^^^^^^^^ - | -note: same as this - --> $DIR/match_same_arms2.rs:101:29 - | -LL | (Ok(x), Some(_)) => println!("ok {}", x), - | ^^^^^^^^^^^^^^^^^^^^ -help: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))` +error: this match arm has an identical body to another arm --> $DIR/match_same_arms2.rs:101:9 | LL | (Ok(x), Some(_)) => println!("ok {}", x), - | ^^^^^^^^^^^^^^^^ - = help: ...or consider changing the match arm bodies - = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) + | ----------------^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: try merging the arm patterns: `(Ok(x), Some(_)) | (Ok(_), Some(x))` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:102:9 + | +LL | (Ok(_), Some(x)) => println!("ok {}", x), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:117:18 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:117:9 | LL | Ok(_) => println!("ok"), - | ^^^^^^^^^^^^^^ + | -----^^^^^^^^^^^^^^^^^^ + | | + | help: try merging the arm patterns: `Ok(_) | Ok(3)` | -note: same as this - --> $DIR/match_same_arms2.rs:116:18 - | -LL | Ok(3) => println!("ok"), - | ^^^^^^^^^^^^^^ -help: consider refactoring into `Ok(3) | Ok(_)` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms2.rs:116:9 | LL | Ok(3) => println!("ok"), - | ^^^^^ - = help: ...or consider changing the match arm bodies - = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) + | ^^^^^^^^^^^^^^^^^^^^^^^ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms2.rs:144:14 +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:144:9 | LL | 1 => { - | ______________^ + | ^ help: try merging the arm patterns: `1 | 0` + | _________| + | | LL | | empty!(0); LL | | }, | |_________^ | -note: same as this - --> $DIR/match_same_arms2.rs:141:14 - | -LL | 0 => { - | ______________^ -LL | | empty!(0); -LL | | }, - | |_________^ -help: consider refactoring into `0 | 1` + = help: or try changing either arm body +note: other arm here --> $DIR/match_same_arms2.rs:141:9 | -LL | 0 => { - | ^ - = help: ...or consider changing the match arm bodies +LL | / 0 => { +LL | | empty!(0); +LL | | }, + | |_________^ error: match expression looks like `matches!` macro --> $DIR/match_same_arms2.rs:162:16 @@ -184,5 +147,50 @@ LL | | }; | = note: `-D clippy::match-like-matches-macro` implied by `-D warnings` -error: aborting due to 9 previous errors +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:194:9 + | +LL | Foo::X(0) => 1, + | ---------^^^^^ + | | + | help: try merging the arm patterns: `Foo::X(0) | Foo::Z(_)` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:196:9 + | +LL | Foo::Z(_) => 1, + | ^^^^^^^^^^^^^^ + +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:204:9 + | +LL | Foo::Z(_) => 1, + | ---------^^^^^ + | | + | help: try merging the arm patterns: `Foo::Z(_) | Foo::X(0)` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:202:9 + | +LL | Foo::X(0) => 1, + | ^^^^^^^^^^^^^^ + +error: this match arm has an identical body to another arm + --> $DIR/match_same_arms2.rs:227:9 + | +LL | Some(Bar { y: 0, x: 5, .. }) => 1, + | ----------------------------^^^^^ + | | + | help: try merging the arm patterns: `Some(Bar { y: 0, x: 5, .. }) | Some(Bar { x: 0, y: 5, .. })` + | + = help: or try changing either arm body +note: other arm here + --> $DIR/match_same_arms2.rs:224:9 + | +LL | Some(Bar { x: 0, y: 5, .. }) => 1, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 12 previous errors