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

Increase verbosity when suggesting subtle code changes #69942

Merged
merged 4 commits into from
Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -952,7 +952,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 @@ -2316,8 +2321,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 @@ -1583,13 +1583,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 @@ -1614,7 +1615,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 @@ -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
Loading