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

7 files changed, 346 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0d5d10c7ae5..72183f68ee0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/README.md b/README.md
index 3c056171f02..8af7a20f66d 100644
--- a/README.md
+++ b/README.md
@@ -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:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 56cd2ca3e92..863c0623487 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -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,
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 31982706497..4b83c6e6b11 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -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 {
diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop.rs
index 2a70149f246..513a3c0ee42 100644
--- a/tests/ui/for_loop.rs
+++ b/tests/ui/for_loop.rs
@@ -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;
 
diff --git a/tests/ui/into_iter_on_ref.rs b/tests/ui/into_iter_on_ref.rs
new file mode 100644
index 00000000000..72aa6341a50
--- /dev/null
+++ b/tests/ui/into_iter_on_ref.rs
@@ -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()
+}
diff --git a/tests/ui/into_iter_on_ref.stderr b/tests/ui/into_iter_on_ref.stderr
new file mode 100644
index 00000000000..39055423048
--- /dev/null
+++ b/tests/ui/into_iter_on_ref.stderr
@@ -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
+