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 12, 2024
1 parent 6d19ac3 commit 5da1b41
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 13 deletions.
24 changes: 24 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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, _), .. })
Expand Down
71 changes: 62 additions & 9 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 @@ -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,
);
Expand All @@ -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 {
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
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/no-gat-position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub trait Iter {

fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
//~^ 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 () {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/no-gat-position.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0229]: associated item constraints are not allowed here
LL | fn next<'a>(&'a mut self) -> Option<Self::Item<'a, As1: Copy>>;
| ^^^^^^^^^ 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<Self::Item<'a, As1: Copy>>;
| ~~~~~~~~~~~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {}
| ~~~~~~~~~~~~~
Expand Down

0 comments on commit 5da1b41

Please sign in to comment.