Skip to content

Commit da56c35

Browse files
borsbors
committed
Auto merge of #10933 - y21:issue2262-followup, r=Centri3
[`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice Fixes #2262 (well, actually my PR over at #10901 did do most of the stuff, but this PR implements the one last other case mentioned in the comments that my PR didn't fix) Before this change, it would lint `(&vec![1]).iter().sum::<i32>()`, but not `vec![1].iter().sum::<i32>()`. This PR handles this case. This also refactors a few things that I wanted to do in my other PR but forgot about. changelog: [`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice
2 parents b095247 + 50ba459 commit da56c35

16 files changed

+124
-117
lines changed

‎clippy_lints/src/vec.rs‎

Lines changed: 42 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use clippy_utils::ty::is_copy;
88
use clippy_utils::visitors::for_each_local_use_after_expr;
99
use clippy_utils::{get_parent_expr, higher, is_trait_method};
1010
use if_chain::if_chain;
11-
use rustc_ast::BindingAnnotation;
1211
use rustc_errors::Applicability;
1312
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Node, PatKind};
1413
use rustc_lint::{LateContext, LateLintPass};
@@ -54,11 +53,7 @@ declare_clippy_lint! {
5453
impl_lint_pass!(UselessVec => [USELESS_VEC]);
5554

5655
fn adjusts_to_slice(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
57-
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty_adjusted(e).kind() {
58-
ty.is_slice()
59-
} else {
60-
false
61-
}
56+
matches!(cx.typeck_results().expr_ty_adjusted(e).kind(), ty::Ref(_, ty, _) if ty.is_slice())
6257
}
6358

6459
/// Checks if the given expression is a method call to a `Vec` method
@@ -76,13 +71,21 @@ fn is_allowed_vec_method(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
7671

7772
impl<'tcx> LateLintPass<'tcx> for UselessVec {
7873
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
79-
// search for `&vec![_]` expressions where the adjusted type is `&[_]`
74+
// search for `&vec![_]` or `vec![_]` expressions where the adjusted type is `&[_]`
8075
if_chain! {
8176
if adjusts_to_slice(cx, expr);
82-
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, addressee) = expr.kind;
83-
if let Some(vec_args) = higher::VecArgs::hir(cx, addressee);
77+
if let Some(vec_args) = higher::VecArgs::hir(cx, expr.peel_borrows());
8478
then {
85-
self.check_vec_macro(cx, &vec_args, mutability, expr.span, SuggestSlice::Yes);
79+
let (suggest_slice, span) = if let ExprKind::AddrOf(BorrowKind::Ref, mutability, _) = expr.kind {
80+
// `expr` is `&vec![_]`, so suggest `&[_]` (or `&mut[_]` resp.)
81+
(SuggestedType::SliceRef(mutability), expr.span)
82+
} else {
83+
// `expr` is the `vec![_]` expansion, so suggest `[_]`
84+
// and also use the span of the actual `vec![_]` expression
85+
(SuggestedType::Array, expr.span.ctxt().outer_expn_data().call_site)
86+
};
87+
88+
self.check_vec_macro(cx, &vec_args, span, suggest_slice);
8689
}
8790
}
8891

@@ -93,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
9396
// for now ignore locals with type annotations.
9497
// this is to avoid compile errors when doing the suggestion here: let _: Vec<_> = vec![..];
9598
&& local.ty.is_none()
96-
&& let PatKind::Binding(BindingAnnotation(_, mutbl), id, ..) = local.pat.kind
99+
&& let PatKind::Binding(_, id, ..) = local.pat.kind
97100
&& is_copy(cx, vec_type(cx.typeck_results().expr_ty_adjusted(expr)))
98101
{
99102
let only_slice_uses = for_each_local_use_after_expr(cx, id, expr.hir_id, |expr| {
@@ -113,9 +116,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
113116
self.check_vec_macro(
114117
cx,
115118
&vec_args,
116-
mutbl,
117119
expr.span.ctxt().outer_expn_data().call_site,
118-
SuggestSlice::No
120+
SuggestedType::Array
119121
);
120122
}
121123
}
@@ -128,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
128130
then {
129131
// report the error around the `vec!` not inside `<std macros>:`
130132
let span = arg.span.ctxt().outer_expn_data().call_site;
131-
self.check_vec_macro(cx, &vec_args, Mutability::Not, span, SuggestSlice::No);
133+
self.check_vec_macro(cx, &vec_args, span, SuggestedType::Array);
132134
}
133135
}
134136
}
@@ -137,29 +139,23 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
137139
}
138140

139141
#[derive(Copy, Clone)]
140-
enum SuggestSlice {
142+
enum SuggestedType {
141143
/// Suggest using a slice `&[..]` / `&mut [..]`
142-
Yes,
144+
SliceRef(Mutability),
143145
/// Suggest using an array: `[..]`
144-
No,
146+
Array,
145147
}
146148

147149
impl UselessVec {
148150
fn check_vec_macro<'tcx>(
149151
&mut self,
150152
cx: &LateContext<'tcx>,
151153
vec_args: &higher::VecArgs<'tcx>,
152-
mutability: Mutability,
153154
span: Span,
154-
suggest_slice: SuggestSlice,
155+
suggest_slice: SuggestedType,
155156
) {
156157
let mut applicability = Applicability::MachineApplicable;
157158

158-
let (borrow_prefix_mut, borrow_prefix) = match suggest_slice {
159-
SuggestSlice::Yes => ("&mut ", "&"),
160-
SuggestSlice::No => ("", ""),
161-
};
162-
163159
let snippet = match *vec_args {
164160
higher::VecArgs::Repeat(elem, len) => {
165161
if let Some(Constant::Int(len_constant)) = constant(cx, cx.typeck_results(), len) {
@@ -168,21 +164,13 @@ impl UselessVec {
168164
return;
169165
}
170166

171-
match mutability {
172-
Mutability::Mut => {
173-
format!(
174-
"{borrow_prefix_mut}[{}; {}]",
175-
snippet_with_applicability(cx, elem.span, "elem", &mut applicability),
176-
snippet_with_applicability(cx, len.span, "len", &mut applicability)
177-
)
178-
},
179-
Mutability::Not => {
180-
format!(
181-
"{borrow_prefix}[{}; {}]",
182-
snippet_with_applicability(cx, elem.span, "elem", &mut applicability),
183-
snippet_with_applicability(cx, len.span, "len", &mut applicability)
184-
)
185-
},
167+
let elem = snippet_with_applicability(cx, elem.span, "elem", &mut applicability);
168+
let len = snippet_with_applicability(cx, len.span, "len", &mut applicability);
169+
170+
match suggest_slice {
171+
SuggestedType::SliceRef(Mutability::Mut) => format!("&mut [{elem}; {len}]"),
172+
SuggestedType::SliceRef(Mutability::Not) => format!("&[{elem}; {len}]"),
173+
SuggestedType::Array => format!("[{elem}; {len}]"),
186174
}
187175
} else {
188176
return;
@@ -194,25 +182,24 @@ impl UselessVec {
194182
return;
195183
}
196184
let span = args[0].span.to(last.span);
185+
let args = snippet_with_applicability(cx, span, "..", &mut applicability);
197186

198-
match mutability {
199-
Mutability::Mut => {
200-
format!(
201-
"{borrow_prefix_mut}[{}]",
202-
snippet_with_applicability(cx, span, "..", &mut applicability)
203-
)
187+
match suggest_slice {
188+
SuggestedType::SliceRef(Mutability::Mut) => {
189+
format!("&mut [{args}]")
190+
},
191+
SuggestedType::SliceRef(Mutability::Not) => {
192+
format!("&[{args}]")
204193
},
205-
Mutability::Not => {
206-
format!(
207-
"{borrow_prefix}[{}]",
208-
snippet_with_applicability(cx, span, "..", &mut applicability)
209-
)
194+
SuggestedType::Array => {
195+
format!("[{args}]")
210196
},
211197
}
212198
} else {
213-
match mutability {
214-
Mutability::Mut => format!("{borrow_prefix_mut}[]"),
215-
Mutability::Not => format!("{borrow_prefix}[]"),
199+
match suggest_slice {
200+
SuggestedType::SliceRef(Mutability::Mut) => "&mut []".to_owned(),
201+
SuggestedType::SliceRef(Mutability::Not) => "&[]".to_owned(),
202+
SuggestedType::Array => "[]".to_owned(),
216203
}
217204
}
218205
},
@@ -226,8 +213,8 @@ impl UselessVec {
226213
&format!(
227214
"you can use {} directly",
228215
match suggest_slice {
229-
SuggestSlice::Yes => "a slice",
230-
SuggestSlice::No => "an array",
216+
SuggestedType::SliceRef(_) => "a slice",
217+
SuggestedType::Array => "an array",
231218
}
232219
),
233220
snippet,

‎tests/ui/cloned_instead_of_copied.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![warn(clippy::cloned_instead_of_copied)]
44
#![allow(unused)]
5+
#![allow(clippy::useless_vec)]
56

67
fn main() {
78
// yay

‎tests/ui/cloned_instead_of_copied.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#![warn(clippy::cloned_instead_of_copied)]
44
#![allow(unused)]
5+
#![allow(clippy::useless_vec)]
56

67
fn main() {
78
// yay

‎tests/ui/cloned_instead_of_copied.stderr‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,49 @@
11
error: used `cloned` where `copied` could be used instead
2-
--> $DIR/cloned_instead_of_copied.rs:8:24
2+
--> $DIR/cloned_instead_of_copied.rs:9:24
33
|
44
LL | let _ = [1].iter().cloned();
55
| ^^^^^^ help: try: `copied`
66
|
77
= note: `-D clippy::cloned-instead-of-copied` implied by `-D warnings`
88

99
error: used `cloned` where `copied` could be used instead
10-
--> $DIR/cloned_instead_of_copied.rs:9:31
10+
--> $DIR/cloned_instead_of_copied.rs:10:31
1111
|
1212
LL | let _ = vec!["hi"].iter().cloned();
1313
| ^^^^^^ help: try: `copied`
1414

1515
error: used `cloned` where `copied` could be used instead
16-
--> $DIR/cloned_instead_of_copied.rs:10:22
16+
--> $DIR/cloned_instead_of_copied.rs:11:22
1717
|
1818
LL | let _ = Some(&1).cloned();
1919
| ^^^^^^ help: try: `copied`
2020

2121
error: used `cloned` where `copied` could be used instead
22-
--> $DIR/cloned_instead_of_copied.rs:11:34
22+
--> $DIR/cloned_instead_of_copied.rs:12:34
2323
|
2424
LL | let _ = Box::new([1].iter()).cloned();
2525
| ^^^^^^ help: try: `copied`
2626

2727
error: used `cloned` where `copied` could be used instead
28-
--> $DIR/cloned_instead_of_copied.rs:12:32
28+
--> $DIR/cloned_instead_of_copied.rs:13:32
2929
|
3030
LL | let _ = Box::new(Some(&1)).cloned();
3131
| ^^^^^^ help: try: `copied`
3232

3333
error: used `cloned` where `copied` could be used instead
34-
--> $DIR/cloned_instead_of_copied.rs:28:22
34+
--> $DIR/cloned_instead_of_copied.rs:29:22
3535
|
3636
LL | let _ = Some(&1).cloned(); // Option::copied needs 1.35
3737
| ^^^^^^ help: try: `copied`
3838

3939
error: used `cloned` where `copied` could be used instead
40-
--> $DIR/cloned_instead_of_copied.rs:33:24
40+
--> $DIR/cloned_instead_of_copied.rs:34:24
4141
|
4242
LL | let _ = [1].iter().cloned(); // Iterator::copied needs 1.36
4343
| ^^^^^^ help: try: `copied`
4444

4545
error: used `cloned` where `copied` could be used instead
46-
--> $DIR/cloned_instead_of_copied.rs:34:22
46+
--> $DIR/cloned_instead_of_copied.rs:35:22
4747
|
4848
LL | let _ = Some(&1).cloned();
4949
| ^^^^^^ help: try: `copied`

‎tests/ui/eta.fixed‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
clippy::no_effect,
88
clippy::option_map_unit_fn,
99
clippy::redundant_closure_call,
10-
clippy::uninlined_format_args
10+
clippy::uninlined_format_args,
11+
clippy::useless_vec
1112
)]
1213

1314
use std::path::{Path, PathBuf};

‎tests/ui/eta.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
clippy::no_effect,
88
clippy::option_map_unit_fn,
99
clippy::redundant_closure_call,
10-
clippy::uninlined_format_args
10+
clippy::uninlined_format_args,
11+
clippy::useless_vec
1112
)]
1213

1314
use std::path::{Path, PathBuf};

0 commit comments

Comments
 (0)