Skip to content

Commit 5f3e48b

Browse files
committed
Suggest how to fix with unconstrained type parameters
Unconstrained type parameters made no suggestion and it makes difficult to know how to fix (#107295). To make fixing easier, we output suggestions how to fix.
1 parent 1b41e84 commit 5f3e48b

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

compiler/rustc_hir/src/hir.rs

+55
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,61 @@ impl<'hir> Generics<'hir> {
845845
bound_span.with_lo(bounds[bound_pos - 1].span().hi())
846846
}
847847
}
848+
849+
pub fn span_for_param_removal(&self, param_index: usize) -> Span {
850+
if param_index >= self.params.len() {
851+
return self.span.shrink_to_hi();
852+
}
853+
854+
let is_impl_generic = |par: &&GenericParam<'_>| match par.kind {
855+
GenericParamKind::Type { .. }
856+
| GenericParamKind::Const { .. }
857+
| GenericParamKind::Lifetime { kind: LifetimeParamKind::Explicit } => true,
858+
_ => false,
859+
};
860+
861+
// Find the span of the type parameter.
862+
if let Some(next) = self.params[param_index + 1..].iter().find(is_impl_generic) {
863+
self.params[param_index].span.until(next.span)
864+
} else if let Some(prev) = self.params[..param_index].iter().rfind(is_impl_generic) {
865+
let mut prev_span = prev.span;
866+
// Consider the span of the bounds with the previous generic parameter when there is.
867+
if let Some(prev_bounds_span) = self.span_for_param_bounds(prev) {
868+
prev_span = prev_span.to(prev_bounds_span);
869+
}
870+
871+
// Consider the span of the bounds with the current generic parameter when there is.
872+
prev_span.shrink_to_hi().to(
873+
if let Some(cur_bounds_span) = self.span_for_param_bounds(&self.params[param_index])
874+
{
875+
cur_bounds_span
876+
} else {
877+
self.params[param_index].span
878+
},
879+
)
880+
} else {
881+
// Remove also angle brackets <> when there is just ONE generic parameter.
882+
self.span
883+
}
884+
}
885+
886+
fn span_for_param_bounds(&self, param: &GenericParam<'hir>) -> Option<Span> {
887+
self.predicates
888+
.iter()
889+
.find(|pred| {
890+
if let WherePredicateKind::BoundPredicate(WhereBoundPredicate {
891+
origin: PredicateOrigin::GenericParam,
892+
bounded_ty,
893+
..
894+
}) = pred.kind
895+
{
896+
bounded_ty.span == param.span
897+
} else {
898+
false
899+
}
900+
})
901+
.map(|pred| pred.span)
902+
}
848903
}
849904

850905
/// A single predicate in a where-clause.

compiler/rustc_hir_analysis/src/impl_wf_check.rs

+71-1
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ use std::assert_matches::debug_assert_matches;
1313
use min_specialization::check_min_specialization;
1414
use rustc_data_structures::fx::FxHashSet;
1515
use rustc_errors::codes::*;
16+
use rustc_errors::{Applicability, Diag};
1617
use rustc_hir::def::DefKind;
1718
use rustc_hir::def_id::LocalDefId;
18-
use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
19+
use rustc_hir::{Path, QPath, Ty, TyKind};
20+
use rustc_middle::ty::{self, GenericParamDef, TyCtxt, TypeVisitableExt};
1921
use rustc_span::ErrorGuaranteed;
2022

2123
use crate::constrained_generic_params as cgp;
@@ -128,6 +130,21 @@ pub(crate) fn enforce_impl_lifetime_params_are_constrained(
128130
for param in &impl_generics.own_params {
129131
match param.kind {
130132
ty::GenericParamDefKind::Lifetime => {
133+
let param_lt = cgp::Parameter::from(param.to_early_bound_region_data());
134+
if lifetimes_in_associated_types.contains(&param_lt) // (*)
135+
&& !input_parameters.contains(&param_lt)
136+
{
137+
let mut diag = tcx.dcx().create_err(UnconstrainedGenericParameter {
138+
span: tcx.def_span(param.def_id),
139+
param_name: param.name,
140+
param_def_kind: tcx.def_descr(param.def_id),
141+
const_param_note: false,
142+
const_param_note2: false,
143+
});
144+
diag.code(E0207);
145+
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, param);
146+
res = Err(diag.emit());
147+
}
131148
// This is a horrible concession to reality. I think it'd be
132149
// better to just ban unconstrained lifetimes outright, but in
133150
// practice people do non-hygienic macros like:
@@ -158,6 +175,7 @@ pub(crate) fn enforce_impl_lifetime_params_are_constrained(
158175
const_param_note2: false,
159176
});
160177
diag.code(E0207);
178+
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, param);
161179
res = Err(diag.emit());
162180
}
163181
}
@@ -229,8 +247,60 @@ pub(crate) fn enforce_impl_non_lifetime_params_are_constrained(
229247
const_param_note2: const_param_note,
230248
});
231249
diag.code(E0207);
250+
suggest_to_remove_or_use_generic(tcx, &mut diag, impl_def_id, &param);
232251
res = Err(diag.emit());
233252
}
234253
}
235254
res
236255
}
256+
257+
fn suggest_to_remove_or_use_generic(
258+
tcx: TyCtxt<'_>,
259+
diag: &mut Diag<'_>,
260+
impl_def_id: LocalDefId,
261+
param: &GenericParamDef,
262+
) {
263+
let node = tcx.hir_node_by_def_id(impl_def_id);
264+
let hir_impl = node.expect_item().expect_impl();
265+
266+
let Some((index, _)) = hir_impl
267+
.generics
268+
.params
269+
.iter()
270+
.enumerate()
271+
.find(|(_, par)| par.def_id.to_def_id() == param.def_id)
272+
else {
273+
return;
274+
};
275+
276+
let mut suggestions = vec![];
277+
278+
// Suggestion for removing the type parameter.
279+
suggestions.push(vec![(hir_impl.generics.span_for_param_removal(index), String::new())]);
280+
281+
// Suggestion for making use of the type parameter.
282+
if let Some(path) = extract_ty_as_path(hir_impl.self_ty) {
283+
let seg = path.segments.last().unwrap();
284+
if let Some(args) = seg.args {
285+
suggestions
286+
.push(vec![(args.span().unwrap().shrink_to_hi(), format!(", {}", param.name))]);
287+
} else {
288+
suggestions.push(vec![(seg.ident.span.shrink_to_hi(), format!("<{}>", param.name))]);
289+
}
290+
}
291+
292+
diag.multipart_suggestions(
293+
format!("either remove the type parameter {}, or make use of it, for example", param.name),
294+
suggestions,
295+
Applicability::MaybeIncorrect,
296+
);
297+
}
298+
299+
fn extract_ty_as_path<'hir>(ty: &Ty<'hir>) -> Option<&'hir Path<'hir>> {
300+
match ty.kind {
301+
TyKind::Path(QPath::Resolved(_, path)) => Some(path),
302+
TyKind::Slice(ty) | TyKind::Array(ty, _) => extract_ty_as_path(ty),
303+
TyKind::Ptr(ty) | TyKind::Ref(_, ty) => extract_ty_as_path(ty.ty),
304+
_ => None,
305+
}
306+
}

0 commit comments

Comments
 (0)