Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add future incompatibility lint for array.into_iter() #66017

Merged
merged 3 commits into from
Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libcore/iter/traits/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
91 changes: 91 additions & 0 deletions src/librustc_lint/array_into_iter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
use crate::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I believe we prefer single line imports.

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,
}

LukasKalbertodt marked this conversation as resolved.
Show resolved Hide resolved
// 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();
}
}
}
4 changes: 4 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#[macro_use]
extern crate rustc;

mod array_into_iter;
mod error_codes;
mod nonstandard_style;
mod redundant_semicolon;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
]);
)
}
Expand Down
4 changes: 2 additions & 2 deletions src/libtest/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/iterators/into-iter-on-arrays-lint.fixed
Original file line number Diff line number Diff line change
@@ -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]);
}
33 changes: 33 additions & 0 deletions src/test/ui/iterators/into-iter-on-arrays-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// run-pass
LukasKalbertodt marked this conversation as resolved.
Show resolved Hide resolved
// run-rustfix

fn main() {
let small = [1, 2];
let big = [0u8; 33];

// Expressions that should trigger the lint
small.into_iter();
LukasKalbertodt marked this conversation as resolved.
Show resolved Hide resolved
//~^ 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]);
}
37 changes: 37 additions & 0 deletions src/test/ui/iterators/into-iter-on-arrays-lint.stderr
Original file line number Diff line number Diff line change
@@ -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>