Skip to content

Commit

Permalink
.last() to .next_back() requires a mutable receiver
Browse files Browse the repository at this point in the history
In the case where `iter` is a `DoubleEndedIterator`, replacing a call to
`iter.last()` (which consumes `iter`) by `iter.next_back()` (which
requires a mutable reference to `iter`) cannot be done when `iter` is a
non-mutable binding which is not a mutable reference.
  • Loading branch information
samueltardieu committed Feb 2, 2025
1 parent 6b92d40 commit 5dfa493
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 2 deletions.
4 changes: 3 additions & 1 deletion clippy_lints/src/methods/double_ended_iterator_last.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/double_ended_iterator_last.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
20 changes: 20 additions & 0 deletions tests/ui/double_ended_iterator_last.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
20 changes: 19 additions & 1 deletion tests/ui/double_ended_iterator_last.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 5dfa493

Please sign in to comment.