Skip to content

Commit 2c51951

Browse files
llogiqllogiq
authored
add manual_slice_fill lint (#14082)
close #13856 changelog: [`manual_slice_fill`]: new lint
2 parents 6d1482c + 07ede9c commit 2c51951

14 files changed

+433
-35
lines changed

Diff for: ‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5773,6 +5773,7 @@ Released 2018-09-13
57735773
[`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
57745774
[`manual_rotate`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rotate
57755775
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
5776+
[`manual_slice_fill`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill
57765777
[`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
57775778
[`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
57785779
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat

Diff for: ‎book/src/lint_configuration.md

+1
Original file line numberDiff line numberDiff line change
@@ -752,6 +752,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
752752
* [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid)
753753
* [`manual_repeat_n`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_repeat_n)
754754
* [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain)
755+
* [`manual_slice_fill`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_fill)
755756
* [`manual_split_once`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once)
756757
* [`manual_str_repeat`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat)
757758
* [`manual_strip`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip)

Diff for: ‎clippy_config/src/conf.rs

+1
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,7 @@ define_Conf! {
621621
manual_rem_euclid,
622622
manual_repeat_n,
623623
manual_retain,
624+
manual_slice_fill,
624625
manual_split_once,
625626
manual_str_repeat,
626627
manual_strip,

Diff for: ‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
292292
crate::loops::MANUAL_FIND_INFO,
293293
crate::loops::MANUAL_FLATTEN_INFO,
294294
crate::loops::MANUAL_MEMCPY_INFO,
295+
crate::loops::MANUAL_SLICE_FILL_INFO,
295296
crate::loops::MANUAL_WHILE_LET_SOME_INFO,
296297
crate::loops::MISSING_SPIN_LOOP_INFO,
297298
crate::loops::MUT_RANGE_BOUND_INFO,

Diff for: ‎clippy_lints/src/loops/manual_slice_fill.rs

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
3+
use clippy_utils::macros::span_is_local;
4+
use clippy_utils::msrvs::{self, Msrv};
5+
use clippy_utils::source::{HasSession, snippet_with_applicability};
6+
use clippy_utils::ty::implements_trait;
7+
use clippy_utils::{higher, peel_blocks_with_stmt, span_contains_comment};
8+
use rustc_ast::ast::LitKind;
9+
use rustc_ast::{RangeLimits, UnOp};
10+
use rustc_data_structures::packed::Pu128;
11+
use rustc_errors::Applicability;
12+
use rustc_hir::QPath::Resolved;
13+
use rustc_hir::def::Res;
14+
use rustc_hir::{Expr, ExprKind, Pat};
15+
use rustc_lint::LateContext;
16+
use rustc_span::source_map::Spanned;
17+
use rustc_span::sym;
18+
19+
use super::MANUAL_SLICE_FILL;
20+
21+
pub(super) fn check<'tcx>(
22+
cx: &LateContext<'tcx>,
23+
pat: &'tcx Pat<'_>,
24+
arg: &'tcx Expr<'_>,
25+
body: &'tcx Expr<'_>,
26+
expr: &'tcx Expr<'_>,
27+
msrv: &Msrv,
28+
) {
29+
if !msrv.meets(msrvs::SLICE_FILL) {
30+
return;
31+
}
32+
33+
// `for _ in 0..slice.len() { slice[_] = value; }`
34+
if let Some(higher::Range {
35+
start: Some(start),
36+
end: Some(end),
37+
limits: RangeLimits::HalfOpen,
38+
}) = higher::Range::hir(arg)
39+
&& let ExprKind::Lit(Spanned {
40+
node: LitKind::Int(Pu128(0), _),
41+
..
42+
}) = start.kind
43+
&& let ExprKind::Block(..) = body.kind
44+
// Check if the body is an assignment to a slice element.
45+
&& let ExprKind::Assign(assignee, assignval, _) = peel_blocks_with_stmt(body).kind
46+
&& let ExprKind::Index(slice, _, _) = assignee.kind
47+
// Check if `len()` is used for the range end.
48+
&& let ExprKind::MethodCall(path, recv,..) = end.kind
49+
&& path.ident.name == sym::len
50+
// Check if the slice which is being assigned to is the same as the one being iterated over.
51+
&& let ExprKind::Path(Resolved(_, recv_path)) = recv.kind
52+
&& let ExprKind::Path(Resolved(_, slice_path)) = slice.kind
53+
&& recv_path.res == slice_path.res
54+
&& !assignval.span.from_expansion()
55+
// It is generally not equivalent to use the `fill` method if `assignval` can have side effects
56+
&& switch_to_eager_eval(cx, assignval)
57+
&& span_is_local(assignval.span)
58+
// The `fill` method requires that the slice's element type implements the `Clone` trait.
59+
&& let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
60+
&& implements_trait(cx, cx.typeck_results().expr_ty(slice), clone_trait, &[])
61+
{
62+
sugg(cx, body, expr, slice.span, assignval.span);
63+
}
64+
// `for _ in &mut slice { *_ = value; }`
65+
else if let ExprKind::AddrOf(_, _, recv) = arg.kind
66+
// Check if the body is an assignment to a slice element.
67+
&& let ExprKind::Assign(assignee, assignval, _) = peel_blocks_with_stmt(body).kind
68+
&& let ExprKind::Unary(UnOp::Deref, slice_iter) = assignee.kind
69+
&& let ExprKind::Path(Resolved(_, recv_path)) = recv.kind
70+
// Check if the slice which is being assigned to is the same as the one being iterated over.
71+
&& let ExprKind::Path(Resolved(_, slice_path)) = slice_iter.kind
72+
&& let Res::Local(local) = slice_path.res
73+
&& local == pat.hir_id
74+
&& !assignval.span.from_expansion()
75+
&& switch_to_eager_eval(cx, assignval)
76+
&& span_is_local(assignval.span)
77+
// The `fill` method cannot be used if the slice's element type does not implement the `Clone` trait.
78+
&& let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
79+
&& implements_trait(cx, cx.typeck_results().expr_ty(recv), clone_trait, &[])
80+
{
81+
sugg(cx, body, expr, recv_path.span, assignval.span);
82+
}
83+
}
84+
85+
fn sugg<'tcx>(
86+
cx: &LateContext<'tcx>,
87+
body: &'tcx Expr<'_>,
88+
expr: &'tcx Expr<'_>,
89+
slice_span: rustc_span::Span,
90+
assignval_span: rustc_span::Span,
91+
) {
92+
let mut app = if span_contains_comment(cx.sess().source_map(), body.span) {
93+
Applicability::MaybeIncorrect // Comments may be informational.
94+
} else {
95+
Applicability::MachineApplicable
96+
};
97+
98+
span_lint_and_sugg(
99+
cx,
100+
MANUAL_SLICE_FILL,
101+
expr.span,
102+
"manually filling a slice",
103+
"try",
104+
format!(
105+
"{}.fill({});",
106+
snippet_with_applicability(cx, slice_span, "..", &mut app),
107+
snippet_with_applicability(cx, assignval_span, "..", &mut app),
108+
),
109+
app,
110+
);
111+
}

Diff for: ‎clippy_lints/src/loops/mod.rs

+28
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ mod iter_next_loop;
88
mod manual_find;
99
mod manual_flatten;
1010
mod manual_memcpy;
11+
mod manual_slice_fill;
1112
mod manual_while_let_some;
1213
mod missing_spin_loop;
1314
mod mut_range_bound;
@@ -714,6 +715,31 @@ declare_clippy_lint! {
714715
"possibly unintended infinite loop"
715716
}
716717

718+
declare_clippy_lint! {
719+
/// ### What it does
720+
/// Checks for manually filling a slice with a value.
721+
///
722+
/// ### Why is this bad?
723+
/// Using the `fill` method is more idiomatic and concise.
724+
///
725+
/// ### Example
726+
/// ```no_run
727+
/// let mut some_slice = [1, 2, 3, 4, 5];
728+
/// for i in 0..some_slice.len() {
729+
/// some_slice[i] = 0;
730+
/// }
731+
/// ```
732+
/// Use instead:
733+
/// ```no_run
734+
/// let mut some_slice = [1, 2, 3, 4, 5];
735+
/// some_slice.fill(0);
736+
/// ```
737+
#[clippy::version = "1.86.0"]
738+
pub MANUAL_SLICE_FILL,
739+
style,
740+
"manually filling a slice with a value"
741+
}
742+
717743
pub struct Loops {
718744
msrv: Msrv,
719745
enforce_iter_loop_reborrow: bool,
@@ -750,6 +776,7 @@ impl_lint_pass!(Loops => [
750776
MANUAL_WHILE_LET_SOME,
751777
UNUSED_ENUMERATE_INDEX,
752778
INFINITE_LOOP,
779+
MANUAL_SLICE_FILL,
753780
]);
754781

755782
impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -823,6 +850,7 @@ impl Loops {
823850
) {
824851
let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
825852
if !is_manual_memcpy_triggered {
853+
manual_slice_fill::check(cx, pat, arg, body, expr, &self.msrv);
826854
needless_range_loop::check(cx, pat, arg, body, expr);
827855
explicit_counter_loop::check(cx, pat, arg, body, expr, label);
828856
}

Diff for: ‎clippy_utils/src/msrvs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ msrv_aliases! {
4040
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
4141
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
4242
1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS }
43-
1,50,0 { BOOL_THEN, CLAMP }
43+
1,50,0 { BOOL_THEN, CLAMP, SLICE_FILL }
4444
1,47,0 { TAU, IS_ASCII_DIGIT_CONST, ARRAY_IMPL_ANY_LEN, SATURATING_SUB_CONST }
4545
1,46,0 { CONST_IF_MATCH }
4646
1,45,0 { STR_STRIP_PREFIX }

Diff for: ‎tests/ui/manual_memcpy/without_loop_counters.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
#![warn(clippy::manual_memcpy)]
2-
#![allow(clippy::assigning_clones, clippy::useless_vec, clippy::needless_range_loop)]
2+
#![allow(
3+
clippy::assigning_clones,
4+
clippy::useless_vec,
5+
clippy::needless_range_loop,
6+
clippy::manual_slice_fill
7+
)]
38

49
//@no-rustfix
510
const LOOP_OFFSET: usize = 5000;

Diff for: ‎tests/ui/manual_memcpy/without_loop_counters.stderr

+18-18
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: it looks like you're manually copying between slices
2-
--> tests/ui/manual_memcpy/without_loop_counters.rs:9:5
2+
--> tests/ui/manual_memcpy/without_loop_counters.rs:14:5
33
|
44
LL | / for i in 0..src.len() {
55
LL | |
@@ -12,7 +12,7 @@ LL | | }
1212
= help: to override `-D warnings` add `#[allow(clippy::manual_memcpy)]`
1313

1414
error: it looks like you're manually copying between slices
15-
--> tests/ui/manual_memcpy/without_loop_counters.rs:16:5
15+
--> tests/ui/manual_memcpy/without_loop_counters.rs:21:5
1616
|
1717
LL | / for i in 0..src.len() {
1818
LL | |
@@ -21,7 +21,7 @@ LL | | }
2121
| |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].copy_from_slice(&src[..]);`
2222

2323
error: it looks like you're manually copying between slices
24-
--> tests/ui/manual_memcpy/without_loop_counters.rs:22:5
24+
--> tests/ui/manual_memcpy/without_loop_counters.rs:27:5
2525
|
2626
LL | / for i in 0..src.len() {
2727
LL | |
@@ -30,7 +30,7 @@ LL | | }
3030
| |_____^ help: try replacing the loop by: `dst[..src.len()].copy_from_slice(&src[10..(src.len() + 10)]);`
3131

3232
error: it looks like you're manually copying between slices
33-
--> tests/ui/manual_memcpy/without_loop_counters.rs:28:5
33+
--> tests/ui/manual_memcpy/without_loop_counters.rs:33:5
3434
|
3535
LL | / for i in 11..src.len() {
3636
LL | |
@@ -39,7 +39,7 @@ LL | | }
3939
| |_____^ help: try replacing the loop by: `dst[11..src.len()].copy_from_slice(&src[(11 - 10)..(src.len() - 10)]);`
4040

4141
error: it looks like you're manually copying between slices
42-
--> tests/ui/manual_memcpy/without_loop_counters.rs:34:5
42+
--> tests/ui/manual_memcpy/without_loop_counters.rs:39:5
4343
|
4444
LL | / for i in 0..dst.len() {
4545
LL | |
@@ -48,7 +48,7 @@ LL | | }
4848
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..dst.len()]);`
4949

5050
error: it looks like you're manually copying between slices
51-
--> tests/ui/manual_memcpy/without_loop_counters.rs:48:5
51+
--> tests/ui/manual_memcpy/without_loop_counters.rs:53:5
5252
|
5353
LL | / for i in 10..256 {
5454
LL | |
@@ -64,7 +64,7 @@ LL + dst2[(10 + 500)..(256 + 500)].copy_from_slice(&src[10..256]);
6464
|
6565

6666
error: it looks like you're manually copying between slices
67-
--> tests/ui/manual_memcpy/without_loop_counters.rs:61:5
67+
--> tests/ui/manual_memcpy/without_loop_counters.rs:66:5
6868
|
6969
LL | / for i in 10..LOOP_OFFSET {
7070
LL | |
@@ -73,7 +73,7 @@ LL | | }
7373
| |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].copy_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);`
7474

7575
error: it looks like you're manually copying between slices
76-
--> tests/ui/manual_memcpy/without_loop_counters.rs:75:5
76+
--> tests/ui/manual_memcpy/without_loop_counters.rs:80:5
7777
|
7878
LL | / for i in 0..src_vec.len() {
7979
LL | |
@@ -82,7 +82,7 @@ LL | | }
8282
| |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].copy_from_slice(&src_vec[..]);`
8383

8484
error: it looks like you're manually copying between slices
85-
--> tests/ui/manual_memcpy/without_loop_counters.rs:105:5
85+
--> tests/ui/manual_memcpy/without_loop_counters.rs:110:5
8686
|
8787
LL | / for i in from..from + src.len() {
8888
LL | |
@@ -91,7 +91,7 @@ LL | | }
9191
| |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].copy_from_slice(&src[..(from + src.len() - from)]);`
9292

9393
error: it looks like you're manually copying between slices
94-
--> tests/ui/manual_memcpy/without_loop_counters.rs:110:5
94+
--> tests/ui/manual_memcpy/without_loop_counters.rs:115:5
9595
|
9696
LL | / for i in from..from + 3 {
9797
LL | |
@@ -100,7 +100,7 @@ LL | | }
100100
| |_____^ help: try replacing the loop by: `dst[from..(from + 3)].copy_from_slice(&src[..(from + 3 - from)]);`
101101

102102
error: it looks like you're manually copying between slices
103-
--> tests/ui/manual_memcpy/without_loop_counters.rs:116:5
103+
--> tests/ui/manual_memcpy/without_loop_counters.rs:121:5
104104
|
105105
LL | / for i in 0..5 {
106106
LL | |
@@ -109,7 +109,7 @@ LL | | }
109109
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
110110

111111
error: it looks like you're manually copying between slices
112-
--> tests/ui/manual_memcpy/without_loop_counters.rs:122:5
112+
--> tests/ui/manual_memcpy/without_loop_counters.rs:127:5
113113
|
114114
LL | / for i in 0..0 {
115115
LL | |
@@ -118,7 +118,7 @@ LL | | }
118118
| |_____^ help: try replacing the loop by: `dst[..0].copy_from_slice(&src[..0]);`
119119

120120
error: it looks like you're manually copying between slices
121-
--> tests/ui/manual_memcpy/without_loop_counters.rs:146:5
121+
--> tests/ui/manual_memcpy/without_loop_counters.rs:151:5
122122
|
123123
LL | / for i in 0..4 {
124124
LL | |
@@ -127,7 +127,7 @@ LL | | }
127127
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[..4]);`
128128

129129
error: it looks like you're manually copying between slices
130-
--> tests/ui/manual_memcpy/without_loop_counters.rs:152:5
130+
--> tests/ui/manual_memcpy/without_loop_counters.rs:157:5
131131
|
132132
LL | / for i in 0..5 {
133133
LL | |
@@ -136,7 +136,7 @@ LL | | }
136136
| |_____^ help: try replacing the loop by: `dst[..5].copy_from_slice(&src);`
137137

138138
error: it looks like you're manually copying between slices
139-
--> tests/ui/manual_memcpy/without_loop_counters.rs:158:5
139+
--> tests/ui/manual_memcpy/without_loop_counters.rs:163:5
140140
|
141141
LL | / for i in 0..5 {
142142
LL | |
@@ -145,7 +145,7 @@ LL | | }
145145
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src);`
146146

147147
error: it looks like you're manually copying between slices
148-
--> tests/ui/manual_memcpy/without_loop_counters.rs:205:5
148+
--> tests/ui/manual_memcpy/without_loop_counters.rs:210:5
149149
|
150150
LL | / for i in 0..5 {
151151
LL | |
@@ -154,7 +154,7 @@ LL | | }
154154
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0]);`
155155

156156
error: it looks like you're manually copying between slices
157-
--> tests/ui/manual_memcpy/without_loop_counters.rs:211:5
157+
--> tests/ui/manual_memcpy/without_loop_counters.rs:216:5
158158
|
159159
LL | / for i in 0..5 {
160160
LL | |
@@ -163,7 +163,7 @@ LL | | }
163163
| |_____^ help: try replacing the loop by: `dst.copy_from_slice(&src[0][1]);`
164164

165165
error: it looks like you're manually copying between slices
166-
--> tests/ui/manual_memcpy/without_loop_counters.rs:219:5
166+
--> tests/ui/manual_memcpy/without_loop_counters.rs:224:5
167167
|
168168
LL | / for i in 0..src.len() {
169169
LL | |

0 commit comments

Comments
 (0)