Skip to content

Commit

Permalink
Rollup merge of rust-lang#69942 - estebank:sized-verbose-sugg, r=matt…
Browse files Browse the repository at this point in the history
…hewjasper

Increase verbosity when suggesting subtle code changes

Do not suggest changes that are actually quite small inline, to minimize the likelihood of confusion.

Fix rust-lang#69243.
  • Loading branch information
Dylan-DPC authored Mar 14, 2020
2 parents af0cdbf + 7995a08 commit b8d85a3
Show file tree
Hide file tree
Showing 32 changed files with 361 additions and 240 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
13 changes: 6 additions & 7 deletions src/librustc_infer/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc::ty::{
};
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
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_id::{DefId, LOCAL_CRATE};
use rustc_hir::{QPath, TyKind, WhereBoundPredicate, WherePredicate};
Expand Down Expand Up @@ -1186,15 +1186,14 @@ impl<'a, 'tcx> 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 @@ -1356,7 +1355,7 @@ impl<'a, 'tcx> 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
24 changes: 13 additions & 11 deletions src/librustc_infer/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl<'a, 'tcx> 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 @@ -276,7 +276,8 @@ impl<'a, 'tcx> 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 @@ -297,7 +298,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
})
.collect::<Vec<_>>()
.join(", ");
format!("{}({})", ident, args)
let sugg = format!("({})", args);
(format!("{}{}", ident, sugg), sugg)
}
_ => return,
};
Expand All @@ -306,10 +308,10 @@ impl<'a, 'tcx> 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 @@ -494,7 +496,7 @@ impl<'a, 'tcx> 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 Expand Up @@ -898,11 +900,11 @@ impl<'a, 'tcx> 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 @@ -916,7 +918,7 @@ impl<'a, 'tcx> 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 @@ -953,7 +955,7 @@ impl<'a, 'tcx> 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
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 @@ -136,7 +136,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 @@ -146,26 +146,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 @@ -4951,15 +4951,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 @@ -752,17 +752,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
Loading

0 comments on commit b8d85a3

Please sign in to comment.