Skip to content

Commit bb7b6be

Browse files
borsbors
committed
Auto merge of #8133 - surechen:fix_8128, r=xFrednet
Fix 8128 Fixes #8128 changelog: Fix error suggestion of `skip(..).next()` for immutable variable.
2 parents 547efad + 4ffd660 commit bb7b6be

File tree

6 files changed

+138
-10
lines changed

6 files changed

+138
-10
lines changed

‎clippy_lints/src/methods/iter_skip_next.rs‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::is_trait_method;
3+
use clippy_utils::path_to_local;
34
use clippy_utils::source::snippet;
45
use rustc_errors::Applicability;
56
use rustc_hir as hir;
7+
use rustc_hir::{BindingAnnotation, Node, PatKind};
68
use rustc_lint::LateContext;
79
use rustc_span::sym;
810

@@ -11,14 +13,34 @@ use super::ITER_SKIP_NEXT;
1113
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
1214
// lint if caller of skip is an Iterator
1315
if is_trait_method(cx, expr, sym::Iterator) {
14-
span_lint_and_sugg(
16+
let mut application = Applicability::MachineApplicable;
17+
span_lint_and_then(
1518
cx,
1619
ITER_SKIP_NEXT,
1720
expr.span.trim_start(recv.span).unwrap(),
1821
"called `skip(..).next()` on an iterator",
19-
"use `nth` instead",
20-
format!(".nth({})", snippet(cx, arg.span, "..")),
21-
Applicability::MachineApplicable,
22+
|diag| {
23+
if_chain! {
24+
if let Some(id) = path_to_local(recv);
25+
if let Node::Binding(pat) = cx.tcx.hir().get(id);
26+
if let PatKind::Binding(ann, _, _, _) = pat.kind;
27+
if ann != BindingAnnotation::Mutable;
28+
then {
29+
application = Applicability::Unspecified;
30+
diag.span_help(
31+
pat.span,
32+
&format!("for this change `{}` has to be mutable", snippet(cx, pat.span, "..")),
33+
);
34+
}
35+
}
36+
37+
diag.span_suggestion(
38+
expr.span.trim_start(recv.span).unwrap(),
39+
"use `nth` instead",
40+
format!(".nth({})", snippet(cx, arg.span, "..")),
41+
application,
42+
);
43+
},
2244
);
2345
}
2446
}

‎tests/ui/iter_skip_next.fixed‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![warn(clippy::iter_skip_next)]
55
#![allow(clippy::blacklisted_name)]
66
#![allow(clippy::iter_nth)]
7+
#![allow(unused_mut, dead_code)]
78

89
extern crate option_helpers;
910

@@ -19,4 +20,18 @@ fn main() {
1920
let foo = IteratorFalsePositives { foo: 0 };
2021
let _ = foo.skip(42).next();
2122
let _ = foo.filter().skip(42).next();
23+
24+
// fix #8128
25+
let test_string = "1|1 2";
26+
let mut sp = test_string.split('|').map(|s| s.trim());
27+
let _: Vec<&str> = sp.nth(1).unwrap().split(' ').collect();
28+
if let Some(mut s) = Some(test_string.split('|').map(|s| s.trim())) {
29+
let _: Vec<&str> = s.nth(1).unwrap().split(' ').collect();
30+
};
31+
fn check<T>(mut s: T)
32+
where
33+
T: Iterator<Item = String>,
34+
{
35+
let _: Vec<&str> = s.nth(1).unwrap().split(' ').collect();
36+
}
2237
}

‎tests/ui/iter_skip_next.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![warn(clippy::iter_skip_next)]
55
#![allow(clippy::blacklisted_name)]
66
#![allow(clippy::iter_nth)]
7+
#![allow(unused_mut, dead_code)]
78

89
extern crate option_helpers;
910

@@ -19,4 +20,18 @@ fn main() {
1920
let foo = IteratorFalsePositives { foo: 0 };
2021
let _ = foo.skip(42).next();
2122
let _ = foo.filter().skip(42).next();
23+
24+
// fix #8128
25+
let test_string = "1|1 2";
26+
let mut sp = test_string.split('|').map(|s| s.trim());
27+
let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();
28+
if let Some(mut s) = Some(test_string.split('|').map(|s| s.trim())) {
29+
let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
30+
};
31+
fn check<T>(mut s: T)
32+
where
33+
T: Iterator<Item = String>,
34+
{
35+
let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
36+
}
2237
}

‎tests/ui/iter_skip_next.stderr‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,46 @@
11
error: called `skip(..).next()` on an iterator
2-
--> $DIR/iter_skip_next.rs:15:28
2+
--> $DIR/iter_skip_next.rs:16:28
33
|
44
LL | let _ = some_vec.iter().skip(42).next();
55
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)`
66
|
77
= note: `-D clippy::iter-skip-next` implied by `-D warnings`
88

99
error: called `skip(..).next()` on an iterator
10-
--> $DIR/iter_skip_next.rs:16:36
10+
--> $DIR/iter_skip_next.rs:17:36
1111
|
1212
LL | let _ = some_vec.iter().cycle().skip(42).next();
1313
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(42)`
1414

1515
error: called `skip(..).next()` on an iterator
16-
--> $DIR/iter_skip_next.rs:17:20
16+
--> $DIR/iter_skip_next.rs:18:20
1717
|
1818
LL | let _ = (1..10).skip(10).next();
1919
| ^^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(10)`
2020

2121
error: called `skip(..).next()` on an iterator
22-
--> $DIR/iter_skip_next.rs:18:33
22+
--> $DIR/iter_skip_next.rs:19:33
2323
|
2424
LL | let _ = &some_vec[..].iter().skip(3).next();
2525
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(3)`
2626

27-
error: aborting due to 4 previous errors
27+
error: called `skip(..).next()` on an iterator
28+
--> $DIR/iter_skip_next.rs:27:26
29+
|
30+
LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();
31+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
32+
33+
error: called `skip(..).next()` on an iterator
34+
--> $DIR/iter_skip_next.rs:29:29
35+
|
36+
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
37+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
38+
39+
error: called `skip(..).next()` on an iterator
40+
--> $DIR/iter_skip_next.rs:35:29
41+
|
42+
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
43+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
44+
45+
error: aborting due to 7 previous errors
2846

‎tests/ui/iter_skip_next_unfixable.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#![warn(clippy::iter_skip_next)]
2+
#![allow(dead_code)]
3+
4+
/// Checks implementation of `ITER_SKIP_NEXT` lint
5+
fn main() {
6+
// fix #8128
7+
let test_string = "1|1 2";
8+
let sp = test_string.split('|').map(|s| s.trim());
9+
let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();
10+
if let Some(s) = Some(test_string.split('|').map(|s| s.trim())) {
11+
let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
12+
};
13+
fn check<T>(s: T)
14+
where
15+
T: Iterator<Item = String>,
16+
{
17+
let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
18+
}
19+
}

‎tests/ui/iter_skip_next_unfixable.stderr‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
error: called `skip(..).next()` on an iterator
2+
--> $DIR/iter_skip_next_unfixable.rs:9:26
3+
|
4+
LL | let _: Vec<&str> = sp.skip(1).next().unwrap().split(' ').collect();
5+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
6+
|
7+
= note: `-D clippy::iter-skip-next` implied by `-D warnings`
8+
help: for this change `sp` has to be mutable
9+
--> $DIR/iter_skip_next_unfixable.rs:8:9
10+
|
11+
LL | let sp = test_string.split('|').map(|s| s.trim());
12+
| ^^
13+
14+
error: called `skip(..).next()` on an iterator
15+
--> $DIR/iter_skip_next_unfixable.rs:11:29
16+
|
17+
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
18+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
19+
|
20+
help: for this change `s` has to be mutable
21+
--> $DIR/iter_skip_next_unfixable.rs:10:17
22+
|
23+
LL | if let Some(s) = Some(test_string.split('|').map(|s| s.trim())) {
24+
| ^
25+
26+
error: called `skip(..).next()` on an iterator
27+
--> $DIR/iter_skip_next_unfixable.rs:17:29
28+
|
29+
LL | let _: Vec<&str> = s.skip(1).next().unwrap().split(' ').collect();
30+
| ^^^^^^^^^^^^^^^ help: use `nth` instead: `.nth(1)`
31+
|
32+
help: for this change `s` has to be mutable
33+
--> $DIR/iter_skip_next_unfixable.rs:13:17
34+
|
35+
LL | fn check<T>(s: T)
36+
| ^
37+
38+
error: aborting due to 3 previous errors
39+

0 commit comments

Comments
 (0)