Added lints `into_iter_on_ref` and `into_iter_on_array`. Fix #1565.

This commit is contained in:
kennytm 2018-10-21 04:03:38 +08:00
parent a8d258454f
commit 2d1c9313b0
No known key found for this signature in database
GPG Key ID: FEF6C8051D0E013C
7 changed files with 346 additions and 3 deletions

View File

@ -713,6 +713,8 @@ All notable changes to this project will be documented in this file.
[`inline_fn_without_body`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#inline_fn_without_body
[`int_plus_one`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#int_plus_one
[`integer_arithmetic`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#integer_arithmetic
[`into_iter_on_array`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#into_iter_on_array
[`into_iter_on_ref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#into_iter_on_ref
[`invalid_ref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#invalid_ref
[`invalid_regex`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#invalid_regex
[`invalid_upcast_comparisons`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#invalid_upcast_comparisons

View File

@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 284 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
[There are 286 lints included in this crate!](https://rust-lang-nursery.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:

View File

@ -628,6 +628,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
methods::EXPECT_FUN_CALL,
methods::FILTER_NEXT,
methods::GET_UNWRAP,
methods::INTO_ITER_ON_ARRAY,
methods::INTO_ITER_ON_REF,
methods::ITER_CLONED_COLLECT,
methods::ITER_NTH,
methods::ITER_SKIP_NEXT,
@ -784,6 +786,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
methods::CHARS_LAST_CMP,
methods::GET_UNWRAP,
methods::INTO_ITER_ON_REF,
methods::ITER_CLONED_COLLECT,
methods::ITER_SKIP_NEXT,
methods::NEW_RET_NO_SELF,
@ -935,6 +938,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
loops::WHILE_IMMUTABLE_CONDITION,
mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
methods::CLONE_DOUBLE_REF,
methods::INTO_ITER_ON_ARRAY,
methods::TEMPORARY_CSTRING_AS_PTR,
minmax::MIN_MAX,
misc::CMP_NAN,

View File

@ -745,6 +745,51 @@ declare_clippy_lint! {
"using `filter_map` when a more succinct alternative exists"
}
/// **What it does:** Checks for `into_iter` calls on types which should be replaced by `iter` or
/// `iter_mut`.
///
/// **Why is this bad?** Arrays and `PathBuf` do not yet have an `into_iter` method which move out
/// their content into an iterator. Calling `into_iter` instead just forwards to `iter` or
/// `iter_mut` due to auto-referencing, of which only yield references. Furthermore, when the
/// standard library actually [implements the `into_iter` method][25725] which moves the content out
/// of the array, the original use of `into_iter` got inferred with the wrong type and the code will
/// be broken.
///
/// **Known problems:** None
///
/// **Example:**
///
/// ```rust
/// let _ = [1, 2, 3].into_iter().map(|x| *x).collect::<Vec<u32>>();
/// ```
///
/// [25725]: https://github.com/rust-lang/rust/issues/25725
declare_clippy_lint! {
pub INTO_ITER_ON_ARRAY,
correctness,
"using `.into_iter()` on an array"
}
/// **What it does:** Checks for `into_iter` calls on references which should be replaced by `iter`
/// or `iter_mut`.
///
/// **Why is this bad?** Readability. Calling `into_iter` on a reference will not move out its
/// content into the resulting iterator, which is confusing. It is better just call `iter` or
/// `iter_mut` directly.
///
/// **Known problems:** None
///
/// **Example:**
///
/// ```rust
/// let _ = (&vec![3, 4, 5]).into_iter();
/// ```
declare_clippy_lint! {
pub INTO_ITER_ON_REF,
style,
"using `.into_iter()` on a reference"
}
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(
@ -779,7 +824,9 @@ impl LintPass for Pass {
ITER_CLONED_COLLECT,
USELESS_ASREF,
UNNECESSARY_FOLD,
UNNECESSARY_FILTER_MAP
UNNECESSARY_FILTER_MAP,
INTO_ITER_ON_ARRAY,
INTO_ITER_ON_REF,
)
}
}
@ -843,6 +890,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
lint_single_char_pattern(cx, expr, &args[pos]);
}
},
ty::Ref(..) if method_call.ident.name == "into_iter" => {
lint_into_iter(cx, expr, self_ty, *method_span);
},
_ => (),
}
},
@ -2084,6 +2134,71 @@ fn lint_asref(cx: &LateContext<'_, '_>, expr: &hir::Expr, call_name: &str, as_re
}
}
fn ty_has_iter_method(cx: &LateContext<'_, '_>, self_ref_ty: ty::Ty<'_>) -> Option<(&'static Lint, &'static str, &'static str)> {
let (self_ty, mutbl) = match self_ref_ty.sty {
ty::TyKind::Ref(_, self_ty, mutbl) => (self_ty, mutbl),
_ => unreachable!(),
};
let method_name = match mutbl {
hir::MutImmutable => "iter",
hir::MutMutable => "iter_mut",
};
let def_id = match self_ty.sty {
ty::TyKind::Array(..) => return Some((INTO_ITER_ON_ARRAY, "array", method_name)),
ty::TyKind::Slice(..) => return Some((INTO_ITER_ON_REF, "slice", method_name)),
ty::Adt(adt, _) => adt.did,
_ => return None,
};
// FIXME: instead of this hard-coded list, we should check if `<adt>::iter`
// exists and has the desired signature. Unfortunately FnCtxt is not exported
// so we can't use its `lookup_method` method.
static INTO_ITER_COLLECTIONS: [(&Lint, &[&str]); 13] = [
(INTO_ITER_ON_REF, &paths::VEC),
(INTO_ITER_ON_REF, &paths::OPTION),
(INTO_ITER_ON_REF, &paths::RESULT),
(INTO_ITER_ON_REF, &paths::BTREESET),
(INTO_ITER_ON_REF, &paths::BTREEMAP),
(INTO_ITER_ON_REF, &paths::VEC_DEQUE),
(INTO_ITER_ON_REF, &paths::LINKED_LIST),
(INTO_ITER_ON_REF, &paths::BINARY_HEAP),
(INTO_ITER_ON_REF, &paths::HASHSET),
(INTO_ITER_ON_REF, &paths::HASHMAP),
(INTO_ITER_ON_ARRAY, &["std", "path", "PathBuf"]),
(INTO_ITER_ON_REF, &["std", "path", "Path"]),
(INTO_ITER_ON_REF, &["std", "sync", "mpsc", "Receiver"]),
];
for (lint, path) in &INTO_ITER_COLLECTIONS {
if match_def_path(cx.tcx, def_id, path) {
return Some((lint, path.last().unwrap(), method_name))
}
}
None
}
fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: ty::Ty<'_>, method_span: Span) {
if !match_trait_method(cx, expr, &paths::INTO_ITERATOR) {
return;
}
if let Some((lint, kind, method_name)) = ty_has_iter_method(cx, self_ref_ty) {
span_lint_and_sugg(
cx,
lint,
method_span,
&format!(
"this .into_iter() call is equivalent to .{}() and will not move the {}",
method_name,
kind,
),
"call directly",
method_name.to_owned(),
);
}
}
/// Given a `Result<T, E>` type, return its error type (`E`).
fn get_error_type<'a>(cx: &LateContext<'_, '_>, ty: Ty<'a>) -> Option<Ty<'a>> {
if let ty::Adt(_, substs) = ty.sty {

View File

@ -29,7 +29,7 @@ impl Unrelated {
clippy::for_kv_map)]
#[warn(clippy::unused_collect)]
#[allow(clippy::linkedlist, clippy::shadow_unrelated, clippy::unnecessary_mut_passed, clippy::cyclomatic_complexity, clippy::similar_names)]
#[allow(clippy::many_single_char_names, unused_variables)]
#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)]
fn main() {
const MAX_LEN: usize = 42;

View File

@ -0,0 +1,44 @@
#![warn(clippy::into_iter_on_ref)]
#![deny(clippy::into_iter_on_array)]
struct X;
use std::collections::*;
fn main() {
for _ in &[1,2,3] {}
for _ in vec![X, X] {}
for _ in &vec![X, X] {}
for _ in [1,2,3].into_iter() {} //~ ERROR equivalent to .iter()
let _ = [1,2,3].into_iter(); //~ ERROR equivalent to .iter()
let _ = vec![1,2,3].into_iter();
let _ = (&vec![1,2,3]).into_iter(); //~ WARN equivalent to .iter()
let _ = vec![1,2,3].into_boxed_slice().into_iter(); //~ WARN equivalent to .iter()
let _ = std::rc::Rc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
let _ = std::sync::Arc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
let _ = (&&&&&&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
let _ = (&&&&mut &&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
let _ = (&mut &mut &mut [1,2,3]).into_iter(); //~ ERROR equivalent to .iter_mut()
let _ = (&Some(4)).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut Some(5)).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&Ok::<_, i32>(6)).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut Err::<i32, _>(7)).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&mut HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
let _ = (&BTreeSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&BinaryHeap::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = (&HashSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter()
let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
}

View File

@ -0,0 +1,178 @@
error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:11:22
|
11 | for _ in [1,2,3].into_iter() {} //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
|
note: lint level defined here
--> $DIR/into_iter_on_ref.rs:2:9
|
2 | #![deny(clippy::into_iter_on_array)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:13:21
|
13 | let _ = [1,2,3].into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the Vec
--> $DIR/into_iter_on_ref.rs:15:28
|
15 | let _ = (&vec![1,2,3]).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
|
= note: `-D clippy::into-iter-on-ref` implied by `-D warnings`
error: this .into_iter() call is equivalent to .iter() and will not move the slice
--> $DIR/into_iter_on_ref.rs:16:44
|
16 | let _ = vec![1,2,3].into_boxed_slice().into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the slice
--> $DIR/into_iter_on_ref.rs:17:41
|
17 | let _ = std::rc::Rc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the slice
--> $DIR/into_iter_on_ref.rs:18:44
|
18 | let _ = std::sync::Arc::from(&[X][..]).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:20:30
|
20 | let _ = (&&&&&&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the array
--> $DIR/into_iter_on_ref.rs:21:34
|
21 | let _ = (&&&&mut &&&[1,2,3]).into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the array
--> $DIR/into_iter_on_ref.rs:22:38
|
22 | let _ = (&mut &mut &mut [1,2,3]).into_iter(); //~ ERROR equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the Option
--> $DIR/into_iter_on_ref.rs:24:24
|
24 | let _ = (&Some(4)).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Option
--> $DIR/into_iter_on_ref.rs:25:28
|
25 | let _ = (&mut Some(5)).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the Result
--> $DIR/into_iter_on_ref.rs:26:32
|
26 | let _ = (&Ok::<_, i32>(6)).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Result
--> $DIR/into_iter_on_ref.rs:27:37
|
27 | let _ = (&mut Err::<i32, _>(7)).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the Vec
--> $DIR/into_iter_on_ref.rs:28:34
|
28 | let _ = (&Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the Vec
--> $DIR/into_iter_on_ref.rs:29:38
|
29 | let _ = (&mut Vec::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the BTreeMap
--> $DIR/into_iter_on_ref.rs:30:44
|
30 | let _ = (&BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the BTreeMap
--> $DIR/into_iter_on_ref.rs:31:48
|
31 | let _ = (&mut BTreeMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the VecDeque
--> $DIR/into_iter_on_ref.rs:32:39
|
32 | let _ = (&VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the VecDeque
--> $DIR/into_iter_on_ref.rs:33:43
|
33 | let _ = (&mut VecDeque::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the LinkedList
--> $DIR/into_iter_on_ref.rs:34:41
|
34 | let _ = (&LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the LinkedList
--> $DIR/into_iter_on_ref.rs:35:45
|
35 | let _ = (&mut LinkedList::<i32>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the HashMap
--> $DIR/into_iter_on_ref.rs:36:43
|
36 | let _ = (&HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter_mut() and will not move the HashMap
--> $DIR/into_iter_on_ref.rs:37:47
|
37 | let _ = (&mut HashMap::<i32, u64>::new()).into_iter(); //~ WARN equivalent to .iter_mut()
| ^^^^^^^^^ help: call directly: `iter_mut`
error: this .into_iter() call is equivalent to .iter() and will not move the BTreeSet
--> $DIR/into_iter_on_ref.rs:39:39
|
39 | let _ = (&BTreeSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the BinaryHeap
--> $DIR/into_iter_on_ref.rs:40:41
|
40 | let _ = (&BinaryHeap::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the HashSet
--> $DIR/into_iter_on_ref.rs:41:38
|
41 | let _ = (&HashSet::<i32>::new()).into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the Path
--> $DIR/into_iter_on_ref.rs:42:43
|
42 | let _ = std::path::Path::new("12/34").into_iter(); //~ WARN equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: this .into_iter() call is equivalent to .iter() and will not move the PathBuf
--> $DIR/into_iter_on_ref.rs:43:47
|
43 | let _ = std::path::PathBuf::from("12/34").into_iter(); //~ ERROR equivalent to .iter()
| ^^^^^^^^^ help: call directly: `iter`
error: aborting due to 28 previous errors