bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #126054 - veera-sivarajan:bugfix-113073-bound-on-generics-2, r=fee1-dead

Browse source 
`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration

Fixes #113073

This PR suggests  `impl<T: Bound> Trait<T> for Foo` when finding `impl Trait<T: Bound> for Foo`. Tangentially, it also improves a handful of other error messages.

It accomplishes this in two steps:
1. Check if constrained arguments and parameter names appear in the same order and delay emitting "incorrect number of generic arguments" error because it can be confusing for the programmer to see `0 generic arguments provided` when there are `n` constrained generic arguments.

2. Inside `E0229`, suggest declaring the type parameter right after the `impl` keyword by finding the relevant impl block's span for type parameter declaration. This also handles lifetime declarations correctly.

Also, the multi part suggestion doesn't use the fluent error mechanism because translating all the errors to fluent style feels outside the scope of this PR. I will handle it in a separate PR if this gets approved.
This commit is contained in:
Matthias Krüger 2024-06-14 12:23:36 +02:00 • committed by GitHub
commit bfe032334f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 263 additions and 101 deletions
Download patch file Download diff file Expand all files Collapse all files

71
compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file

@ -1217,7 +1217,6 @@ pub fn prohibit_assoc_item_constraint(
// otherwise suggest the removal of the binding.
if let Some((def_id, segment, _)) = segment
&& segment.args().parenthesized == hir::GenericArgsParentheses::No
&& let hir::AssocItemConstraintKind::Equality { term } = constraint.kind
{
// Suggests removal of the offending binding
let suggest_removal = |e: &mut Diag<'_>| {
@ -1263,7 +1262,7 @@ pub fn prohibit_assoc_item_constraint(
if let Ok(suggestion) = tcx.sess.source_map().span_to_snippet(removal_span) {
e.span_suggestion_verbose(
removal_span,
"consider removing this associated item binding",
format!("consider removing this associated item {}", constraint.kind.descr()),
suggestion,
Applicability::MaybeIncorrect,
);
@ -1286,19 +1285,73 @@ pub fn prohibit_assoc_item_constraint(
// Check if the type has a generic param with the same name
// as the assoc type name in the associated item binding.
let generics = tcx.generics_of(def_id);
let matching_param =
generics.own_params.iter().find(|p| p.name.as_str() == constraint.ident.as_str());
let matching_param = generics.own_params.iter().find(|p| p.name == constraint.ident.name);
// Now emit the appropriate suggestion
if let Some(matching_param) = matching_param {
match (&matching_param.kind, term) {
(GenericParamDefKind::Type { .. }, hir::Term::Ty(ty)) => {
suggest_direct_use(&mut err, ty.span);
}
(GenericParamDefKind::Const { .. }, hir::Term::Const(c)) => {
match (constraint.kind, &matching_param.kind) {
(
hir::AssocItemConstraintKind::Equality { term: hir::Term::Ty(ty) },
GenericParamDefKind::Type { .. },
) => suggest_direct_use(&mut err, ty.span),
(
hir::AssocItemConstraintKind::Equality { term: hir::Term::Const(c) },
GenericParamDefKind::Const { .. },
) => {
let span = tcx.hir().span(c.hir_id);
suggest_direct_use(&mut err, span);
}
(hir::AssocItemConstraintKind::Bound { bounds }, _) => {
// Suggest `impl<T: Bound> Trait<T> for Foo` when finding
// `impl Trait<T: Bound> for Foo`
// Get the parent impl block based on the binding we have
// and the trait DefId
let impl_block = tcx
.hir()
.parent_iter(constraint.hir_id)
.find_map(|(_, node)| node.impl_block_of_trait(def_id));
let type_with_constraints =
tcx.sess.source_map().span_to_snippet(constraint.span);
if let Some(impl_block) = impl_block
&& let Ok(type_with_constraints) = type_with_constraints
{
// Filter out the lifetime parameters because
// they should be declared before the type parameter
let lifetimes: String = bounds
.iter()
.filter_map(|bound| {
if let hir::GenericBound::Outlives(lifetime) = bound {
Some(format!("{lifetime}, "))
} else {
None
}
})
.collect();
// Figure out a span and suggestion string based on
// whether there are any existing parameters
let param_decl = if let Some(param_span) =
impl_block.generics.span_for_param_suggestion()
{
(param_span, format!(", {lifetimes}{type_with_constraints}"))
} else {
(
impl_block.generics.span.shrink_to_lo(),
format!("<{lifetimes}{type_with_constraints}>"),
)
};
let suggestions =
vec![param_decl, (constraint.span, format!("{}", matching_param.name))];
err.multipart_suggestion_verbose(
format!("declare the type parameter right after the `impl` keyword"),
suggestions,
Applicability::MaybeIncorrect,
);
}
}
_ => suggest_removal(&mut err),
}
} else {

17
compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs
View file

@ -531,6 +531,7 @@ pub(crate) fn check_generic_arg_count(
let num_default_params = expected_max - expected_min;
let mut all_params_are_binded = false;
let gen_args_info = if provided > expected_max {
invalid_args.extend((expected_max..provided).map(|i| i + args_offset));
let num_redundant_args = provided - expected_max;
@ -547,6 +548,20 @@ pub(crate) fn check_generic_arg_count(
} else {
let num_missing_args = expected_max - provided;
let constraint_names: Vec<_> =
gen_args.constraints.iter().map(|b| b.ident.name).collect();
let param_names: Vec<_> = gen_params
.own_params
.iter()
.filter(|param| !has_self || param.index != 0) // Assumes `Self` will always be the first parameter
.map(|param| param.name)
.collect();
if constraint_names == param_names {
// We set this to true and delay emitting `WrongNumberOfGenericArgs`
// to provide a succinct error for cases like issue #113073
all_params_are_binded = true;
};
GenericArgsInfo::MissingTypesOrConsts {
num_missing_args,
num_default_params,
@ -567,7 +582,7 @@ pub(crate) fn check_generic_arg_count(
def_id,
)
.diagnostic()
.emit()
.emit_unless(all_params_are_binded)
});
Err(reported)