From 95c369fa9168d2eee08d1a693dc1c63872fe46c0 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Mon, 20 Jan 2020 10:54:54 +0900
Subject: [PATCH] Add `skip_while_next` lint

---
 CHANGELOG.md                         |  1 +
 README.md                            |  2 +-
 clippy_lints/src/lib.rs              |  3 +++
 clippy_lints/src/methods/mod.rs      | 39 ++++++++++++++++++++++++++++
 src/lintlist/mod.rs                  |  9 ++++++-
 tests/ui/auxiliary/option_helpers.rs |  4 +++
 tests/ui/skip_while_next.rs          | 29 +++++++++++++++++++++
 tests/ui/skip_while_next.stderr      | 20 ++++++++++++++
 8 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 tests/ui/skip_while_next.rs
 create mode 100644 tests/ui/skip_while_next.stderr

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 11d745a4c23..f7e8a4534de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1285,6 +1285,7 @@ Released 2018-09-13
 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
+[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
diff --git a/README.md b/README.md
index 7b1d14e9afc..d752e5b4cc1 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 347 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 348 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 497146772a5..962ff38eb6c 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -645,6 +645,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::SEARCH_IS_SOME,
         &methods::SHOULD_IMPLEMENT_TRAIT,
         &methods::SINGLE_CHAR_PATTERN,
+        &methods::SKIP_WHILE_NEXT,
         &methods::STRING_EXTEND_CHARS,
         &methods::SUSPICIOUS_MAP,
         &methods::TEMPORARY_CSTRING_AS_PTR,
@@ -1223,6 +1224,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
         LintId::of(&methods::SINGLE_CHAR_PATTERN),
+        LintId::of(&methods::SKIP_WHILE_NEXT),
         LintId::of(&methods::STRING_EXTEND_CHARS),
         LintId::of(&methods::SUSPICIOUS_MAP),
         LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
@@ -1475,6 +1477,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::FLAT_MAP_IDENTITY),
         LintId::of(&methods::OPTION_AND_THEN_SOME),
         LintId::of(&methods::SEARCH_IS_SOME),
+        LintId::of(&methods::SKIP_WHILE_NEXT),
         LintId::of(&methods::SUSPICIOUS_MAP),
         LintId::of(&methods::UNNECESSARY_FILTER_MAP),
         LintId::of(&methods::USELESS_ASREF),
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index ed37d3411b5..8dfebb56f30 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -375,6 +375,29 @@ declare_clippy_lint! {
     "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.skip_while(condition).next()`.
+    ///
+    /// **Why is this bad?** Readability, this can be written more concisely as
+    /// `_.find(!condition)`.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// # let vec = vec![1];
+    /// vec.iter().skip_while(|x| **x == 0).next();
+    /// ```
+    /// Could be written as
+    /// ```rust
+    /// # let vec = vec![1];
+    /// vec.iter().find(|x| **x != 0);
+    /// ```
+    pub SKIP_WHILE_NEXT,
+    complexity,
+    "using `skip_while(p).next()`, which is more succinctly expressed as `.find(!p)`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.map(_).flatten(_)`,
     ///
@@ -1192,6 +1215,7 @@ declare_lint_pass!(Methods => [
     SEARCH_IS_SOME,
     TEMPORARY_CSTRING_AS_PTR,
     FILTER_NEXT,
+    SKIP_WHILE_NEXT,
     FILTER_MAP,
     FILTER_MAP_NEXT,
     FLAT_MAP_IDENTITY,
@@ -1237,6 +1261,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
             ["and_then", ..] => lint_option_and_then_some(cx, expr, arg_lists[0]),
             ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
+            ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
             ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]),
@@ -2530,6 +2555,20 @@ fn lint_filter_next<'a, 'tcx>(
     }
 }
 
+/// lint use of `skip_while().next()` for `Iterators`
+fn lint_skip_while_next<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    _skip_while_args: &'tcx [hir::Expr<'_>],
+) {
+    // lint if caller of `.skip_while().next()` is an Iterator
+    if match_trait_method(cx, expr, &paths::ITERATOR) {
+        let msg = "called `skip_while(p).next()` on an `Iterator`. \
+                   This is more succinctly expressed by calling `.find(!p)` instead.";
+        span_lint(cx, SKIP_WHILE_NEXT, expr.span, msg);
+    }
+}
+
 /// lint use of `filter().map()` for `Iterators`
 fn lint_filter_map<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index c6339daf2eb..dcdfd015393 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 347] = [
+pub const ALL_LINTS: [Lint; 348] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1862,6 +1862,13 @@ pub const ALL_LINTS: [Lint; 347] = [
         deprecation: None,
         module: "matches",
     },
+    Lint {
+        name: "skip_while_next",
+        group: "complexity",
+        desc: "using `skip_while(p).next()`, which is more succinctly expressed as `.find(!p)`",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "slow_vector_initialization",
         group: "perf",
diff --git a/tests/ui/auxiliary/option_helpers.rs b/tests/ui/auxiliary/option_helpers.rs
index 33195211968..ed11c41e21c 100644
--- a/tests/ui/auxiliary/option_helpers.rs
+++ b/tests/ui/auxiliary/option_helpers.rs
@@ -44,4 +44,8 @@ impl IteratorFalsePositives {
     pub fn skip(self, _: usize) -> IteratorFalsePositives {
         self
     }
+
+    pub fn skip_while(self) -> IteratorFalsePositives {
+        self
+    }
 }
diff --git a/tests/ui/skip_while_next.rs b/tests/ui/skip_while_next.rs
new file mode 100644
index 00000000000..a522c0f08b2
--- /dev/null
+++ b/tests/ui/skip_while_next.rs
@@ -0,0 +1,29 @@
+// aux-build:option_helpers.rs
+
+#![warn(clippy::skip_while_next)]
+#![allow(clippy::blacklisted_name)]
+
+extern crate option_helpers;
+use option_helpers::IteratorFalsePositives;
+
+#[rustfmt::skip]
+fn skip_while_next() {
+    let v = vec![3, 2, 1, 0, -1, -2, -3];
+
+    // Single-line case.
+    let _ = v.iter().skip_while(|&x| *x < 0).next();
+
+    // Multi-line case.
+    let _ = v.iter().skip_while(|&x| {
+                                *x < 0
+                            }
+                   ).next();
+
+    // Check that hat we don't lint if the caller is not an `Iterator`.
+    let foo = IteratorFalsePositives { foo: 0 };
+    let _ = foo.skip_while().next();
+}
+
+fn main() {
+    skip_while_next();
+}
diff --git a/tests/ui/skip_while_next.stderr b/tests/ui/skip_while_next.stderr
new file mode 100644
index 00000000000..2ce88ac23a5
--- /dev/null
+++ b/tests/ui/skip_while_next.stderr
@@ -0,0 +1,20 @@
+error: called `skip_while(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(!p)` instead.
+  --> $DIR/skip_while_next.rs:14:13
+   |
+LL |     let _ = v.iter().skip_while(|&x| *x < 0).next();
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::skip-while-next` implied by `-D warnings`
+
+error: called `skip_while(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(!p)` instead.
+  --> $DIR/skip_while_next.rs:17:13
+   |
+LL |       let _ = v.iter().skip_while(|&x| {
+   |  _____________^
+LL | |                                 *x < 0
+LL | |                             }
+LL | |                    ).next();
+   | |___________________________^
+
+error: aborting due to 2 previous errors
+