Make iter_nth work for Vecs too

This commit is contained in:
Devon Hollowood 2016-06-16 02:02:00 -07:00
parent 7764dc5ef4
commit 74025be59d
5 changed files with 46 additions and 19 deletions

View File

@ -172,6 +172,7 @@ All notable changes to this project will be documented in this file.
[`invalid_upcast_comparisons`]: https://github.com/Manishearth/rust-clippy/wiki#invalid_upcast_comparisons
[`items_after_statements`]: https://github.com/Manishearth/rust-clippy/wiki#items_after_statements
[`iter_next_loop`]: https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop
[`iter_nth`]: https://github.com/Manishearth/rust-clippy/wiki#iter_nth
[`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
[`let_and_return`]: https://github.com/Manishearth/rust-clippy/wiki#let_and_return
@ -234,7 +235,6 @@ All notable changes to this project will be documented in this file.
[`single_char_pattern`]: https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern
[`single_match`]: https://github.com/Manishearth/rust-clippy/wiki#single_match
[`single_match_else`]: https://github.com/Manishearth/rust-clippy/wiki#single_match_else
[`slice_iter_nth`]: https://github.com/Manishearth/rust-clippy/wiki#slice_iter_nth
[`str_to_string`]: https://github.com/Manishearth/rust-clippy/wiki#str_to_string
[`string_add`]: https://github.com/Manishearth/rust-clippy/wiki#string_add
[`string_add_assign`]: https://github.com/Manishearth/rust-clippy/wiki#string_add_assign

View File

@ -78,6 +78,7 @@ name
[invalid_upcast_comparisons](https://github.com/Manishearth/rust-clippy/wiki#invalid_upcast_comparisons) | allow | a comparison involving an upcast which is always true or false
[items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | allow | finds blocks where an item comes after a statement
[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 slice or Vec
[len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()`
[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
[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
@ -140,7 +141,6 @@ name
[single_char_pattern](https://github.com/Manishearth/rust-clippy/wiki#single_char_pattern) | warn | using a single-character str where a char could be used, e.g. `_.split("x")`
[single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead
[single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else) | allow | a match statement with a two arms where the second arm's pattern is a wildcard; recommends `if let` instead
[slice_iter_nth](https://github.com/Manishearth/rust-clippy/wiki#slice_iter_nth) | warn | using `.iter().nth()` on a slice
[string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
[string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
[string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead

View File

@ -349,6 +349,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
methods::CLONE_ON_COPY,
methods::EXTEND_FROM_SLICE,
methods::FILTER_NEXT,
methods::ITER_NTH,
methods::NEW_RET_NO_SELF,
methods::OK_EXPECT,
methods::OPTION_MAP_UNWRAP_OR,
@ -357,7 +358,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
methods::SEARCH_IS_SOME,
methods::SHOULD_IMPLEMENT_TRAIT,
methods::SINGLE_CHAR_PATTERN,
methods::SLICE_ITER_NTH,
methods::TEMPORARY_CSTRING_AS_PTR,
methods::WRONG_SELF_CONVENTION,
minmax::MIN_MAX,

View File

@ -312,7 +312,7 @@ declare_lint! {
"getting the inner pointer of a temporary `CString`"
}
/// **What it does:** This lint checks for use of `.iter().nth()` on a slice.
/// **What it does:** This lint checks for use of `.iter().nth()` on a slice or Vec.
///
/// **Why is this bad?** `.get()` is more efficient and more readable.
///
@ -320,18 +320,20 @@ declare_lint! {
///
/// **Example:**
/// ```rust
/// let some_slice = &[0, 1, 2, 3][..];
/// let third_elem = some_slice.iter().nth(3);
/// let some_vec = vec![0, 1, 2, 3];
/// let bad_vec = some_vec.iter().nth(3);
/// let bad_slice = &some_vec[..].iter().nth(3);
/// ```
/// The correct use would be:
/// ```rust
/// let some_slice = &[0, 1, 2, 3][..];
/// let third_elem = some_slice.get(3);
/// let some_vec = vec![0, 1, 2, 3];
/// let bad_vec = some_vec.get(3);
/// let bad_slice = &some_vec[..].get(3);
/// ```
declare_lint! {
pub SLICE_ITER_NTH,
pub ITER_NTH,
Warn,
"using `.iter().nth()` on a slice"
"using `.iter().nth()` on a slice or Vec"
}
impl LintPass for MethodsPass {
@ -353,7 +355,7 @@ impl LintPass for MethodsPass {
SINGLE_CHAR_PATTERN,
SEARCH_IS_SOME,
TEMPORARY_CSTRING_AS_PTR,
SLICE_ITER_NTH)
ITER_NTH)
}
}
@ -387,7 +389,7 @@ impl LateLintPass for MethodsPass {
} else if let Some(arglists) = method_chain_args(expr, &["unwrap", "as_ptr"]) {
lint_cstring_as_ptr(cx, expr, &arglists[0][0], &arglists[1][0]);
} else if let Some(arglists) = method_chain_args(expr, &["iter", "nth"]) {
lint_slice_iter_nth(cx, expr, arglists[0]);
lint_iter_nth(cx, expr, arglists[0]);
}
lint_or_fun_call(cx, expr, &name.node.as_str(), args);
@ -643,14 +645,21 @@ fn lint_cstring_as_ptr(cx: &LateContext, expr: &hir::Expr, new: &hir::Expr, unwr
#[allow(ptr_arg)]
// Type of MethodArgs is potentially a Vec
fn lint_slice_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &MethodArgs){
// lint if the caller of `.iter().nth` is a `slice`
fn lint_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &MethodArgs){
// lint if the caller of `.iter().nth()` is a `slice`
if let Some(_) = derefs_to_slice(cx, &iter_args[0], &cx.tcx.expr_ty(&iter_args[0])) {
span_lint(cx,
SLICE_ITER_NTH,
ITER_NTH,
expr.span,
"called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable");
}
// lint if the caller of `.iter().nth()` is a `Vec`
else if match_type(cx, cx.tcx.expr_ty(&iter_args[0]), &paths::VEC) {
span_lint(cx,
ITER_NTH,
expr.span,
"called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable");
}
}
fn derefs_to_slice(cx: &LateContext, expr: &hir::Expr, ty: &ty::Ty) -> Option<(Span, &'static str)> {

View File

@ -128,6 +128,16 @@ fn option_methods() {
}
/// Struct to generate false positives for things with .iter()
#[derive(Copy, Clone)]
struct HasIter;
impl HasIter {
fn iter(self) -> IteratorFalsePositives {
IteratorFalsePositives { foo: 0 }
}
}
/// Struct to generate false positive for Iterator-based lints
#[derive(Copy, Clone)]
struct IteratorFalsePositives {
@ -154,6 +164,10 @@ impl IteratorFalsePositives {
fn rposition(self) -> Option<u32> {
Some(self.foo)
}
fn nth(self, n: usize) -> Option<u32> {
Some(self.foo)
}
}
/// Checks implementation of `FILTER_NEXT` lint
@ -309,13 +323,17 @@ fn or_fun_call() {
//~|SUGGESTION btree.entry(42).or_insert_with(String::new);
}
/// Checks implementation of `SLICE_ITER_NTH` lint
fn slice_iter_nth() {
/// Checks implementation of `ITER_NTH` lint
fn iter_nth() {
let some_vec = vec![0, 1, 2, 3];
let bad = &some_vec[..].iter().nth(3);
let bad_vec = some_vec.iter().nth(3);
//~^ERROR called `.iter().nth()` on a Vec.
let bad_slice = &some_vec[..].iter().nth(3);
//~^ERROR called `.iter().nth()` on a slice.
let ok = some_vec.iter().nth(3); // This should be okay, since some_vec is not a slice
let false_positive = HasIter;
let ok = false_positive.iter().nth(3);
// ^This should be okay, because false_positive is not a slice or Vec
}
#[allow(similar_names)]