Skip to content

Commit

Permalink
Add lint for calling last() on DoubleEndedIterator
Browse files Browse the repository at this point in the history
  • Loading branch information
qsantos committed Jan 1, 2025
1 parent 33a6590 commit 707653f
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5493,6 +5493,7 @@ Released 2018-09-13
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
[`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
[`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last
[`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use
[`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
[`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::methods::CLONE_ON_REF_PTR_INFO,
crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
crate::methods::CONST_IS_EMPTY_INFO,
crate::methods::DOUBLE_ENDED_ITERATOR_LAST_INFO,
crate::methods::DRAIN_COLLECT_INFO,
crate::methods::ERR_EXPECT_INFO,
crate::methods::EXPECT_FUN_CALL_INFO,
Expand Down
42 changes: 42 additions & 0 deletions clippy_lints/src/methods/double_ended_iterator_last.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_trait_method;
use clippy_utils::ty::implements_trait;
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::{Span, sym};

use super::DOUBLE_ENDED_ITERATOR_LAST;

pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Expr<'_>, call_span: Span) {
let typeck = cx.typeck_results();

// Check if the current "last" method is that of the Iterator trait
if !is_trait_method(cx, expr, sym::Iterator) {
return;
}

// Find id for DoubleEndedIterator trait
let Some(deiter_id) = cx.tcx.get_diagnostic_item(sym::DoubleEndedIterator) else {
return;
};

// Find the type of self
let self_type = typeck.expr_ty(self_expr).peel_refs();

// Check that the object implements the DoubleEndedIterator trait
if !implements_trait(cx, self_type, deiter_id, &[]) {
return;
}

// Emit lint
span_lint_and_sugg(
cx,
DOUBLE_ENDED_ITERATOR_LAST,
call_span,
"called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator",
"try",
"next_back()".to_string(),
Applicability::MachineApplicable,
);
}
29 changes: 29 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod clone_on_copy;
mod clone_on_ref_ptr;
mod cloned_instead_of_copied;
mod collapsible_str_replace;
mod double_ended_iterator_last;
mod drain_collect;
mod err_expect;
mod expect_fun_call;
Expand Down Expand Up @@ -4284,6 +4285,32 @@ declare_clippy_lint! {
"map of a trivial closure (not dependent on parameter) over a range"
}

declare_clippy_lint! {
/// ### What it does
///
/// Checks for `Iterator::last` being called on a `DoubleEndedIterator`, which can be replaced
/// with `DoubleEndedIterator::next_back`.
///
/// ### Why is this bad?
///
/// `Iterator::last` is implemented by consuming the iterator, which is unnecessary if
/// the iterator is a `DoubleEndedIterator`. Since Rust traits do not allow specialization,
/// `Iterator::last` cannot be optimized for `DoubleEndedIterator`.
///
/// ### Example
/// ```no_run
/// let last_arg = "echo hello world".split(' ').last();
/// ```
/// Use instead:
/// ```no_run
/// let last_arg = "echo hello world".split(' ').next_back();
/// ```
#[clippy::version = "1.85.0"]
pub DOUBLE_ENDED_ITERATOR_LAST,
perf,
"using `Iterator::last` on a `DoubleEndedIterator`"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -4449,6 +4476,7 @@ impl_lint_pass!(Methods => [
MAP_ALL_ANY_IDENTITY,
MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
UNNECESSARY_MAP_OR,
DOUBLE_ENDED_ITERATOR_LAST,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4931,6 +4959,7 @@ impl Methods {
false,
);
}
double_ended_iterator_last::check(cx, expr, recv, call_span);
},
("len", []) => {
if let Some(("as_bytes", prev_recv, [], _, _)) = method_call(recv) {
Expand Down
35 changes: 35 additions & 0 deletions tests/ui/double_ended_iterator_last.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#![warn(clippy::double_ended_iterator_last)]

// Typical case
pub fn last_arg(s: &str) -> Option<&str> {
s.split(' ').next_back()
}

fn main() {
// General case
struct DeIterator;
impl Iterator for DeIterator {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
Some(())
}
}
impl DoubleEndedIterator for DeIterator {
fn next_back(&mut self) -> Option<Self::Item> {
Some(())
}
}
let _ = DeIterator.next_back();
// Should not apply to other methods of Iterator
let _ = DeIterator.count();

// Should not apply to simple iterators
struct SimpleIterator;
impl Iterator for SimpleIterator {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
Some(())
}
}
let _ = SimpleIterator.last();
}
35 changes: 35 additions & 0 deletions tests/ui/double_ended_iterator_last.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#![warn(clippy::double_ended_iterator_last)]

// Typical case
pub fn last_arg(s: &str) -> Option<&str> {
s.split(' ').last()
}

fn main() {
// General case
struct DeIterator;
impl Iterator for DeIterator {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
Some(())
}
}
impl DoubleEndedIterator for DeIterator {
fn next_back(&mut self) -> Option<Self::Item> {
Some(())
}
}
let _ = DeIterator.last();
// Should not apply to other methods of Iterator
let _ = DeIterator.count();

// Should not apply to simple iterators
struct SimpleIterator;
impl Iterator for SimpleIterator {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
Some(())
}
}
let _ = SimpleIterator.last();
}
17 changes: 17 additions & 0 deletions tests/ui/double_ended_iterator_last.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
--> tests/ui/double_ended_iterator_last.rs:5:18
|
LL | s.split(' ').last()
| ^^^^^^ 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:22:24
|
LL | let _ = DeIterator.last();
| ^^^^^^ help: try: `next_back()`

error: aborting due to 2 previous errors

0 comments on commit 707653f

Please sign in to comment.