Skip to content

Commit

Permalink
Increase verbosity when suggesting subtle code changes
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Mar 22, 2020
1 parent 5ae85f4 commit 52fbd3e
Show file tree
Hide file tree
Showing 32 changed files with 360 additions and 239 deletions.
17 changes: 6 additions & 11 deletions src/librustc_infer/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::infer::InferCtxt;
use rustc::hir::map::Map;
use rustc::ty::print::Print;
use rustc::ty::{self, DefIdTree, Infer, Ty, TyVar};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Namespace};
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
Expand Down Expand Up @@ -462,24 +462,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
e: &Expr<'_>,
err: &mut DiagnosticBuilder<'_>,
) {
if let (Ok(snippet), Some(tables), None) = (
self.tcx.sess.source_map().span_to_snippet(segment.ident.span),
self.in_progress_tables,
&segment.args,
) {
if let (Some(tables), None) = (self.in_progress_tables, &segment.args) {
let borrow = tables.borrow();
if let Some((DefKind::AssocFn, did)) = borrow.type_dependent_def(e.hir_id) {
let generics = self.tcx.generics_of(did);
if !generics.params.is_empty() {
err.span_suggestion(
segment.ident.span,
err.span_suggestion_verbose(
segment.ident.span.shrink_to_hi(),
&format!(
"consider specifying the type argument{} in the method call",
if generics.params.len() > 1 { "s" } else { "" },
pluralize!(generics.params.len()),
),
format!(
"{}::<{}>",
snippet,
"::<{}>",
generics
.params
.iter()
Expand Down
19 changes: 9 additions & 10 deletions src/librustc_trait_selection/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,11 +815,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// For example, if `expected_args_length` is 2, suggest `|_, _|`.
if found_args.is_empty() && is_closure {
let underscores = vec!["_"; expected_args.len()].join(", ");
err.span_suggestion(
err.span_suggestion_verbose(
pipe_span,
&format!(
"consider changing the closure to take and ignore the expected argument{}",
if expected_args.len() < 2 { "" } else { "s" }
pluralize!(expected_args.len())
),
format!("|{}|", underscores),
Applicability::MachineApplicable,
Expand All @@ -833,7 +833,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
.map(|(name, _)| name.to_owned())
.collect::<Vec<String>>()
.join(", ");
err.span_suggestion(
err.span_suggestion_verbose(
found_span,
"change the closure to take multiple arguments instead of a single tuple",
format!("|{}|", sugg),
Expand Down Expand Up @@ -870,7 +870,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
String::new()
},
);
err.span_suggestion(
err.span_suggestion_verbose(
found_span,
"change the closure to accept a tuple instead of individual arguments",
sugg,
Expand Down Expand Up @@ -1420,15 +1420,14 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
// |
// = note: cannot resolve `_: Tt`

err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_hi(),
&format!(
"consider specifying the type argument{} in the function call",
if generics.params.len() > 1 { "s" } else { "" },
pluralize!(generics.params.len()),
),
format!(
"{}::<{}>",
snippet,
"::<{}>",
generics
.params
.iter()
Expand Down Expand Up @@ -1590,7 +1589,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
[] => (span.shrink_to_hi(), ":"),
[.., bound] => (bound.span().shrink_to_hi(), " + "),
};
err.span_suggestion(
err.span_suggestion_verbose(
span,
"consider relaxing the implicit `Sized` restriction",
format!("{} ?Sized", separator),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
let hir = self.tcx.hir();
// Get the name of the callable and the arguments to be used in the suggestion.
let snippet = match hir.get_if_local(def_id) {
let (snippet, sugg) = match hir.get_if_local(def_id) {
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Closure(_, decl, _, span, ..),
..
Expand All @@ -401,7 +401,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
None => return,
};
let args = decl.inputs.iter().map(|_| "_").collect::<Vec<_>>().join(", ");
format!("{}({})", name, args)
let sugg = format!("({})", args);
(format!("{}{}", name, sugg), sugg)
}
Some(hir::Node::Item(hir::Item {
ident,
Expand All @@ -422,7 +423,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
})
.collect::<Vec<_>>()
.join(", ");
format!("{}({})", ident, args)
let sugg = format!("({})", args);
(format!("{}{}", ident, sugg), sugg)
}
_ => return,
};
Expand All @@ -431,10 +433,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// an argument, the `obligation.cause.span` points at the expression
// of the argument, so we can provide a suggestion. This is signaled
// by `points_at_arg`. Otherwise, we give a more general note.
err.span_suggestion(
obligation.cause.span,
err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(),
&msg,
snippet,
sugg,
Applicability::HasPlaceholders,
);
} else {
Expand Down Expand Up @@ -619,7 +621,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
.source_map()
.span_take_while(span, |c| c.is_whitespace() || *c == '&');
if points_at_arg && mutability == hir::Mutability::Not && refs_number > 0 {
err.span_suggestion(
err.span_suggestion_verbose(
sp,
"consider changing this borrow's mutability",
"&mut ".to_string(),
Expand Down
28 changes: 11 additions & 17 deletions src/librustc_typeck/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self_ty: Ty<'tcx>,
call_expr: &hir::Expr<'_>,
) {
let has_params = self
let params = self
.probe_for_name(
method_name.span,
probe::Mode::MethodCall,
Expand All @@ -147,26 +147,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
call_expr.hir_id,
ProbeScope::TraitsInScope,
)
.and_then(|pick| {
.map(|pick| {
let sig = self.tcx.fn_sig(pick.item.def_id);
Ok(sig.inputs().skip_binder().len() > 1)
});
sig.inputs().skip_binder().len().saturating_sub(1)
})
.unwrap_or(0);

// Account for `foo.bar<T>`;
let sugg_span = method_name.span.with_hi(call_expr.span.hi());
let snippet = self
.tcx
.sess
.source_map()
.span_to_snippet(sugg_span)
.unwrap_or_else(|_| method_name.to_string());
let (suggestion, applicability) = if has_params.unwrap_or_default() {
(format!("{}(...)", snippet), Applicability::HasPlaceholders)
} else {
(format!("{}()", snippet), Applicability::MaybeIncorrect)
};
let sugg_span = call_expr.span.shrink_to_hi();
let (suggestion, applicability) = (
format!("({})", (0..params).map(|_| "_").collect::<Vec<_>>().join(", ")),
if params > 0 { Applicability::HasPlaceholders } else { Applicability::MaybeIncorrect },
);

err.span_suggestion(sugg_span, msg, suggestion, applicability);
err.span_suggestion_verbose(sugg_span, msg, suggestion, applicability);
}

/// Performs method lookup. If lookup is successful, it will return the callee
Expand Down
16 changes: 7 additions & 9 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4941,15 +4941,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => {}
}
if let Ok(code) = self.sess().source_map().span_to_snippet(expr.span) {
err.span_suggestion(
expr.span,
&format!("use parentheses to {}", msg),
format!("{}({})", code, sugg_call),
applicability,
);
return true;
}
err.span_suggestion_verbose(
expr.span.shrink_to_hi(),
&format!("use parentheses to {}", msg),
format!("({})", sugg_call),
applicability,
);
return true;
}
false
}
Expand Down
24 changes: 14 additions & 10 deletions src/librustc_typeck/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,17 +753,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
res.descr(),
),
);
let (msg, sugg) = match parent_pat {
Some(Pat { kind: hir::PatKind::Struct(..), .. }) => (
"bind the struct field to a different name instead",
format!("{}: other_{}", ident, ident.as_str().to_lowercase()),
),
_ => (
"introduce a new binding instead",
format!("other_{}", ident.as_str().to_lowercase()),
),
match parent_pat {
Some(Pat { kind: hir::PatKind::Struct(..), .. }) => {
e.span_suggestion_verbose(
ident.span.shrink_to_hi(),
"bind the struct field to a different name instead",
format!(": other_{}", ident.as_str().to_lowercase()),
Applicability::HasPlaceholders,
);
}
_ => {
let msg = "introduce a new binding instead";
let sugg = format!("other_{}", ident.as_str().to_lowercase());
e.span_suggestion(ident.span, msg, sugg, Applicability::HasPlaceholders);
}
};
e.span_suggestion(ident.span, msg, sugg, Applicability::HasPlaceholders);
}
}
e.emit();
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/error-codes/E0615.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ error[E0615]: attempted to take value of method `method` on type `Foo`
--> $DIR/E0615.rs:11:7
|
LL | f.method;
| ^^^^^^ help: use parentheses to call the method: `method()`
| ^^^^^^
|
help: use parentheses to call the method
|
LL | f.method();
| ^^

error: aborting due to previous error

Expand Down
8 changes: 5 additions & 3 deletions src/test/ui/extern/extern-types-unsized.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ error[E0277]: the size for values of type `A` cannot be known at compilation tim
--> $DIR/extern-types-unsized.rs:22:20
|
LL | fn assert_sized<T>() { }
| ------------ -- help: consider relaxing the implicit `Sized` restriction: `: ?Sized`
| |
| required by this bound in `assert_sized`
| ------------ - required by this bound in `assert_sized`
...
LL | assert_sized::<A>();
| ^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `A`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
help: consider relaxing the implicit `Sized` restriction
|
LL | fn assert_sized<T: ?Sized>() { }
| ^^^^^^^^

error[E0277]: the size for values of type `A` cannot be known at compilation time
--> $DIR/extern-types-unsized.rs:25:5
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/implicit-method-bind.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ error[E0615]: attempted to take value of method `abs` on type `i32`
--> $DIR/implicit-method-bind.rs:2:20
|
LL | let _f = 10i32.abs;
| ^^^ help: use parentheses to call the method: `abs()`
| ^^^
|
help: use parentheses to call the method
|
LL | let _f = 10i32.abs();
| ^^

error: aborting due to previous error

Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/issues/issue-13853-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ error[E0615]: attempted to take value of method `get` on type `std::boxed::Box<(
--> $DIR/issue-13853-2.rs:5:43
|
LL | fn foo(res : Box<dyn ResponseHook>) { res.get }
| ^^^ help: use parentheses to call the method: `get()`
| ^^^
|
help: use parentheses to call the method
|
LL | fn foo(res : Box<dyn ResponseHook>) { res.get() }
| ^^

error: aborting due to previous error

Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/issues/issue-26472.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ error[E0616]: field `len` of struct `sub::S` is private
--> $DIR/issue-26472.rs:11:13
|
LL | let v = s.len;
| ^^---
| |
| help: a method `len` also exists, call it with parentheses: `len()`
| ^^^^^
|
help: a method `len` also exists, call it with parentheses
|
LL | let v = s.len();
| ^^

error[E0616]: field `len` of struct `sub::S` is private
--> $DIR/issue-26472.rs:12:5
Expand Down
10 changes: 6 additions & 4 deletions src/test/ui/issues/issue-35241.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ LL | struct Foo(u32);
| ---------------- fn(u32) -> Foo {Foo} defined here
LL |
LL | fn test() -> Foo { Foo }
| --- ^^^
| | |
| | expected struct `Foo`, found fn item
| | help: use parentheses to instantiate this tuple struct: `Foo(_)`
| --- ^^^ expected struct `Foo`, found fn item
| |
| expected `Foo` because of return type
|
= note: expected struct `Foo`
found fn item `fn(u32) -> Foo {Foo}`
help: use parentheses to instantiate this tuple struct
|
LL | fn test() -> Foo { Foo(_) }
| ^^^

error: aborting due to previous error

Expand Down
14 changes: 12 additions & 2 deletions src/test/ui/methods/method-missing-call.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,23 @@ error[E0615]: attempted to take value of method `get_x` on type `Point`
--> $DIR/method-missing-call.rs:22:26
|
LL | .get_x;
| ^^^^^ help: use parentheses to call the method: `get_x()`
| ^^^^^
|
help: use parentheses to call the method
|
LL | .get_x();
| ^^

error[E0615]: attempted to take value of method `filter_map` on type `std::iter::Filter<std::iter::Map<std::slice::Iter<'_, {integer}>, [closure@$DIR/method-missing-call.rs:27:20: 27:25]>, [closure@$DIR/method-missing-call.rs:28:23: 28:35]>`
--> $DIR/method-missing-call.rs:29:16
|
LL | .filter_map;
| ^^^^^^^^^^ help: use parentheses to call the method: `filter_map(...)`
| ^^^^^^^^^^
|
help: use parentheses to call the method
|
LL | .filter_map(_);
| ^^^

error: aborting due to 2 previous errors

Expand Down
9 changes: 5 additions & 4 deletions src/test/ui/question-mark-type-infer.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ error[E0284]: type annotations needed
--> $DIR/question-mark-type-infer.rs:12:21
|
LL | l.iter().map(f).collect()?
| ^^^^^^^
| |
| cannot infer type
| help: consider specifying the type argument in the method call: `collect::<B>`
| ^^^^^^^ cannot infer type
|
= note: cannot resolve `<_ as std::ops::Try>::Ok == _`
help: consider specifying the type argument in the method call
|
LL | l.iter().map(f).collect::<B>()?
| ^^^^^

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 52fbd3e

Please sign in to comment.