From 5da1b4189e8b6820624ea548efe90c2cc35d576c 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 | 24 +++++++ .../src/hir_ty_lowering/errors.rs | 71 ++++++++++++++++--- .../src/hir_ty_lowering/generics.rs | 17 ++++- .../associated-type-bounds/no-gat-position.rs | 2 +- .../no-gat-position.stderr | 2 +- .../rtn-in-impl-signature.stderr | 2 +- 6 files changed, 105 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e971d0e3c14..a021847db26 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2456,6 +2456,15 @@ pub enum AssocItemConstraintKind<'hir> { Bound { bounds: &'hir [GenericBound<'hir>] }, } +impl<'hir> AssocItemConstraintKind<'hir> { + pub fn descr(&self) -> &'static str { + match self { + AssocItemConstraintKind::Equality { .. } => "binding", + AssocItemConstraintKind::Bound { .. } => "constraint", + } + } +} + #[derive(Debug, Clone, Copy, HashStable_Generic)] pub struct Ty<'hir> { pub hir_id: HirId, @@ -3735,6 +3744,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 3a9ef244fd3..2d240699105 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<'_>| { @@ -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 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 { 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 26cabb69d25..3f888c4e272 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) diff --git a/tests/ui/associated-type-bounds/no-gat-position.rs b/tests/ui/associated-type-bounds/no-gat-position.rs index 249812ebbff..ec9214897fa 100644 --- a/tests/ui/associated-type-bounds/no-gat-position.rs +++ b/tests/ui/associated-type-bounds/no-gat-position.rs @@ -5,7 +5,7 @@ pub trait Iter { fn next<'a>(&'a mut self) -> Option>; //~^ ERROR associated item constraints are not allowed here - //~| HELP consider removing this associated item binding + //~| HELP consider removing this associated item constraint } impl Iter for () { diff --git a/tests/ui/associated-type-bounds/no-gat-position.stderr b/tests/ui/associated-type-bounds/no-gat-position.stderr index b8e466b8d84..39a2f89f9ac 100644 --- a/tests/ui/associated-type-bounds/no-gat-position.stderr +++ b/tests/ui/associated-type-bounds/no-gat-position.stderr @@ -4,7 +4,7 @@ error[E0229]: associated item constraints are not allowed here LL | fn next<'a>(&'a mut self) -> Option>; | ^^^^^^^^^ associated item constraint not allowed here | -help: consider removing this associated item binding +help: consider removing this associated item constraint | LL | fn next<'a>(&'a mut self) -> Option>; | ~~~~~~~~~~~ diff --git a/tests/ui/async-await/return-type-notation/rtn-in-impl-signature.stderr b/tests/ui/async-await/return-type-notation/rtn-in-impl-signature.stderr index b0c89b05925..54960ae60bc 100644 --- a/tests/ui/async-await/return-type-notation/rtn-in-impl-signature.stderr +++ b/tests/ui/async-await/return-type-notation/rtn-in-impl-signature.stderr @@ -13,7 +13,7 @@ error[E0229]: associated item constraints are not allowed here LL | impl Super1<'_, bar(): Send> for () {} | ^^^^^^^^^^^ associated item constraint not allowed here | -help: consider removing this associated item binding +help: consider removing this associated item constraint | LL | impl Super1<'_, bar(): Send> for () {} | ~~~~~~~~~~~~~