Skip to content

Commit d019fd9

Browse files
borsbors
committed
Auto merge of rust-lang#9855 - Alexendoo:needless-borrowed-ref-extra, r=xFrednet
Extend `needless_borrowed_reference` to structs and tuples, ignore _ changelog: [`needless_borrowed_reference`]: Lint struct and tuple patterns, and patterns containing `_` Now lints patterns like ```rust &(ref a, ref b) &Tuple(ref a, ref b) &Struct { ref a, ref b } &(ref a, _) ```
2 parents 01f40ce + f75fc85 commit d019fd9

13 files changed

+356
-114
lines changed

‎clippy_lints/src/enum_variants.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ impl LateLintPass<'_> for EnumVariantNames {
250250
let item_name = item.ident.name.as_str();
251251
let item_camel = to_camel_case(item_name);
252252
if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
253-
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
253+
if let Some((mod_name, mod_camel)) = self.modules.last() {
254254
// constants don't have surrounding modules
255255
if !mod_camel.is_empty() {
256256
if mod_name == &item.ident.name {

‎clippy_lints/src/int_plus_one.rs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,58 +62,54 @@ impl IntPlusOne {
6262
fn check_binop(cx: &EarlyContext<'_>, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<String> {
6363
match (binop, &lhs.kind, &rhs.kind) {
6464
// case where `x - 1 >= ...` or `-1 + x >= ...`
65-
(BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => {
65+
(BinOpKind::Ge, ExprKind::Binary(lhskind, lhslhs, lhsrhs), _) => {
6666
match (lhskind.node, &lhslhs.kind, &lhsrhs.kind) {
6767
// `-1 + x`
68-
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
68+
(BinOpKind::Add, ExprKind::Lit(lit), _) if Self::check_lit(lit, -1) => {
6969
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::Lhs)
7070
},
7171
// `x - 1`
72-
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
72+
(BinOpKind::Sub, _, ExprKind::Lit(lit)) if Self::check_lit(lit, 1) => {
7373
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::Lhs)
7474
},
7575
_ => None,
7676
}
7777
},
7878
// case where `... >= y + 1` or `... >= 1 + y`
79-
(BinOpKind::Ge, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs))
80-
if rhskind.node == BinOpKind::Add =>
81-
{
79+
(BinOpKind::Ge, _, ExprKind::Binary(rhskind, rhslhs, rhsrhs)) if rhskind.node == BinOpKind::Add => {
8280
match (&rhslhs.kind, &rhsrhs.kind) {
8381
// `y + 1` and `1 + y`
84-
(&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
82+
(ExprKind::Lit(lit), _) if Self::check_lit(lit, 1) => {
8583
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::Rhs)
8684
},
87-
(_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
85+
(_, ExprKind::Lit(lit)) if Self::check_lit(lit, 1) => {
8886
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::Rhs)
8987
},
9088
_ => None,
9189
}
9290
},
9391
// case where `x + 1 <= ...` or `1 + x <= ...`
94-
(BinOpKind::Le, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _)
95-
if lhskind.node == BinOpKind::Add =>
96-
{
92+
(BinOpKind::Le, ExprKind::Binary(lhskind, lhslhs, lhsrhs), _) if lhskind.node == BinOpKind::Add => {
9793
match (&lhslhs.kind, &lhsrhs.kind) {
9894
// `1 + x` and `x + 1`
99-
(&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
95+
(ExprKind::Lit(lit), _) if Self::check_lit(lit, 1) => {
10096
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::Lhs)
10197
},
102-
(_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
98+
(_, ExprKind::Lit(lit)) if Self::check_lit(lit, 1) => {
10399
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::Lhs)
104100
},
105101
_ => None,
106102
}
107103
},
108104
// case where `... >= y - 1` or `... >= -1 + y`
109-
(BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => {
105+
(BinOpKind::Le, _, ExprKind::Binary(rhskind, rhslhs, rhsrhs)) => {
110106
match (rhskind.node, &rhslhs.kind, &rhsrhs.kind) {
111107
// `-1 + y`
112-
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
108+
(BinOpKind::Add, ExprKind::Lit(lit), _) if Self::check_lit(lit, -1) => {
113109
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::Rhs)
114110
},
115111
// `y - 1`
116-
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
112+
(BinOpKind::Sub, _, ExprKind::Lit(lit)) if Self::check_lit(lit, 1) => {
117113
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::Rhs)
118114
},
119115
_ => None,

‎clippy_lints/src/len_zero.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,8 +366,7 @@ fn check_for_is_empty<'tcx>(
366366
}
367367

368368
fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {
369-
if let (&ExprKind::MethodCall(method_path, receiver, args, _), &ExprKind::Lit(ref lit)) = (&method.kind, &lit.kind)
370-
{
369+
if let (&ExprKind::MethodCall(method_path, receiver, args, _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
371370
// check if we are in an is_empty() method
372371
if let Some(name) = get_item_name(cx, method) {
373372
if name.as_str() == "is_empty" {

‎clippy_lints/src/needless_borrowed_ref.rs

Lines changed: 69 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ declare_clippy_lint! {
3636
declare_lint_pass!(NeedlessBorrowedRef => [NEEDLESS_BORROWED_REFERENCE]);
3737

3838
impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef {
39-
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
40-
if pat.span.from_expansion() {
39+
fn check_pat(&mut self, cx: &LateContext<'tcx>, ref_pat: &'tcx Pat<'_>) {
40+
if ref_pat.span.from_expansion() {
4141
// OK, simple enough, lints doesn't check in macro.
4242
return;
4343
}
4444

4545
// Do not lint patterns that are part of an OR `|` pattern, the binding mode must match in all arms
46-
for (_, node) in cx.tcx.hir().parent_iter(pat.hir_id) {
46+
for (_, node) in cx.tcx.hir().parent_iter(ref_pat.hir_id) {
4747
let Node::Pat(pat) = node else { break };
4848

4949
if matches!(pat.kind, PatKind::Or(_)) {
@@ -52,20 +52,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef {
5252
}
5353

5454
// Only lint immutable refs, because `&mut ref T` may be useful.
55-
let PatKind::Ref(sub_pat, Mutability::Not) = pat.kind else { return };
55+
let PatKind::Ref(pat, Mutability::Not) = ref_pat.kind else { return };
5656

57-
match sub_pat.kind {
57+
match pat.kind {
5858
// Check sub_pat got a `ref` keyword (excluding `ref mut`).
5959
PatKind::Binding(BindingAnnotation::REF, _, ident, None) => {
6060
span_lint_and_then(
6161
cx,
6262
NEEDLESS_BORROWED_REFERENCE,
63-
pat.span,
63+
ref_pat.span,
6464
"this pattern takes a reference on something that is being dereferenced",
6565
|diag| {
6666
// `&ref ident`
6767
// ^^^^^
68-
let span = pat.span.until(ident.span);
68+
let span = ref_pat.span.until(ident.span);
6969
diag.span_suggestion_verbose(
7070
span,
7171
"try removing the `&ref` part",
@@ -84,41 +84,71 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef {
8484
}),
8585
after,
8686
) => {
87-
let mut suggestions = Vec::new();
88-
89-
for element_pat in itertools::chain(before, after) {
90-
if let PatKind::Binding(BindingAnnotation::REF, _, ident, None) = element_pat.kind {
91-
// `&[..., ref ident, ...]`
92-
// ^^^^
93-
let span = element_pat.span.until(ident.span);
94-
suggestions.push((span, String::new()));
95-
} else {
96-
return;
97-
}
98-
}
87+
check_subpatterns(
88+
cx,
89+
"dereferencing a slice pattern where every element takes a reference",
90+
ref_pat,
91+
pat,
92+
itertools::chain(before, after),
93+
);
94+
},
95+
PatKind::Tuple(subpatterns, _) | PatKind::TupleStruct(_, subpatterns, _) => {
96+
check_subpatterns(
97+
cx,
98+
"dereferencing a tuple pattern where every element takes a reference",
99+
ref_pat,
100+
pat,
101+
subpatterns,
102+
);
103+
},
104+
PatKind::Struct(_, fields, _) => {
105+
check_subpatterns(
106+
cx,
107+
"dereferencing a struct pattern where every field's pattern takes a reference",
108+
ref_pat,
109+
pat,
110+
fields.iter().map(|field| field.pat),
111+
);
112+
},
113+
_ => {},
114+
}
115+
}
116+
}
99117

100-
if !suggestions.is_empty() {
101-
span_lint_and_then(
102-
cx,
103-
NEEDLESS_BORROWED_REFERENCE,
104-
pat.span,
105-
"dereferencing a slice pattern where every element takes a reference",
106-
|diag| {
107-
// `&[...]`
108-
// ^
109-
let span = pat.span.until(sub_pat.span);
110-
suggestions.push((span, String::new()));
118+
fn check_subpatterns<'tcx>(
119+
cx: &LateContext<'tcx>,
120+
message: &str,
121+
ref_pat: &Pat<'_>,
122+
pat: &Pat<'_>,
123+
subpatterns: impl IntoIterator<Item = &'tcx Pat<'tcx>>,
124+
) {
125+
let mut suggestions = Vec::new();
111126

112-
diag.multipart_suggestion(
113-
"try removing the `&` and `ref` parts",
114-
suggestions,
115-
Applicability::MachineApplicable,
116-
);
117-
},
118-
);
119-
}
127+
for subpattern in subpatterns {
128+
match subpattern.kind {
129+
PatKind::Binding(BindingAnnotation::REF, _, ident, None) => {
130+
// `ref ident`
131+
// ^^^^
132+
let span = subpattern.span.until(ident.span);
133+
suggestions.push((span, String::new()));
120134
},
121-
_ => {},
135+
PatKind::Wild => {},
136+
_ => return,
122137
}
123138
}
139+
140+
if !suggestions.is_empty() {
141+
span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, ref_pat.span, message, |diag| {
142+
// `&pat`
143+
// ^
144+
let span = ref_pat.span.until(pat.span);
145+
suggestions.push((span, String::new()));
146+
147+
diag.multipart_suggestion(
148+
"try removing the `&` and `ref` parts",
149+
suggestions,
150+
Applicability::MachineApplicable,
151+
);
152+
});
153+
}
124154
}

‎clippy_lints/src/unsafe_removed_from_name.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ fn check_use_tree(use_tree: &UseTree, cx: &EarlyContext<'_>, span: Span) {
5050
},
5151
UseTreeKind::Simple(None, ..) | UseTreeKind::Glob => {},
5252
UseTreeKind::Nested(ref nested_use_tree) => {
53-
for &(ref use_tree, _) in nested_use_tree {
53+
for (use_tree, _) in nested_use_tree {
5454
check_use_tree(use_tree, cx, span);
5555
}
5656
},

‎clippy_utils/src/consts.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ pub enum Constant {
5151
impl PartialEq for Constant {
5252
fn eq(&self, other: &Self) -> bool {
5353
match (self, other) {
54-
(&Self::Str(ref ls), &Self::Str(ref rs)) => ls == rs,
55-
(&Self::Binary(ref l), &Self::Binary(ref r)) => l == r,
54+
(Self::Str(ls), Self::Str(rs)) => ls == rs,
55+
(Self::Binary(l), Self::Binary(r)) => l == r,
5656
(&Self::Char(l), &Self::Char(r)) => l == r,
5757
(&Self::Int(l), &Self::Int(r)) => l == r,
5858
(&Self::F64(l), &Self::F64(r)) => {
@@ -69,8 +69,8 @@ impl PartialEq for Constant {
6969
},
7070
(&Self::Bool(l), &Self::Bool(r)) => l == r,
7171
(&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => l == r,
72-
(&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => ls == rs && lv == rv,
73-
(&Self::Ref(ref lb), &Self::Ref(ref rb)) => *lb == *rb,
72+
(Self::Repeat(lv, ls), Self::Repeat(rv, rs)) => ls == rs && lv == rv,
73+
(Self::Ref(lb), Self::Ref(rb)) => *lb == *rb,
7474
// TODO: are there inter-type equalities?
7575
_ => false,
7676
}
@@ -126,17 +126,17 @@ impl Hash for Constant {
126126
impl Constant {
127127
pub fn partial_cmp(tcx: TyCtxt<'_>, cmp_type: Ty<'_>, left: &Self, right: &Self) -> Option<Ordering> {
128128
match (left, right) {
129-
(&Self::Str(ref ls), &Self::Str(ref rs)) => Some(ls.cmp(rs)),
130-
(&Self::Char(ref l), &Self::Char(ref r)) => Some(l.cmp(r)),
129+
(Self::Str(ls), Self::Str(rs)) => Some(ls.cmp(rs)),
130+
(Self::Char(l), Self::Char(r)) => Some(l.cmp(r)),
131131
(&Self::Int(l), &Self::Int(r)) => match *cmp_type.kind() {
132132
ty::Int(int_ty) => Some(sext(tcx, l, int_ty).cmp(&sext(tcx, r, int_ty))),
133133
ty::Uint(_) => Some(l.cmp(&r)),
134134
_ => bug!("Not an int type"),
135135
},
136136
(&Self::F64(l), &Self::F64(r)) => l.partial_cmp(&r),
137137
(&Self::F32(l), &Self::F32(r)) => l.partial_cmp(&r),
138-
(&Self::Bool(ref l), &Self::Bool(ref r)) => Some(l.cmp(r)),
139-
(&Self::Tuple(ref l), &Self::Tuple(ref r)) if l.len() == r.len() => match *cmp_type.kind() {
138+
(Self::Bool(l), Self::Bool(r)) => Some(l.cmp(r)),
139+
(Self::Tuple(l), Self::Tuple(r)) if l.len() == r.len() => match *cmp_type.kind() {
140140
ty::Tuple(tys) if tys.len() == l.len() => l
141141
.iter()
142142
.zip(r)
@@ -146,7 +146,7 @@ impl Constant {
146146
.unwrap_or_else(|| Some(l.len().cmp(&r.len()))),
147147
_ => None,
148148
},
149-
(&Self::Vec(ref l), &Self::Vec(ref r)) => {
149+
(Self::Vec(l), Self::Vec(r)) => {
150150
let (ty::Array(cmp_type, _) | ty::Slice(cmp_type)) = *cmp_type.kind() else {
151151
return None
152152
};
@@ -155,7 +155,7 @@ impl Constant {
155155
.find(|r| r.map_or(true, |o| o != Ordering::Equal))
156156
.unwrap_or_else(|| Some(l.len().cmp(&r.len())))
157157
},
158-
(&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => {
158+
(Self::Repeat(lv, ls), Self::Repeat(rv, rs)) => {
159159
match Self::partial_cmp(
160160
tcx,
161161
match *cmp_type.kind() {
@@ -169,7 +169,7 @@ impl Constant {
169169
x => x,
170170
}
171171
},
172-
(&Self::Ref(ref lb), &Self::Ref(ref rb)) => Self::partial_cmp(
172+
(Self::Ref(lb), Self::Ref(rb)) => Self::partial_cmp(
173173
tcx,
174174
match *cmp_type.kind() {
175175
ty::Ref(_, ty, _) => ty,

‎clippy_utils/src/hir_utils.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ impl HirEqInterExpr<'_, '_, '_> {
266266
(&ExprKind::Let(l), &ExprKind::Let(r)) => {
267267
self.eq_pat(l.pat, r.pat) && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && self.eq_expr(l.init, r.init)
268268
},
269-
(&ExprKind::Lit(ref l), &ExprKind::Lit(ref r)) => l.node == r.node,
269+
(ExprKind::Lit(l), ExprKind::Lit(r)) => l.node == r.node,
270270
(&ExprKind::Loop(lb, ref ll, ref lls, _), &ExprKind::Loop(rb, ref rl, ref rls, _)) => {
271271
lls == rls && self.eq_block(lb, rb) && both(ll, rl, |l, r| l.ident.name == r.ident.name)
272272
},
@@ -291,8 +291,8 @@ impl HirEqInterExpr<'_, '_, '_> {
291291
(&ExprKind::Repeat(le, ll), &ExprKind::Repeat(re, rl)) => {
292292
self.eq_expr(le, re) && self.eq_array_length(ll, rl)
293293
},
294-
(&ExprKind::Ret(ref l), &ExprKind::Ret(ref r)) => both(l, r, |l, r| self.eq_expr(l, r)),
295-
(&ExprKind::Path(ref l), &ExprKind::Path(ref r)) => self.eq_qpath(l, r),
294+
(ExprKind::Ret(l), ExprKind::Ret(r)) => both(l, r, |l, r| self.eq_expr(l, r)),
295+
(ExprKind::Path(l), ExprKind::Path(r)) => self.eq_qpath(l, r),
296296
(&ExprKind::Struct(l_path, lf, ref lo), &ExprKind::Struct(r_path, rf, ref ro)) => {
297297
self.eq_qpath(l_path, r_path)
298298
&& both(lo, ro, |l, r| self.eq_expr(l, r))
@@ -362,7 +362,7 @@ impl HirEqInterExpr<'_, '_, '_> {
362362
}
363363
eq
364364
},
365-
(&PatKind::Path(ref l), &PatKind::Path(ref r)) => self.eq_qpath(l, r),
365+
(PatKind::Path(l), PatKind::Path(r)) => self.eq_qpath(l, r),
366366
(&PatKind::Lit(l), &PatKind::Lit(r)) => self.eq_expr(l, r),
367367
(&PatKind::Tuple(l, ls), &PatKind::Tuple(r, rs)) => ls == rs && over(l, r, |l, r| self.eq_pat(l, r)),
368368
(&PatKind::Range(ref ls, ref le, li), &PatKind::Range(ref rs, ref re, ri)) => {
@@ -429,13 +429,11 @@ impl HirEqInterExpr<'_, '_, '_> {
429429
match (&left.kind, &right.kind) {
430430
(&TyKind::Slice(l_vec), &TyKind::Slice(r_vec)) => self.eq_ty(l_vec, r_vec),
431431
(&TyKind::Array(lt, ll), &TyKind::Array(rt, rl)) => self.eq_ty(lt, rt) && self.eq_array_length(ll, rl),
432-
(&TyKind::Ptr(ref l_mut), &TyKind::Ptr(ref r_mut)) => {
433-
l_mut.mutbl == r_mut.mutbl && self.eq_ty(l_mut.ty, r_mut.ty)
434-
},
435-
(&TyKind::Rptr(_, ref l_rmut), &TyKind::Rptr(_, ref r_rmut)) => {
432+
(TyKind::Ptr(l_mut), TyKind::Ptr(r_mut)) => l_mut.mutbl == r_mut.mutbl && self.eq_ty(l_mut.ty, r_mut.ty),
433+
(TyKind::Rptr(_, l_rmut), TyKind::Rptr(_, r_rmut)) => {
436434
l_rmut.mutbl == r_rmut.mutbl && self.eq_ty(l_rmut.ty, r_rmut.ty)
437435
},
438-
(&TyKind::Path(ref l), &TyKind::Path(ref r)) => self.eq_qpath(l, r),
436+
(TyKind::Path(l), TyKind::Path(r)) => self.eq_qpath(l, r),
439437
(&TyKind::Tup(l), &TyKind::Tup(r)) => over(l, r, |l, r| self.eq_ty(l, r)),
440438
(&TyKind::Infer, &TyKind::Infer) => true,
441439
_ => false,

‎tests/ui/match_expr_like_matches_macro.fixed

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22

33
#![feature(custom_inner_attributes)]
44
#![warn(clippy::match_like_matches_macro)]
5-
#![allow(unreachable_patterns, dead_code, clippy::equatable_if_let)]
5+
#![allow(
6+
unreachable_patterns,
7+
dead_code,
8+
clippy::equatable_if_let,
9+
clippy::needless_borrowed_reference
10+
)]
611

712
fn main() {
813
let x = Some(5);

‎tests/ui/match_expr_like_matches_macro.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22

33
#![feature(custom_inner_attributes)]
44
#![warn(clippy::match_like_matches_macro)]
5-
#![allow(unreachable_patterns, dead_code, clippy::equatable_if_let)]
5+
#![allow(
6+
unreachable_patterns,
7+
dead_code,
8+
clippy::equatable_if_let,
9+
clippy::needless_borrowed_reference
10+
)]
611

712
fn main() {
813
let x = Some(5);

0 commit comments

Comments
 (0)