diff --git a/clippy_lints/src/methods/double_ended_iterator_last.rs b/clippy_lints/src/methods/double_ended_iterator_last.rs index 208172980c9f..29cfa7cb37b1 100644 --- a/clippy_lints/src/methods/double_ended_iterator_last.rs +++ b/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, diff --git a/clippy_lints/src/unnecessary_struct_initialization.rs b/clippy_lints/src/unnecessary_struct_initialization.rs index 1df229c330eb..5792b6b3178d 100644 --- a/clippy_lints/src/unnecessary_struct_initialization.rs +++ b/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) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 7f06a04cbd2d..a340b41a0062 100644 --- a/clippy_utils/src/lib.rs +++ b/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 + } +} diff --git a/tests/ui/double_ended_iterator_last.fixed b/tests/ui/double_ended_iterator_last.fixed index 06c48e337537..3f5291c05727 100644 --- a/tests/ui/double_ended_iterator_last.fixed +++ b/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` +} diff --git a/tests/ui/double_ended_iterator_last.rs b/tests/ui/double_ended_iterator_last.rs index 9c13b496d117..4767f183c37c 100644 --- a/tests/ui/double_ended_iterator_last.rs +++ b/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` +} diff --git a/tests/ui/double_ended_iterator_last.stderr b/tests/ui/double_ended_iterator_last.stderr index b795c18a736e..eaae6cd93579 100644 --- a/tests/ui/double_ended_iterator_last.stderr +++ b/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