Add lint for calling last() on DoubleEndedIterator

qsantos · qsantos · commit 707653f268a2 · 2025-01-01T18:47:23.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5493,6 +5493,7 @@ Released 2018-09-13
 [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
 [`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
 [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
+[`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last
 [`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use
 [`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
 [`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -372,6 +372,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::CLONE_ON_REF_PTR_INFO,
     crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
     crate::methods::CONST_IS_EMPTY_INFO,
+    crate::methods::DOUBLE_ENDED_ITERATOR_LAST_INFO,
     crate::methods::DRAIN_COLLECT_INFO,
     crate::methods::ERR_EXPECT_INFO,
     crate::methods::EXPECT_FUN_CALL_INFO,
diff --git a/clippy_lints/src/methods/double_ended_iterator_last.rs b/clippy_lints/src/methods/double_ended_iterator_last.rs
@@ -0,0 +1,42 @@
+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::{Span, sym};
+
+use super::DOUBLE_ENDED_ITERATOR_LAST;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Expr<'_>, call_span: Span) {
+    let typeck = cx.typeck_results();
+
+    // Check if the current "last" method is that of the Iterator trait
+    if !is_trait_method(cx, expr, sym::Iterator) {
+        return;
+    }
+
+    // Find id for DoubleEndedIterator trait
+    let Some(deiter_id) = cx.tcx.get_diagnostic_item(sym::DoubleEndedIterator) else {
+        return;
+    };
+
+    // Find the type of self
+    let self_type = typeck.expr_ty(self_expr).peel_refs();
+
+    // Check that the object implements the DoubleEndedIterator trait
+    if !implements_trait(cx, self_type, deiter_id, &[]) {
+        return;
+    }
+
+    // Emit lint
+    span_lint_and_sugg(
+        cx,
+        DOUBLE_ENDED_ITERATOR_LAST,
+        call_span,
+        "called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator",
+        "try",
+        "next_back()".to_string(),
+        Applicability::MachineApplicable,
+    );
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -14,6 +14,7 @@ mod clone_on_copy;
 mod clone_on_ref_ptr;
 mod cloned_instead_of_copied;
 mod collapsible_str_replace;
+mod double_ended_iterator_last;
 mod drain_collect;
 mod err_expect;
 mod expect_fun_call;
@@ -4284,6 +4285,32 @@ declare_clippy_lint! {
     "map of a trivial closure (not dependent on parameter) over a range"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for `Iterator::last` being called on a  `DoubleEndedIterator`, which can be replaced
+    /// with `DoubleEndedIterator::next_back`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// `Iterator::last` is implemented by consuming the iterator, which is unnecessary if
+    /// the iterator is a `DoubleEndedIterator`. Since Rust traits do not allow specialization,
+    /// `Iterator::last` cannot be optimized for `DoubleEndedIterator`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let last_arg = "echo hello world".split(' ').last();
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let last_arg = "echo hello world".split(' ').next_back();
+    /// ```
+    #[clippy::version = "1.85.0"]
+    pub DOUBLE_ENDED_ITERATOR_LAST,
+    perf,
+    "using `Iterator::last` on a `DoubleEndedIterator`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4449,6 +4476,7 @@ impl_lint_pass!(Methods => [
     MAP_ALL_ANY_IDENTITY,
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     UNNECESSARY_MAP_OR,
+    DOUBLE_ENDED_ITERATOR_LAST,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4931,6 +4959,7 @@ impl Methods {
                             false,
                         );
                     }
+                    double_ended_iterator_last::check(cx, expr, recv, call_span);
                 },
                 ("len", []) => {
                     if let Some(("as_bytes", prev_recv, [], _, _)) = method_call(recv) {
diff --git a/tests/ui/double_ended_iterator_last.fixed b/tests/ui/double_ended_iterator_last.fixed
@@ -0,0 +1,35 @@
+#![warn(clippy::double_ended_iterator_last)]
+
+// Typical case
+pub fn last_arg(s: &str) -> Option<&str> {
+    s.split(' ').next_back()
+}
+
+fn main() {
+    // General case
+    struct DeIterator;
+    impl Iterator for DeIterator {
+        type Item = ();
+        fn next(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    impl DoubleEndedIterator for DeIterator {
+        fn next_back(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    let _ = DeIterator.next_back();
+    // Should not apply to other methods of Iterator
+    let _ = DeIterator.count();
+
+    // Should not apply to simple iterators
+    struct SimpleIterator;
+    impl Iterator for SimpleIterator {
+        type Item = ();
+        fn next(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    let _ = SimpleIterator.last();
+}
diff --git a/tests/ui/double_ended_iterator_last.rs b/tests/ui/double_ended_iterator_last.rs
@@ -0,0 +1,35 @@
+#![warn(clippy::double_ended_iterator_last)]
+
+// Typical case
+pub fn last_arg(s: &str) -> Option<&str> {
+    s.split(' ').last()
+}
+
+fn main() {
+    // General case
+    struct DeIterator;
+    impl Iterator for DeIterator {
+        type Item = ();
+        fn next(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    impl DoubleEndedIterator for DeIterator {
+        fn next_back(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    let _ = DeIterator.last();
+    // Should not apply to other methods of Iterator
+    let _ = DeIterator.count();
+
+    // Should not apply to simple iterators
+    struct SimpleIterator;
+    impl Iterator for SimpleIterator {
+        type Item = ();
+        fn next(&mut self) -> Option<Self::Item> {
+            Some(())
+        }
+    }
+    let _ = SimpleIterator.last();
+}
diff --git a/tests/ui/double_ended_iterator_last.stderr b/tests/ui/double_ended_iterator_last.stderr
@@ -0,0 +1,17 @@
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:5:18
+   |
+LL |     s.split(' ').last()
+   |                  ^^^^^^ help: try: `next_back()`
+   |
+   = note: `-D clippy::double-ended-iterator-last` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::double_ended_iterator_last)]`
+
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:22:24
+   |
+LL |     let _ = DeIterator.last();
+   |                        ^^^^^^ help: try: `next_back()`
+
+error: aborting due to 2 previous errors
+
