Add lint for calling last() on DoubleEndedIterator
This commit is contained in:
parent
33a6590ce1
commit
707653f268
7 changed files with 160 additions and 0 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
42
clippy_lints/src/methods/double_ended_iterator_last.rs
Normal file
42
clippy_lints/src/methods/double_ended_iterator_last.rs
Normal file
|
|
@ -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,
|
||||
);
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
@ -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,
|
||||
|
|
@ -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.
|
||||
|
|
@ -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) {
|
||||
|
|
|
|||
35
tests/ui/double_ended_iterator_last.fixed
Normal file
35
tests/ui/double_ended_iterator_last.fixed
Normal file
|
|
@ -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
tests/ui/double_ended_iterator_last.rs
Normal file
35
tests/ui/double_ended_iterator_last.rs
Normal file
|
|
@ -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
tests/ui/double_ended_iterator_last.stderr
Normal file
17
tests/ui/double_ended_iterator_last.stderr
Normal file
|
|
@ -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
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue