squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Replace Range::step_by checking with Iterator::step_by

Browse source
This commit is contained in:
Oliver Schneider 2017-06-18 15:14:46 +02:00
parent 27727c4aff
commit 38925a55b7
7 changed files with 70 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -3,6 +3,8 @@ All notable changes to this project will be documented in this file.
## 0.0.141
* Rewrite of the `doc_markdown` lint.
* Deprecated [`range_step_by_zero`]
* New lint: [`iterator_step_by_zero`]
## 0.0.140 - 2017-06-16
* Update to *rustc 1.19.0-nightly (258ae6dd9 2017-06-15)*
@ -439,6 +441,7 @@ All notable changes to this project will be documented in this file.
[`iter_next_loop`]: https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop
[`iter_nth`]: https://github.com/Manishearth/rust-clippy/wiki#iter_nth
[`iter_skip_next`]: https://github.com/Manishearth/rust-clippy/wiki#iter_skip_next
[`iterator_step_by_zero`]: https://github.com/Manishearth/rust-clippy/wiki#iterator_step_by_zero
[`large_enum_variant`]: https://github.com/Manishearth/rust-clippy/wiki#large_enum_variant
[`len_without_is_empty`]: https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty
[`len_zero`]: https://github.com/Manishearth/rust-clippy/wiki#len_zero

2
README.md
View file

@ -260,6 +260,7 @@ name
[iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended
[iter_nth](https://github.com/Manishearth/rust-clippy/wiki#iter_nth) | warn | using `.iter().nth()` on a standard library type with O(1) element access
[iter_skip_next](https://github.com/Manishearth/rust-clippy/wiki#iter_skip_next) | warn | using `.skip(x).next()` on an iterator
[iterator_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#iterator_step_by_zero) | warn | using `Iterator::step_by(0)`, which produces an infinite iterator
[large_enum_variant](https://github.com/Manishearth/rust-clippy/wiki#large_enum_variant) | warn | large size difference between variants on an enum
[len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits or impls with a public `len` method but no corresponding `is_empty` method
[len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
@ -321,7 +322,6 @@ name
[print_with_newline](https://github.com/Manishearth/rust-clippy/wiki#print_with_newline) | warn | using `print!()` with a format string that ends in a newline
[ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg) | warn | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
[pub_enum_variant_names](https://github.com/Manishearth/rust-clippy/wiki#pub_enum_variant_names) | allow | enums where all variants share a prefix/postfix
[range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero) | warn | using `Range::step_by(0)`, which produces an infinite iterator
[range_zip_with_len](https://github.com/Manishearth/rust-clippy/wiki#range_zip_with_len) | warn | zipping iterator with a range when `enumerate()` would do
[redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure) | warn | redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)
[redundant_closure_call](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure_call) | warn | throwaway closures called in the expression they are defined

10
clippy_lints/src/deprecated_lints.rs
View file

@ -14,6 +14,16 @@ declare_deprecated_lint! {
"`.extend_from_slice(_)` is a faster way to extend a Vec by a slice"
}
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** `Range::step_by(0)` used to be linted since it's
/// an infinite iterator, which is better expressed by `iter::repeat`,
/// but the method has been removed for `Iterator::step_by` which panics
/// if given a zero
declare_deprecated_lint! {
pub RANGE_STEP_BY_ZERO,
"`iterator.step_by(0)` panics nowadays"
}
/// **What it does:** Nothing. This lint has been deprecated.
///

6
clippy_lints/src/lib.rs
View file

@ -194,6 +194,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
"extend_from_slice",
"`.extend_from_slice(_)` is a faster way to extend a Vec by a slice",
);
store.register_removed(
"range_step_by_zero",
"`iterator.step_by(0)` panics nowadays",
);
store.register_removed(
"unstable_as_slice",
"`Vec::as_slice` has been stabilized in 1.7",
@ -490,7 +494,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
ptr::CMP_NULL,
ptr::MUT_FROM_REF,
ptr::PTR_ARG,
ranges::RANGE_STEP_BY_ZERO,
ranges::ITERATOR_STEP_BY_ZERO,
ranges::RANGE_ZIP_WITH_LEN,
reference::DEREF_ADDROF,
regex::INVALID_REGEX,

40
clippy_lints/src/ranges.rs
View file

@ -1,10 +1,10 @@
use rustc::lint::*;
use rustc::hir::*;
use syntax::codemap::Spanned;
use utils::{is_integer_literal, match_type, paths, snippet, span_lint};
use utils::higher;
use utils::{is_integer_literal, paths, snippet, span_lint};
use utils::{higher, implements_trait, get_trait_def_id};
/// **What it does:** Checks for iterating over ranges with a `.step_by(0)`,
/// **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
@ -17,10 +17,11 @@ use utils::higher;
/// for x in (5..5).step_by(0) { .. }
/// ```
declare_lint! {
pub RANGE_STEP_BY_ZERO,
pub ITERATOR_STEP_BY_ZERO,
Warn,
"using `Range::step_by(0)`, which produces an infinite iterator"
"using `Iterator::step_by(0)`, which produces an infinite iterator"
}
/// **What it does:** Checks for zipping a collection with the range of `0.._.len()`.
///
/// **Why is this bad?** The code is better expressed with `.enumerate()`.
@ -42,7 +43,7 @@ pub struct StepByZero;
impl LintPass for StepByZero {
fn get_lints(&self) -> LintArray {
lint_array!(RANGE_STEP_BY_ZERO, RANGE_ZIP_WITH_LEN)
lint_array!(ITERATOR_STEP_BY_ZERO, RANGE_ZIP_WITH_LEN)
}
}
@ -52,12 +53,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StepByZero {
let name = name.as_str();
// Range with step_by(0).
if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) && is_integer_literal(&args[1], 0) {
span_lint(cx,
RANGE_STEP_BY_ZERO,
if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
use consts::{Constant, constant};
use rustc_const_math::ConstInt::Usize;
if let Some((Constant::Int(Usize(us)), _)) = constant(cx, &args[1]) {
if us.as_u64(cx.sess().target.uint_type) == 0 {
span_lint(
cx,
ITERATOR_STEP_BY_ZERO,
expr.span,
"Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \
instead");
"Iterator::step_by(0) will panic at runtime",
);
}
}
} else if name == "zip" && args.len() == 2 {
let iter = &args[0].node;
let zip_arg = &args[1];
@ -90,9 +98,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StepByZero {
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(expr);
let ty = cx.tables.expr_ty_adjusted(expr);
// Note: `RangeTo`, `RangeToInclusive` and `RangeFull` don't have step_by
match_type(cx, ty, &paths::RANGE) || match_type(cx, ty, &paths::RANGE_FROM) ||
match_type(cx, ty, &paths::RANGE_INCLUSIVE)
get_trait_def_id(cx, &paths::ITERATOR)
.map_or(
false,
|iterator_trait| implements_trait(cx, ty, iterator_trait, &[])
)
}

6
clippy_tests/examples/range.rs
View file

@ -1,3 +1,4 @@
#![feature(iterator_step_by)]
#![feature(step_by)]
#![feature(inclusive_range_syntax)]
#![feature(plugin)]
@ -8,7 +9,7 @@ impl NotARange {
fn step_by(&self, _: u32) {}
}
#[warn(range_step_by_zero, range_zip_with_len)]
#[warn(iterator_step_by_zero, range_zip_with_len)]
fn main() {
(0..1).step_by(0);
// No warning for non-zero step
@ -28,4 +29,7 @@ fn main() {
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);
}

32
clippy_tests/examples/range.stderr
View file

@ -1,51 +1,59 @@
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:13:12
--> range.rs:14:12
|
13 | (0..1).step_by(0);
14 | (0..1).step_by(0);
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:15:12
--> range.rs:16:12
|
15 | (0..1).step_by(1);
16 | (0..1).step_by(1);
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:17:11
--> range.rs:18:11
|
17 | (1..).step_by(0);
18 | (1..).step_by(0);
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:18:13
--> range.rs:19:13
|
18 | (1...2).step_by(0);
19 | (1...2).step_by(0);
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`
error: use of deprecated item: replaced by `Iterator::step_by`
--> range.rs:21:7
--> range.rs:22:7
|
21 | x.step_by(0);
22 | x.step_by(0);
| ^^^^^^^
|
= note: `-D deprecated` implied by `-D warnings`
error: It is more idiomatic to use v1.iter().enumerate()
--> range.rs:29:14
--> range.rs:30:14
|
29 | let _x = v1.iter().zip(0..v1.len());
30 | let _x = v1.iter().zip(0..v1.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D range-zip-with-len` implied by `-D warnings`
error: Iterator::step_by(0) will panic at runtime
--> range.rs:34:13
|
34 | let _ = v1.iter().step_by(2/3);
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D iterator-step-by-zero` implied by `-D warnings`
error: aborting due to previous error(s)
error: Could not compile `clippy_tests`.