Standardize arg suggestions between typeck and trait selection

This commit is contained in:
Michael Goulet 2022-10-09 21:39:57 +00:00
parent b3edd9f775
commit f5336a969c
4 changed files with 60 additions and 54 deletions

View file

@ -90,7 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if ty.is_suggestable(self.tcx, false) { if ty.is_suggestable(self.tcx, false) {
format!("/* {ty} */") format!("/* {ty} */")
} else { } else {
"".to_string() "/* value */".to_string()
} }
}) })
.collect::<Vec<_>>() .collect::<Vec<_>>()

View file

@ -21,6 +21,7 @@ use rustc_hir::lang_items::LangItem;
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node}; use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
use rustc_infer::infer::error_reporting::TypeErrCtxt; use rustc_infer::infer::error_reporting::TypeErrCtxt;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::hir::map; use rustc_middle::hir::map;
use rustc_middle::ty::{ use rustc_middle::ty::{
self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree, self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree,
@ -28,7 +29,7 @@ use rustc_middle::ty::{
ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable,
}; };
use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_middle::ty::{TypeAndMut, TypeckResults};
use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{BytePos, DesugaringKind, ExpnKind, Span, DUMMY_SP}; use rustc_span::{BytePos, DesugaringKind, ExpnKind, Span, DUMMY_SP};
use rustc_target::spec::abi; use rustc_target::spec::abi;
use std::fmt; use std::fmt;
@ -814,80 +815,85 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// Skipping binder here, remapping below // Skipping binder here, remapping below
let self_ty = trait_pred.self_ty().skip_binder(); let self_ty = trait_pred.self_ty().skip_binder();
let (def_id, output_ty, callable) = match *self_ty.kind() { let (def_id, inputs, output, kind) = match *self_ty.kind() {
ty::Closure(def_id, substs) => (def_id, substs.as_closure().sig().output(), "closure"), ty::Closure(def_id, substs) => {
ty::FnDef(def_id, _) => ( let sig = substs.as_closure().sig();
def_id, (def_id, sig.inputs().map_bound(|inputs| &inputs[1..]), sig.output(), "closure")
self_ty.fn_sig(self.tcx).output(), }
match self.tcx.def_kind(def_id) { ty::FnDef(def_id, _) => {
DefKind::Ctor(..) => "constructor", let sig = self_ty.fn_sig(self.tcx);
_ => "function", (
}, def_id,
), sig.inputs(),
sig.output(),
match self.tcx.def_kind(def_id) {
DefKind::Ctor(..) => "constructor",
_ => "function",
},
)
}
_ => return false, _ => return false,
}; };
let msg = format!("use parentheses to call the {}", callable); let output = self.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
// "We should really create a single list of bound vars from the combined vars LateBoundRegionConversionTime::FnCall,
// from the predicate and function, but instead we just liberate the function bound vars" output,
let output_ty = self.tcx.liberate_late_bound_regions(def_id, output_ty); );
let inputs = inputs.skip_binder().iter().map(|ty| {
self.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
LateBoundRegionConversionTime::FnCall,
inputs.rebind(*ty),
)
});
// Remapping bound vars here // Remapping bound vars here
let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, output_ty)); let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, output));
let new_obligation = let new_obligation =
self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred_and_self); self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred_and_self);
if !self.predicate_must_hold_modulo_regions(&new_obligation) { if !self.predicate_must_hold_modulo_regions(&new_obligation) {
return false; return false;
} }
let hir = self.tcx.hir();
// Get the name of the callable and the arguments to be used in the suggestion. // Get the name of the callable and the arguments to be used in the suggestion.
let (snippet, sugg) = match hir.get_if_local(def_id) { let hir = self.tcx.hir();
let msg = format!("use parentheses to call the {}", kind);
let args = inputs
.map(|ty| {
if ty.is_suggestable(self.tcx, false) {
format!("/* {ty} */")
} else {
"/* value */".to_string()
}
})
.collect::<Vec<_>>()
.join(", ");
let name = match hir.get_if_local(def_id) {
Some(hir::Node::Expr(hir::Expr { Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { fn_decl, fn_decl_span, .. }), kind: hir::ExprKind::Closure(hir::Closure { fn_decl_span, .. }),
.. ..
})) => { })) => {
err.span_label(*fn_decl_span, "consider calling this closure"); err.span_label(*fn_decl_span, "consider calling this closure");
let Some(name) = self.get_closure_name(def_id, err, &msg) else { let Some(name) = self.get_closure_name(def_id, err, &msg) else {
return false; return false;
}; };
let args = fn_decl.inputs.iter().map(|_| "_").collect::<Vec<_>>().join(", "); name.to_string()
let sugg = format!("({})", args);
(format!("{}{}", name, sugg), sugg)
} }
Some(hir::Node::Item(hir::Item { Some(hir::Node::Item(hir::Item { ident, kind: hir::ItemKind::Fn(..), .. })) => {
ident,
kind: hir::ItemKind::Fn(.., body_id),
..
})) => {
err.span_label(ident.span, "consider calling this function"); err.span_label(ident.span, "consider calling this function");
let body = hir.body(*body_id); ident.to_string()
let args = body
.params
.iter()
.map(|arg| match &arg.pat.kind {
hir::PatKind::Binding(_, _, ident, None)
// FIXME: provide a better suggestion when encountering `SelfLower`, it
// should suggest a method call.
if ident.name != kw::SelfLower => ident.to_string(),
_ => "_".to_string(),
})
.collect::<Vec<_>>()
.join(", ");
let sugg = format!("({})", args);
(format!("{}{}", ident, sugg), sugg)
} }
Some(hir::Node::Ctor(data)) => { Some(hir::Node::Ctor(..)) => {
let name = self.tcx.def_path_str(def_id); let name = self.tcx.def_path_str(def_id);
err.span_label( err.span_label(
self.tcx.def_span(def_id), self.tcx.def_span(def_id),
format!("consider calling the constructor for `{}`", name), format!("consider calling the constructor for `{}`", name),
); );
let args = data.fields().iter().map(|_| "_").collect::<Vec<_>>().join(", "); name
let sugg = format!("({})", args);
(format!("{name}{sugg}"), sugg)
} }
_ => return false, _ => return false,
}; };
@ -901,11 +907,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err.span_suggestion_verbose( err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(), obligation.cause.span.shrink_to_hi(),
&msg, &msg,
sugg, format!("({args})"),
Applicability::HasPlaceholders, Applicability::HasPlaceholders,
); );
} else { } else {
err.help(&format!("{}: `{}`", msg, snippet)); err.help(&format!("{msg}: `{name}({args})`"));
} }
true true
} }

View file

@ -19,7 +19,7 @@ LL | assert_eq!(foo, y);
| ^^^^^^^^^^^^^^^^^^ `for<'a> fn(&'a i32) -> &'a i32 {foo}` cannot be formatted using `{:?}` because it doesn't implement `Debug` | ^^^^^^^^^^^^^^^^^^ `for<'a> fn(&'a i32) -> &'a i32 {foo}` cannot be formatted using `{:?}` because it doesn't implement `Debug`
| |
= help: the trait `Debug` is not implemented for fn item `for<'a> fn(&'a i32) -> &'a i32 {foo}` = help: the trait `Debug` is not implemented for fn item `for<'a> fn(&'a i32) -> &'a i32 {foo}`
= help: use parentheses to call the function: `foo(s)` = help: use parentheses to call the function: `foo(/* &i32 */)`
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to 2 previous errors error: aborting due to 2 previous errors

View file

@ -16,8 +16,8 @@ LL | fn insert_resource<R: Resource>(resource: R) {}
| ^^^^^^^^ required by this bound in `insert_resource` | ^^^^^^^^ required by this bound in `insert_resource`
help: use parentheses to call the constructor help: use parentheses to call the constructor
| |
LL | insert_resource(Time(_)); LL | insert_resource(Time(/* u32 */));
| +++ | +++++++++++
error: aborting due to previous error error: aborting due to previous error