-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add the iter_last_slice lint
#15496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add the iter_last_slice lint
#15496
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| use super::ITER_LAST_SLICE; | ||
| use super::utils::derefs_to_slice; | ||
| use clippy_utils::diagnostics::span_lint_and_sugg; | ||
| use clippy_utils::higher; | ||
| use clippy_utils::source::snippet_with_applicability; | ||
| use rustc_ast::ast; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir as hir; | ||
| use rustc_lint::LateContext; | ||
|
|
||
| pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, caller_expr: &'tcx hir::Expr<'_>) { | ||
| if derefs_to_slice(cx, caller_expr, cx.typeck_results().expr_ty(caller_expr)).is_some() { | ||
| // caller is a Slice | ||
| if let hir::ExprKind::Index(caller_var, index_expr, _) = &caller_expr.kind | ||
| && let Some(higher::Range { | ||
| start: _, | ||
| end: Some(end_expr), | ||
| limits, | ||
| }) = higher::Range::hir(index_expr) | ||
| && let hir::ExprKind::Lit(end_lit) = &end_expr.kind | ||
| && let ast::LitKind::Int(end_idx, _) = end_lit.node | ||
| { | ||
| let mut applicability = Applicability::MachineApplicable; | ||
| let suggest = if end_idx == 0 { | ||
| // This corresponds to `$slice[..0].iter().last()`, which | ||
| // evaluates to `None`. Maybe this is worth its own lint? | ||
| format!( | ||
| "{}.last()", | ||
| snippet_with_applicability(cx, caller_var.span, "..", &mut applicability) | ||
| ) | ||
| } else { | ||
| let idx = match limits { | ||
| ast::RangeLimits::HalfOpen => end_idx.get() - 1, | ||
| ast::RangeLimits::Closed => end_idx.get(), | ||
| }; | ||
|
|
||
| format!( | ||
| "{}.get({idx})", | ||
| snippet_with_applicability(cx, caller_var.span, "..", &mut applicability) | ||
| ) | ||
| }; | ||
| span_lint_and_sugg( | ||
| cx, | ||
| ITER_LAST_SLICE, | ||
| expr.span, | ||
| "using `.iter().last()` on a slice without end index", | ||
| "try calling", | ||
| suggest, | ||
| applicability, | ||
| ); | ||
| } | ||
| } else if super::iter_next_slice::is_vec_or_array(cx, caller_expr) { | ||
| // caller is a Vec or an Array | ||
| let mut applicability = Applicability::MachineApplicable; | ||
| span_lint_and_sugg( | ||
| cx, | ||
| ITER_LAST_SLICE, | ||
| expr.span, | ||
| "using `.iter().last()` on an array", | ||
| "try calling", | ||
| format!( | ||
| "{}.last()", | ||
| snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability) | ||
| ), | ||
| applicability, | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ mod iter_cloned_collect; | |
| mod iter_count; | ||
| mod iter_filter; | ||
| mod iter_kv_map; | ||
| mod iter_last_slice; | ||
| mod iter_next_slice; | ||
| mod iter_nth; | ||
| mod iter_nth_zero; | ||
|
|
@@ -1820,6 +1821,33 @@ declare_clippy_lint! { | |
| "using `.iter().next()` on a sliced array, which can be shortened to just `.get()`" | ||
| } | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Checks for usage of `iter().last()` on a Slice or an Array. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// These can be shortened into `.last()`. | ||
| /// | ||
| /// ### Example | ||
| /// ```no_run | ||
| /// # let a = [1, 2, 3]; | ||
| /// # let b = vec![1, 2, 3]; | ||
| /// a[..2].iter().last(); | ||
| /// b.iter().last(); | ||
| /// ``` | ||
| /// should be written as: | ||
| /// ```no_run | ||
| /// # let a = [1, 2, 3]; | ||
| /// # let b = vec![1, 2, 3]; | ||
| /// a.get(1); | ||
| /// b.get(0); | ||
| /// ``` | ||
| #[clippy::version = "1.46.0"] | ||
| pub ITER_LAST_SLICE, | ||
| style, | ||
| "using `.iter().next()` on a slice, array or `Vec`, which can be shortened to just `.get()`" | ||
| } | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Warns when using `push_str`/`insert_str` with a single-character string literal | ||
|
|
@@ -4644,6 +4672,7 @@ impl_lint_pass!(Methods => [ | |
| FLAT_MAP_IDENTITY, | ||
| MAP_FLATTEN, | ||
| ITERATOR_STEP_BY_ZERO, | ||
| ITER_LAST_SLICE, | ||
| ITER_NEXT_SLICE, | ||
| ITER_COUNT, | ||
| ITER_NTH, | ||
|
|
@@ -5228,17 +5257,27 @@ impl Methods { | |
| } | ||
| }, | ||
| (sym::last, []) => { | ||
| if let Some((sym::cloned, recv2, [], _span2, _)) = method_call(recv) { | ||
| iter_overeager_cloned::check( | ||
| cx, | ||
| expr, | ||
| recv, | ||
| recv2, | ||
| iter_overeager_cloned::Op::LaterCloned, | ||
| false, | ||
| ); | ||
| let method_call = method_call(recv); | ||
|
|
||
| match method_call { | ||
| Some((sym::cloned, recv2, [], _, _)) => { | ||
| iter_overeager_cloned::check( | ||
| cx, | ||
| expr, | ||
| recv, | ||
| recv2, | ||
| iter_overeager_cloned::Op::LaterCloned, | ||
| false, | ||
| ); | ||
| double_ended_iterator_last::check(cx, expr, recv, call_span); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why repeat this check in two match arms? I think leaving it outside the |
||
| }, | ||
|
|
||
| Some((sym::iter, recv2, [], _, _)) => iter_last_slice::check(cx, expr, recv2), | ||
|
|
||
| _ => { | ||
| double_ended_iterator_last::check(cx, expr, recv, call_span); | ||
| }, | ||
| } | ||
| double_ended_iterator_last::check(cx, expr, recv, call_span); | ||
| }, | ||
| (sym::len, []) => { | ||
| if let Some((prev_method @ (sym::as_bytes | sym::bytes), prev_recv, [], _, _)) = method_call(recv) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #![warn(clippy::iter_last_slice)] | ||
| #![allow(clippy::useless_vec)] | ||
|
|
||
| fn main() { | ||
| // test code goes here | ||
| let s = [1, 2, 3]; | ||
| let v = vec![1, 2, 3]; | ||
|
|
||
| let _ = s.last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.last() | ||
|
|
||
| let _ = s.get(1); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.get(1) | ||
| let _ = s.get(2); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.get(2) | ||
|
|
||
| let _ = v.get(4); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.get(4) | ||
| let _ = v.get(5); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.get(5) | ||
|
|
||
| let _ = v.last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.last() | ||
|
|
||
| let o = Some(5); | ||
| o.iter().last(); | ||
| // Shouldn't be linted since this is not a Slice or an Array | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #![warn(clippy::iter_last_slice)] | ||
| #![allow(clippy::useless_vec)] | ||
|
|
||
| fn main() { | ||
| // test code goes here | ||
| let s = [1, 2, 3]; | ||
| let v = vec![1, 2, 3]; | ||
|
|
||
| let _ = s.iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.last() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these comments could be left out -- the |
||
|
|
||
| let _ = s[..2].iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.get(1) | ||
| let _ = s[..=2].iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by s.get(2) | ||
|
|
||
| let _ = v[..5].iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.get(4) | ||
| let _ = v[..=5].iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.get(5) | ||
|
|
||
| let _ = v.iter().last(); | ||
| //~^ iter_last_slice | ||
| // Should be replaced by v.last() | ||
|
|
||
| let o = Some(5); | ||
| o.iter().last(); | ||
| // Shouldn't be linted since this is not a Slice or an Array | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| error: using `.iter().last()` on an array | ||
| --> tests/ui/iter_last_slice.rs:9:13 | ||
| | | ||
| LL | let _ = s.iter().last(); | ||
| | ^^^^^^^^^^^^^^^ help: try calling: `s.last()` | ||
| | | ||
| = note: `-D clippy::iter-last-slice` implied by `-D warnings` | ||
| = help: to override `-D warnings` add `#[allow(clippy::iter_last_slice)]` | ||
|
|
||
| error: using `.iter().last()` on a slice without end index | ||
| --> tests/ui/iter_last_slice.rs:13:13 | ||
| | | ||
| LL | let _ = s[..2].iter().last(); | ||
| | ^^^^^^^^^^^^^^^^^^^^ help: try calling: `s.get(1)` | ||
|
|
||
| error: using `.iter().last()` on a slice without end index | ||
| --> tests/ui/iter_last_slice.rs:16:13 | ||
| | | ||
| LL | let _ = s[..=2].iter().last(); | ||
| | ^^^^^^^^^^^^^^^^^^^^^ help: try calling: `s.get(2)` | ||
|
|
||
| error: using `.iter().last()` on a slice without end index | ||
| --> tests/ui/iter_last_slice.rs:20:13 | ||
| | | ||
| LL | let _ = v[..5].iter().last(); | ||
| | ^^^^^^^^^^^^^^^^^^^^ help: try calling: `v.get(4)` | ||
|
|
||
| error: using `.iter().last()` on a slice without end index | ||
| --> tests/ui/iter_last_slice.rs:23:13 | ||
| | | ||
| LL | let _ = v[..=5].iter().last(); | ||
| | ^^^^^^^^^^^^^^^^^^^^^ help: try calling: `v.get(5)` | ||
|
|
||
| error: using `.iter().last()` on an array | ||
| --> tests/ui/iter_last_slice.rs:27:13 | ||
| | | ||
| LL | let _ = v.iter().last(); | ||
| | ^^^^^^^^^^^^^^^ help: try calling: `v.last()` | ||
|
|
||
| error: aborting due to 6 previous errors | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.