From 4c79d8ace5bf19d6f839b52138af014da570284f Mon Sep 17 00:00:00 2001
From: Catherine <114838443+Centri3@users.noreply.github.com>
Date: Thu, 29 Jun 2023 04:55:07 -0500
Subject: [PATCH] new lint `iter_skip_zero`
---
CHANGELOG.md | 1 +
clippy_lints/src/casts/unnecessary_cast.rs | 2 +-
clippy_lints/src/declared_lints.rs | 1 +
clippy_lints/src/methods/iter_skip_zero.rs | 34 +++++++++++++++++
clippy_lints/src/methods/mod.rs | 34 ++++++++++++++++-
tests/ui/iter_skip_zero.fixed | 25 +++++++++++++
tests/ui/iter_skip_zero.rs | 25 +++++++++++++
tests/ui/iter_skip_zero.stderr | 43 ++++++++++++++++++++++
8 files changed, 163 insertions(+), 2 deletions(-)
create mode 100644 clippy_lints/src/methods/iter_skip_zero.rs
create mode 100644 tests/ui/iter_skip_zero.fixed
create mode 100644 tests/ui/iter_skip_zero.rs
create mode 100644 tests/ui/iter_skip_zero.stderr
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ece1111213c..acc9d8ffbf7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4939,6 +4939,7 @@ Released 2018-09-13
[`iter_on_single_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_on_single_items
[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
+[`iter_skip_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_zero
[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs
index ae56f38d9ad..4b45b0c9eac 100644
--- a/clippy_lints/src/casts/unnecessary_cast.rs
+++ b/clippy_lints/src/casts/unnecessary_cast.rs
@@ -257,7 +257,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
// Will this work for more complex types? Probably not!
if !snippet
.split("->")
- .skip(0)
+ .skip(1)
.map(|s| {
s.trim() == cast_from.to_string()
|| s.split("where").any(|ty| ty.trim() == cast_from.to_string())
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index 168dc9bf4e1..47995fea9f9 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -360,6 +360,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::ITER_ON_SINGLE_ITEMS_INFO,
crate::methods::ITER_OVEREAGER_CLONED_INFO,
crate::methods::ITER_SKIP_NEXT_INFO,
+ crate::methods::ITER_SKIP_ZERO_INFO,
crate::methods::ITER_WITH_DRAIN_INFO,
crate::methods::MANUAL_FILTER_MAP_INFO,
crate::methods::MANUAL_FIND_MAP_INFO,
diff --git a/clippy_lints/src/methods/iter_skip_zero.rs b/clippy_lints/src/methods/iter_skip_zero.rs
new file mode 100644
index 00000000000..6b696b42a69
--- /dev/null
+++ b/clippy_lints/src/methods/iter_skip_zero.rs
@@ -0,0 +1,34 @@
+use clippy_utils::consts::{constant, Constant};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::{is_from_proc_macro, is_trait_method};
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::ITER_SKIP_ZERO;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg_expr: &Expr<'_>) {
+ if !expr.span.from_expansion()
+ && is_trait_method(cx, expr, sym::Iterator)
+ && let Some(arg) = constant(cx, cx.typeck_results(), arg_expr).and_then(|constant| {
+ if let Constant::Int(arg) = constant {
+ Some(arg)
+ } else {
+ None
+ }
+ })
+ && arg == 0
+ && !is_from_proc_macro(cx, expr)
+ {
+ span_lint_and_then(cx, ITER_SKIP_ZERO, arg_expr.span, "usage of `.skip(0)`", |diag| {
+ diag.span_suggestion(
+ arg_expr.span,
+ "if you meant to skip the first element, use",
+ "1",
+ Applicability::MaybeIncorrect,
+ )
+ .note("this call to `skip` does nothing and is useless; remove it");
+ });
+ }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 6efeff92091..be71072acd6 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -45,6 +45,7 @@ mod iter_nth_zero;
mod iter_on_single_or_empty_collections;
mod iter_overeager_cloned;
mod iter_skip_next;
+mod iter_skip_zero;
mod iter_with_drain;
mod iterator_step_by_zero;
mod manual_next_back;
@@ -3443,6 +3444,27 @@ declare_clippy_lint! {
"`format!`ing every element in a collection, then collecting the strings into a new `String`"
}
+declare_clippy_lint! {
+ /// ### What it does
+ /// Checks for usage of `.skip(0)` on iterators.
+ ///
+ /// ### Why is this bad?
+ /// This was likely intended to be `.skip(1)` to skip the first element, as `.skip(0)` does
+ /// nothing. If not, the call should be removed.
+ ///
+ /// ### Example
+ /// ```rust
+ /// let v = vec![1, 2, 3];
+ /// let x = v.iter().skip(0).collect::<Vec<_>>();
+ /// let y = v.iter().collect::<Vec<_>>();
+ /// assert_eq!(x, y);
+ /// ```
+ #[clippy::version = "1.72.0"]
+ pub ITER_SKIP_ZERO,
+ correctness,
+ "disallows `.skip(0)`"
+}
+
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
@@ -3579,6 +3601,7 @@ impl_lint_pass!(Methods => [
MANUAL_TRY_FOLD,
FORMAT_COLLECT,
STRING_LIT_CHARS_ANY,
+ ITER_SKIP_ZERO,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3901,7 +3924,16 @@ impl Methods {
unnecessary_join::check(cx, expr, recv, join_arg, span);
}
},
- ("last", []) | ("skip", [_]) => {
+ ("skip", [arg]) => {
+ iter_skip_zero::check(cx, expr, arg);
+
+ if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
+ if let ("cloned", []) = (name2, args2) {
+ iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
+ }
+ }
+ }
+ ("last", []) => {
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
diff --git a/tests/ui/iter_skip_zero.fixed b/tests/ui/iter_skip_zero.fixed
new file mode 100644
index 00000000000..1eb0984fe07
--- /dev/null
+++ b/tests/ui/iter_skip_zero.fixed
@@ -0,0 +1,25 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![allow(clippy::useless_vec, unused)]
+#![warn(clippy::iter_skip_zero)]
+
+#[macro_use]
+extern crate proc_macros;
+
+use std::iter::once;
+
+fn main() {
+ let _ = [1, 2, 3].iter().skip(1);
+ let _ = vec![1, 2, 3].iter().skip(1);
+ let _ = once([1, 2, 3]).skip(1);
+ let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(1)).skip(1);
+ // Don't lint
+ let _ = [1, 2, 3].iter().skip(1);
+ let _ = vec![1, 2, 3].iter().skip(1);
+ external! {
+ let _ = [1, 2, 3].iter().skip(0);
+ }
+ with_span! {
+ let _ = [1, 2, 3].iter().skip(0);
+ }
+}
diff --git a/tests/ui/iter_skip_zero.rs b/tests/ui/iter_skip_zero.rs
new file mode 100644
index 00000000000..8c103ab1d5b
--- /dev/null
+++ b/tests/ui/iter_skip_zero.rs
@@ -0,0 +1,25 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![allow(clippy::useless_vec, unused)]
+#![warn(clippy::iter_skip_zero)]
+
+#[macro_use]
+extern crate proc_macros;
+
+use std::iter::once;
+
+fn main() {
+ let _ = [1, 2, 3].iter().skip(0);
+ let _ = vec![1, 2, 3].iter().skip(0);
+ let _ = once([1, 2, 3]).skip(0);
+ let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
+ // Don't lint
+ let _ = [1, 2, 3].iter().skip(1);
+ let _ = vec![1, 2, 3].iter().skip(1);
+ external! {
+ let _ = [1, 2, 3].iter().skip(0);
+ }
+ with_span! {
+ let _ = [1, 2, 3].iter().skip(0);
+ }
+}
diff --git a/tests/ui/iter_skip_zero.stderr b/tests/ui/iter_skip_zero.stderr
new file mode 100644
index 00000000000..80fecd59e6d
--- /dev/null
+++ b/tests/ui/iter_skip_zero.stderr
@@ -0,0 +1,43 @@
+error: usage of `.skip(0)`
+ --> $DIR/iter_skip_zero.rs:12:35
+ |
+LL | let _ = [1, 2, 3].iter().skip(0);
+ | ^ help: if you meant to skip the first element, use: `1`
+ |
+ = note: this call to `skip` does nothing and is useless; remove it
+ = note: `-D clippy::iter-skip-zero` implied by `-D warnings`
+
+error: usage of `.skip(0)`
+ --> $DIR/iter_skip_zero.rs:13:39
+ |
+LL | let _ = vec![1, 2, 3].iter().skip(0);
+ | ^ help: if you meant to skip the first element, use: `1`
+ |
+ = note: this call to `skip` does nothing and is useless; remove it
+
+error: usage of `.skip(0)`
+ --> $DIR/iter_skip_zero.rs:14:34
+ |
+LL | let _ = once([1, 2, 3]).skip(0);
+ | ^ help: if you meant to skip the first element, use: `1`
+ |
+ = note: this call to `skip` does nothing and is useless; remove it
+
+error: usage of `.skip(0)`
+ --> $DIR/iter_skip_zero.rs:15:71
+ |
+LL | let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
+ | ^ help: if you meant to skip the first element, use: `1`
+ |
+ = note: this call to `skip` does nothing and is useless; remove it
+
+error: usage of `.skip(0)`
+ --> $DIR/iter_skip_zero.rs:15:62
+ |
+LL | let _ = vec![1, 2, 3].iter().chain([1, 2, 3].iter().skip(0)).skip(0);
+ | ^ help: if you meant to skip the first element, use: `1`
+ |
+ = note: this call to `skip` does nothing and is useless; remove it
+
+error: aborting due to 5 previous errors
+