Skip to content

Commit

Permalink
Rollup merge of #69942 - estebank:sized-verbose-sugg, r=matthewjasper
Browse files Browse the repository at this point in the history
Increase verbosity when suggesting subtle code changes

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

Fix #69243.
  • Loading branch information
Centril authored Mar 23, 2020
2 parents 61a56fb + 9175940 commit 906b399
Show file tree
Hide file tree
Showing 137 changed files with 818 additions and 663 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
16 changes: 12 additions & 4 deletions src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ impl<'a, 'tcx> NamePrivacyVisitor<'a, 'tcx> {
def.variant_descr(),
self.tcx.def_path_str(def.did)
)
.span_label(span, format!("field `{}` is private", field.ident))
.span_label(span, "private field")
.emit();
}
}
Expand Down Expand Up @@ -1180,7 +1180,11 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, 'tcx> {
fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool {
let is_error = !self.item_is_accessible(def_id);
if is_error {
self.tcx.sess.span_err(self.span, &format!("{} `{}` is private", kind, descr));
self.tcx
.sess
.struct_span_err(self.span, &format!("{} `{}` is private", kind, descr))
.span_label(self.span, &format!("private {}", kind))
.emit();
}
is_error
}
Expand Down Expand Up @@ -1313,8 +1317,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
hir::QPath::Resolved(_, ref path) => path.to_string(),
hir::QPath::TypeRelative(_, ref segment) => segment.ident.to_string(),
};
let msg = format!("{} `{}` is private", kind.descr(def_id), name);
self.tcx.sess.span_err(span, &msg);
let kind = kind.descr(def_id);
self.tcx
.sess
.struct_span_err(span, &format!("{} `{}` is private", kind, name))
.span_label(span, &format!("private {}", kind))
.emit();
return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ impl<'a> Resolver<'a> {
let descr = get_descr(binding);
let mut err =
struct_span_err!(self.session, ident.span, E0603, "{} `{}` is private", descr, ident);
err.span_label(ident.span, &format!("this {} is private", descr));
err.span_label(ident.span, &format!("private {}", descr));
if let Some(span) = ctor_fields_span {
err.span_label(span, "a constructor is private if any of the fields is private");
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_resolve/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,10 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {

match (res, source) {
(Res::Def(DefKind::Macro(MacroKind::Bang), _), _) => {
err.span_suggestion(
span,
err.span_suggestion_verbose(
span.shrink_to_hi(),
"use `!` to invoke the macro",
format!("{}!", path_str),
"!".to_string(),
Applicability::MaybeIncorrect,
);
if path_str == "try" && span.rust_2015() {
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
17 changes: 13 additions & 4 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1452,8 +1452,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
.expect("missing associated type");

if !assoc_ty.vis.is_accessible_from(def_scope, tcx) {
let msg = format!("associated type `{}` is private", binding.item_name);
tcx.sess.span_err(binding.span, &msg);
tcx.sess
.struct_span_err(
binding.span,
&format!("associated type `{}` is private", binding.item_name),
)
.span_label(binding.span, "private associated type")
.emit();
}
tcx.check_stability(assoc_ty.def_id, Some(hir_ref_id), binding.span);

Expand Down Expand Up @@ -2315,8 +2320,12 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {

let kind = DefKind::AssocTy;
if !item.vis.is_accessible_from(def_scope, tcx) {
let msg = format!("{} `{}` is private", kind.descr(item.def_id), assoc_ident);
tcx.sess.span_err(span, &msg);
let kind = kind.descr(item.def_id);
let msg = format!("{} `{}` is private", kind, assoc_ident);
tcx.sess
.struct_span_err(span, &msg)
.span_label(span, &format!("private {}", kind))
.emit();
}
tcx.check_stability(item.def_id, Some(hir_ref_id), span);

Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1580,13 +1580,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
let mut err = struct_span_err!(
self.tcx().sess,
expr.span,
field.span,
E0616,
"field `{}` of {} `{}` is private",
field,
kind_name,
struct_path
);
err.span_label(field.span, "private field");
// Also check if an accessible method exists, which is often what is meant.
if self.method_exists(field, expr_t, expr.hir_id, false) && !self.expr_in_place(expr.hir_id)
{
Expand All @@ -1611,7 +1612,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
field,
expr_t
);

err.span_label(field.span, "method, not a field");
if !self.expr_in_place(expr.hir_id) {
self.suggest_method_call(
&mut err,
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
10 changes: 6 additions & 4 deletions src/librustc_typeck/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,25 +758,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
MethodError::Ambiguity(sources) => {
let mut err = struct_span_err!(
self.sess(),
span,
item_name.span,
E0034,
"multiple applicable items in scope"
);
err.span_label(span, format!("multiple `{}` found", item_name));
err.span_label(item_name.span, format!("multiple `{}` found", item_name));

report_candidates(span, &mut err, sources, sugg_span);
err.emit();
}

MethodError::PrivateMatch(kind, def_id, out_of_scope_traits) => {
let kind = kind.descr(def_id);
let mut err = struct_span_err!(
self.tcx.sess,
span,
item_name.span,
E0624,
"{} `{}` is private",
kind.descr(def_id),
kind,
item_name
);
err.span_label(item_name.span, &format!("private {}", kind));
self.suggest_valid_traits(&mut err, out_of_scope_traits);
err.emit();
}
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 @@ -4939,15 +4939,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
Loading

0 comments on commit 906b399

Please sign in to comment.