diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 4775e07d24bd..a7fc399bfe22 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1,4 +1,3 @@ -use clippy_utils::consts::{constant, constant_full_int, miri_to_const, FullInt}; use clippy_utils::diagnostics::{ multispan_sugg, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, }; @@ -19,18 +18,18 @@ use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::{ self as hir, Arm, BindingAnnotation, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, - PatKind, PathSegment, QPath, RangeEnd, TyKind, + PatKind, PathSegment, QPath, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, Ty, VariantDef}; +use rustc_middle::ty::{self, VariantDef}; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{sym, symbol::kw, Span}; -use std::cmp::Ordering; +use rustc_span::{sym, symbol::kw}; mod match_bool; mod match_like_matches; mod match_same_arms; +mod overlapping_arms; mod redundant_pattern_match; mod single_match; @@ -632,7 +631,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { if let ExprKind::Match(ex, arms, MatchSource::Normal) = expr.kind { single_match::check(cx, ex, arms, expr); match_bool::check(cx, ex, arms, expr); - check_overlapping_arms(cx, ex, arms); + overlapping_arms::check(cx, ex, arms); check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); @@ -710,24 +709,6 @@ impl<'tcx> LateLintPass<'tcx> for Matches { extract_msrv_attr!(LateContext); } -fn check_overlapping_arms<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { - if arms.len() >= 2 && cx.typeck_results().expr_ty(ex).is_integral() { - let ranges = all_ranges(cx, arms, cx.typeck_results().expr_ty(ex)); - if !ranges.is_empty() { - if let Some((start, end)) = overlapping(&ranges) { - span_lint_and_note( - cx, - MATCH_OVERLAPPING_ARM, - start.span, - "some ranges overlap", - Some(end.span), - "overlaps with this", - ); - } - } - } -} - fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm<'tcx>]) { let ex_ty = cx.typeck_results().expr_ty(ex).peel_refs(); if is_type_diagnostic_item(cx, ex_ty, sym::Result) { @@ -1219,59 +1200,6 @@ fn opt_parent_let<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<&'a Local<' None } -/// Gets the ranges for each range pattern arm. Applies `ty` bounds for open ranges. -fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec> { - arms.iter() - .filter_map(|arm| { - if let Arm { pat, guard: None, .. } = *arm { - if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind { - let lhs_const = match lhs { - Some(lhs) => constant(cx, cx.typeck_results(), lhs)?.0, - None => miri_to_const(ty.numeric_min_val(cx.tcx)?)?, - }; - let rhs_const = match rhs { - Some(rhs) => constant(cx, cx.typeck_results(), rhs)?.0, - None => miri_to_const(ty.numeric_max_val(cx.tcx)?)?, - }; - - let lhs_val = lhs_const.int_value(cx, ty)?; - let rhs_val = rhs_const.int_value(cx, ty)?; - - let rhs_bound = match range_end { - RangeEnd::Included => EndBound::Included(rhs_val), - RangeEnd::Excluded => EndBound::Excluded(rhs_val), - }; - return Some(SpannedRange { - span: pat.span, - node: (lhs_val, rhs_bound), - }); - } - - if let PatKind::Lit(value) = pat.kind { - let value = constant_full_int(cx, cx.typeck_results(), value)?; - return Some(SpannedRange { - span: pat.span, - node: (value, EndBound::Included(value)), - }); - } - } - None - }) - .collect() -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum EndBound { - Included(T), - Excluded(T), -} - -#[derive(Debug, Eq, PartialEq)] -struct SpannedRange { - pub span: Span, - pub node: (T, EndBound), -} - // Checks if arm has the form `None => None` fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone)) @@ -1317,104 +1245,3 @@ where } ref_count > 1 } - -fn overlapping(ranges: &[SpannedRange]) -> Option<(&SpannedRange, &SpannedRange)> -where - T: Copy + Ord, -{ - #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] - enum BoundKind { - EndExcluded, - Start, - EndIncluded, - } - - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - struct RangeBound<'a, T>(T, BoundKind, &'a SpannedRange); - - impl<'a, T: Copy + Ord> PartialOrd for RangeBound<'a, T> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } - } - - impl<'a, T: Copy + Ord> Ord for RangeBound<'a, T> { - fn cmp(&self, RangeBound(other_value, other_kind, _): &Self) -> Ordering { - let RangeBound(self_value, self_kind, _) = *self; - (self_value, self_kind).cmp(&(*other_value, *other_kind)) - } - } - - let mut values = Vec::with_capacity(2 * ranges.len()); - - for r @ SpannedRange { node: (start, end), .. } in ranges { - values.push(RangeBound(*start, BoundKind::Start, r)); - values.push(match end { - EndBound::Excluded(val) => RangeBound(*val, BoundKind::EndExcluded, r), - EndBound::Included(val) => RangeBound(*val, BoundKind::EndIncluded, r), - }); - } - - values.sort(); - - let mut started = vec![]; - - for RangeBound(_, kind, range) in values { - match kind { - BoundKind::Start => started.push(range), - BoundKind::EndExcluded | BoundKind::EndIncluded => { - let mut overlap = None; - - while let Some(last_started) = started.pop() { - if last_started == range { - break; - } - overlap = Some(last_started); - } - - if let Some(first_overlapping) = overlap { - return Some((range, first_overlapping)); - } - }, - } - } - - None -} - -#[test] -fn test_overlapping() { - use rustc_span::source_map::DUMMY_SP; - - let sp = |s, e| SpannedRange { - span: DUMMY_SP, - node: (s, e), - }; - - assert_eq!(None, overlapping::(&[])); - assert_eq!(None, overlapping(&[sp(1, EndBound::Included(4))])); - assert_eq!( - None, - overlapping(&[sp(1, EndBound::Included(4)), sp(5, EndBound::Included(6))]) - ); - assert_eq!( - None, - overlapping(&[ - sp(1, EndBound::Included(4)), - sp(5, EndBound::Included(6)), - sp(10, EndBound::Included(11)) - ],) - ); - assert_eq!( - Some((&sp(1, EndBound::Included(4)), &sp(3, EndBound::Included(6)))), - overlapping(&[sp(1, EndBound::Included(4)), sp(3, EndBound::Included(6))]) - ); - assert_eq!( - Some((&sp(5, EndBound::Included(6)), &sp(6, EndBound::Included(11)))), - overlapping(&[ - sp(1, EndBound::Included(4)), - sp(5, EndBound::Included(6)), - sp(6, EndBound::Included(11)) - ],) - ); -} diff --git a/clippy_lints/src/matches/overlapping_arms.rs b/clippy_lints/src/matches/overlapping_arms.rs new file mode 100644 index 000000000000..7e6581266902 --- /dev/null +++ b/clippy_lints/src/matches/overlapping_arms.rs @@ -0,0 +1,181 @@ +use clippy_utils::consts::{constant, constant_full_int, miri_to_const, FullInt}; +use clippy_utils::diagnostics::span_lint_and_note; +use core::cmp::Ordering; +use rustc_hir::{Arm, Expr, PatKind, RangeEnd}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_span::Span; + +use super::MATCH_OVERLAPPING_ARM; + +pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { + if arms.len() >= 2 && cx.typeck_results().expr_ty(ex).is_integral() { + let ranges = all_ranges(cx, arms, cx.typeck_results().expr_ty(ex)); + if !ranges.is_empty() { + if let Some((start, end)) = overlapping(&ranges) { + span_lint_and_note( + cx, + MATCH_OVERLAPPING_ARM, + start.span, + "some ranges overlap", + Some(end.span), + "overlaps with this", + ); + } + } + } +} + +/// Gets the ranges for each range pattern arm. Applies `ty` bounds for open ranges. +fn all_ranges<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>], ty: Ty<'tcx>) -> Vec> { + arms.iter() + .filter_map(|arm| { + if let Arm { pat, guard: None, .. } = *arm { + if let PatKind::Range(ref lhs, ref rhs, range_end) = pat.kind { + let lhs_const = match lhs { + Some(lhs) => constant(cx, cx.typeck_results(), lhs)?.0, + None => miri_to_const(ty.numeric_min_val(cx.tcx)?)?, + }; + let rhs_const = match rhs { + Some(rhs) => constant(cx, cx.typeck_results(), rhs)?.0, + None => miri_to_const(ty.numeric_max_val(cx.tcx)?)?, + }; + + let lhs_val = lhs_const.int_value(cx, ty)?; + let rhs_val = rhs_const.int_value(cx, ty)?; + + let rhs_bound = match range_end { + RangeEnd::Included => EndBound::Included(rhs_val), + RangeEnd::Excluded => EndBound::Excluded(rhs_val), + }; + return Some(SpannedRange { + span: pat.span, + node: (lhs_val, rhs_bound), + }); + } + + if let PatKind::Lit(value) = pat.kind { + let value = constant_full_int(cx, cx.typeck_results(), value)?; + return Some(SpannedRange { + span: pat.span, + node: (value, EndBound::Included(value)), + }); + } + } + None + }) + .collect() +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum EndBound { + Included(T), + Excluded(T), +} + +#[derive(Debug, Eq, PartialEq)] +struct SpannedRange { + pub span: Span, + pub node: (T, EndBound), +} + +fn overlapping(ranges: &[SpannedRange]) -> Option<(&SpannedRange, &SpannedRange)> +where + T: Copy + Ord, +{ + #[derive(Copy, Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] + enum BoundKind { + EndExcluded, + Start, + EndIncluded, + } + + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + struct RangeBound<'a, T>(T, BoundKind, &'a SpannedRange); + + impl<'a, T: Copy + Ord> PartialOrd for RangeBound<'a, T> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + impl<'a, T: Copy + Ord> Ord for RangeBound<'a, T> { + fn cmp(&self, RangeBound(other_value, other_kind, _): &Self) -> Ordering { + let RangeBound(self_value, self_kind, _) = *self; + (self_value, self_kind).cmp(&(*other_value, *other_kind)) + } + } + + let mut values = Vec::with_capacity(2 * ranges.len()); + + for r @ SpannedRange { node: (start, end), .. } in ranges { + values.push(RangeBound(*start, BoundKind::Start, r)); + values.push(match end { + EndBound::Excluded(val) => RangeBound(*val, BoundKind::EndExcluded, r), + EndBound::Included(val) => RangeBound(*val, BoundKind::EndIncluded, r), + }); + } + + values.sort(); + + let mut started = vec![]; + + for RangeBound(_, kind, range) in values { + match kind { + BoundKind::Start => started.push(range), + BoundKind::EndExcluded | BoundKind::EndIncluded => { + let mut overlap = None; + + while let Some(last_started) = started.pop() { + if last_started == range { + break; + } + overlap = Some(last_started); + } + + if let Some(first_overlapping) = overlap { + return Some((range, first_overlapping)); + } + }, + } + } + + None +} + +#[test] +fn test_overlapping() { + use rustc_span::source_map::DUMMY_SP; + + let sp = |s, e| SpannedRange { + span: DUMMY_SP, + node: (s, e), + }; + + assert_eq!(None, overlapping::(&[])); + assert_eq!(None, overlapping(&[sp(1, EndBound::Included(4))])); + assert_eq!( + None, + overlapping(&[sp(1, EndBound::Included(4)), sp(5, EndBound::Included(6))]) + ); + assert_eq!( + None, + overlapping(&[ + sp(1, EndBound::Included(4)), + sp(5, EndBound::Included(6)), + sp(10, EndBound::Included(11)) + ],) + ); + assert_eq!( + Some((&sp(1, EndBound::Included(4)), &sp(3, EndBound::Included(6)))), + overlapping(&[sp(1, EndBound::Included(4)), sp(3, EndBound::Included(6))]) + ); + assert_eq!( + Some((&sp(5, EndBound::Included(6)), &sp(6, EndBound::Included(11)))), + overlapping(&[ + sp(1, EndBound::Included(4)), + sp(5, EndBound::Included(6)), + sp(6, EndBound::Included(11)) + ],) + ); +}