diff --git a/CHANGELOG.md b/CHANGELOG.md index 50a7f44ad8e6..f64da95fc7b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/README.md b/README.md index 01fc20f0f27d..215ad6f56301 100644 --- a/README.md +++ b/README.md @@ -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 342 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 343 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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 49101695a58b..bbcf0a5a63d6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -617,6 +617,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, @@ -1195,6 +1196,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), @@ -1372,6 +1374,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), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 585c55601070..13878e817787 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -20,6 +20,7 @@ use syntax::ast; use syntax::source_map::Span; use syntax::symbol::{sym, Symbol, 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, @@ -756,6 +757,32 @@ 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,ignore + /// // 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. @@ -1136,6 +1163,7 @@ declare_lint_pass!(Methods => [ MAP_FLATTEN, ITERATOR_STEP_BY_ZERO, ITER_NTH, + ITER_NTH_ZERO, ITER_SKIP_NEXT, GET_UNWRAP, STRING_EXTEND_CHARS, @@ -1191,8 +1219,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]), @@ -1983,7 +2012,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, @@ -1998,9 +2026,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" @@ -2009,6 +2038,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 }; @@ -2023,6 +2054,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<'_>, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 08cbff404442..97a13a9bbf5a 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -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; 342] = [ +pub const ALL_LINTS: [Lint; 343] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -875,6 +875,13 @@ pub const ALL_LINTS: [Lint; 342] = [ deprecation: None, module: "methods", }, + Lint { + 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", diff --git a/tests/ui/iter_nth_zero.fixed b/tests/ui/iter_nth_zero.fixed new file mode 100644 index 000000000000..b54147c94d19 --- /dev/null +++ b/tests/ui/iter_nth_zero.fixed @@ -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(); +} diff --git a/tests/ui/iter_nth_zero.rs b/tests/ui/iter_nth_zero.rs new file mode 100644 index 000000000000..b92c7d18adb4 --- /dev/null +++ b/tests/ui/iter_nth_zero.rs @@ -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().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(); +} diff --git a/tests/ui/iter_nth_zero.stderr b/tests/ui/iter_nth_zero.stderr new file mode 100644 index 000000000000..2b20a4ceb4ab --- /dev/null +++ b/tests/ui/iter_nth_zero.stderr @@ -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 +