Skip to content

Commit

Permalink
Rollup merge of rust-lang#66017 - LukasKalbertodt:array-into-iter-lin…
Browse files Browse the repository at this point in the history
…t, r=matthewjasper

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

This is for rust-lang#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 rust-lang#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](rust-lang#65819 (comment)) since I already started implementing this before they commented.

### TODO

- [x] Decide if we want this lint -> apparently [we want](rust-lang#65819 (comment))
- [x] Open a lint-tracking-issue and add the correct issue number in the code -> rust-lang#66145
  • Loading branch information
Centril authored Nov 7, 2019
2 parents e19cb40 + 761ba89 commit c9eae9e
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 2 deletions.
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::{
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();
}
}
}
4 changes: 4 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#[macro_use]
extern crate rustc;

mod array_into_iter;
mod error_codes;
mod nonstandard_style;
mod redundant_semicolon;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
]);
)
}
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
// 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]);
}
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>

0 comments on commit c9eae9e

Please sign in to comment.