Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-26 22:50:56 +00:00
Code Issues Projects Releases Packages Wiki Activity

Update iterator_step_by_zero

Browse source 
Move `iterator_step_by_zero` into `methods` since it applies to all
iterators and not just ranges. Simplify the code while doing so.
This commit is contained in:
Michael Wright 2019-12-18 18:59:43 +02:00
parent c62396dbf4
commit e097fca4df
7 changed files with 119 additions and 104 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
clippy_lints/src/lib.rs
View file

@ -606,6 +606,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&methods::GET_UNWRAP,
&methods::INEFFICIENT_TO_STRING,
&methods::INTO_ITER_ON_REF,
&methods::ITERATOR_STEP_BY_ZERO,
&methods::ITER_CLONED_COLLECT,
&methods::ITER_NTH,
&methods::ITER_SKIP_NEXT,
@ -699,7 +700,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&ptr::PTR_ARG,
&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST,
&question_mark::QUESTION_MARK,
&ranges::ITERATOR_STEP_BY_ZERO,
&ranges::RANGE_MINUS_ONE,
&ranges::RANGE_PLUS_ONE,
&ranges::RANGE_ZIP_WITH_LEN,
@ -1179,6 +1179,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&methods::FLAT_MAP_IDENTITY),
LintId::of(&methods::INEFFICIENT_TO_STRING),
LintId::of(&methods::INTO_ITER_ON_REF),
LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_NTH),
LintId::of(&methods::ITER_SKIP_NEXT),
@ -1244,7 +1245,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&ptr::PTR_ARG),
LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(&question_mark::QUESTION_MARK),
LintId::of(&ranges::ITERATOR_STEP_BY_ZERO),
LintId::of(&ranges::RANGE_MINUS_ONE),
LintId::of(&ranges::RANGE_PLUS_ONE),
LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
@ -1521,6 +1521,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
LintId::of(&methods::CLONE_DOUBLE_REF),
LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
LintId::of(&methods::TEMPORARY_CSTRING_AS_PTR),
LintId::of(&methods::UNINIT_ASSUMED_INIT),
LintId::of(&methods::ZST_OFFSET),
@ -1533,7 +1534,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(&ptr::MUT_FROM_REF),
LintId::of(&ranges::ITERATOR_STEP_BY_ZERO),
LintId::of(&regex::INVALID_REGEX),
LintId::of(&serde_api::SERDE_API_MISUSE),
LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),

36
clippy_lints/src/methods/mod.rs
View file

@ -737,6 +737,26 @@ declare_clippy_lint! {
"getting the inner pointer of a temporary `CString`"
}
declare_clippy_lint! {
/// **What it does:** Checks for calling `.step_by(0)` on iterators,
/// which never terminates.
///
/// **Why is this bad?** This very much looks like an oversight, since with
/// `loop { .. }` there is an obvious better way to endlessly loop.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// for x in (5..5).step_by(0) {
/// ..
/// }
/// ```
pub ITERATOR_STEP_BY_ZERO,
correctness,
"using `Iterator::step_by(0)`, which produces an infinite iterator"
}
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.
@ -1115,6 +1135,7 @@ declare_lint_pass!(Methods => [
FLAT_MAP_IDENTITY,
FIND_MAP,
MAP_FLATTEN,
ITERATOR_STEP_BY_ZERO,
ITER_NTH,
ITER_SKIP_NEXT,
GET_UNWRAP,
@ -1173,6 +1194,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
},
["nth", "iter"] => lint_iter_nth(cx, expr, arg_lists[1], false),
["nth", "iter_mut"] => lint_iter_nth(cx, expr, arg_lists[1], true),
["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]),
["as_ref"] => lint_asref(cx, expr, "as_ref", arg_lists[0]),
@ -1950,6 +1972,20 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args:
}
}
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,
ITERATOR_STEP_BY_ZERO,
expr.span,
"Iterator::step_by(0) will panic at runtime",
);
}
}
}
fn lint_iter_nth<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Expr, iter_args: &'tcx [hir::Expr], is_mut: bool) {
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() {

47
clippy_lints/src/ranges.rs
View file

@ -8,28 +8,8 @@ use syntax::ast::RangeLimits;
use syntax::source_map::Spanned;
use crate::utils::sugg::Sugg;
use crate::utils::{get_trait_def_id, higher, implements_trait, SpanlessEq};
use crate::utils::{is_integer_const, paths, snippet, snippet_opt, span_lint, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for calling `.step_by(0)` on iterators,
/// which never terminates.
///
/// **Why is this bad?** This very much looks like an oversight, since with
/// `loop { .. }` there is an obvious better way to endlessly loop.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// for x in (5..5).step_by(0) {
/// ..
/// }
/// ```
pub ITERATOR_STEP_BY_ZERO,
correctness,
"using `Iterator::step_by(0)`, which produces an infinite iterator"
}
use crate::utils::{higher, SpanlessEq};
use crate::utils::{is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then};
declare_clippy_lint! {
/// **What it does:** Checks for zipping a collection with the range of
@ -102,7 +82,6 @@ declare_clippy_lint! {
}
declare_lint_pass!(Ranges => [
ITERATOR_STEP_BY_ZERO,
RANGE_ZIP_WITH_LEN,
RANGE_PLUS_ONE,
RANGE_MINUS_ONE
@ -112,19 +91,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind {
let name = path.ident.as_str();
// Range with step_by(0).
if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
use crate::consts::{constant, Constant};
if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &args[1]) {
span_lint(
cx,
ITERATOR_STEP_BY_ZERO,
expr.span,
"Iterator::step_by(0) will panic at runtime",
);
}
} else if name == "zip" && args.len() == 2 {
if name == "zip" && args.len() == 2 {
let iter = &args[0].kind;
let zip_arg = &args[1];
if_chain! {
@ -232,14 +199,6 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
}
}
fn has_step_by(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
// No need for `walk_ptrs_ty` here because `step_by` moves `self`, so it
// can't be called on a borrowed range.
let ty = cx.tables.expr_ty_adjusted(expr);
get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
}
fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
match expr.kind {
ExprKind::Binary(

28
tests/ui/iterator_step_by_zero.rs Normal file
View file

@ -0,0 +1,28 @@
#[warn(clippy::iterator_step_by_zero)]
fn main() {
let _ = vec!["A", "B", "B"].iter().step_by(0);
let _ = "XXX".chars().step_by(0);
let _ = (0..1).step_by(0);
// No error, not an iterator.
let y = NotIterator;
y.step_by(0);
// No warning for non-zero step
let _ = (0..1).step_by(1);
let _ = (1..).step_by(0);
let _ = (1..=2).step_by(0);
let x = 0..1;
let _ = x.step_by(0);
// check const eval
let v1 = vec![1, 2, 3];
let _ = v1.iter().step_by(2 / 3);
}
struct NotIterator;
impl NotIterator {
fn step_by(&self, _: u32) {}
}

46
tests/ui/iterator_step_by_zero.stderr Normal file
View file

@ -0,0 +1,46 @@
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:3:13
|
LL | let _ = vec!["A", "B", "B"].iter().step_by(0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::iterator-step-by-zero` implied by `-D warnings`
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:4:13
|
LL | let _ = "XXX".chars().step_by(0);
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:5:13
|
LL | let _ = (0..1).step_by(0);
| ^^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:14:13
|
LL | let _ = (1..).step_by(0);
| ^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:15:13
|
LL | let _ = (1..=2).step_by(0);
| ^^^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:18:13
|
LL | let _ = x.step_by(0);
| ^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/iterator_step_by_zero.rs:22:13
|
LL | let _ = v1.iter().step_by(2 / 3);
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 7 previous errors

24
tests/ui/range.rs
View file

@ -1,31 +1,9 @@
struct NotARange;
impl NotARange {
fn step_by(&self, _: u32) {}
}
#[warn(clippy::iterator_step_by_zero, clippy::range_zip_with_len)]
#[warn(clippy::range_zip_with_len)]
fn main() {
let _ = (0..1).step_by(0);
// No warning for non-zero step
let _ = (0..1).step_by(1);
let _ = (1..).step_by(0);
let _ = (1..=2).step_by(0);
let x = 0..1;
let _ = x.step_by(0);
// No error, not a range.
let y = NotARange;
y.step_by(0);
let v1 = vec![1, 2, 3];
let v2 = vec![4, 5];
let _x = v1.iter().zip(0..v1.len());
let _y = v1.iter().zip(0..v2.len()); // No error
// check const eval
let _ = v1.iter().step_by(2 / 3);
}
#[allow(unused)]

36
tests/ui/range.stderr
View file

@ -1,42 +1,10 @@
error: Iterator::step_by(0) will panic at runtime
--> $DIR/range.rs:8:13
|
LL | let _ = (0..1).step_by(0);
| ^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::iterator-step-by-zero` implied by `-D warnings`
error: Iterator::step_by(0) will panic at runtime
--> $DIR/range.rs:12:13
|
LL | let _ = (1..).step_by(0);
| ^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/range.rs:13:13
|
LL | let _ = (1..=2).step_by(0);
| ^^^^^^^^^^^^^^^^^^
error: Iterator::step_by(0) will panic at runtime
--> $DIR/range.rs:16:13
|
LL | let _ = x.step_by(0);
| ^^^^^^^^^^^^
error: It is more idiomatic to use v1.iter().enumerate()
--> $DIR/range.rs:24:14
--> $DIR/range.rs:5:14
|
LL | let _x = v1.iter().zip(0..v1.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::range-zip-with-len` implied by `-D warnings`
error: Iterator::step_by(0) will panic at runtime
--> $DIR/range.rs:28:13
|
LL | let _ = v1.iter().step_by(2 / 3);
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 6 previous errors
error: aborting due to previous error