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

Suggest removing &s #48834

Merged
merged 12 commits into from
Mar 20, 2018
49 changes: 49 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand Down Expand Up @@ -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
Expand Down
96 changes: 50 additions & 46 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/suggest-remove-refs-1.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
15 changes: 15 additions & 0 deletions src/test/ui/suggest-remove-refs-1.stderr
Original file line number Diff line number Diff line change
@@ -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`.
18 changes: 18 additions & 0 deletions src/test/ui/suggest-remove-refs-2.rs
Original file line number Diff line number Diff line change
@@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also add another test where the above uses multiple lines?

for (i, n) in & & & &v
    .iter()
    .enumerate()
{
    println!("{} {}", i, n);
}

//~^ ERROR the trait bound
println!("{}", i);
}
}
15 changes: 15 additions & 0 deletions src/test/ui/suggest-remove-refs-2.stderr
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

The output is great!

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, this is good

|
= 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`.
21 changes: 21 additions & 0 deletions src/test/ui/suggest-remove-refs-3.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
19 changes: 19 additions & 0 deletions src/test/ui/suggest-remove-refs-3.stderr
Original file line number Diff line number Diff line change
@@ -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`.