From acf90a868e1a04751d1aad7e842c37e306e9987d Mon Sep 17 00:00:00 2001 From: Veera Date: Wed, 5 Jun 2024 20:12:16 -0400 Subject: [PATCH] E0229: Suggest Moving Type Constraints to Type Parameter Declaration --- compiler/rustc_hir/src/hir.rs | 15 ++++ .../src/hir_ty_lowering/errors.rs | 73 ++++++++++++++++--- .../src/hir_ty_lowering/generics.rs | 17 ++++- 3 files changed, 94 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e971d0e3c1435..993cfa878b63c 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -3735,6 +3735,21 @@ impl<'hir> Node<'hir> { } } + /// Get a `hir::Impl` if the node is an impl block for the given `trait_def_id`. + pub fn impl_block_of_trait(self, trait_def_id: DefId) -> Option<&'hir Impl<'hir>> { + match self { + Node::Item(Item { kind: ItemKind::Impl(impl_block), .. }) + if impl_block + .of_trait + .and_then(|trait_ref| trait_ref.trait_def_id()) + .is_some_and(|trait_id| trait_id == trait_def_id) => + { + Some(impl_block) + } + _ => None, + } + } + pub fn fn_sig(self) -> Option<&'hir FnSig<'hir>> { match self { Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(fn_sig, _), .. }) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index 3a9ef244fd3f5..edfffc68c1f52 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -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<'_>| { @@ -1286,20 +1285,74 @@ 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); + match constraint.kind { + hir::AssocItemConstraintKind::Equality { term } => { + match (&matching_param.kind, term) { + (GenericParamDefKind::Type { .. }, hir::Term::Ty(ty)) => { + suggest_direct_use(&mut err, ty.span); + } + (GenericParamDefKind::Const { .. }, hir::Term::Const(c)) => { + let span = tcx.hir().span(c.hir_id); + suggest_direct_use(&mut err, span); + } + _ => suggest_removal(&mut err), + } } - (GenericParamDefKind::Const { .. }, hir::Term::Const(c)) => { - let span = tcx.hir().span(c.hir_id); - suggest_direct_use(&mut err, span); + hir::AssocItemConstraintKind::Bound { bounds } => { + // Suggest `impl Trait for Foo` when finding + // `impl Trait 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 { suggest_removal(&mut err); diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs index 26cabb69d25c7..3f888c4e2722b 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs @@ -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)