Skip to content

Commit

Permalink
E0229: Suggest Moving Type Constraints to Type Parameter Declaration
Browse files Browse the repository at this point in the history
  • Loading branch information
veera-sivarajan committed Jun 6, 2024
1 parent 6d19ac3 commit acf90a8
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 11 deletions.
15 changes: 15 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, _), .. })
Expand Down
73 changes: 63 additions & 10 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>| {
Expand Down Expand Up @@ -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<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 {
suggest_removal(&mut err);
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -567,7 +582,7 @@ pub(crate) fn check_generic_arg_count(
def_id,
)
.diagnostic()
.emit()
.emit_unless(all_params_are_binded)
});

Err(reported)
Expand Down

0 comments on commit acf90a8

Please sign in to comment.