.last() to .next_back() requires a mutable receiver
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 not a mutable binding or a mutable reference. When `iter` is a local binding, it can be made mutable by fixing its definition site.
This commit is contained in:
parent
510d3b69fc
commit
45f7a60d31
6 changed files with 169 additions and 21 deletions
|
|
@ -1,8 +1,8 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::is_trait_method;
|
||||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::ty::implements_trait;
|
||||
use clippy_utils::{is_mutable, is_trait_method, path_to_local};
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::Expr;
|
||||
use rustc_hir::{Expr, Node, PatKind};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty::Instance;
|
||||
use rustc_span::{Span, sym};
|
||||
|
|
@ -28,14 +28,40 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Exp
|
|||
// if the resolved method is the same as the provided definition
|
||||
&& fn_def.def_id() == last_def.def_id
|
||||
{
|
||||
span_lint_and_sugg(
|
||||
let mut sugg = vec![(call_span, String::from("next_back()"))];
|
||||
let mut dont_apply = false;
|
||||
// if `self_expr` is a reference, it is mutable because it is used for `.last()`
|
||||
if !(is_mutable(cx, self_expr) || self_type.is_ref()) {
|
||||
if let Some(hir_id) = path_to_local(self_expr)
|
||||
&& let Node::Pat(pat) = cx.tcx.hir_node(hir_id)
|
||||
&& let PatKind::Binding(_, _, ident, _) = pat.kind
|
||||
{
|
||||
sugg.push((ident.span.shrink_to_lo(), String::from("mut ")));
|
||||
} else {
|
||||
// If we can't make the binding mutable, make the suggestion `Unspecified` to prevent it from being
|
||||
// automatically applied, and add a complementary help message.
|
||||
dont_apply = true;
|
||||
}
|
||||
}
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
DOUBLE_ENDED_ITERATOR_LAST,
|
||||
call_span,
|
||||
expr.span,
|
||||
"called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator",
|
||||
"try",
|
||||
"next_back()".to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
|diag| {
|
||||
diag.multipart_suggestion(
|
||||
"try",
|
||||
sugg,
|
||||
if dont_apply {
|
||||
Applicability::Unspecified
|
||||
} else {
|
||||
Applicability::MachineApplicable
|
||||
},
|
||||
);
|
||||
if dont_apply {
|
||||
diag.span_note(self_expr.span, "this must be made mutable to use `.next_back()`");
|
||||
}
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,8 +2,7 @@
|
|||
|
||||
// Typical case
|
||||
pub fn last_arg(s: &str) -> Option<&str> {
|
||||
s.split(' ').next_back()
|
||||
//~^ double_ended_iterator_last
|
||||
s.split(' ').next_back() //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
|
@ -20,8 +19,7 @@ fn main() {
|
|||
Some(())
|
||||
}
|
||||
}
|
||||
let _ = DeIterator.next_back();
|
||||
//~^ double_ended_iterator_last
|
||||
let _ = DeIterator.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
// Should not apply to other methods of Iterator
|
||||
let _ = DeIterator.count();
|
||||
|
||||
|
|
@ -53,3 +51,27 @@ fn main() {
|
|||
}
|
||||
let _ = CustomLast.last();
|
||||
}
|
||||
|
||||
fn issue_14139() {
|
||||
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.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`
|
||||
|
||||
let mut index = [true, true, false, false, false, true].iter();
|
||||
let (mut subindex, _) = (index.by_ref().take(3), 42);
|
||||
let _ = subindex.next_back(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,8 +2,7 @@
|
|||
|
||||
// Typical case
|
||||
pub fn last_arg(s: &str) -> Option<&str> {
|
||||
s.split(' ').last()
|
||||
//~^ double_ended_iterator_last
|
||||
s.split(' ').last() //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
}
|
||||
|
||||
fn main() {
|
||||
|
|
@ -20,8 +19,7 @@ fn main() {
|
|||
Some(())
|
||||
}
|
||||
}
|
||||
let _ = DeIterator.last();
|
||||
//~^ double_ended_iterator_last
|
||||
let _ = DeIterator.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
// Should not apply to other methods of Iterator
|
||||
let _ = DeIterator.count();
|
||||
|
||||
|
|
@ -53,3 +51,27 @@ 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(); //~ 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.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`
|
||||
|
||||
let mut index = [true, true, false, false, false, true].iter();
|
||||
let (subindex, _) = (index.by_ref().take(3), 42);
|
||||
let _ = subindex.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,17 +1,69 @@
|
|||
error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
|
||||
--> tests/ui/double_ended_iterator_last.rs:5:18
|
||||
--> tests/ui/double_ended_iterator_last.rs:5:5
|
||||
|
|
||||
LL | s.split(' ').last()
|
||||
| ^^^^^^ help: try: `next_back()`
|
||||
| ^^^^^^^^^^^^^------
|
||||
| |
|
||||
| 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:23:24
|
||||
--> tests/ui/double_ended_iterator_last.rs:22:13
|
||||
|
|
||||
LL | let _ = DeIterator.last();
|
||||
| ^^^^^^ help: try: `next_back()`
|
||||
| ^^^^^^^^^^^------
|
||||
| |
|
||||
| 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:58:13
|
||||
|
|
||||
LL | let _ = subindex.last();
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: try
|
||||
|
|
||||
LL ~ let mut subindex = index.by_ref().take(3);
|
||||
LL ~ let _ = subindex.next_back();
|
||||
|
|
||||
|
||||
error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
|
||||
--> tests/ui/double_ended_iterator_last.rs:62:13
|
||||
|
|
||||
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:13
|
||||
|
|
||||
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:13
|
||||
|
|
||||
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:76:13
|
||||
|
|
||||
LL | let _ = subindex.last();
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: try
|
||||
|
|
||||
LL ~ let (mut subindex, _) = (index.by_ref().take(3), 42);
|
||||
LL ~ let _ = subindex.next_back();
|
||||
|
|
||||
|
||||
error: aborting due to 7 previous errors
|
||||
|
||||
|
|
|
|||
8
tests/ui/double_ended_iterator_last_unfixable.rs
Normal file
8
tests/ui/double_ended_iterator_last_unfixable.rs
Normal file
|
|
@ -0,0 +1,8 @@
|
|||
//@no-rustfix
|
||||
#![warn(clippy::double_ended_iterator_last)]
|
||||
|
||||
fn main() {
|
||||
let mut index = [true, true, false, false, false, true].iter();
|
||||
let subindex = (index.by_ref().take(3), 42);
|
||||
let _ = subindex.0.last(); //~ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
|
||||
}
|
||||
18
tests/ui/double_ended_iterator_last_unfixable.stderr
Normal file
18
tests/ui/double_ended_iterator_last_unfixable.stderr
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
|
||||
--> tests/ui/double_ended_iterator_last_unfixable.rs:7:13
|
||||
|
|
||||
LL | let _ = subindex.0.last();
|
||||
| ^^^^^^^^^^^------
|
||||
| |
|
||||
| help: try: `next_back()`
|
||||
|
|
||||
note: this must be made mutable to use `.next_back()`
|
||||
--> tests/ui/double_ended_iterator_last_unfixable.rs:7:13
|
||||
|
|
||||
LL | let _ = subindex.0.last();
|
||||
| ^^^^^^^^^^
|
||||
= note: `-D clippy::double-ended-iterator-last` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::double_ended_iterator_last)]`
|
||||
|
||||
error: aborting due to 1 previous error
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue