add byref checking for the guard's local

This commit is contained in:
mojave2 2023-09-13 11:13:51 +08:00
parent 69fcbfdac0
commit 7f870201d3
No known key found for this signature in database
4 changed files with 176 additions and 21 deletions

View File

@ -34,24 +34,45 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
], ],
MatchSource::Normal, MatchSource::Normal,
) = if_expr.kind ) = 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( emit_redundant_guards(
cx, cx,
outer_arm, outer_arm,
if_expr.span, if_expr.span,
scrutinee, pat_span,
arm.pat.span, binding_span,
is_field,
arm.guard, arm.guard,
); );
} }
// `Some(x) if let Some(2) = x` // `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( emit_redundant_guards(
cx, cx,
outer_arm, outer_arm,
let_expr.span, let_expr.span,
let_expr.init, pat_span,
let_expr.pat.span, binding_span,
is_field,
None, 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. // 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) && 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( emit_redundant_guards(
cx, cx,
outer_arm, outer_arm,
if_expr.span, if_expr.span,
local, pat_span,
pat.span, binding_span,
is_field,
None, 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) { if let Some(local) = path_to_local(guard_expr) && !is_local_used(cx, outer_arm.body, local) {
let mut span = None; let mut span = None;
let mut multiple_bindings = false; let mut multiple_bindings = false;
let mut is_byref = false;
// `each_binding` gives the `HirId` of the `Pat` itself, not the binding // `each_binding` gives the `HirId` of the `Pat` itself, not the binding
outer_arm.pat.walk(|pat| { 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 && hir_id == local
&& span.replace(pat.span).is_some()
{ {
multiple_bindings = true; is_byref = matches!(bind_annot.0, rustc_ast::ByRef::Yes);
return false; if span.replace(pat.span).is_some() {
multiple_bindings = true;
return false;
}
} }
true true
@ -102,7 +140,8 @@ fn get_pat_binding<'tcx>(cx: &LateContext<'tcx>, guard_expr: &Expr<'_>, outer_ar
return span.map(|span| { return span.map(|span| {
( (
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>, cx: &LateContext<'tcx>,
outer_arm: &Arm<'tcx>, outer_arm: &Arm<'tcx>,
guard_span: Span, guard_span: Span,
local: &Expr<'_>,
pat_span: Span, pat_span: Span,
binding_span: Span,
field_binding: bool,
inner_guard: Option<Guard<'_>>, inner_guard: Option<Guard<'_>>,
) { ) {
let mut app = Applicability::MaybeIncorrect; 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( span_lint_and_then(
cx, cx,
@ -134,10 +171,10 @@ fn emit_redundant_guards<'tcx>(
diag.multipart_suggestion_verbose( diag.multipart_suggestion_verbose(
"try", "try",
vec![ vec![
if can_use_shorthand { if field_binding {
(pat_binding, binding_replacement.into_owned()) (binding_span.shrink_to_hi(), format!(": {binding_replacement}"))
} else { } 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()), guard_span.source_callsite().with_lo(outer_arm.pat.span.hi()),

View File

@ -143,3 +143,44 @@ fn g(opt_s: Option<S>) {
_ => {}, _ => {},
} }
} }
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) => {},
_ => {},
}
}
}

View File

@ -143,3 +143,44 @@ fn g(opt_s: Option<S>) {
_ => {}, _ => {},
} }
} }
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) => {},
_ => {},
}
}
}

View File

@ -94,5 +94,41 @@ LL - x if matches!(x, Some(0)) => ..,
LL + 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