Skip to content

Commit 94beb9d

Browse files
committed
apply comments
1 parent 7d38cba commit 94beb9d

13 files changed

+194
-146
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5110,7 +5110,7 @@ Released 2018-09-13
51105110
[`redundant_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else
51115111
[`redundant_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_feature_names
51125112
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
5113-
[`redundant_guard`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_guard
5113+
[`redundant_guards`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_guards
51145114
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
51155115
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
51165116
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
301301
crate::matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS_INFO,
302302
crate::matches::MATCH_WILD_ERR_ARM_INFO,
303303
crate::matches::NEEDLESS_MATCH_INFO,
304-
crate::matches::REDUNDANT_GUARD_INFO,
304+
crate::matches::REDUNDANT_GUARDS_INFO,
305305
crate::matches::REDUNDANT_PATTERN_MATCHING_INFO,
306306
crate::matches::REST_PAT_IN_FULLY_BOUND_STRUCTS_INFO,
307307
crate::matches::SIGNIFICANT_DROP_IN_SCRUTINEE_INFO,

clippy_lints/src/matches/mod.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ mod match_wild_enum;
1616
mod match_wild_err_arm;
1717
mod needless_match;
1818
mod overlapping_arms;
19-
mod redundant_guard;
19+
mod redundant_guards;
2020
mod redundant_pattern_match;
2121
mod rest_pat_in_fully_bound_struct;
2222
mod significant_drop_in_scrutinee;
@@ -934,7 +934,8 @@ declare_clippy_lint! {
934934
/// Checks for unnecessary guards in match expressions.
935935
///
936936
/// ### Why is this bad?
937-
/// It's more complex and much less readable.
937+
/// It's more complex and much less readable. Making it part of the pattern can improve
938+
/// exhaustiveness checking as well.
938939
///
939940
/// ### Example
940941
/// ```rust,ignore
@@ -953,7 +954,7 @@ declare_clippy_lint! {
953954
/// }
954955
/// ```
955956
#[clippy::version = "1.72.0"]
956-
pub REDUNDANT_GUARD,
957+
pub REDUNDANT_GUARDS,
957958
complexity,
958959
"checks for unnecessary guards in match expressions"
959960
}
@@ -1000,7 +1001,7 @@ impl_lint_pass!(Matches => [
10001001
TRY_ERR,
10011002
MANUAL_MAP,
10021003
MANUAL_FILTER,
1003-
REDUNDANT_GUARD,
1004+
REDUNDANT_GUARDS,
10041005
]);
10051006

10061007
impl<'tcx> LateLintPass<'tcx> for Matches {
@@ -1048,7 +1049,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
10481049
needless_match::check_match(cx, ex, arms, expr);
10491050
match_on_vec_items::check(cx, ex);
10501051
match_str_case_mismatch::check(cx, ex, arms);
1051-
redundant_guard::check(cx, arms);
1052+
redundant_guards::check(cx, arms);
10521053

10531054
if !in_constant(cx, expr.hir_id) {
10541055
manual_unwrap_or::check(cx, expr, ex, arms);

clippy_lints/src/matches/redundant_guard.rs renamed to clippy_lints/src/matches/redundant_guards.rs

Lines changed: 51 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,17 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::path_to_local;
23
use clippy_utils::source::snippet_with_applicability;
3-
use clippy_utils::visitors::for_each_expr;
4+
use clippy_utils::visitors::{for_each_expr, is_local_used};
45
use rustc_errors::Applicability;
56
use rustc_hir::def::{DefKind, Res};
6-
use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, HirId, MatchSource, Node, Pat, PatField, PatKind};
7+
use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind};
78
use rustc_lint::LateContext;
8-
use rustc_span::symbol::Ident;
99
use rustc_span::Span;
10-
use std::borrow::Cow;
1110
use std::ops::ControlFlow;
1211

13-
use super::REDUNDANT_GUARD;
12+
use super::REDUNDANT_GUARDS;
1413

15-
pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
14+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
1615
for outer_arm in arms {
1716
let mut app = Applicability::MaybeIncorrect;
1817
let Some(guard) = outer_arm.guard else {
@@ -35,14 +34,14 @@ pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
3534
],
3635
MatchSource::Normal,
3736
) = if_expr.kind
38-
&& let Some((pat_binding, field_name)) = get_pat_binding(cx, scrutinee, outer_arm)
37+
&& let Some((pat_binding, can_use_shorthand)) = get_pat_binding(cx, scrutinee, outer_arm)
3938
{
40-
emit_redundant_guard(
39+
emit_redundant_guards(
4140
cx,
4241
outer_arm,
4342
if_expr.span,
4443
pat_binding,
45-
field_name,
44+
can_use_shorthand,
4645
arm.pat.span,
4746
arm.guard,
4847
&mut app,
@@ -51,14 +50,14 @@ pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
5150
// `Some(x) if let Some(2) = x`
5251
else if let Guard::IfLet(let_expr) = guard
5352
&& let pat = let_expr.pat
54-
&& let Some((pat_binding, field_name)) = get_pat_binding(cx, let_expr.init, outer_arm)
53+
&& let Some((pat_binding, can_use_shorthand)) = get_pat_binding(cx, let_expr.init, outer_arm)
5554
{
56-
emit_redundant_guard(
55+
emit_redundant_guards(
5756
cx,
5857
outer_arm,
5958
let_expr.span,
6059
pat_binding,
61-
field_name,
60+
can_use_shorthand,
6261
pat.span,
6362
None,
6463
&mut app,
@@ -68,7 +67,7 @@ pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
6867
else if let Guard::If(if_expr) = guard
6968
&& let ExprKind::Binary(bin_op, local, pat) = if_expr.kind
7069
&& matches!(bin_op.node, BinOpKind::Eq)
71-
&& let Some((pat_binding, field_name)) = get_pat_binding(cx, local, outer_arm)
70+
&& let Some((pat_binding, can_use_shorthand)) = get_pat_binding(cx, local, outer_arm)
7271
&& expr_can_be_pat(cx, pat)
7372
// Ensure they have the same type. If they don't, we'd need deref coercion which isn't
7473
// possible (currently) in a pattern. In some cases, you can use something like
@@ -78,12 +77,12 @@ pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
7877
// This isn't necessary in the other two checks, as they must be a pattern already.
7978
&& cx.typeck_results().expr_ty(local) == cx.typeck_results().expr_ty(pat)
8079
{
81-
emit_redundant_guard(
80+
emit_redundant_guards(
8281
cx,
8382
outer_arm,
8483
if_expr.span,
8584
pat_binding,
86-
field_name,
85+
can_use_shorthand,
8786
pat.span,
8887
None,
8988
&mut app,
@@ -92,103 +91,75 @@ pub(super) fn check(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
9291
}
9392
}
9493

95-
fn remove_binding(
96-
cx: &LateContext<'_>,
97-
pat: &Pat<'_>,
98-
span: Span,
99-
app: &mut Applicability,
100-
) -> (Cow<'static, str>, Cow<'static, str>) {
101-
let pat_start = snippet_with_applicability(cx, pat.span.with_hi(span.lo()), "<start>", app);
102-
let pat_end = snippet_with_applicability(cx, pat.span.with_lo(span.hi()), "<end>", app);
103-
(pat_start, pat_end)
104-
}
105-
106-
fn get_pat_binding(cx: &LateContext<'_>, guard_expr: &Expr<'_>, outer_arm: &Arm<'_>) -> Option<(Span, Option<Ident>)> {
107-
if let ExprKind::Path(qpath) = guard_expr.kind
108-
&& let res = cx.qpath_res(&qpath, guard_expr.hir_id)
109-
&& let Res::Local(hir_id) = res
110-
&& local_came_from_and_not_used(cx, &res, outer_arm.pat.hir_id, outer_arm.body)
111-
{
112-
return cx.tcx.hir().res_span(res).map(|span| {
113-
(
114-
span,
115-
match cx.tcx.hir().get_parent(hir_id) {
116-
Node::PatField(PatField { ident, .. }) => Some(*ident),
117-
_ => None,
118-
},
119-
)
94+
fn get_pat_binding<'tcx>(cx: &LateContext<'tcx>, guard_expr: &Expr<'_>, outer_arm: &Arm<'tcx>) -> Option<(Span, bool)> {
95+
if let Some(local) = path_to_local(guard_expr) && !is_local_used(cx, outer_arm.body, local) {
96+
let mut span = None;
97+
// FIXME: hir_id == local is only true for the first or, this will always be false
98+
let mut multiple_bindings = false;
99+
outer_arm.pat.each_binding(|_, hir_id, binding_span, _| {
100+
if hir_id == local && span.replace(binding_span).is_some() {
101+
multiple_bindings = true;
102+
}
120103
});
104+
105+
if !multiple_bindings {
106+
return span.map(|span| {
107+
(
108+
span,
109+
!matches!(cx.tcx.hir().get_parent(local), Node::PatField(_)),
110+
)
111+
});
112+
}
121113
}
122114

123115
None
124116
}
125117

126-
fn get_guard(cx: &LateContext<'_>, guard: Option<Guard<'_>>, app: &mut Applicability) -> Cow<'static, str> {
118+
fn get_guard(cx: &LateContext<'_>, guard: Option<Guard<'_>>, app: &mut Applicability) -> String {
127119
guard.map_or_else(
128-
|| Cow::Borrowed(""),
120+
|| String::new(),
129121
|guard| {
130122
let (prefix, span) = match guard {
131123
Guard::If(e) => ("if", e.span),
132124
Guard::IfLet(l) => ("if let", l.span),
133125
};
134126

135-
Cow::Owned(format!(
136-
" {prefix} {}",
137-
snippet_with_applicability(cx, span, "<guard>", app),
138-
))
127+
format!(" {prefix} {}", snippet_with_applicability(cx, span, "<guard>", app))
139128
},
140129
)
141130
}
142131

143-
fn local_came_from_and_not_used(cx: &LateContext<'_>, local: &Res, target: HirId, body: &Expr<'_>) -> bool {
144-
if let Res::Local(local) = local
145-
&& cx.tcx.hir().parent_iter(*local).any(|(p, _)| p == target)
146-
&& for_each_expr(body, |expr| {
147-
if let ExprKind::Path(qpath) = expr.kind
148-
&& let Res::Local(local_ref) = cx.qpath_res(&qpath, expr.hir_id)
149-
&& *local == local_ref
150-
{
151-
return ControlFlow::Break(());
152-
}
153-
154-
ControlFlow::Continue(())
155-
})
156-
.is_none()
157-
{
158-
return true;
159-
}
160-
161-
false
162-
}
163-
164132
#[expect(clippy::too_many_arguments)]
165-
fn emit_redundant_guard(
133+
fn emit_redundant_guards(
166134
cx: &LateContext<'_>,
167135
outer_arm: &Arm<'_>,
168136
guard_span: Span,
169137
pat_binding: Span,
170-
field_name: Option<Ident>,
138+
can_use_shorthand: bool,
171139
pat_span: Span,
172140
inner_guard: Option<Guard<'_>>,
173141
app: &mut Applicability,
174142
) {
175-
let (pat_start, pat_end) = remove_binding(cx, outer_arm.pat, pat_binding, app);
176-
let binding_replacement = snippet_with_applicability(cx, pat_span, "<binding_repl>", app);
177-
178143
span_lint_and_then(
179144
cx,
180-
REDUNDANT_GUARD,
145+
REDUNDANT_GUARDS,
181146
guard_span.source_callsite(),
182147
"redundant guard",
183148
|diag| {
184-
diag.span_suggestion(
185-
outer_arm.span.with_hi(guard_span.source_callsite().hi()),
149+
let binding_replacement = snippet_with_applicability(cx, pat_span, "<binding_repl>", app);
150+
diag.multipart_suggestion_verbose(
186151
"try",
187-
format!(
188-
"{pat_start}{}{binding_replacement}{pat_end}{}",
189-
field_name.map_or_else(|| Cow::Borrowed(""), |i| format!("{}: ", i.as_str()).into()),
190-
get_guard(cx, inner_guard, app),
191-
),
152+
vec![
153+
if can_use_shorthand {
154+
(pat_binding, binding_replacement.into_owned())
155+
} else {
156+
(pat_binding.shrink_to_hi(), format!(": {binding_replacement}"))
157+
},
158+
(
159+
guard_span.source_callsite().with_lo(outer_arm.pat.span.hi()),
160+
get_guard(cx, inner_guard, app),
161+
),
162+
],
192163
*app,
193164
);
194165
},

tests/ui/match_expr_like_matches_macro.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
dead_code,
77
clippy::equatable_if_let,
88
clippy::needless_borrowed_reference,
9-
clippy::redundant_guard
9+
clippy::redundant_guards
1010
)]
1111

1212
fn main() {

tests/ui/match_expr_like_matches_macro.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
dead_code,
77
clippy::equatable_if_let,
88
clippy::needless_borrowed_reference,
9-
clippy::redundant_guard
9+
clippy::redundant_guards
1010
)]
1111

1212
fn main() {

tests/ui/redundant_guard.stderr

Lines changed: 0 additions & 52 deletions
This file was deleted.

0 commit comments

Comments
 (0)