Auto merge of #54946 - estebank:iterator, r=varkor

Add filtering option to `rustc_on_unimplemented` and reword `Iterator` E0277 errors

 - Add more targetting filters for arrays to `rustc_on_unimplemented` (Fix #53766)
 - Detect one element array of `Range` type, which is potentially a typo:
   `for _ in [0..10] {}` where iterating between `0` and `10` was intended.
   (Fix #23141)
 - Suggest `.bytes()` and `.chars()` for `String`.
 - Suggest borrowing or `.iter()` on arrays (Fix #36391)
 - Suggest using range literal when iterating on integers (Fix #34353)
 - Do not suggest `.iter()` by default (Fix #50773, fix #46806)
 - Add regression test (Fix #22872)

Author: bors

src/libcore/iter/iterator.rs

@@ -30,11 +30,72 @@ fn _assert_is_object_safe(_: &dyn Iterator<Item=()>) {}
 /// [impl]: index.html#implementing-iterator
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_on_unimplemented(
+    on(
+        _Self="[std::ops::Range<Idx>; 1]",
+        label="if you meant to iterate between two values, remove the square brackets",
+        note="`[start..end]` is an array of one `Range`; you might have meant to have a `Range` \
+              without the brackets: `start..end`"
+    ),
+    on(
+        _Self="[std::ops::RangeFrom<Idx>; 1]",
+        label="if you meant to iterate from a value onwards, remove the square brackets",
+        note="`[start..]` is an array of one `RangeFrom`; you might have meant to have a \
+              `RangeFrom` without the brackets: `start..`, keeping in mind that iterating over an \
+              unbounded iterator will run forever unless you `break` or `return` from within the \
+              loop"
+    ),
+    on(
+        _Self="[std::ops::RangeTo<Idx>; 1]",
+        label="if you meant to iterate until a value, remove the square brackets and add a \
+               starting value",
+        note="`[..end]` is an array of one `RangeTo`; you might have meant to have a bounded \
+              `Range` without the brackets: `0..end`"
+    ),
+    on(
+        _Self="[std::ops::RangeInclusive<Idx>; 1]",
+        label="if you meant to iterate between two values, remove the square brackets",
+        note="`[start..=end]` is an array of one `RangeInclusive`; you might have meant to have a \
+              `RangeInclusive` without the brackets: `start..=end`"
+    ),
+    on(
+        _Self="[std::ops::RangeToInclusive<Idx>; 1]",
+        label="if you meant to iterate until a value (including it), remove the square brackets \
+               and add a starting value",
+        note="`[..=end]` is an array of one `RangeToInclusive`; you might have meant to have a \
+              bounded `RangeInclusive` without the brackets: `0..=end`"
+    ),
+    on(
+        _Self="std::ops::RangeTo<Idx>",
+        label="if you meant to iterate until a value, add a starting value",
+        note="`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \
+              bounded `Range`: `0..end`"
+    ),
+    on(
+        _Self="std::ops::RangeToInclusive<Idx>",
+        label="if you meant to iterate until a value (including it), add a starting value",
+        note="`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \
+              to have a bounded `RangeInclusive`: `0..=end`"
+    ),
     on(
         _Self="&str",
         label="`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
     ),
-    label="`{Self}` is not an iterator; maybe try calling `.iter()` or a similar method"
+    on(
+        _Self="std::string::String",
+        label="`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
+    ),
+    on(
+        _Self="[]",
+        label="borrow the array with `&` or call `.iter()` on it to iterate over it",
+        note="arrays are not an iterators, but slices like the following are: `&[1, 2, 3]`"
+    ),
+    on(
+        _Self="{integral}",
+        note="if you want to iterate between `start` until a value `end`, use the exclusive range \
+              syntax `start..end` or the inclusive range syntax `start..=end`"
+    ),
+    label="`{Self}` is not an iterator",
+    message="`{Self}` is not an iterator"
 )]
 #[doc(spotlight)]
 pub trait Iterator {

src/librustc/traits/error_reporting.rs

@@ -349,9 +349,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     fn on_unimplemented_note(
         &self,
         trait_ref: ty::PolyTraitRef<'tcx>,
-        obligation: &PredicateObligation<'tcx>) ->
-        OnUnimplementedNote
-    {
+        obligation: &PredicateObligation<'tcx>,
+    ) -> OnUnimplementedNote {
         let def_id = self.impl_similar_to(trait_ref, obligation)
             .unwrap_or(trait_ref.def_id());
         let trait_ref = *trait_ref.skip_binder();
@@ -410,6 +409,38 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             flags.push(("crate_local".to_owned(), None));
         }
 
+        // Allow targetting all integers using `{integral}`, even if the exact type was resolved
+        if self_ty.is_integral() {
+            flags.push(("_Self".to_owned(), Some("{integral}".to_owned())));
+        }
+
+        if let ty::Array(aty, len) = self_ty.sty {
+            flags.push(("_Self".to_owned(), Some("[]".to_owned())));
+            flags.push(("_Self".to_owned(), Some(format!("[{}]", aty))));
+            if let Some(def) = aty.ty_adt_def() {
+                // We also want to be able to select the array's type's original
+                // signature with no type arguments resolved
+                flags.push((
+                    "_Self".to_owned(),
+                    Some(format!("[{}]", self.tcx.type_of(def.did).to_string())),
+                ));
+                let tcx = self.tcx;
+                if let Some(len) = len.val.try_to_scalar().and_then(|scalar| {
+                    scalar.to_usize(tcx).ok()
+                }) {
+                    flags.push((
+                        "_Self".to_owned(),
+                        Some(format!("[{}; {}]", self.tcx.type_of(def.did).to_string(), len)),
+                    ));
+                } else {
+                    flags.push((
+                        "_Self".to_owned(),
+                        Some(format!("[{}; _]", self.tcx.type_of(def.did).to_string())),
+                    ));
+                }
+            }
+        }
+
         if let Ok(Some(command)) = OnUnimplementedDirective::of_item(
             self.tcx, trait_ref.def_id, def_id
         ) {

src/test/ui/conservative_impl_trait.rs

@@ -11,7 +11,7 @@
 // #39872, #39553
 
 fn will_ice(something: &u32) -> impl Iterator<Item = &u32> {
-    //~^ ERROR the trait bound `(): std::iter::Iterator` is not satisfied [E0277]
+    //~^ ERROR `()` is not an iterator
 }
 
 fn main() {}

src/test/ui/conservative_impl_trait.stderr

@@ -1,8 +1,8 @@
-error[E0277]: the trait bound `(): std::iter::Iterator` is not satisfied
+error[E0277]: `()` is not an iterator
   --> $DIR/conservative_impl_trait.rs:13:33
    |
 LL | fn will_ice(something: &u32) -> impl Iterator<Item = &u32> {
-   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not an iterator; maybe try calling `.iter()` or a similar method
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not an iterator
    |
    = help: the trait `std::iter::Iterator` is not implemented for `()`
    = note: the return type of a function must have a statically known size

src/test/ui/feature-gates/feature-gate-trivial_bounds.stderr

@@ -75,15 +75,16 @@ LL | | }
    = help: see issue #48214
    = help: add #![feature(trivial_bounds)] to the crate attributes to enable
 
-error[E0277]: the trait bound `i32: std::iter::Iterator` is not satisfied
+error[E0277]: `i32` is not an iterator
   --> $DIR/feature-gate-trivial_bounds.rs:50:1
    |
 LL | / fn use_for() where i32: Iterator { //~ ERROR
 LL | |     for _ in 2i32 {}
 LL | | }
-   | |_^ `i32` is not an iterator; maybe try calling `.iter()` or a similar method
+   | |_^ `i32` is not an iterator
    |
    = help: the trait `std::iter::Iterator` is not implemented for `i32`
+   = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end`
    = help: see issue #48214
    = help: add #![feature(trivial_bounds)] to the crate attributes to enable
 

src/test/ui/for/for-c-in-str.rs

@@ -12,7 +12,7 @@
 
 fn main() {
     for c in "asdf" {
-    //~^ ERROR the trait bound `&str: std::iter::Iterator` is not satisfied
+    //~^ ERROR `&str` is not an iterator
     //~| NOTE `&str` is not an iterator
     //~| HELP the trait `std::iter::Iterator` is not implemented for `&str`
     //~| NOTE required by `std::iter::IntoIterator::into_iter`

src/test/ui/for/for-c-in-str.stderr

@@ -1,4 +1,4 @@
-error[E0277]: the trait bound `&str: std::iter::Iterator` is not satisfied
+error[E0277]: `&str` is not an iterator
   --> $DIR/for-c-in-str.rs:14:14
    |
 LL |     for c in "asdf" {

src/test/ui/for/for-loop-bogosity.rs

@@ -24,7 +24,8 @@ pub fn main() {
         x: 1,
         y: 2,
     };
-    for x in bogus { //~ ERROR `MyStruct: std::iter::Iterator` is not satisfied
+    for x in bogus {
+    //~^ ERROR `MyStruct` is not an iterator
         drop(x);
     }
 }

src/test/ui/for/for-loop-bogosity.stderr

@@ -1,8 +1,8 @@
-error[E0277]: the trait bound `MyStruct: std::iter::Iterator` is not satisfied
+error[E0277]: `MyStruct` is not an iterator
   --> $DIR/for-loop-bogosity.rs:27:14
    |
-LL |     for x in bogus { //~ ERROR `MyStruct: std::iter::Iterator` is not satisfied
-   |              ^^^^^ `MyStruct` is not an iterator; maybe try calling `.iter()` or a similar method
+LL |     for x in bogus {
+   |              ^^^^^ `MyStruct` is not an iterator
    |
    = help: the trait `std::iter::Iterator` is not implemented for `MyStruct`
    = note: required by `std::iter::IntoIterator::into_iter`

src/test/ui/issues/issue-22872.rs

@@ -0,0 +1,23 @@
+trait Wrap<'b> {
+    fn foo(&'b mut self);
+}
+
+struct Wrapper<P>(P);
+
+impl<'b, P> Wrap<'b> for Wrapper<P>
+where P: Process<'b>,
+      <P as Process<'b>>::Item: Iterator {
+    fn foo(&mut self) {}
+}
+
+
+pub trait Process<'a> {
+    type Item;
+    fn bar(&'a self);
+}
+
+fn push_process<P>(process: P) where P: Process<'static> {
+    let _: Box<for<'b> Wrap<'b>> = Box::new(Wrapper(process));
+}
+
+fn main() {}