Skip to content

Commit a5bad06

Browse files
esamuderaesamudera
committed
WIP - slice iter next
1 parent ce86f90 commit a5bad06

File tree

10 files changed

+176
-23
lines changed

10 files changed

+176
-23
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1554,6 +1554,7 @@ Released 2018-09-13
15541554
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
15551555
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
15561556
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
1557+
[`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice
15571558
[`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
15581559
[`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
15591560
[`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
669669
&methods::INTO_ITER_ON_REF,
670670
&methods::ITERATOR_STEP_BY_ZERO,
671671
&methods::ITER_CLONED_COLLECT,
672+
&methods::ITER_NEXT_SLICE,
672673
&methods::ITER_NTH,
673674
&methods::ITER_NTH_ZERO,
674675
&methods::ITER_SKIP_NEXT,
@@ -1317,6 +1318,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13171318
LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
13181319
LintId::of(&methods::SINGLE_CHAR_PATTERN),
13191320
LintId::of(&methods::SKIP_WHILE_NEXT),
1321+
LintId::of(&methods::ITER_NEXT_SLICE),
13201322
LintId::of(&methods::STRING_EXTEND_CHARS),
13211323
LintId::of(&methods::SUSPICIOUS_MAP),
13221324
LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
@@ -1491,6 +1493,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14911493
LintId::of(&methods::OPTION_MAP_OR_NONE),
14921494
LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION),
14931495
LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
1496+
LintId::of(&methods::ITER_NEXT_SLICE),
14941497
LintId::of(&methods::STRING_EXTEND_CHARS),
14951498
LintId::of(&methods::UNNECESSARY_FOLD),
14961499
LintId::of(&methods::WRONG_SELF_CONVENTION),

‎clippy_lints/src/loops.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use rustc_middle::middle::region;
2727
use rustc_middle::ty::{self, Ty};
2828
use rustc_session::{declare_lint_pass, declare_tool_lint};
2929
use rustc_span::source_map::Span;
30+
use rustc_span::symbol::Symbol;
3031
use rustc_span::BytePos;
3132
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, Place, PlaceBase};
3233
use std::iter::{once, Iterator};
@@ -2381,32 +2382,32 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
23812382
match_type(cx, ty, &paths::BTREEMAP) ||
23822383
is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) {
23832384
if method.ident.name == sym!(len) {
2384-
let span = shorten_needless_collect_span(expr);
2385+
let span = shorten_span(expr, sym!(collect));
23852386
span_lint_and_sugg(
23862387
cx,
23872388
NEEDLESS_COLLECT,
23882389
span,
23892390
NEEDLESS_COLLECT_MSG,
23902391
"replace with",
2391-
".count()".to_string(),
2392+
"count()".to_string(),
23922393
Applicability::MachineApplicable,
23932394
);
23942395
}
23952396
if method.ident.name == sym!(is_empty) {
2396-
let span = shorten_needless_collect_span(expr);
2397+
let span = shorten_span(expr, sym!(iter));
23972398
span_lint_and_sugg(
23982399
cx,
23992400
NEEDLESS_COLLECT,
24002401
span,
24012402
NEEDLESS_COLLECT_MSG,
24022403
"replace with",
2403-
".next().is_none()".to_string(),
2404+
"get(0).is_none()".to_string(),
24042405
Applicability::MachineApplicable,
24052406
);
24062407
}
24072408
if method.ident.name == sym!(contains) {
24082409
let contains_arg = snippet(cx, args[1].span, "??");
2409-
let span = shorten_needless_collect_span(expr);
2410+
let span = shorten_span(expr, sym!(collect));
24102411
span_lint_and_then(
24112412
cx,
24122413
NEEDLESS_COLLECT,
@@ -2422,7 +2423,7 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
24222423
span,
24232424
"replace with",
24242425
format!(
2425-
".any(|{}| x == {})",
2426+
"any(|{}| x == {})",
24262427
arg, pred
24272428
),
24282429
Applicability::MachineApplicable,
@@ -2435,13 +2436,13 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'a, '
24352436
}
24362437
}
24372438

2438-
fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span {
2439-
if_chain! {
2440-
if let ExprKind::MethodCall(_, _, ref args) = expr.kind;
2441-
if let ExprKind::MethodCall(_, ref span, _) = args[0].kind;
2442-
then {
2443-
return expr.span.with_lo(span.lo() - BytePos(1));
2439+
fn shorten_span(expr: &Expr<'_>, target_fn_name: Symbol) -> Span {
2440+
let mut current_expr = expr;
2441+
while let ExprKind::MethodCall(ref path, ref span, ref args) = current_expr.kind {
2442+
if path.ident.name == target_fn_name {
2443+
return expr.span.with_lo(span.lo());
24442444
}
2445+
current_expr = &args[0];
24452446
}
24462447
unreachable!()
24472448
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use rustc_span::symbol::{sym, SymbolStr};
2626
use crate::consts::{constant, Constant};
2727
use crate::utils::usage::mutated_variables;
2828
use crate::utils::{
29-
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy,
29+
get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, in_macro, is_copy,
3030
is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment,
3131
match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, method_chain_args, paths,
3232
remove_blocks, return_ty, same_tys, single_segment_path, snippet, snippet_with_applicability,
@@ -1242,6 +1242,26 @@ declare_clippy_lint! {
12421242
"using `as_ref().map(Deref::deref)`, which is more succinctly expressed as `as_deref()`"
12431243
}
12441244

1245+
declare_clippy_lint! {
1246+
/// **What it does:** TODO
1247+
///
1248+
/// **Why is this bad?** TODO
1249+
///
1250+
/// **Known problems:** None.
1251+
///
1252+
/// **Example:**
1253+
/// ```rust
1254+
/// TODO
1255+
/// ```
1256+
/// The correct use would be:
1257+
/// ```rust
1258+
/// TODO
1259+
/// ```
1260+
pub ITER_NEXT_SLICE,
1261+
style,
1262+
"using `.iter().next()` on a sliced array, which can be shortened to just `.get()`"
1263+
}
1264+
12451265
declare_lint_pass!(Methods => [
12461266
UNWRAP_USED,
12471267
EXPECT_USED,
@@ -1273,6 +1293,7 @@ declare_lint_pass!(Methods => [
12731293
FIND_MAP,
12741294
MAP_FLATTEN,
12751295
ITERATOR_STEP_BY_ZERO,
1296+
ITER_NEXT_SLICE,
12761297
ITER_NTH,
12771298
ITER_NTH_ZERO,
12781299
ITER_SKIP_NEXT,
@@ -1320,6 +1341,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
13201341
},
13211342
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
13221343
["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
1344+
["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
13231345
["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]),
13241346
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
13251347
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]),
@@ -2184,6 +2206,58 @@ fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, args
21842206
}
21852207
}
21862208

2209+
fn lint_iter_next<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
2210+
let caller_expr = &iter_args[0];
2211+
2212+
let mut parent_expr_opt = get_parent_expr(cx, expr);
2213+
while let Some(parent_expr) = parent_expr_opt {
2214+
if higher::for_loop(parent_expr).is_some() {
2215+
return;
2216+
}
2217+
parent_expr_opt = get_parent_expr(cx, parent_expr);
2218+
}
2219+
2220+
if derefs_to_slice(cx, caller_expr, cx.tables.expr_ty(caller_expr)).is_some() {
2221+
// caller is a Slice
2222+
if_chain! {
2223+
if let hir::ExprKind::Index(ref caller_var, ref index_expr) = &caller_expr.kind;
2224+
if let Some(higher::Range { start: Some(start_expr), end: None, limits: ast::RangeLimits::HalfOpen })
2225+
= higher::range(cx, index_expr);
2226+
if let hir::ExprKind::Lit(ref start_lit) = &start_expr.kind;
2227+
if let ast::LitKind::Int(start_idx, _) = start_lit.node;
2228+
then {
2229+
let mut applicability = Applicability::MachineApplicable;
2230+
span_lint_and_sugg(
2231+
cx,
2232+
ITER_NEXT_SLICE,
2233+
expr.span,
2234+
"Using `.iter().next()` on a Slice without end index.",
2235+
"try calling",
2236+
format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx),
2237+
applicability,
2238+
);
2239+
}
2240+
}
2241+
} else if is_type_diagnostic_item(cx, cx.tables.expr_ty(caller_expr), sym!(vec_type))
2242+
|| matches!(&walk_ptrs_ty(cx.tables.expr_ty(caller_expr)).kind, ty::Array(_, _))
2243+
{
2244+
// caller is a Vec or an Array
2245+
let mut applicability = Applicability::MachineApplicable;
2246+
span_lint_and_sugg(
2247+
cx,
2248+
ITER_NEXT_SLICE,
2249+
expr.span,
2250+
"Using `.iter().next()` on an array",
2251+
"try calling",
2252+
format!(
2253+
"{}.get(0)",
2254+
snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability)
2255+
),
2256+
applicability,
2257+
);
2258+
}
2259+
}
2260+
21872261
fn lint_iter_nth<'a, 'tcx>(
21882262
cx: &LateContext<'a, 'tcx>,
21892263
expr: &hir::Expr<'_>,

‎clippy_lints/src/needless_continue.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ fn erode_from_back(s: &str) -> String {
424424
}
425425

426426
fn span_of_first_expr_in_block(block: &ast::Block) -> Option<Span> {
427-
block.stmts.iter().next().map(|stmt| stmt.span)
427+
block.stmts.get(0).map(|stmt| stmt.span)
428428
}
429429

430430
#[cfg(test)]

‎src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,6 +1977,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
19771977
deprecation: None,
19781978
module: "methods",
19791979
},
1980+
Lint {
1981+
name: "iter_next_slice",
1982+
group: "style",
1983+
desc: "default lint description",
1984+
deprecation: None,
1985+
module: "methods",
1986+
},
19801987
Lint {
19811988
name: "slow_vector_initialization",
19821989
group: "perf",

‎tests/ui/iter_next_slice.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#![warn(clippy::iter_next_slice)]
2+
3+
fn main() {
4+
// test code goes here
5+
let s = [1, 2, 3];
6+
let v = vec![1, 2, 3];
7+
let o = Some(5);
8+
9+
s.iter().next();
10+
// Should be replaced by s.get(0)
11+
12+
// s.into_iter().next();
13+
// Probaby can rely on the default `array_into_iter`/`clippy::into_iter_on_ref` lint
14+
15+
s[2..].iter().next();
16+
// Should be replaced by s.get(2)
17+
18+
// s[1..].into_iter().next();
19+
// Probaby can rely on the default `clippy::into_iter_on_ref` lint
20+
21+
v[5..].iter().next();
22+
// Should be replaced by v.get(5)
23+
24+
// v[1..].into_iter().next();
25+
// Probaby can rely on the default `clippy::into_iter_on_ref` lint
26+
27+
v.iter().next();
28+
// Should be replaced by v.get(0)
29+
30+
// v.into_iter().next();
31+
// Tricky one—I don't think there's any other (short) way to say this (take ownership of first value)
32+
// So this should probably not get any warnings
33+
34+
o.iter().next();
35+
// Replace with o.as_ref()
36+
37+
// o.into_iter().next();
38+
// Replace with o
39+
}

‎tests/ui/iter_next_slice.stderr

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: Using `.iter().next()` on an array
2+
--> $DIR/iter_next_slice.rs:9:5
3+
|
4+
LL | s.iter().next();
5+
| ^^^^^^^^^^^^^^^ help: try calling: `s.get(0)`
6+
|
7+
= note: `-D clippy::iter-next-slice` implied by `-D warnings`
8+
9+
error: Using `.iter().next()` on a Slice without end index.
10+
--> $DIR/iter_next_slice.rs:15:5
11+
|
12+
LL | s[2..].iter().next();
13+
| ^^^^^^^^^^^^^^^^^^^^ help: try calling: `s.get(2)`
14+
15+
error: Using `.iter().next()` on a Slice without end index.
16+
--> $DIR/iter_next_slice.rs:21:5
17+
|
18+
LL | v[5..].iter().next();
19+
| ^^^^^^^^^^^^^^^^^^^^ help: try calling: `v.get(5)`
20+
21+
error: Using `.iter().next()` on an array
22+
--> $DIR/iter_next_slice.rs:27:5
23+
|
24+
LL | v.iter().next();
25+
| ^^^^^^^^^^^^^^^ help: try calling: `v.get(0)`
26+
27+
error: aborting due to 4 previous errors
28+

‎tests/ui/needless_collect.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::collections::{BTreeSet, HashMap, HashSet};
99
fn main() {
1010
let sample = [1; 5];
1111
let len = sample.iter().count();
12-
if sample.iter().next().is_none() {
12+
if sample.get(0).is_none() {
1313
// Empty
1414
}
1515
sample.iter().cloned().any(|x| x == 1);

‎tests/ui/needless_collect.stderr

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11
error: avoid using `collect()` when not needed
2-
--> $DIR/needless_collect.rs:11:28
2+
--> $DIR/needless_collect.rs:11:29
33
|
44
LL | let len = sample.iter().collect::<Vec<_>>().len();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`
66
|
77
= note: `-D clippy::needless-collect` implied by `-D warnings`
88

99
error: avoid using `collect()` when not needed
10-
--> $DIR/needless_collect.rs:12:21
10+
--> $DIR/needless_collect.rs:12:15
1111
|
1212
LL | if sample.iter().collect::<Vec<_>>().is_empty() {
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()`
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get(0).is_none()`
1414

1515
error: avoid using `collect()` when not needed
16-
--> $DIR/needless_collect.rs:15:27
16+
--> $DIR/needless_collect.rs:15:28
1717
|
1818
LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|x| x == 1)`
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`
2020

2121
error: avoid using `collect()` when not needed
22-
--> $DIR/needless_collect.rs:16:34
22+
--> $DIR/needless_collect.rs:16:35
2323
|
2424
LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().len();
25-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()`
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`
2626

2727
error: aborting due to 4 previous errors
2828

0 commit comments

Comments
 (0)