Auto merge of rust-lang#10769 - Icxolu:manual_next_back, r=giraffate

add lint `manual_next_back`

changelog: [`manual_next_back`]: checks for manual reverse iteration (`.rev().next()`) of a `DoubleEndedIterator`

fixes rust-lang#10274

Author: bors

CHANGELOG.md

@@ -4785,6 +4785,7 @@ Released 2018-09-13
 [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str
 [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
+[`manual_next_back`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_next_back
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
 [`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains

clippy_lints/src/declared_lints.rs

@@ -351,6 +351,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::ITER_WITH_DRAIN_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
+    crate::methods::MANUAL_NEXT_BACK_INFO,
     crate::methods::MANUAL_OK_OR_INFO,
     crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO,
     crate::methods::MANUAL_SPLIT_ONCE_INFO,

clippy_lints/src/methods/manual_next_back.rs

@@ -0,0 +1,38 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_trait_method;
+use clippy_utils::ty::implements_trait;
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_lint::LateContext;
+use rustc_span::symbol::sym;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'_>,
+    rev_call: &'tcx Expr<'_>,
+    rev_recv: &'tcx Expr<'_>,
+) {
+    let rev_recv_ty = cx.typeck_results().expr_ty(rev_recv);
+
+    // check that the receiver of `rev` implements `DoubleEndedIterator` and
+    // that `rev` and `next` come from `Iterator`
+    if cx
+        .tcx
+        .get_diagnostic_item(sym::DoubleEndedIterator)
+        .map_or(false, |double_ended_iterator| {
+            implements_trait(cx, rev_recv_ty, double_ended_iterator, &[])
+        })
+        && is_trait_method(cx, rev_call, sym::Iterator)
+        && is_trait_method(cx, expr, sym::Iterator)
+    {
+        span_lint_and_sugg(
+            cx,
+            super::MANUAL_NEXT_BACK,
+            expr.span.with_lo(rev_recv.span.hi()),
+            "manual backwards iteration",
+            "use",
+            String::from(".next_back()"),
+            Applicability::MachineApplicable,
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -45,6 +45,7 @@ mod iter_overeager_cloned;
 mod iter_skip_next;
 mod iter_with_drain;
 mod iterator_step_by_zero;
+mod manual_next_back;
 mod manual_ok_or;
 mod manual_saturating_arithmetic;
 mod manual_str_repeat;
@@ -3193,6 +3194,29 @@ declare_clippy_lint! {
     "calling `drain` in order to `clear` a container"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `.rev().next()` on a `DoubleEndedIterator`
+    ///
+    /// ### Why is this bad?
+    /// `.next_back()` is cleaner.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let foo = [0; 10];
+    /// foo.iter().rev().next();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # let foo = [0; 10];
+    /// foo.iter().next_back();
+    /// ```
+    #[clippy::version = "1.71.0"]
+    pub MANUAL_NEXT_BACK,
+    style,
+    "manual reverse iteration of `DoubleEndedIterator`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3321,6 +3345,7 @@ impl_lint_pass!(Methods => [
     NEEDLESS_COLLECT,
     SUSPICIOUS_COMMAND_ARG_SPACE,
     CLEAR_WITH_DRAIN,
+    MANUAL_NEXT_BACK,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3677,6 +3702,7 @@ impl Methods {
                             ("iter", []) => iter_next_slice::check(cx, expr, recv2),
                             ("skip", [arg]) => iter_skip_next::check(cx, expr, recv2, arg),
                             ("skip_while", [_]) => skip_while_next::check(cx, expr),
+                            ("rev", [])=> manual_next_back::check(cx, expr, recv, recv2),
                             _ => {},
                         }
                     }

tests/ui/manual_next_back.fixed

@@ -0,0 +1,36 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::manual_next_back)]
+
+struct FakeIter(std::ops::Range<i32>);
+
+impl FakeIter {
+    fn rev(self) -> Self {
+        self
+    }
+
+    fn next(&self) {}
+}
+
+impl DoubleEndedIterator for FakeIter {
+    fn next_back(&mut self) -> Option<Self::Item> {
+        self.0.next_back()
+    }
+}
+
+impl Iterator for FakeIter {
+    type Item = i32;
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+}
+
+fn main() {
+    // should not lint
+    FakeIter(0..10).rev().next();
+
+    // should lint
+    let _ = (0..10).next_back().unwrap();
+    let _ = "something".bytes().next_back();
+}

tests/ui/manual_next_back.rs

@@ -0,0 +1,36 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::manual_next_back)]
+
+struct FakeIter(std::ops::Range<i32>);
+
+impl FakeIter {
+    fn rev(self) -> Self {
+        self
+    }
+
+    fn next(&self) {}
+}
+
+impl DoubleEndedIterator for FakeIter {
+    fn next_back(&mut self) -> Option<Self::Item> {
+        self.0.next_back()
+    }
+}
+
+impl Iterator for FakeIter {
+    type Item = i32;
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+}
+
+fn main() {
+    // should not lint
+    FakeIter(0..10).rev().next();
+
+    // should lint
+    let _ = (0..10).rev().next().unwrap();
+    let _ = "something".bytes().rev().next();
+}

tests/ui/manual_next_back.stderr

@@ -0,0 +1,16 @@
+error: manual backwards iteration
+  --> $DIR/manual_next_back.rs:34:20
+   |
+LL |     let _ = (0..10).rev().next().unwrap();
+   |                    ^^^^^^^^^^^^^ help: use: `.next_back()`
+   |
+   = note: `-D clippy::manual-next-back` implied by `-D warnings`
+
+error: manual backwards iteration
+  --> $DIR/manual_next_back.rs:35:32
+   |
+LL |     let _ = "something".bytes().rev().next();
+   |                                ^^^^^^^^^^^^^ help: use: `.next_back()`
+
+error: aborting due to 2 previous errors
+