.last() to .next_back() requires a mutable receiver

clippy_lints/src/methods/double_ended_iterator_last.rs

@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::is_trait_method;
 use clippy_utils::ty::implements_trait;
+use clippy_utils::{is_mutable, is_trait_method};
 use rustc_errors::Applicability;
 use rustc_hir::Expr;
 use rustc_lint::LateContext;
@@ -27,6 +27,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Exp
         && let Some(last_def) = cx.tcx.provided_trait_methods(item).find(|m| m.name.as_str() == "last")
         // if the resolved method is the same as the provided definition
         && fn_def.def_id() == last_def.def_id
+        // if `selx_expr` is a reference, it is mutable because it is used for `.last()`
+        && (is_mutable(cx, self_expr) || cx.typeck_results().expr_ty(self_expr).is_ref())
     {
         span_lint_and_sugg(
             cx,

clippy_lints/src/unnecessary_struct_initialization.rs

@@ -1,8 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet;
 use clippy_utils::ty::is_copy;
-use clippy_utils::{get_parent_expr, path_to_local};
-use rustc_hir::{BindingMode, Expr, ExprField, ExprKind, Node, PatKind, Path, QPath, StructTailExpr, UnOp};
+use clippy_utils::{get_parent_expr, is_mutable, path_to_local};
+use rustc_hir::{Expr, ExprField, ExprKind, Path, QPath, StructTailExpr, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 
@@ -157,16 +157,6 @@ fn same_path_in_all_fields<'tcx>(
     }
 }
 
-fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    if let Some(hir_id) = path_to_local(expr)
-        && let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
-    {
-        matches!(pat.kind, PatKind::Binding(BindingMode::MUT, ..))
-    } else {
-        true
-    }
-}
-
 fn check_references(cx: &LateContext<'_>, expr_a: &Expr<'_>, expr_b: &Expr<'_>) -> bool {
     if let Some(parent) = get_parent_expr(cx, expr_a)
         && let parent_ty = cx.typeck_results().expr_ty_adjusted(parent)

clippy_utils/src/lib.rs

@@ -3481,3 +3481,22 @@ pub fn leaks_droppable_temporary_with_limited_lifetime<'tcx>(cx: &LateContext<'t
     })
     .is_break()
 }
+
+/// Returns `true` if `expr` designates a mutable static, a mutable local binding, or an expression
+/// that can be owned.
+pub fn is_mutable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    if let Some(hir_id) = path_to_local(expr)
+        && let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
+    {
+        matches!(pat.kind, PatKind::Binding(BindingMode::MUT, ..))
+    } else if let ExprKind::Path(p) = &expr.kind
+        && let Some(mutability) = cx
+            .qpath_res(p, expr.hir_id)
+            .opt_def_id()
+            .and_then(|id| cx.tcx.static_mutability(id))
+    {
+        mutability == Mutability::Mut
+    } else {
+        true
+    }
+}

tests/ui/double_ended_iterator_last.fixed

@@ -2,7 +2,7 @@
 
 // Typical case
 pub fn last_arg(s: &str) -> Option<&str> {
-    s.split(' ').next_back()
+    s.split(' ').next_back() //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
 }
 
 fn main() {
@@ -19,7 +19,7 @@ fn main() {
             Some(())
         }
     }
-    let _ = DeIterator.next_back();
+    let _ = DeIterator.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
     // Should not apply to other methods of Iterator
     let _ = DeIterator.count();
 
@@ -51,3 +51,23 @@ fn main() {
     }
     let _ = CustomLast.last();
 }
+
+fn issue_14139() {
+    let mut index = [true, true, false, false, false, true].iter();
+    let subindex = index.by_ref().take(3);
+    let _ = subindex.last(); // No lint, `subindex` is not mutable
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let _ = subindex.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let subindex = &mut subindex;
+    let _ = subindex.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let subindex = &mut subindex;
+    let _ = subindex.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+}

tests/ui/double_ended_iterator_last.rs

@@ -2,7 +2,7 @@
 
 // Typical case
 pub fn last_arg(s: &str) -> Option<&str> {
-    s.split(' ').last()
+    s.split(' ').last() //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
 }
 
 fn main() {
@@ -19,7 +19,7 @@ fn main() {
             Some(())
         }
     }
-    let _ = DeIterator.last();
+    let _ = DeIterator.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
     // Should not apply to other methods of Iterator
     let _ = DeIterator.count();
 
@@ -51,3 +51,23 @@ fn main() {
     }
     let _ = CustomLast.last();
 }
+
+fn issue_14139() {
+    let mut index = [true, true, false, false, false, true].iter();
+    let subindex = index.by_ref().take(3);
+    let _ = subindex.last(); // No lint, `subindex` is not mutable
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let _ = subindex.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let subindex = &mut subindex;
+    let _ = subindex.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+
+    let mut index = [true, true, false, false, false, true].iter();
+    let mut subindex = index.by_ref().take(3);
+    let subindex = &mut subindex;
+    let _ = subindex.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
+}

tests/ui/double_ended_iterator_last.stderr

@@ -13,5 +13,23 @@ error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly
 LL |     let _ = DeIterator.last();
    |                        ^^^^^^ help: try: `next_back()`
 
-error: aborting due to 2 previous errors
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:62:22
+   |
+LL |     let _ = subindex.last();
+   |                      ^^^^^^ help: try: `next_back()`
+
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:67:22
+   |
+LL |     let _ = subindex.last();
+   |                      ^^^^^^ help: try: `next_back()`
+
+error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
+  --> tests/ui/double_ended_iterator_last.rs:72:22
+   |
+LL |     let _ = subindex.last();
+   |                      ^^^^^^ help: try: `next_back()`
+
+error: aborting due to 5 previous errors