From b492c97a31432adde450c9cc8c9d369bbd4d83f2 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Fri, 1 Nov 2019 12:04:18 +0100 Subject: [PATCH 1/3] Add future incompatibility lint for `array.into_iter()` As we might want to add `IntoIterator` impls for arrays in the future, and since that introduces a breaking change, this lint warns and suggests using `iter()` instead (which is shorter and more explicit). --- src/libcore/iter/traits/collect.rs | 1 + src/librustc_lint/array_into_iter.rs | 91 ++++++++++++++++++++++++++++ src/librustc_lint/lib.rs | 4 ++ 3 files changed, 96 insertions(+) create mode 100644 src/librustc_lint/array_into_iter.rs diff --git a/src/libcore/iter/traits/collect.rs b/src/libcore/iter/traits/collect.rs index 00a864170583e..bbdb169cac0fc 100644 --- a/src/libcore/iter/traits/collect.rs +++ b/src/libcore/iter/traits/collect.rs @@ -205,6 +205,7 @@ pub trait FromIterator: Sized { /// .collect() /// } /// ``` +#[rustc_diagnostic_item = "IntoIterator"] #[stable(feature = "rust1", since = "1.0.0")] pub trait IntoIterator { /// The type of the elements being iterated over. diff --git a/src/librustc_lint/array_into_iter.rs b/src/librustc_lint/array_into_iter.rs new file mode 100644 index 0000000000000..e73414174fb35 --- /dev/null +++ b/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 ", + 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(); + } + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index b1beef04c5929..473f8d4d3eb3d 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -21,6 +21,7 @@ #[macro_use] extern crate rustc; +mod array_into_iter; mod error_codes; mod nonstandard_style; mod redundant_semicolon; @@ -56,6 +57,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; @@ -130,6 +132,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, ]); ) } From 8fd09d9db68437d5ff48ab8889d949d08a8bb3f9 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Fri, 1 Nov 2019 12:04:31 +0100 Subject: [PATCH 2/3] Add UI test for `array.into_iter()` lint --- .../iterators/into-iter-on-arrays-lint.fixed | 33 +++++++++++++++++ .../ui/iterators/into-iter-on-arrays-lint.rs | 33 +++++++++++++++++ .../iterators/into-iter-on-arrays-lint.stderr | 37 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 src/test/ui/iterators/into-iter-on-arrays-lint.fixed create mode 100644 src/test/ui/iterators/into-iter-on-arrays-lint.rs create mode 100644 src/test/ui/iterators/into-iter-on-arrays-lint.stderr diff --git a/src/test/ui/iterators/into-iter-on-arrays-lint.fixed b/src/test/ui/iterators/into-iter-on-arrays-lint.fixed new file mode 100644 index 0000000000000..f88a52d315918 --- /dev/null +++ b/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]); +} diff --git a/src/test/ui/iterators/into-iter-on-arrays-lint.rs b/src/test/ui/iterators/into-iter-on-arrays-lint.rs new file mode 100644 index 0000000000000..e1a4b535f3832 --- /dev/null +++ b/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]); +} diff --git a/src/test/ui/iterators/into-iter-on-arrays-lint.stderr b/src/test/ui/iterators/into-iter-on-arrays-lint.stderr new file mode 100644 index 0000000000000..b5964bd44bff7 --- /dev/null +++ b/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 + +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 + +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 + +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 + From 761ba89ffd73498c3014d3b43b5bc0b4f592a284 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Fri, 1 Nov 2019 13:43:44 +0100 Subject: [PATCH 3/3] Replace `array.into_iter()` with `iter()` in `libtest/tests.rs` --- src/libtest/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libtest/tests.rs b/src/libtest/tests.rs index 9de774555e9cc..e0e211444cff5 100644 --- a/src/libtest/tests.rs +++ b/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);