diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 29af4812351..1f61e4df30d 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -34,24 +34,45 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { ], MatchSource::Normal, ) = if_expr.kind + && let Some((binding_span, is_field, is_byref)) = get_pat_binding(cx, scrutinee, outer_arm) { + if is_field && is_byref { return; } + let pat_span = if let PatKind::Ref(pat, _) = arm.pat.kind { + if is_byref { pat.span } else { continue; } + } else { + if is_byref { continue; } + arm.pat.span + }; + emit_redundant_guards( cx, outer_arm, if_expr.span, - scrutinee, - arm.pat.span, + pat_span, + binding_span, + is_field, arm.guard, ); } // `Some(x) if let Some(2) = x` - else if let Guard::IfLet(let_expr) = guard { + else if let Guard::IfLet(let_expr) = guard + && let Some((binding_span, is_field, is_byref)) = get_pat_binding(cx, let_expr.init, outer_arm) + { + if is_field && is_byref { return; } + let pat_span = if let PatKind::Ref(pat, _) = let_expr.pat.kind { + if is_byref && !is_field { pat.span } else { continue; } + } else { + if is_byref { continue; } + let_expr.pat.span + }; + emit_redundant_guards( cx, outer_arm, let_expr.span, - let_expr.init, - let_expr.pat.span, + pat_span, + binding_span, + is_field, None, ); } @@ -67,31 +88,48 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { // // This isn't necessary in the other two checks, as they must be a pattern already. && cx.typeck_results().expr_ty(local) == cx.typeck_results().expr_ty(pat) + && let Some((binding_span, is_field, is_byref)) = get_pat_binding(cx, local, outer_arm) { + if is_field && is_byref { return; } + let pat_span = if let ExprKind::AddrOf(rustc_ast::BorrowKind::Ref, _, expr) = pat.kind { + if is_byref { expr.span } else { continue; } + } else { + if is_byref { continue; } + pat.span + }; + emit_redundant_guards( cx, outer_arm, if_expr.span, - local, - pat.span, + pat_span, + binding_span, + is_field, None, ); } } } -fn get_pat_binding<'tcx>(cx: &LateContext<'tcx>, guard_expr: &Expr<'_>, outer_arm: &Arm<'tcx>) -> Option<(Span, bool)> { +fn get_pat_binding<'tcx>( + cx: &LateContext<'tcx>, + guard_expr: &Expr<'_>, + outer_arm: &Arm<'tcx>, +) -> Option<(Span, bool, bool)> { if let Some(local) = path_to_local(guard_expr) && !is_local_used(cx, outer_arm.body, local) { let mut span = None; let mut multiple_bindings = false; + let mut is_byref = false; // `each_binding` gives the `HirId` of the `Pat` itself, not the binding outer_arm.pat.walk(|pat| { - if let PatKind::Binding(_, hir_id, _, _) = pat.kind + if let PatKind::Binding(bind_annot, hir_id, _, _) = pat.kind && hir_id == local - && span.replace(pat.span).is_some() { - multiple_bindings = true; - return false; + is_byref = matches!(bind_annot.0, rustc_ast::ByRef::Yes); + if span.replace(pat.span).is_some() { + multiple_bindings = true; + return false; + } } true @@ -102,7 +140,8 @@ fn get_pat_binding<'tcx>(cx: &LateContext<'tcx>, guard_expr: &Expr<'_>, outer_ar return span.map(|span| { ( span, - !matches!(cx.tcx.hir().get_parent(local), Node::PatField(_)), + matches!(cx.tcx.hir().get_parent(local), Node::PatField(_)), + is_byref, ) }); } @@ -115,14 +154,12 @@ fn emit_redundant_guards<'tcx>( cx: &LateContext<'tcx>, outer_arm: &Arm<'tcx>, guard_span: Span, - local: &Expr<'_>, pat_span: Span, + binding_span: Span, + field_binding: bool, inner_guard: Option>, ) { let mut app = Applicability::MaybeIncorrect; - let Some((pat_binding, can_use_shorthand)) = get_pat_binding(cx, local, outer_arm) else { - return; - }; span_lint_and_then( cx, @@ -134,10 +171,10 @@ fn emit_redundant_guards<'tcx>( diag.multipart_suggestion_verbose( "try", vec![ - if can_use_shorthand { - (pat_binding, binding_replacement.into_owned()) + if field_binding { + (binding_span.shrink_to_hi(), format!(": {binding_replacement}")) } else { - (pat_binding.shrink_to_hi(), format!(": {binding_replacement}")) + (binding_span, binding_replacement.into_owned()) }, ( guard_span.source_callsite().with_lo(outer_arm.pat.span.hi()), diff --git a/tests/ui/redundant_guards.fixed b/tests/ui/redundant_guards.fixed index 9a1ec3a4d36..6793b9c0f6d 100644 --- a/tests/ui/redundant_guards.fixed +++ b/tests/ui/redundant_guards.fixed @@ -143,3 +143,44 @@ fn g(opt_s: Option) { _ => {}, } } + +mod issue11465 { + enum A { + Foo([u8; 3]), + } + + struct B { + b: String, + c: i32, + } + + fn issue11465() { + let c = Some(1); + match c { + Some(1) => {}, + Some(2) => {}, + Some(3) => {}, + _ => {}, + }; + + let enum_a = A::Foo([98, 97, 114]); + match enum_a { + A::Foo(ref arr) if arr == b"foo" => {}, + A::Foo(ref arr) if let b"bar" = arr => {}, + A::Foo(ref arr) if matches!(arr, b"baz") => {}, + _ => {}, + }; + + let struct_b = B { + b: "bar".to_string(), + c: 42, + }; + match struct_b { + B { ref b, .. } if b == "bar" => {}, + B { ref c, .. } if c == &1 => {}, + B { ref c, .. } if let &1 = c => {}, + B { ref c, .. } if matches!(c, &1) => {}, + _ => {}, + } + } +} diff --git a/tests/ui/redundant_guards.rs b/tests/ui/redundant_guards.rs index e2e0ee816c5..e63cd29e8af 100644 --- a/tests/ui/redundant_guards.rs +++ b/tests/ui/redundant_guards.rs @@ -143,3 +143,44 @@ fn g(opt_s: Option) { _ => {}, } } + +mod issue11465 { + enum A { + Foo([u8; 3]), + } + + struct B { + b: String, + c: i32, + } + + fn issue11465() { + let c = Some(1); + match c { + Some(ref x) if x == &1 => {}, + Some(ref x) if let &2 = x => {}, + Some(ref x) if matches!(x, &3) => {}, + _ => {}, + }; + + let enum_a = A::Foo([98, 97, 114]); + match enum_a { + A::Foo(ref arr) if arr == b"foo" => {}, + A::Foo(ref arr) if let b"bar" = arr => {}, + A::Foo(ref arr) if matches!(arr, b"baz") => {}, + _ => {}, + }; + + let struct_b = B { + b: "bar".to_string(), + c: 42, + }; + match struct_b { + B { ref b, .. } if b == "bar" => {}, + B { ref c, .. } if c == &1 => {}, + B { ref c, .. } if let &1 = c => {}, + B { ref c, .. } if matches!(c, &1) => {}, + _ => {}, + } + } +} diff --git a/tests/ui/redundant_guards.stderr b/tests/ui/redundant_guards.stderr index 50077234382..54b5f2464e6 100644 --- a/tests/ui/redundant_guards.stderr +++ b/tests/ui/redundant_guards.stderr @@ -94,5 +94,41 @@ LL - x if matches!(x, Some(0)) => .., LL + Some(0) => .., | -error: aborting due to 8 previous errors +error: redundant guard + --> $DIR/redundant_guards.rs:160:28 + | +LL | Some(ref x) if x == &1 => {}, + | ^^^^^^^ + | +help: try + | +LL - Some(ref x) if x == &1 => {}, +LL + Some(1) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:161:28 + | +LL | Some(ref x) if let &2 = x => {}, + | ^^^^^^^^^^ + | +help: try + | +LL - Some(ref x) if let &2 = x => {}, +LL + Some(2) => {}, + | + +error: redundant guard + --> $DIR/redundant_guards.rs:162:28 + | +LL | Some(ref x) if matches!(x, &3) => {}, + | ^^^^^^^^^^^^^^^ + | +help: try + | +LL - Some(ref x) if matches!(x, &3) => {}, +LL + Some(3) => {}, + | + +error: aborting due to 11 previous errors