dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #5597 - esamudera:slice_iter_next, r=flip1995 

New lint: iter_next_slice

Hello, this is a work-in-progress PR for issue: https://github.com/rust-lang/rust-clippy/issues/5572

I have implemented lint to replace `iter().next()` for `slice[index..]` and `array` with `get(index)` and `get(0)` respectively. However since I made a lot of changes, I would like to request some feedback before continuing so that I could fix mistakes.

Thank you!

---

changelog: implement `iter_next_slice` lint and test, and modify `needless_continues`, `for_loop_over_options_result` UI tests since they have `iter().next()`
This commit is contained in:
bors 2020-06-02 11:42:22 +00:00
parent bfafb8e0d6 32fde0b511
commit f760d77bdb
14 changed files with 204 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1401,6 +1401,7 @@ Released 2018-09-13
[`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
[`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
[`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop
[`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice
[`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
[`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
[`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next

3
clippy_lints/src/lib.rs
View File

@ -666,6 +666,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::INTO_ITER_ON_REF,
&methods::ITERATOR_STEP_BY_ZERO,
&methods::ITER_CLONED_COLLECT,
&methods::ITER_NEXT_SLICE,
&methods::ITER_NTH,
&methods::ITER_NTH_ZERO,
&methods::ITER_SKIP_NEXT,
@ -1310,6 +1311,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::INTO_ITER_ON_REF),
LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_NEXT_SLICE),
LintId::of(&methods::ITER_NTH),
LintId::of(&methods::ITER_NTH_ZERO),
LintId::of(&methods::ITER_SKIP_NEXT),
@ -1491,6 +1493,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::CHARS_NEXT_CMP),
LintId::of(&methods::INTO_ITER_ON_REF),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_NEXT_SLICE),
LintId::of(&methods::ITER_NTH_ZERO),
LintId::of(&methods::ITER_SKIP_NEXT),
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),

26
clippy_lints/src/loops.rs
View File

@ -27,7 +27,7 @@ use rustc_middle::middle::region;
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::BytePos;
use rustc_span::symbol::Symbol;
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase};
use std::iter::{once, Iterator};
use std::mem;
@ -2381,32 +2381,32 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
match_type(cx, ty, &paths::BTREEMAP) ||
is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) {
if method.ident.name == sym!(len) {
let span = shorten_needless_collect_span(expr);
let span = shorten_span(expr, sym!(collect));
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
"replace with",
".count()".to_string(),
"count()".to_string(),
Applicability::MachineApplicable,
);
}
if method.ident.name == sym!(is_empty) {
let span = shorten_needless_collect_span(expr);
let span = shorten_span(expr, sym!(iter));
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
"replace with",
".next().is_none()".to_string(),
"get(0).is_none()".to_string(),
Applicability::MachineApplicable,
);
}
if method.ident.name == sym!(contains) {
let contains_arg = snippet(cx, args[1].span, "??");
let span = shorten_needless_collect_span(expr);
let span = shorten_span(expr, sym!(collect));
span_lint_and_then(
cx,
NEEDLESS_COLLECT,
@ -2422,7 +2422,7 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
span,
"replace with",
format!(
".any(|{}| x == {})",
"any(|{}| x == {})",
arg, pred
),
Applicability::MachineApplicable,
@ -2435,13 +2435,13 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
}
}
fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span {
if_chain! {
if let ExprKind::MethodCall(_, _, ref args) = expr.kind;
if let ExprKind::MethodCall(_, ref span, _) = args[0].kind;
then {
return expr.span.with_lo(span.lo() - BytePos(1));
fn shorten_span(expr: &Expr<'_>, target_fn_name: Symbol) -> Span {
let mut current_expr = expr;
while let ExprKind::MethodCall(ref path, ref span, ref args) = current_expr.kind {
if path.ident.name == target_fn_name {
return expr.span.with_lo(span.lo());
}
current_expr = &args[0];
}
unreachable!()
}

84
clippy_lints/src/methods/mod.rs
View File

@ -26,7 +26,7 @@ use rustc_span::symbol::{sym, SymbolStr};
use crate::consts::{constant, Constant};
use crate::utils::usage::mutated_variables;
use crate::utils::{
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, in_macro, is_copy,
is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment,
match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths,
remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability,
@ -1242,6 +1242,32 @@ declare_clippy_lint! {
"using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`"
}
declare_clippy_lint! {
/// **What it does:** Checks for usage of `iter().next()` on a Slice or an Array
///
/// **Why is this bad?** These can be shortened into `.get()`
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # let a = [1, 2, 3];
/// # let b = vec![1, 2, 3];
/// a[2..].iter().next();
/// b.iter().next();
/// ```
/// should be written as:
/// ```rust
/// # let a = [1, 2, 3];
/// # let b = vec![1, 2, 3];
/// a.get(2);
/// b.get(0);
/// ```
pub ITER_NEXT_SLICE,
style,
"using `.iter().next()` on a sliced array, which can be shortened to just `.get()`"
}
declare_lint_pass!(Methods => [
UNWRAP_USED,
EXPECT_USED,
@ -1273,6 +1299,7 @@ declare_lint_pass!(Methods => [
FIND_MAP,
MAP_FLATTEN,
ITERATOR_STEP_BY_ZERO,
ITER_NEXT_SLICE,
ITER_NTH,
ITER_NTH_ZERO,
ITER_SKIP_NEXT,
@ -1320,6 +1347,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
},
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]),
@ -2199,6 +2227,60 @@ fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, args
}
}
fn lint_iter_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
let caller_expr = &iter_args[0];
// Skip lint if the `iter().next()` expression is a for loop argument,
// since it is already covered by `&loops::ITER_NEXT_LOOP`
let mut parent_expr_opt = get_parent_expr(cx, expr);
while let Some(parent_expr) = parent_expr_opt {
if higher::for_loop(parent_expr).is_some() {
return;
}
parent_expr_opt = get_parent_expr(cx, parent_expr);
}
if derefs_to_slice(cx, caller_expr, cx.tables.expr_ty(caller_expr)).is_some() {
// caller is a Slice
if_chain! {
if let hir::ExprKind::Index(ref caller_var, ref index_expr) = &caller_expr.kind;
if let Some(higher::Range { start: Some(start_expr), end: None, limits: ast::RangeLimits::HalfOpen })
= higher::range(cx, index_expr);
if let hir::ExprKind::Lit(ref start_lit) = &start_expr.kind;
if let ast::LitKind::Int(start_idx, _) = start_lit.node;
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_NEXT_SLICE,
expr.span,
"Using `.iter().next()` on a Slice without end index.",
"try calling",
format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx),
applicability,
);
}
}
} else if is_type_diagnostic_item(cx, cx.tables.expr_ty(caller_expr), sym!(vec_type))
|| matches!(&walk_ptrs_ty(cx.tables.expr_ty(caller_expr)).kind, ty::Array(_, _))
{
// caller is a Vec or an Array
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_NEXT_SLICE,
expr.span,
"Using `.iter().next()` on an array",
"try calling",
format!(
"{}.get(0)",
snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability)
),
applicability,
);
}
}
fn lint_iter_nth<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &hir::Expr<'_>,

2
clippy_lints/src/needless_continue.rs
View File

@ -424,7 +424,7 @@ fn erode_from_back(s: &str) -> String {
}
fn span_of_first_expr_in_block(block: &ast::Block) -> Option<Span> {
block.stmts.iter().next().map(|stmt| stmt.span)
block.stmts.get(0).map(|stmt| stmt.span)
}
#[cfg(test)]

7
src/lintlist/mod.rs
View File

@ -934,6 +934,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "loops",
},
Lint {
name: "iter_next_slice",
group: "style",
desc: "using `.iter().next()` on a sliced array, which can be shortened to just `.get()`",
deprecation: None,
module: "methods",
},
Lint {
name: "iter_nth",
group: "perf",

2
tests/ui/into_iter_on_ref.fixed
View File

@ -40,4 +40,6 @@ fn main() {
let _ = (&HashSet::<i32>::new()).iter(); //~ WARN equivalent to .iter()
let _ = std::path::Path::new("12/34").iter(); //~ WARN equivalent to .iter()
let _ = std::path::PathBuf::from("12/34").iter(); //~ ERROR equivalent to .iter()
let _ = (&[1, 2, 3]).iter().next(); //~ WARN equivalent to .iter()
}

2
tests/ui/into_iter_on_ref.rs
View File

@ -40,4 +40,6 @@ fn main() {
let _ = (&HashSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter()
let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
let _ = (&[1, 2, 3]).into_iter().next(); //~ WARN equivalent to .iter()
}

8
tests/ui/into_iter_on_ref.stderr
View File

@ -156,5 +156,11 @@ error: this `.into_iter()` call is equivalent to `.iter()` and will not move the
LL | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: aborting due to 26 previous errors
error: this `.into_iter()` call is equivalent to `.iter()` and will not move the `array`
--> $DIR/into_iter_on_ref.rs:44:26
|
LL | let _ = (&[1, 2, 3]).into_iter().next(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: aborting due to 27 previous errors

24
tests/ui/iter_next_slice.fixed Normal file
View File

@ -0,0 +1,24 @@
// run-rustfix
#![warn(clippy::iter_next_slice)]
fn main() {
// test code goes here
let s = [1, 2, 3];
let v = vec![1, 2, 3];
s.get(0);
// Should be replaced by s.get(0)
s.get(2);
// Should be replaced by s.get(2)
v.get(5);
// Should be replaced by v.get(5)
v.get(0);
// Should be replaced by v.get(0)
let o = Some(5);
o.iter().next();
// Shouldn't be linted since this is not a Slice or an Array
}

24
tests/ui/iter_next_slice.rs Normal file
View File

@ -0,0 +1,24 @@
// run-rustfix
#![warn(clippy::iter_next_slice)]
fn main() {
// test code goes here
let s = [1, 2, 3];
let v = vec![1, 2, 3];
s.iter().next();
// Should be replaced by s.get(0)
s[2..].iter().next();
// Should be replaced by s.get(2)
v[5..].iter().next();
// Should be replaced by v.get(5)
v.iter().next();
// Should be replaced by v.get(0)
let o = Some(5);
o.iter().next();
// Shouldn't be linted since this is not a Slice or an Array
}

28
tests/ui/iter_next_slice.stderr Normal file
View File

@ -0,0 +1,28 @@
error: Using `.iter().next()` on an array
--> $DIR/iter_next_slice.rs:9:5
|
LL | s.iter().next();
| ^^^^^^^^^^^^^^^ help: try calling: `s.get(0)`
|
= note: `-D clippy::iter-next-slice` implied by `-D warnings`
error: Using `.iter().next()` on a Slice without end index.
--> $DIR/iter_next_slice.rs:12:5
|
LL | s[2..].iter().next();
| ^^^^^^^^^^^^^^^^^^^^ help: try calling: `s.get(2)`
error: Using `.iter().next()` on a Slice without end index.
--> $DIR/iter_next_slice.rs:15:5
|
LL | v[5..].iter().next();
| ^^^^^^^^^^^^^^^^^^^^ help: try calling: `v.get(5)`
error: Using `.iter().next()` on an array
--> $DIR/iter_next_slice.rs:18:5
|
LL | v.iter().next();
| ^^^^^^^^^^^^^^^ help: try calling: `v.get(0)`
error: aborting due to 4 previous errors

2
tests/ui/needless_collect.fixed
View File

@ -9,7 +9,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
fn main() {
let sample = [1; 5];
let len = sample.iter().count();
if sample.iter().next().is_none() {
if sample.get(0).is_none() {
// Empty
}
sample.iter().cloned().any(|x| x == 1);

16
tests/ui/needless_collect.stderr
View File

@ -1,28 +1,28 @@
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:11:28
--> $DIR/needless_collect.rs:11:29
|
LL | let len = sample.iter().collect::<Vec<_>>().len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`
|
= note: `-D clippy::needless-collect` implied by `-D warnings`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:12:21
--> $DIR/needless_collect.rs:12:15
|
LL | if sample.iter().collect::<Vec<_>>().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get(0).is_none()`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:15:27
--> $DIR/needless_collect.rs:15:28
|
LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|x| x == 1)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`
error: avoid using `collect()` when not needed
--> $DIR/needless_collect.rs:16:34
--> $DIR/needless_collect.rs:16:35
|
LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`
error: aborting due to 4 previous errors