Rollup merge of rust-lang#48834 - ysiraichi:suggest-remove-ref, r=estebank

Suggest removing `&`s

This implements the error message discussed in rust-lang#47744.
We check whether removing each `&` yields a type that satisfies the requested obligation.
Also, it was created a new `NodeId` field in `ObligationCause` in order to iterate through the `&`s. The way it's implemented now, it iterates through the obligation snippet and counts the number of `&`.

r? @estebank

Author: kennytm

src/librustc/traits/error_reporting.rs

@@ -606,6 +606,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                         }
 
                         self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
+                        self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
 
                         // Try to report a help message
                         if !trait_ref.has_infer_types() &&
@@ -844,6 +845,54 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    /// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`,
+    /// suggest removing these references until we reach a type that implements the trait.
+    fn suggest_remove_reference(&self,
+                                obligation: &PredicateObligation<'tcx>,
+                                err: &mut DiagnosticBuilder<'tcx>,
+                                trait_ref: &ty::Binder<ty::TraitRef<'tcx>>) {
+        let ty::Binder(trait_ref) = trait_ref;
+        let span = obligation.cause.span;
+
+        if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) {
+            let refs_number = snippet.chars()
+                .filter(|c| !c.is_whitespace())
+                .take_while(|c| *c == '&')
+                .count();
+
+            let mut trait_type = trait_ref.self_ty();
+            let mut selcx = SelectionContext::new(self);
+
+            for refs_remaining in 0..refs_number {
+                if let ty::TypeVariants::TyRef(_, ty::TypeAndMut{ ty: t_type, mutbl: _ }) =
+                    trait_type.sty {
+
+                    trait_type = t_type;
+
+                    let substs = self.tcx.mk_substs_trait(trait_type, &[]);
+                    let new_trait_ref = ty::TraitRef::new(trait_ref.def_id, substs);
+                    let new_obligation = Obligation::new(ObligationCause::dummy(),
+                                                         obligation.param_env,
+                                                         new_trait_ref.to_predicate());
+
+                    if selcx.evaluate_obligation(&new_obligation) {
+                        let sp = self.tcx.sess.codemap()
+                            .span_take_while(span, |c| c.is_whitespace() || *c == '&');
+
+                        let remove_refs = refs_remaining + 1;
+                        let format_str = format!("consider removing {} leading `&`-references",
+                                                 remove_refs);
+
+                        err.span_suggestion_short(sp, &format_str, String::from(""));
+                        break;
+                    }
+                } else {
+                    break;
+                }
+            }
+        }
+    }
+
     /// Given some node representing a fn-like thing in the HIR map,
     /// returns a span and `ArgKind` information that describes the
     /// arguments it expects. This can be supplied to

src/libsyntax/codemap.rs

@@ -597,21 +597,6 @@ impl CodeMap {
         self.span_to_source(sp, |src, start_index, _| src[..start_index].to_string())
     }
 
-    /// Given a `Span`, try to get a shorter span ending before the first occurrence of `c` `char`
-    pub fn span_until_char(&self, sp: Span, c: char) -> Span {
-        match self.span_to_snippet(sp) {
-            Ok(snippet) => {
-                let snippet = snippet.split(c).nth(0).unwrap_or("").trim_right();
-                if !snippet.is_empty() && !snippet.contains('\n') {
-                    sp.with_hi(BytePos(sp.lo().0 + snippet.len() as u32))
-                } else {
-                    sp
-                }
-            }
-            _ => sp,
-        }
-    }
-
     /// Extend the given `Span` to just after the previous occurrence of `c`. Return the same span
     /// if no character could be found or if an error occurred while retrieving the code snippet.
     pub fn span_extend_to_prev_char(&self, sp: Span, c: char) -> Span {
@@ -646,55 +631,74 @@ impl CodeMap {
         sp
     }
 
+    /// Given a `Span`, try to get a shorter span ending before the first occurrence of `c` `char`
+    pub fn span_until_char(&self, sp: Span, c: char) -> Span {
+        match self.span_to_snippet(sp) {
+            Ok(snippet) => {
+                let snippet = snippet.split(c).nth(0).unwrap_or("").trim_right();
+                if !snippet.is_empty() && !snippet.contains('\n') {
+                    sp.with_hi(BytePos(sp.lo().0 + snippet.len() as u32))
+                } else {
+                    sp
+                }
+            }
+            _ => sp,
+        }
+    }
+
+    /// Given a `Span`, try to get a shorter span ending just after the first occurrence of `char`
+    /// `c`.
+    pub fn span_through_char(&self, sp: Span, c: char) -> Span {
+        if let Ok(snippet) = self.span_to_snippet(sp) {
+            if let Some(offset) = snippet.find(c) {
+                return sp.with_hi(BytePos(sp.lo().0 + (offset + c.len_utf8()) as u32));
+            }
+        }
+        sp
+    }
+
     /// Given a `Span`, get a new `Span` covering the first token and all its trailing whitespace or
     /// the original `Span`.
     ///
     /// If `sp` points to `"let mut x"`, then a span pointing at `"let "` will be returned.
     pub fn span_until_non_whitespace(&self, sp: Span) -> Span {
-        if let Ok(snippet) = self.span_to_snippet(sp) {
-            let mut offset = 0;
-            // get the bytes width of all the non-whitespace characters
-            for c in snippet.chars().take_while(|c| !c.is_whitespace()) {
-                offset += c.len_utf8();
-            }
-            // get the bytes width of all the whitespace characters after that
-            for c in snippet[offset..].chars().take_while(|c| c.is_whitespace()) {
-                offset += c.len_utf8();
+        let mut whitespace_found = false;
+
+        self.span_take_while(sp, |c| {
+            if !whitespace_found && c.is_whitespace() {
+                whitespace_found = true;
             }
-            if offset > 1 {
-                return sp.with_hi(BytePos(sp.lo().0 + offset as u32));
+
+            if whitespace_found && !c.is_whitespace() {
+                false
+            } else {
+                true
             }
-        }
-        sp
+        })
     }
 
     /// Given a `Span`, get a new `Span` covering the first token without its trailing whitespace or
     /// the original `Span` in case of error.
     ///
     /// If `sp` points to `"let mut x"`, then a span pointing at `"let"` will be returned.
     pub fn span_until_whitespace(&self, sp: Span) -> Span {
-        if let Ok(snippet) = self.span_to_snippet(sp) {
-            let mut offset = 0;
-            // Get the bytes width of all the non-whitespace characters
-            for c in snippet.chars().take_while(|c| !c.is_whitespace()) {
-                offset += c.len_utf8();
-            }
-            if offset > 1 {
-                return sp.with_hi(BytePos(sp.lo().0 + offset as u32));
-            }
-        }
-        sp
+        self.span_take_while(sp, |c| !c.is_whitespace())
     }
 
-    /// Given a `Span`, try to get a shorter span ending just after the first occurrence of `char`
-    /// `c`.
-    pub fn span_through_char(&self, sp: Span, c: char) -> Span {
+    /// Given a `Span`, get a shorter one until `predicate` yields false.
+    pub fn span_take_while<P>(&self, sp: Span, predicate: P) -> Span
+        where P: for <'r> FnMut(&'r char) -> bool
+    {
         if let Ok(snippet) = self.span_to_snippet(sp) {
-            if let Some(offset) = snippet.find(c) {
-                return sp.with_hi(BytePos(sp.lo().0 + (offset + c.len_utf8()) as u32));
-            }
+            let offset = snippet.chars()
+                .take_while(predicate)
+                .map(|c| c.len_utf8())
+                .sum::<usize>();
+
+            sp.with_hi(BytePos(sp.lo().0 + (offset as u32)))
+        } else {
+            sp
         }
-        sp
     }
 
     pub fn def_span(&self, sp: Span) -> Span {

src/test/ui/suggest-remove-refs-1.rs

@@ -0,0 +1,18 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let v = vec![0, 1, 2, 3];
+
+    for (i, n) in &v.iter().enumerate() {
+        //~^ ERROR the trait bound
+        println!("{}", i);
+    }
+}

src/test/ui/suggest-remove-refs-1.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>: std::iter::Iterator` is not satisfied
+  --> $DIR/suggest-remove-refs-1.rs:14:19
+   |
+LL |     for (i, n) in &v.iter().enumerate() {
+   |                   -^^^^^^^^^^^^^^^^^^^^
+   |                   |
+   |                   `&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>` is not an iterator; maybe try calling `.iter()` or a similar method
+   |                   help: consider removing 1 leading `&`-references
+   |
+   = help: the trait `std::iter::Iterator` is not implemented for `&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>`
+   = note: required by `std::iter::IntoIterator::into_iter`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggest-remove-refs-2.rs

@@ -0,0 +1,18 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let v = vec![0, 1, 2, 3];
+
+    for (i, n) in & & & & &v.iter().enumerate() {
+        //~^ ERROR the trait bound
+        println!("{}", i);
+    }
+}

src/test/ui/suggest-remove-refs-2.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>: std::iter::Iterator` is not satisfied
+  --> $DIR/suggest-remove-refs-2.rs:14:19
+   |
+LL |     for (i, n) in & & & & &v.iter().enumerate() {
+   |                   ---------^^^^^^^^^^^^^^^^^^^^
+   |                   |
+   |                   `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>` is not an iterator; maybe try calling `.iter()` or a similar method
+   |                   help: consider removing 5 leading `&`-references
+   |
+   = help: the trait `std::iter::Iterator` is not implemented for `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>`
+   = note: required by `std::iter::IntoIterator::into_iter`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggest-remove-refs-3.rs

@@ -0,0 +1,21 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let v = vec![0, 1, 2, 3];
+
+    for (i, n) in & & &
+        & &v
+        .iter()
+        .enumerate() {
+        //~^^^^ ERROR the trait bound
+        println!("{}", i);
+    }
+}

src/test/ui/suggest-remove-refs-3.stderr

@@ -0,0 +1,19 @@
+error[E0277]: the trait bound `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>: std::iter::Iterator` is not satisfied
+  --> $DIR/suggest-remove-refs-3.rs:14:19
+   |
+LL |        for (i, n) in & & &
+   |   ___________________^
+   |  |___________________|
+   | ||
+LL | ||         & &v
+   | ||___________- help: consider removing 5 leading `&`-references
+LL | |          .iter()
+LL | |          .enumerate() {
+   | |_____________________^ `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>` is not an iterator; maybe try calling `.iter()` or a similar method
+   |
+   = help: the trait `std::iter::Iterator` is not implemented for `&&&&&std::iter::Enumerate<std::slice::Iter<'_, {integer}>>`
+   = note: required by `std::iter::IntoIterator::into_iter`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.