Rollup merge of #66017 - LukasKalbertodt:array-into-iter-lint, r=matthewjasper

Add future incompatibility lint for `array.into_iter()`

This is for #65819. This lint warns when calling `into_iter` on an array directly. That's because today the method call resolves to `<&[T] as IntoIterator>::into_iter` but that would change when adding `IntoIterator` impls for arrays. This problem is discussed in detail in #65819.

We still haven't decided how to proceed exactly, but it seems like adding a lint is a good idea regardless?

Also: this is the first time I implement a lint, so there are probably a lot of things I can improve. I used a different strategy than @scottmcm describes [here](https://github.com/rust-lang/rust/pull/65819#issuecomment-548667847) since I already started implementing this before they commented.

### TODO

- [x] Decide if we want this lint -> apparently [we want](https://github.com/rust-lang/rust/pull/65819#issuecomment-548964818)
- [x] Open a lint-tracking-issue and add the correct issue number in the code -> https://github.com/rust-lang/rust/issues/66145

src/libcore/iter/traits/collect.rs

@@ -205,6 +205,7 @@ pub trait FromIterator<A>: Sized {
 ///         .collect()
 /// }
 /// ```
+#[rustc_diagnostic_item = "IntoIterator"]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub trait IntoIterator {
     /// The type of the elements being iterated over.

src/librustc_lint/array_into_iter.rs

@@ -0,0 +1,91 @@
+use crate::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
+use rustc::{
+    lint::FutureIncompatibleInfo,
+    hir,
+    ty::{
+        self,
+        adjustment::{Adjust, Adjustment},
+    },
+};
+use syntax::{
+    errors::Applicability,
+    symbol::sym,
+};
+
+
+declare_lint! {
+    pub ARRAY_INTO_ITER,
+    Warn,
+    "detects calling `into_iter` on arrays",
+    @future_incompatible = FutureIncompatibleInfo {
+        reference: "issue #66145 <https://github.com/rust-lang/rust/issues/66145>",
+        edition: None,
+    };
+}
+
+declare_lint_pass!(
+    /// Checks for instances of calling `into_iter` on arrays.
+    ArrayIntoIter => [ARRAY_INTO_ITER]
+);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIntoIter {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
+        // We only care about method call expressions.
+        if let hir::ExprKind::MethodCall(call, span, args) = &expr.kind {
+            if call.ident.name != sym::into_iter {
+                return;
+            }
+
+            // Check if the method call actually calls the libcore
+            // `IntoIterator::into_iter`.
+            let def_id = cx.tables.type_dependent_def_id(expr.hir_id).unwrap();
+            match cx.tcx.trait_of_item(def_id) {
+                Some(trait_id) if cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id) => {},
+                _ => return,
+            };
+
+            // As this is a method call expression, we have at least one
+            // argument.
+            let receiver_arg = &args[0];
+
+            // Test if the original `self` type is an array type.
+            match cx.tables.expr_ty(receiver_arg).kind {
+                ty::Array(..) => {}
+                _ => return,
+            }
+
+            // Make sure that the first adjustment is an autoref coercion.
+            match cx.tables.expr_adjustments(receiver_arg).get(0) {
+                Some(Adjustment { kind: Adjust::Borrow(_), .. }) => {}
+                _ => return,
+            }
+
+            // Emit lint diagnostic.
+            let target = match cx.tables.expr_ty_adjusted(receiver_arg).kind {
+                ty::Ref(_, ty::TyS { kind: ty::Array(..), ..}, _) => "[T; N]",
+                ty::Ref(_, ty::TyS { kind: ty::Slice(..), ..}, _) => "[T]",
+
+                // We know the original first argument type is an array type,
+                // we know that the first adjustment was an autoref coercion
+                // and we know that `IntoIterator` is the trait involved. The
+                // array cannot be coerced to something other than a reference
+                // to an array or to a slice.
+                _ => bug!("array type coerced to something other than array or slice"),
+            };
+            let msg = format!(
+                "this method call currently resolves to `<&{} as IntoIterator>::into_iter` (due \
+                    to autoref coercions), but that might change in the future when \
+                    `IntoIterator` impls for arrays are added.",
+                target,
+            );
+            cx.struct_span_lint(ARRAY_INTO_ITER, *span, &msg)
+                .span_suggestion(
+                    call.ident.span,
+                    "use `.iter()` instead of `.into_iter()` to avoid ambiguity",
+                    "iter".into(),
+                    Applicability::MachineApplicable,
+                )
+                .emit();
+        }
+    }
+}

src/librustc_lint/lib.rs

@@ -22,6 +22,7 @@
 #[macro_use]
 extern crate rustc;
 
+mod array_into_iter;
 mod error_codes;
 mod nonstandard_style;
 mod redundant_semicolon;
@@ -57,6 +58,7 @@ use types::*;
 use unused::*;
 use non_ascii_idents::*;
 use rustc::lint::internal::*;
+use array_into_iter::ArrayIntoIter;
 
 /// Useful for other parts of the compiler.
 pub use builtin::SoftLints;
@@ -131,6 +133,8 @@ macro_rules! late_lint_passes {
             // FIXME: Turn the computation of types which implement Debug into a query
             // and change this to a module lint pass
             MissingDebugImplementations: MissingDebugImplementations::default(),
+
+            ArrayIntoIter: ArrayIntoIter,
         ]);
     )
 }

src/libtest/tests.rs

@@ -269,7 +269,7 @@ fn time_test_failure_template(test_type: TestType) -> TestResult {
 fn test_error_on_exceed() {
     let types = [TestType::UnitTest, TestType::IntegrationTest, TestType::DocTest];
 
-    for test_type in types.into_iter() {
+    for test_type in types.iter() {
         let result = time_test_failure_template(*test_type);
 
         assert_eq!(result, TestResult::TrTimedFail);
@@ -320,7 +320,7 @@ fn test_time_options_threshold() {
         (TestType::DocTest, doc.critical.as_millis(), true, true),
     ];
 
-    for (test_type, time, expected_warn, expected_critical) in test_vector.into_iter() {
+    for (test_type, time, expected_warn, expected_critical) in test_vector.iter() {
         let test_desc = typed_test_desc(*test_type);
         let exec_time = test_exec_time(*time as u64);
 

src/test/ui/iterators/into-iter-on-arrays-lint.fixed

@@ -0,0 +1,33 @@
+// run-pass
+// run-rustfix
+
+fn main() {
+    let small = [1, 2];
+    let big = [0u8; 33];
+
+    // Expressions that should trigger the lint
+    small.iter();
+    //~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    [1, 2].iter();
+    //~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    big.iter();
+    //~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    [0u8; 33].iter();
+    //~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+
+
+    // Expressions that should not
+    (&[1, 2]).into_iter();
+    (&small).into_iter();
+    (&[0u8; 33]).into_iter();
+    (&big).into_iter();
+
+    for _ in &[1, 2] {}
+    (&small as &[_]).into_iter();
+    small[..].into_iter();
+    std::iter::IntoIterator::into_iter(&[1, 2]);
+}

src/test/ui/iterators/into-iter-on-arrays-lint.rs

@@ -0,0 +1,33 @@
+// run-pass
+// run-rustfix
+
+fn main() {
+    let small = [1, 2];
+    let big = [0u8; 33];
+
+    // Expressions that should trigger the lint
+    small.into_iter();
+    //~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    [1, 2].into_iter();
+    //~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    big.into_iter();
+    //~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+    [0u8; 33].into_iter();
+    //~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
+    //~| WARNING this was previously accepted by the compiler but is being phased out
+
+
+    // Expressions that should not
+    (&[1, 2]).into_iter();
+    (&small).into_iter();
+    (&[0u8; 33]).into_iter();
+    (&big).into_iter();
+
+    for _ in &[1, 2] {}
+    (&small as &[_]).into_iter();
+    small[..].into_iter();
+    std::iter::IntoIterator::into_iter(&[1, 2]);
+}

src/test/ui/iterators/into-iter-on-arrays-lint.stderr

@@ -0,0 +1,37 @@
+warning: this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added.
+  --> $DIR/into-iter-on-arrays-lint.rs:9:11
+   |
+LL |     small.into_iter();
+   |           ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
+   |
+   = note: `#[warn(array_into_iter)]` on by default
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
+
+warning: this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added.
+  --> $DIR/into-iter-on-arrays-lint.rs:12:12
+   |
+LL |     [1, 2].into_iter();
+   |            ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
+
+warning: this method call currently resolves to `<&[T] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added.
+  --> $DIR/into-iter-on-arrays-lint.rs:15:9
+   |
+LL |     big.into_iter();
+   |         ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
+
+warning: this method call currently resolves to `<&[T] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added.
+  --> $DIR/into-iter-on-arrays-lint.rs:18:15
+   |
+LL |     [0u8; 33].into_iter();
+   |               ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
+