Skip to content

Commit

Permalink
Emit suggestions when equality constraints are wrongly used
Browse files Browse the repository at this point in the history
  • Loading branch information
gurry committed Apr 16, 2024
1 parent 76cf07d commit f7ebad4
Show file tree
Hide file tree
Showing 17 changed files with 707 additions and 29 deletions.
115 changes: 109 additions & 6 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::traits::FulfillmentError;
use rustc_middle::query::Key;
use rustc_middle::ty::GenericParamDefKind;
use rustc_middle::ty::{self, suggest_constraining_type_param};
use rustc_middle::ty::{AdtDef, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{Binder, TraitRef};
Expand Down Expand Up @@ -1200,12 +1201,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Emits an error regarding forbidden type binding associations
pub fn prohibit_assoc_item_binding(
tcx: TyCtxt<'_>,
span: Span,
segment: Option<(&hir::PathSegment<'_>, Span)>,
binding: &hir::TypeBinding<'_>,
segment: Option<(DefId, &hir::PathSegment<'_>, Span)>,
) -> ErrorGuaranteed {
tcx.dcx().emit_err(AssocTypeBindingNotAllowed {
span,
fn_trait_expansion: if let Some((segment, span)) = segment
let mut err = tcx.dcx().create_err(AssocTypeBindingNotAllowed {
span: binding.span,
fn_trait_expansion: if let Some((_, segment, span)) = segment
&& segment.args().parenthesized == hir::GenericArgsParentheses::ParenSugar
{
Some(ParenthesizedFnTraitExpansion {
Expand All @@ -1215,7 +1216,109 @@ pub fn prohibit_assoc_item_binding(
} else {
None
},
})
});

// Emit a suggestion to turn the assoc item binding into a generic arg
// if the relevant item has a generic param whose name matches the binding name;
// otherwise suggest the removal of the binding.
if let Some((def_id, segment, _)) = segment
&& segment.args().parenthesized == hir::GenericArgsParentheses::No
&& let hir::TypeBindingKind::Equality { term } = binding.kind
{
// Suggests removal of the offending binding
let suggest_removal = |e: &mut Diag<'_>| {
let bindings = segment.args().bindings;
let args = segment.args().args;
let binding_span = binding.span;

// Compute the span to remove based on the position
// of the binding. We do that as follows:
// 1. Find the index of the binding in the list of bindings
// 2. Locate the spans preceding and following the binding.
// If it's the first binding the preceding span would be
// that of the last arg
// 3. Using this information work out whether the span
// to remove will start from the end of the preceding span,
// the start of the next span or will simply be the
// span encomassing everything within the generics brackets

let Some(binding_index) = bindings.iter().position(|b| b.hir_id == binding.hir_id)
else {
bug!("a type binding exists but its HIR ID not found in generics");
};

let preceding_span = if binding_index > 0 {
Some(bindings[binding_index - 1].span)
} else {
args.last().map(|a| a.span())
};

let next_span = if binding_index < bindings.len() - 1 {
Some(bindings[binding_index + 1].span)
} else {
None
};

let removal_span = match (preceding_span, next_span) {
(Some(prec), _) => binding_span.with_lo(prec.hi()),
(None, Some(next)) => binding_span.with_hi(next.lo()),
(None, None) => {
let Some(generics_span) = segment.args().span_ext() else {
bug!("a type binding exists but generic span is empty");
};

generics_span
}
};

// Now emit the suggestion
if let Ok(suggestion) = tcx.sess.source_map().span_to_snippet(removal_span) {
e.span_suggestion_verbose(
removal_span,
"consider removing this type binding",
suggestion,
Applicability::MaybeIncorrect,
);
}
};

// Suggest replacing the associated item binding with a generic argument.
// i.e., replacing `<..., T = A, ...>` with `<..., A, ...>`.
let suggest_direct_use = |e: &mut Diag<'_>, sp: Span| {
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(sp) {
e.span_suggestion_verbose(
binding.span,
format!("to use `{snippet}` as a generic argument specify it directly"),
snippet,
Applicability::MaybeIncorrect,
);
}
};

// Check if the type has a generic param with the
// same name as the assoc type name in type binding
let generics = tcx.generics_of(def_id);
let matching_param =
generics.params.iter().find(|p| p.name.as_str() == binding.ident.as_str());

// 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)) => {
let span = tcx.hir().span(c.hir_id);
suggest_direct_use(&mut err, span);
}
_ => suggest_removal(&mut err),
}
} else {
suggest_removal(&mut err);
}
}

err.emit()
}

pub(crate) fn fn_trait_to_string(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ pub(crate) fn check_generic_arg_count(
if gen_pos != GenericArgPosition::Type
&& let Some(b) = gen_args.bindings.first()
{
prohibit_assoc_item_binding(tcx, b.span, None);
prohibit_assoc_item_binding(tcx, b, None);
}

let explicit_late_bound =
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
ty::BoundConstness::NotConst,
);
if let Some(b) = item_segment.args().bindings.first() {
prohibit_assoc_item_binding(self.tcx(), b.span, Some((item_segment, span)));
prohibit_assoc_item_binding(self.tcx(), b, Some((def_id, item_segment, span)));
}
args
}
Expand Down Expand Up @@ -619,7 +619,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
ty::BoundConstness::NotConst,
);
if let Some(b) = item_segment.args().bindings.first() {
prohibit_assoc_item_binding(self.tcx(), b.span, Some((item_segment, span)));
prohibit_assoc_item_binding(self.tcx(), b, Some((item_def_id, item_segment, span)));
}
args
}
Expand Down Expand Up @@ -764,7 +764,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
constness,
);
if let Some(b) = trait_segment.args().bindings.first() {
prohibit_assoc_item_binding(self.tcx(), b.span, Some((trait_segment, span)));
prohibit_assoc_item_binding(self.tcx(), b, Some((trait_def_id, trait_segment, span)));
}
ty::TraitRef::new(self.tcx(), trait_def_id, generic_args)
}
Expand Down Expand Up @@ -1556,7 +1556,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
for segment in segments {
// Only emit the first error to avoid overloading the user with error messages.
if let Some(b) = segment.args().bindings.first() {
return Err(prohibit_assoc_item_binding(self.tcx(), b.span, None));
return Err(prohibit_assoc_item_binding(self.tcx(), b, None));
}
}

Expand Down
9 changes: 9 additions & 0 deletions tests/rustdoc-ui/invalid_associated_const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here
|
LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/invalid_associated_const.rs:4:17
Expand All @@ -11,6 +16,10 @@ LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
9 changes: 9 additions & 0 deletions tests/rustdoc-ui/issue-102467.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here
|
LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102467.rs:7:17
Expand All @@ -11,6 +16,10 @@ LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
9 changes: 9 additions & 0 deletions tests/ui/associated-consts/issue-102335-const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error[E0229]: associated type bindings are not allowed here
|
LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102335-const.rs:4:17
Expand All @@ -11,6 +16,10 @@ LL | type A: S<C<X = 0i32> = 34>;
| ^^^^^^^^ associated type not allowed here
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider removing this type binding
|
LL | type A: S<C<X = 0i32> = 34>;
| ~~~~~~~~~~

error: aborting due to 2 previous errors

Expand Down
8 changes: 7 additions & 1 deletion tests/ui/associated-type-bounds/issue-102335-ty.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
trait T {
type A: S<C<i32 = u32> = ()>;
type A: S<C<i32 = u32> = ()>; // Just one erroneous equality constraint
//~^ ERROR associated type bindings are not allowed here
//~| ERROR associated type bindings are not allowed here
}

trait T2 {
type A: S<C<i32 = u32, X = i32> = ()>; // More than one erroneous equality constraints
//~^ ERROR associated type bindings are not allowed here
//~| ERROR associated type bindings are not allowed here
}
Expand Down
38 changes: 35 additions & 3 deletions tests/ui/associated-type-bounds/issue-102335-ty.stderr
Original file line number Diff line number Diff line change
@@ -1,17 +1,49 @@
error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102335-ty.rs:2:17
|
LL | type A: S<C<i32 = u32> = ()>;
LL | type A: S<C<i32 = u32> = ()>; // Just one erroneous equality constraint
| ^^^^^^^^^ associated type not allowed here
|
help: consider removing this type binding
|
LL | type A: S<C<i32 = u32> = ()>; // Just one erroneous equality constraint
| ~~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102335-ty.rs:2:17
|
LL | type A: S<C<i32 = u32> = ()>;
LL | type A: S<C<i32 = u32> = ()>; // Just one erroneous equality constraint
| ^^^^^^^^^ associated type not allowed here
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider removing this type binding
|
LL | type A: S<C<i32 = u32> = ()>; // Just one erroneous equality constraint
| ~~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102335-ty.rs:8:17
|
LL | type A: S<C<i32 = u32, X = i32> = ()>; // More than one erroneous equality constraints
| ^^^^^^^^^ associated type not allowed here
|
help: consider removing this type binding
|
LL | type A: S<C<i32 = u32, X = i32> = ()>; // More than one erroneous equality constraints
| ~~~~~~~~~~

error[E0229]: associated type bindings are not allowed here
--> $DIR/issue-102335-ty.rs:8:17
|
LL | type A: S<C<i32 = u32, X = i32> = ()>; // More than one erroneous equality constraints
| ^^^^^^^^^ associated type not allowed here
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: consider removing this type binding
|
LL | type A: S<C<i32 = u32, X = i32> = ()>; // More than one erroneous equality constraints
| ~~~~~~~~~~

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0229`.
Loading

0 comments on commit f7ebad4

Please sign in to comment.