Skip to content

Commit c9eae9e

Browse files
CentrilCentril
authored
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](#65819 (comment)) since I already started implementing this before they commented. ### TODO - [x] Decide if we want this lint -> apparently [we want](#65819 (comment)) - [x] Open a lint-tracking-issue and add the correct issue number in the code -> #66145
2 parents e19cb40 + 761ba89 commit c9eae9e

File tree

7 files changed

+201
-2
lines changed

7 files changed

+201
-2
lines changed

Diff for: ‎src/libcore/iter/traits/collect.rs

+1
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ pub trait FromIterator<A>: Sized {
205205
/// .collect()
206206
/// }
207207
/// ```
208+
#[rustc_diagnostic_item = "IntoIterator"]
208209
#[stable(feature = "rust1", since = "1.0.0")]
209210
pub trait IntoIterator {
210211
/// The type of the elements being iterated over.

Diff for: ‎src/librustc_lint/array_into_iter.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
use crate::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
2+
use rustc::{
3+
lint::FutureIncompatibleInfo,
4+
hir,
5+
ty::{
6+
self,
7+
adjustment::{Adjust, Adjustment},
8+
},
9+
};
10+
use syntax::{
11+
errors::Applicability,
12+
symbol::sym,
13+
};
14+
15+
16+
declare_lint! {
17+
pub ARRAY_INTO_ITER,
18+
Warn,
19+
"detects calling `into_iter` on arrays",
20+
@future_incompatible = FutureIncompatibleInfo {
21+
reference: "issue #66145 <https://github.com/rust-lang/rust/issues/66145>",
22+
edition: None,
23+
};
24+
}
25+
26+
declare_lint_pass!(
27+
/// Checks for instances of calling `into_iter` on arrays.
28+
ArrayIntoIter => [ARRAY_INTO_ITER]
29+
);
30+
31+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIntoIter {
32+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
33+
// We only care about method call expressions.
34+
if let hir::ExprKind::MethodCall(call, span, args) = &expr.kind {
35+
if call.ident.name != sym::into_iter {
36+
return;
37+
}
38+
39+
// Check if the method call actually calls the libcore
40+
// `IntoIterator::into_iter`.
41+
let def_id = cx.tables.type_dependent_def_id(expr.hir_id).unwrap();
42+
match cx.tcx.trait_of_item(def_id) {
43+
Some(trait_id) if cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id) => {},
44+
_ => return,
45+
};
46+
47+
// As this is a method call expression, we have at least one
48+
// argument.
49+
let receiver_arg = &args[0];
50+
51+
// Test if the original `self` type is an array type.
52+
match cx.tables.expr_ty(receiver_arg).kind {
53+
ty::Array(..) => {}
54+
_ => return,
55+
}
56+
57+
// Make sure that the first adjustment is an autoref coercion.
58+
match cx.tables.expr_adjustments(receiver_arg).get(0) {
59+
Some(Adjustment { kind: Adjust::Borrow(_), .. }) => {}
60+
_ => return,
61+
}
62+
63+
// Emit lint diagnostic.
64+
let target = match cx.tables.expr_ty_adjusted(receiver_arg).kind {
65+
ty::Ref(_, ty::TyS { kind: ty::Array(..), ..}, _) => "[T; N]",
66+
ty::Ref(_, ty::TyS { kind: ty::Slice(..), ..}, _) => "[T]",
67+
68+
// We know the original first argument type is an array type,
69+
// we know that the first adjustment was an autoref coercion
70+
// and we know that `IntoIterator` is the trait involved. The
71+
// array cannot be coerced to something other than a reference
72+
// to an array or to a slice.
73+
_ => bug!("array type coerced to something other than array or slice"),
74+
};
75+
let msg = format!(
76+
"this method call currently resolves to `<&{} as IntoIterator>::into_iter` (due \
77+
to autoref coercions), but that might change in the future when \
78+
`IntoIterator` impls for arrays are added.",
79+
target,
80+
);
81+
cx.struct_span_lint(ARRAY_INTO_ITER, *span, &msg)
82+
.span_suggestion(
83+
call.ident.span,
84+
"use `.iter()` instead of `.into_iter()` to avoid ambiguity",
85+
"iter".into(),
86+
Applicability::MachineApplicable,
87+
)
88+
.emit();
89+
}
90+
}
91+
}

Diff for: ‎src/librustc_lint/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#[macro_use]
2323
extern crate rustc;
2424

25+
mod array_into_iter;
2526
mod error_codes;
2627
mod nonstandard_style;
2728
mod redundant_semicolon;
@@ -57,6 +58,7 @@ use types::*;
5758
use unused::*;
5859
use non_ascii_idents::*;
5960
use rustc::lint::internal::*;
61+
use array_into_iter::ArrayIntoIter;
6062

6163
/// Useful for other parts of the compiler.
6264
pub use builtin::SoftLints;
@@ -131,6 +133,8 @@ macro_rules! late_lint_passes {
131133
// FIXME: Turn the computation of types which implement Debug into a query
132134
// and change this to a module lint pass
133135
MissingDebugImplementations: MissingDebugImplementations::default(),
136+
137+
ArrayIntoIter: ArrayIntoIter,
134138
]);
135139
)
136140
}

Diff for: ‎src/libtest/tests.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ fn time_test_failure_template(test_type: TestType) -> TestResult {
269269
fn test_error_on_exceed() {
270270
let types = [TestType::UnitTest, TestType::IntegrationTest, TestType::DocTest];
271271

272-
for test_type in types.into_iter() {
272+
for test_type in types.iter() {
273273
let result = time_test_failure_template(*test_type);
274274

275275
assert_eq!(result, TestResult::TrTimedFail);
@@ -320,7 +320,7 @@ fn test_time_options_threshold() {
320320
(TestType::DocTest, doc.critical.as_millis(), true, true),
321321
];
322322

323-
for (test_type, time, expected_warn, expected_critical) in test_vector.into_iter() {
323+
for (test_type, time, expected_warn, expected_critical) in test_vector.iter() {
324324
let test_desc = typed_test_desc(*test_type);
325325
let exec_time = test_exec_time(*time as u64);
326326

Diff for: ‎src/test/ui/iterators/into-iter-on-arrays-lint.fixed

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// run-pass
2+
// run-rustfix
3+
4+
fn main() {
5+
let small = [1, 2];
6+
let big = [0u8; 33];
7+
8+
// Expressions that should trigger the lint
9+
small.iter();
10+
//~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
11+
//~| WARNING this was previously accepted by the compiler but is being phased out
12+
[1, 2].iter();
13+
//~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
14+
//~| WARNING this was previously accepted by the compiler but is being phased out
15+
big.iter();
16+
//~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
17+
//~| WARNING this was previously accepted by the compiler but is being phased out
18+
[0u8; 33].iter();
19+
//~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
20+
//~| WARNING this was previously accepted by the compiler but is being phased out
21+
22+
23+
// Expressions that should not
24+
(&[1, 2]).into_iter();
25+
(&small).into_iter();
26+
(&[0u8; 33]).into_iter();
27+
(&big).into_iter();
28+
29+
for _ in &[1, 2] {}
30+
(&small as &[_]).into_iter();
31+
small[..].into_iter();
32+
std::iter::IntoIterator::into_iter(&[1, 2]);
33+
}

Diff for: ‎src/test/ui/iterators/into-iter-on-arrays-lint.rs

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// run-pass
2+
// run-rustfix
3+
4+
fn main() {
5+
let small = [1, 2];
6+
let big = [0u8; 33];
7+
8+
// Expressions that should trigger the lint
9+
small.into_iter();
10+
//~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
11+
//~| WARNING this was previously accepted by the compiler but is being phased out
12+
[1, 2].into_iter();
13+
//~^ WARNING this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter`
14+
//~| WARNING this was previously accepted by the compiler but is being phased out
15+
big.into_iter();
16+
//~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
17+
//~| WARNING this was previously accepted by the compiler but is being phased out
18+
[0u8; 33].into_iter();
19+
//~^ WARNING this method call currently resolves to `<&[T] as IntoIterator>::into_iter`
20+
//~| WARNING this was previously accepted by the compiler but is being phased out
21+
22+
23+
// Expressions that should not
24+
(&[1, 2]).into_iter();
25+
(&small).into_iter();
26+
(&[0u8; 33]).into_iter();
27+
(&big).into_iter();
28+
29+
for _ in &[1, 2] {}
30+
(&small as &[_]).into_iter();
31+
small[..].into_iter();
32+
std::iter::IntoIterator::into_iter(&[1, 2]);
33+
}

Diff for: ‎src/test/ui/iterators/into-iter-on-arrays-lint.stderr

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
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.
2+
--> $DIR/into-iter-on-arrays-lint.rs:9:11
3+
|
4+
LL | small.into_iter();
5+
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
6+
|
7+
= note: `#[warn(array_into_iter)]` on by default
8+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
9+
= note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
10+
11+
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.
12+
--> $DIR/into-iter-on-arrays-lint.rs:12:12
13+
|
14+
LL | [1, 2].into_iter();
15+
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
16+
|
17+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
18+
= note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
19+
20+
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.
21+
--> $DIR/into-iter-on-arrays-lint.rs:15:9
22+
|
23+
LL | big.into_iter();
24+
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
25+
|
26+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
27+
= note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
28+
29+
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.
30+
--> $DIR/into-iter-on-arrays-lint.rs:18:15
31+
|
32+
LL | [0u8; 33].into_iter();
33+
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
34+
|
35+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
36+
= note: for more information, see issue #66145 <https://github.com/rust-lang/rust/issues/66145>
37+

0 commit comments

Comments
 (0)