Skip to content
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

New Lint: Iter nth zero #4966

Merged
merged 2 commits into from
Jan 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,7 @@ Released 2018-09-13
[`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_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
[`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
[`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 343 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 344 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MultipleInherentImpl {
}

fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx Crate<'_>) {
if let Some(item) = krate.items.values().nth(0) {
if let Some(item) = krate.items.values().next() {
// Retrieve all inherent implementations from the crate, grouped by type
for impls in cx
.tcx
Expand All @@ -71,7 +71,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MultipleInherentImpl {
{
// Filter out implementations that have generic params (type or lifetime)
let mut impl_spans = impls.iter().filter_map(|impl_def| self.impls.get(impl_def));
if let Some(initial_span) = impl_spans.nth(0) {
if let Some(initial_span) = impl_spans.next() {
impl_spans.for_each(|additional_span| {
span_lint_and_then(
cx,
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&methods::ITERATOR_STEP_BY_ZERO,
&methods::ITER_CLONED_COLLECT,
&methods::ITER_NTH,
&methods::ITER_NTH_ZERO,
&methods::ITER_SKIP_NEXT,
&methods::MANUAL_SATURATING_ARITHMETIC,
&methods::MAP_FLATTEN,
Expand Down Expand Up @@ -1197,6 +1198,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_NTH),
LintId::of(&methods::ITER_NTH_ZERO),
LintId::of(&methods::ITER_SKIP_NEXT),
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
LintId::of(&methods::NEW_RET_NO_SELF),
Expand Down Expand Up @@ -1375,6 +1377,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&methods::CHARS_LAST_CMP),
LintId::of(&methods::INTO_ITER_ON_REF),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_NTH_ZERO),
LintId::of(&methods::ITER_SKIP_NEXT),
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
LintId::of(&methods::NEW_RET_NO_SELF),
Expand Down
59 changes: 55 additions & 4 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use rustc_span::source_map::Span;
use rustc_span::symbol::{sym, Symbol, SymbolStr};
use syntax::ast;

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,
Expand Down Expand Up @@ -756,6 +757,33 @@ declare_clippy_lint! {
"using `Iterator::step_by(0)`, which will panic at runtime"
}

declare_clippy_lint! {
/// **What it does:** Checks for the use of `iter.nth(0)`.
///
/// **Why is this bad?** `iter.nth(0)` is unnecessary, and `iter.next()`
/// is more readable.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
bradsherman marked this conversation as resolved.
Show resolved Hide resolved
/// # use std::collections::HashSet;
/// // Bad
/// # let mut s = HashSet::new();
/// # s.insert(1);
/// let x = s.iter().nth(0);
///
/// // Good
/// # let mut s = HashSet::new();
/// # s.insert(1);
/// let x = s.iter().next();
/// ```
pub ITER_NTH_ZERO,
style,
"replace `iter.nth(0)` with `iter.next()`"
}

declare_clippy_lint! {
/// **What it does:** Checks for use of `.iter().nth()` (and the related
/// `.iter_mut().nth()`) on standard library types with O(1) element access.
Expand Down Expand Up @@ -1136,6 +1164,7 @@ declare_lint_pass!(Methods => [
MAP_FLATTEN,
ITERATOR_STEP_BY_ZERO,
ITER_NTH,
ITER_NTH_ZERO,
ITER_SKIP_NEXT,
GET_UNWRAP,
STRING_EXTEND_CHARS,
Expand Down Expand Up @@ -1191,8 +1220,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
["as_ptr", "unwrap"] | ["as_ptr", "expect"] => {
lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0])
},
["nth", "iter"] => lint_iter_nth(cx, expr, arg_lists[1], false),
["nth", "iter_mut"] => lint_iter_nth(cx, expr, arg_lists[1], true),
["nth", "iter"] => lint_iter_nth(cx, expr, &arg_lists, false),
["nth", "iter_mut"] => lint_iter_nth(cx, expr, &arg_lists, true),
["nth", ..] => lint_iter_nth_zero(cx, expr, arg_lists[0]),
["step_by", ..] => lint_step_by(cx, expr, arg_lists[0]),
["next", "skip"] => lint_iter_skip_next(cx, expr),
["collect", "cloned"] => lint_iter_cloned_collect(cx, expr, arg_lists[1]),
Expand Down Expand Up @@ -1983,7 +2013,6 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, fold_ar

fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, args: &'tcx [hir::Expr<'_>]) {
if match_trait_method(cx, expr, &paths::ITERATOR) {
use crate::consts::{constant, Constant};
if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &args[1]) {
span_lint(
cx,
Expand All @@ -1998,9 +2027,10 @@ fn lint_step_by<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, args
fn lint_iter_nth<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &hir::Expr<'_>,
iter_args: &'tcx [hir::Expr<'_>],
nth_and_iter_args: &[&'tcx [hir::Expr<'tcx>]],
is_mut: bool,
) {
let iter_args = nth_and_iter_args[1];
let mut_str = if is_mut { "_mut" } else { "" };
let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])).is_some() {
"slice"
Expand All @@ -2009,6 +2039,8 @@ fn lint_iter_nth<'a, 'tcx>(
} else if match_type(cx, cx.tables.expr_ty(&iter_args[0]), &paths::VEC_DEQUE) {
"VecDeque"
} else {
let nth_args = nth_and_iter_args[0];
lint_iter_nth_zero(cx, expr, &nth_args);
return; // caller is not a type that we want to lint
};

Expand All @@ -2023,6 +2055,25 @@ fn lint_iter_nth<'a, 'tcx>(
);
}

fn lint_iter_nth_zero<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr<'_>, nth_args: &'tcx [hir::Expr<'_>]) {
if_chain! {
if match_trait_method(cx, expr, &paths::ITERATOR);
if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &nth_args[1]);
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_NTH_ZERO,
expr.span,
"called `.nth(0)` on a `std::iter::Iterator`",
"try calling",
format!("{}.next()", snippet_with_applicability(cx, nth_args[0].span, "..", &mut applicability)),
applicability,
);
}
}
}

fn lint_get_unwrap<'a, 'tcx>(
cx: &LateContext<'a, 'tcx>,
expr: &hir::Expr<'_>,
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 343] = [
pub const ALL_LINTS: [Lint; 344] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -875,6 +875,13 @@ pub const ALL_LINTS: [Lint; 343] = [
deprecation: None,
module: "methods",
},
Lint {
bradsherman marked this conversation as resolved.
Show resolved Hide resolved
name: "iter_nth_zero",
group: "style",
desc: "replace `iter.nth(0)` with `iter.next()`",
deprecation: None,
module: "methods",
},
Lint {
name: "iter_skip_next",
group: "style",
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/iter_nth_zero.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// run-rustfix

#![warn(clippy::iter_nth_zero)]
use std::collections::HashSet;

struct Foo {}

impl Foo {
fn nth(&self, index: usize) -> usize {
index + 1
}
}

fn main() {
let f = Foo {};
f.nth(0); // lint does not apply here

let mut s = HashSet::new();
s.insert(1);
let _x = s.iter().next();

let mut s2 = HashSet::new();
s2.insert(2);
let mut iter = s2.iter();
let _y = iter.next();

let mut s3 = HashSet::new();
s3.insert(3);
let mut iter2 = s3.iter();
let _unwrapped = iter2.next().unwrap();
}
31 changes: 31 additions & 0 deletions tests/ui/iter_nth_zero.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// run-rustfix

#![warn(clippy::iter_nth_zero)]
bradsherman marked this conversation as resolved.
Show resolved Hide resolved
use std::collections::HashSet;

struct Foo {}

impl Foo {
fn nth(&self, index: usize) -> usize {
index + 1
}
}

fn main() {
let f = Foo {};
f.nth(0); // lint does not apply here

let mut s = HashSet::new();
s.insert(1);
let _x = s.iter().nth(0);

let mut s2 = HashSet::new();
s2.insert(2);
let mut iter = s2.iter();
let _y = iter.nth(0);

let mut s3 = HashSet::new();
s3.insert(3);
let mut iter2 = s3.iter();
let _unwrapped = iter2.nth(0).unwrap();
}
22 changes: 22 additions & 0 deletions tests/ui/iter_nth_zero.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: called `.nth(0)` on a `std::iter::Iterator`
--> $DIR/iter_nth_zero.rs:20:14
|
LL | let _x = s.iter().nth(0);
| ^^^^^^^^^^^^^^^ help: try calling: `s.iter().next()`
|
= note: `-D clippy::iter-nth-zero` implied by `-D warnings`

error: called `.nth(0)` on a `std::iter::Iterator`
--> $DIR/iter_nth_zero.rs:25:14
|
LL | let _y = iter.nth(0);
| ^^^^^^^^^^^ help: try calling: `iter.next()`

error: called `.nth(0)` on a `std::iter::Iterator`
--> $DIR/iter_nth_zero.rs:30:22
|
LL | let _unwrapped = iter2.nth(0).unwrap();
| ^^^^^^^^^^^^ help: try calling: `iter2.next()`

error: aborting due to 3 previous errors