Skip to content

Commit a0b76d4

Browse files
committed
new lint iter_skip_zero
1 parent d4a6634 commit a0b76d4

File tree

8 files changed

+163
-2
lines changed

8 files changed

+163
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4939,6 +4939,7 @@ Released 2018-09-13
49394939
[`iter_on_single_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_single_items
49404940
[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
49414941
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
4942+
[`iter_skip_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_zero
49424943
[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
49434944
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
49444945
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits

clippy_lints/src/casts/unnecessary_cast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
257257
// Will this work for more complex types? Probably not!
258258
if !snippet
259259
.split("->")
260-
.skip(0)
260+
.skip(1)
261261
.map(|s| {
262262
s.trim() == cast_from.to_string()
263263
|| s.split("where").any(|ty| ty.trim() == cast_from.to_string())

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
360360
crate::methods::ITER_ON_SINGLE_ITEMS_INFO,
361361
crate::methods::ITER_OVEREAGER_CLONED_INFO,
362362
crate::methods::ITER_SKIP_NEXT_INFO,
363+
crate::methods::ITER_SKIP_ZERO_INFO,
363364
crate::methods::ITER_WITH_DRAIN_INFO,
364365
crate::methods::MANUAL_FILTER_MAP_INFO,
365366
crate::methods::MANUAL_FIND_MAP_INFO,
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
use clippy_utils::consts::{constant, Constant};
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::{is_from_proc_macro, is_trait_method};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::Expr;
6+
use rustc_lint::LateContext;
7+
use rustc_span::sym;
8+
9+
use super::ITER_SKIP_ZERO;
10+
11+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg_expr: &Expr<'_>) {
12+
if !expr.span.from_expansion()
13+
&& is_trait_method(cx, expr, sym::Iterator)
14+
&& let Some(arg) = constant(cx, cx.typeck_results(), arg_expr).and_then(|constant| {
15+
if let Constant::Int(arg) = constant {
16+
Some(arg)
17+
} else {
18+
None
19+
}
20+
})
21+
&& arg == 0
22+
&& !is_from_proc_macro(cx, expr)
23+
{
24+
span_lint_and_then(cx, ITER_SKIP_ZERO, arg_expr.span, "usage of `.skip(0)`", |diag| {
25+
diag.span_suggestion(
26+
arg_expr.span,
27+
"if you meant to skip the first element, use",
28+
"1",
29+
Applicability::MaybeIncorrect,
30+
)
31+
.note("this call to `skip` does nothing and is useless; remove it");
32+
});
33+
}
34+
}

clippy_lints/src/methods/mod.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ mod iter_nth_zero;
4545
mod iter_on_single_or_empty_collections;
4646
mod iter_overeager_cloned;
4747
mod iter_skip_next;
48+
mod iter_skip_zero;
4849
mod iter_with_drain;
4950
mod iterator_step_by_zero;
5051
mod manual_next_back;
@@ -3414,6 +3415,27 @@ declare_clippy_lint! {
34143415
"`format!`ing every element in a collection, then collecting the strings into a new `String`"
34153416
}
34163417

3418+
declare_clippy_lint! {
3419+
/// ### What it does
3420+
/// Checks for usage of `.skip(0)` on iterators.
3421+
///
3422+
/// ### Why is this bad?
3423+
/// This was likely intended to be `.skip(1)` to skip the first element, as `.skip(0)` does
3424+
/// nothing. If not, the call should be removed.
3425+
///
3426+
/// ### Example
3427+
/// ```rust
3428+
/// let v = vec![1, 2, 3];
3429+
/// let x = v.iter().skip(0).collect::<Vec<_>>();
3430+
/// let y = v.iter().collect::<Vec<_>>();
3431+
/// assert_eq!(x, y);
3432+
/// ```
3433+
#[clippy::version = "1.72.0"]
3434+
pub ITER_SKIP_ZERO,
3435+
correctness,
3436+
"disallows `.skip(0)`"
3437+
}
3438+
34173439
pub struct Methods {
34183440
avoid_breaking_exported_api: bool,
34193441
msrv: Msrv,
@@ -3549,6 +3571,7 @@ impl_lint_pass!(Methods => [
35493571
DRAIN_COLLECT,
35503572
MANUAL_TRY_FOLD,
35513573
FORMAT_COLLECT,
3574+
ITER_SKIP_ZERO,
35523575
]);
35533576

35543577
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3871,7 +3894,16 @@ impl Methods {
38713894
unnecessary_join::check(cx, expr, recv, join_arg, span);
38723895
}
38733896
},
3874-
("last", []) | ("skip", [_]) => {
3897+
("skip", [arg]) => {
3898+
iter_skip_zero::check(cx, expr, arg);
3899+
3900+
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
3901+
if let ("cloned", []) = (name2, args2) {
3902+
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
3903+
}
3904+
}
3905+
}
3906+
("last", []) => {
38753907
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
38763908
if let ("cloned", []) = (name2, args2) {
38773909
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);

tests/ui/iter_skip_zero.fixed

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::useless_vec, unused)]
4+
#![warn(clippy::iter_skip_zero)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
use std::iter::once;
10+
11+
fn main() {
12+
let _ = [1, 2, 3].iter().skip(1);
13+
let _ = vec![1, 2, 3].iter().skip(1);
14+
let _ = once([1, 2, 3]).skip(1);
15+
let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(1)).skip(1);
16+
// Don't lint
17+
let _ = [1, 2, 3].iter().skip(1);
18+
let _ = vec![1, 2, 3].iter().skip(1);
19+
external! {
20+
let _ = [1, 2, 3].iter().skip(0);
21+
}
22+
with_span! {
23+
let _ = [1, 2, 3].iter().skip(0);
24+
}
25+
}

tests/ui/iter_skip_zero.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::useless_vec, unused)]
4+
#![warn(clippy::iter_skip_zero)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
use std::iter::once;
10+
11+
fn main() {
12+
let _ = [1, 2, 3].iter().skip(0);
13+
let _ = vec![1, 2, 3].iter().skip(0);
14+
let _ = once([1, 2, 3]).skip(0);
15+
let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
16+
// Don't lint
17+
let _ = [1, 2, 3].iter().skip(1);
18+
let _ = vec![1, 2, 3].iter().skip(1);
19+
external! {
20+
let _ = [1, 2, 3].iter().skip(0);
21+
}
22+
with_span! {
23+
let _ = [1, 2, 3].iter().skip(0);
24+
}
25+
}

tests/ui/iter_skip_zero.stderr

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
error: usage of `.skip(0)`
2+
--> $DIR/iter_skip_zero.rs:12:35
3+
|
4+
LL | let _ = [1, 2, 3].iter().skip(0);
5+
| ^ help: if you meant to skip the first element, use: `1`
6+
|
7+
= note: this call to `skip` does nothing and is useless; remove it
8+
= note: `-D clippy::iter-skip-zero` implied by `-D warnings`
9+
10+
error: usage of `.skip(0)`
11+
--> $DIR/iter_skip_zero.rs:13:39
12+
|
13+
LL | let _ = vec![1, 2, 3].iter().skip(0);
14+
| ^ help: if you meant to skip the first element, use: `1`
15+
|
16+
= note: this call to `skip` does nothing and is useless; remove it
17+
18+
error: usage of `.skip(0)`
19+
--> $DIR/iter_skip_zero.rs:14:34
20+
|
21+
LL | let _ = once([1, 2, 3]).skip(0);
22+
| ^ help: if you meant to skip the first element, use: `1`
23+
|
24+
= note: this call to `skip` does nothing and is useless; remove it
25+
26+
error: usage of `.skip(0)`
27+
--> $DIR/iter_skip_zero.rs:15:71
28+
|
29+
LL | let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
30+
| ^ help: if you meant to skip the first element, use: `1`
31+
|
32+
= note: this call to `skip` does nothing and is useless; remove it
33+
34+
error: usage of `.skip(0)`
35+
--> $DIR/iter_skip_zero.rs:15:62
36+
|
37+
LL | let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
38+
| ^ help: if you meant to skip the first element, use: `1`
39+
|
40+
= note: this call to `skip` does nothing and is useless; remove it
41+
42+
error: aborting due to 5 previous errors
43+

0 commit comments

Comments
 (0)