New line: cloned_next

Author: pmnoxx

clippy_lints/src/lib.register_nursery.rs

@@ -15,6 +15,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(future_not_send::FUTURE_NOT_SEND),
     LintId::of(index_refutable_slice::INDEX_REFUTABLE_SLICE),
     LintId::of(let_if_seq::USELESS_LET_IF_SEQ),
+    LintId::of(methods::CLONED_LAST),
     LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
     LintId::of(mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),
     LintId::of(mutex_atomic::MUTEX_INTEGER),

clippy_lints/src/matches.rs

@@ -790,7 +790,7 @@ fn report_single_match_single_pattern(
             // will not: match Some(_) { &Some(_) => () }
             let ref_count_diff = ty_ref_count - pat_ref_count;
 
-            // Try to remove address of expressions first.
+            // try to remove address of expressions first.
             let (ex, removed) = peel_n_hir_expr_refs(ex, ref_count_diff);
             let ref_count_diff = ref_count_diff - removed;
 

clippy_lints/src/methods/cloned_last.rs

@@ -0,0 +1,36 @@
+use clippy_utils::diagnostics::{span_lint_and_sugg};
+use clippy_utils::source::snippet;
+use clippy_utils::ty::implements_trait;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::CLONED_LAST;
+
+/// lint use of `cloned().next()` for `Iterators`
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'_>,
+) {
+    // lint if caller of `.filter().next()` is an Iterator
+    let recv_impls_iterator = cx.tcx.get_diagnostic_item(sym::Iterator).map_or(false, |id| {
+        implements_trait(cx, cx.typeck_results().expr_ty(recv), id, &[])
+    });
+    if recv_impls_iterator {
+        let msg = "called `cloned().last()` on an `Iterator`. It may be more efficient to call
+                   `.last.cloned()` instead";
+        let iter_snippet = snippet(cx, recv.span, "..");
+        // add note if not multi-line
+        span_lint_and_sugg(
+            cx,
+            CLONED_LAST,
+            expr.span,
+            msg,
+            "try this",
+            format!("{}.last().cloned()", iter_snippet),
+            Applicability::MachineApplicable,
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -9,6 +9,7 @@ mod chars_next_cmp_with_unwrap;
 mod clone_on_copy;
 mod clone_on_ref_ptr;
 mod cloned_instead_of_copied;
+mod cloned_last;
 mod expect_fun_call;
 mod expect_used;
 mod extend_with_drain;
@@ -107,6 +108,29 @@ declare_clippy_lint! {
     "used `cloned` where `copied` could be used instead"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `_.cloned().last()`.
+    ///
+    /// ### Why is this bad?
+    /// Performance, clone only need to be executed once.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let vec = vec!["string".to_string()];
+    /// vec.iter().cloned().last();
+    /// ```
+    /// Could be written as
+    /// ```rust
+    /// # let vec = vec!["string".to_string()];
+    /// vec.iter().last().cloned();
+    /// ```
+    #[clippy::version = "1.59.0"]
+    pub CLONED_LAST,
+    complexity,
+    "using `cloned().last()`, which is less efficient than `.last().cloned()`"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usages of `Iterator::flat_map()` where `filter_map()` could be
@@ -1946,6 +1970,7 @@ impl_lint_pass!(Methods => [
     CLONE_ON_COPY,
     CLONE_ON_REF_PTR,
     CLONE_DOUBLE_REF,
+    CLONED_LAST,
     CLONED_INSTEAD_OF_COPIED,
     FLAT_MAP_OPTION,
     INEFFICIENT_TO_STRING,
@@ -2232,7 +2257,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv),
             ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
             ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
-            ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, msrv),
+            ("cloned", []) => {
+                cloned_instead_of_copied::check(cx, expr, recv, span, msrv);
+            },
             ("collect", []) => match method_call!(recv) {
                 Some((name @ ("cloned" | "copied"), [recv2], _)) => {
                     iter_cloned_collect::check(cx, name, expr, recv2);
@@ -2285,6 +2312,14 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             ("is_file", []) => filetype_is_file::check(cx, expr, recv),
             ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
             ("is_some", []) => check_is_some_is_none(cx, expr, recv, true),
+            ("last", []) => {
+                if let Some((name, [recv, args @ ..], _)) = method_call!(recv) {
+                    match (name, args) {
+                        ("cloned", []) => cloned_last::check(cx, expr, recv),
+                        _ => {},
+                    }
+                }
+            },
             ("map", [m_arg]) => {
                 if let Some((name, [recv2, args @ ..], span2)) = method_call!(recv) {
                     match (name, args) {

tests/ui/cloned_last.fixed

@@ -0,0 +1,9 @@
+// run-rustfix
+#![warn(clippy::nursery)]
+
+fn main() {
+
+    #[rustfmt::skip]
+    let _: Option<String> = vec!["1".to_string(), "2".to_string(), "3".to_string()]
+        .iter().last().cloned();
+}

tests/ui/cloned_last.rs

@@ -0,0 +1,9 @@
+// run-rustfix
+#![warn(clippy::nursery)]
+
+fn main() {
+
+    #[rustfmt::skip]
+    let _: Option<String> = vec!["1".to_string(), "2".to_string(), "3".to_string()]
+        .iter().cloned().last();
+}

tests/ui/cloned_last.stderr

@@ -0,0 +1,18 @@
+error: called `cloned().last()` on an `Iterator`. It may be more efficient to call
+                          `.last.cloned()` instead
+  --> $DIR/cloned_last.rs:7:29
+   |
+LL |       let _: Option<String> = vec!["1".to_string(), "2".to_string(), "3".to_string()]
+   |  _____________________________^
+LL | |         .iter().cloned().last();
+   | |_______________________________^
+   |
+   = note: `-D clippy::cloned-last` implied by `-D warnings`
+help: try this
+   |
+LL ~     let _: Option<String> = vec!["1".to_string(), "2".to_string(), "3".to_string()]
+LL ~         .iter().last().cloned();
+   |
+
+error: aborting due to previous error
+