Skip to content

Commit 532671b

Browse files
estebankestebank
committed
More accurately handle suggestions
* Confirm the path segment being modified is an `enum` * Check whether type has type param before suggesting changing `Self` * Wording changes * Add clarifying comments * Suggest removing args from `Self` if the type doesn't have type params
1 parent 9b2d80a commit 532671b

File tree

3 files changed

+106
-54
lines changed

3 files changed

+106
-54
lines changed

‎compiler/rustc_typeck/src/astconv/mod.rs

Lines changed: 80 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,52 +1803,84 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
18031803
tcx.check_stability(variant_def.def_id, Some(hir_ref_id), span, None);
18041804
self.prohibit_generics(slice::from_ref(assoc_segment).iter(), |err| {
18051805
err.note("enum variants can't have type parameters");
1806-
let type_name = tcx.opt_item_name(adt_def.did());
1807-
let the_enum = type_name.map(|n| format!("enum `{n}`")).unwrap_or_else(|| "the enum".to_string());
1808-
let msg = format!("you might have meant to specity type parameters on {the_enum}");
1806+
let type_name = tcx.item_name(adt_def.did());
1807+
let msg = format!(
1808+
"you might have meant to specity type parameters on enum \
1809+
`{type_name}`"
1810+
);
18091811
let Some(args) = assoc_segment.args else { return; };
1812+
// Get the span of the generics args *including* the leading `::`.
18101813
let args_span = assoc_segment.ident.span.shrink_to_hi().to(args.span_ext);
1814+
if tcx.generics_of(adt_def.did()).count() == 0 {
1815+
// FIXME(estebank): we could also verify that the arguments being
1816+
// work for the `enum`, instead of just looking if it takes *any*.
1817+
err.span_suggestion_verbose(
1818+
args_span,
1819+
&format!("{type_name} doesn't have type parameters"),
1820+
String::new(),
1821+
Applicability::MachineApplicable,
1822+
);
1823+
return;
1824+
}
18111825
let Ok(snippet) = tcx.sess.source_map().span_to_snippet(args_span) else {
18121826
err.note(&msg);
18131827
return;
18141828
};
1815-
let (qself_sugg_span, is_self) = if let hir::TyKind::Path(hir::QPath::Resolved(_, ref path)) = qself.kind {
1829+
let (qself_sugg_span, is_self) = if let hir::TyKind::Path(
1830+
hir::QPath::Resolved(_, ref path)
1831+
) = qself.kind {
18161832
// If the path segment already has type params, we want to overwrite
18171833
// them.
18181834
match &path.segments[..] {
1819-
[.., segment, _] => (
1820-
segment.ident.span.shrink_to_hi().to(segment.args.map_or(
1821-
segment.ident.span.shrink_to_hi(),
1835+
// `segment` is the previous to last element on the path,
1836+
// which would normally be the `enum` itself, while the last
1837+
// `_` `PathSegment` corresponds to the variant.
1838+
[.., hir::PathSegment {
1839+
ident,
1840+
args,
1841+
res: Some(Res::Def(DefKind::Enum, _)),
1842+
..
1843+
}, _] => (
1844+
// We need to include the `::` in `Type::Variant::<Args>`
1845+
// to point the span to `::<Args>`, not just `<Args>`.
1846+
ident.span.shrink_to_hi().to(args.map_or(
1847+
ident.span.shrink_to_hi(),
18221848
|a| a.span_ext)),
18231849
false,
18241850
),
18251851
[segment] => (
1852+
// We need to include the `::` in `Type::Variant::<Args>`
1853+
// to point the span to `::<Args>`, not just `<Args>`.
18261854
segment.ident.span.shrink_to_hi().to(segment.args.map_or(
18271855
segment.ident.span.shrink_to_hi(),
18281856
|a| a.span_ext)),
18291857
kw::SelfUpper == segment.ident.name,
18301858
),
1831-
_ => unreachable!(),
1859+
_ => {
1860+
err.note(&msg);
1861+
return;
1862+
}
18321863
}
18331864
} else {
18341865
err.note(&msg);
18351866
return;
18361867
};
1837-
let Some(type_name) = type_name else {
1838-
err.note(&msg);
1839-
return;
1840-
};
18411868
let suggestion = vec![
18421869
if is_self {
18431870
// Account for people writing `Self::Variant::<Args>`, where
1844-
// `Self` is the enum.
1871+
// `Self` is the enum, and suggest replacing `Self` with the
1872+
// appropriate type: `Type::<Args>::Variant`.
18451873
(qself.span, format!("{type_name}{snippet}"))
18461874
} else {
18471875
(qself_sugg_span, snippet)
18481876
},
18491877
(args_span, String::new()),
18501878
];
1851-
err.multipart_suggestion_verbose(&msg, suggestion, Applicability::MaybeIncorrect);
1879+
err.multipart_suggestion_verbose(
1880+
&msg,
1881+
suggestion,
1882+
Applicability::MaybeIncorrect,
1883+
);
18521884
});
18531885
return Ok((qself_ty, DefKind::Variant, variant_def.def_id));
18541886
} else {
@@ -2369,8 +2401,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
23692401
// Try to evaluate any array length constants.
23702402
let ty = tcx.at(span).type_of(def_id);
23712403
let span_of_impl = tcx.span_of_impl(def_id);
2372-
// TODO: confirm that `def_id`'s type accepts type params at all before suggesting
2373-
// using that instead.
23742404
self.prohibit_generics(path.segments.iter(), |err| {
23752405
let def_id = match *ty.kind() {
23762406
ty::Adt(self_def, _) => self_def.did(),
@@ -2379,32 +2409,52 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
23792409

23802410
let type_name = tcx.item_name(def_id);
23812411
let span_of_ty = tcx.def_ident_span(def_id);
2412+
let generics = tcx.generics_of(def_id).count();
23822413

2383-
let msg = format!("the `Self` type is `{ty}`");
2414+
let msg = format!("`Self` is of type `{ty}`");
23842415
if let (Ok(i_sp), Some(t_sp)) = (span_of_impl, span_of_ty) {
23852416
let i_sp = tcx.sess.source_map().guess_head_span(i_sp);
23862417
let mut span: MultiSpan = vec![t_sp].into();
23872418
span.push_span_label(
23882419
i_sp,
2389-
&format!("`Self` is `{type_name}` in this `impl`"),
2420+
&format!("`Self` is or type `{type_name}` in this `impl`"),
2421+
);
2422+
let mut postfix = "";
2423+
if generics == 0 {
2424+
postfix = ", which doesn't have type parameters";
2425+
}
2426+
span.push_span_label(
2427+
t_sp,
2428+
&format!("`Self` corresponds to this type{postfix}"),
23902429
);
2391-
span.push_span_label(t_sp, "`Self` corresponds to this type");
23922430
err.span_note(span, &msg);
23932431
} else {
23942432
err.note(&msg);
23952433
}
23962434
for segment in path.segments {
2397-
if let Some(_args) = segment.args && segment.ident.name == kw::SelfUpper {
2398-
err.span_suggestion_verbose(
2399-
segment.ident.span,
2400-
format!(
2401-
"the `Self` type doesn't accept type parameters, use the \
2402-
concrete type's name `{type_name}` instead if you want to \
2403-
specify its type parameters"
2404-
),
2405-
type_name.to_string(),
2406-
Applicability::MaybeIncorrect,
2407-
);
2435+
if let Some(args) = segment.args && segment.ident.name == kw::SelfUpper {
2436+
if generics == 0 {
2437+
// FIXME(estebank): we could also verify that the arguments being
2438+
// work for the `enum`, instead of just looking if it takes *any*.
2439+
err.span_suggestion_verbose(
2440+
segment.ident.span.shrink_to_hi().to(args.span_ext),
2441+
"the `Self` type doesn't accept type parameters",
2442+
String::new(),
2443+
Applicability::MachineApplicable,
2444+
);
2445+
return;
2446+
} else {
2447+
err.span_suggestion_verbose(
2448+
segment.ident.span,
2449+
format!(
2450+
"the `Self` type doesn't accept type parameters, use the \
2451+
concrete type's name `{type_name}` instead if you want to \
2452+
specify its type parameters"
2453+
),
2454+
type_name.to_string(),
2455+
Applicability::MaybeIncorrect,
2456+
);
2457+
}
24082458
}
24092459
}
24102460
});

‎src/test/ui/structs/struct-path-self.stderr

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,37 +34,39 @@ error[E0109]: type arguments are not allowed for this type
3434
LL | let z = Self::<u8> {};
3535
| ^^ type argument not allowed
3636
|
37-
note: the `Self` type is `S`
37+
note: `Self` is of type `S`
3838
--> $DIR/struct-path-self.rs:1:8
3939
|
4040
LL | struct S;
41-
| ^ `Self` corresponds to this type
41+
| ^ `Self` corresponds to this type, which doesn't have type parameters
4242
...
4343
LL | impl Tr for S {
44-
| ------------- `Self` is `S` in this `impl`
45-
help: the `Self` type doesn't accept type parameters, use the concrete type's name `S` instead if you want to specify its type parameters
44+
| ------------- `Self` is or type `S` in this `impl`
45+
help: the `Self` type doesn't accept type parameters
4646
|
47-
LL | let z = S::<u8> {};
48-
| ~
47+
LL - let z = Self::<u8> {};
48+
LL + let z = Self {};
49+
|
4950

5051
error[E0109]: type arguments are not allowed for this type
5152
--> $DIR/struct-path-self.rs:30:24
5253
|
5354
LL | let z = Self::<u8> {};
5455
| ^^ type argument not allowed
5556
|
56-
note: the `Self` type is `S`
57+
note: `Self` is of type `S`
5758
--> $DIR/struct-path-self.rs:1:8
5859
|
5960
LL | struct S;
60-
| ^ `Self` corresponds to this type
61+
| ^ `Self` corresponds to this type, which doesn't have type parameters
6162
...
6263
LL | impl S {
63-
| ------ `Self` is `S` in this `impl`
64-
help: the `Self` type doesn't accept type parameters, use the concrete type's name `S` instead if you want to specify its type parameters
64+
| ------ `Self` is or type `S` in this `impl`
65+
help: the `Self` type doesn't accept type parameters
6566
|
66-
LL | let z = S::<u8> {};
67-
| ~
67+
LL - let z = Self::<u8> {};
68+
LL + let z = Self {};
69+
|
6870

6971
error: aborting due to 6 previous errors
7072

‎src/test/ui/type-alias-enum-variants/enum-variant-generic-args.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ error[E0109]: type arguments are not allowed for this type
2929
LL | Self::<()>::TSVariant(());
3030
| ^^ type argument not allowed
3131
|
32-
note: the `Self` type is `Enum<T>`
32+
note: `Self` is of type `Enum<T>`
3333
--> $DIR/enum-variant-generic-args.rs:7:6
3434
|
3535
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
3636
| ^^^^ `Self` corresponds to this type
3737
...
3838
LL | impl<T> Enum<T> {
39-
| --------------- `Self` is `Enum` in this `impl`
39+
| --------------- `Self` is or type `Enum` in this `impl`
4040
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
4141
|
4242
LL | Enum::<()>::TSVariant(());
@@ -67,14 +67,14 @@ error[E0109]: type arguments are not allowed for this type
6767
LL | Self::<()>::TSVariant::<()>(());
6868
| ^^ type argument not allowed
6969
|
70-
note: the `Self` type is `Enum<T>`
70+
note: `Self` is of type `Enum<T>`
7171
--> $DIR/enum-variant-generic-args.rs:7:6
7272
|
7373
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
7474
| ^^^^ `Self` corresponds to this type
7575
...
7676
LL | impl<T> Enum<T> {
77-
| --------------- `Self` is `Enum` in this `impl`
77+
| --------------- `Self` is or type `Enum` in this `impl`
7878
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
7979
|
8080
LL | Enum::<()>::TSVariant::<()>(());
@@ -129,14 +129,14 @@ error[E0109]: type arguments are not allowed for this type
129129
LL | Self::<()>::SVariant { v: () };
130130
| ^^ type argument not allowed
131131
|
132-
note: the `Self` type is `Enum<T>`
132+
note: `Self` is of type `Enum<T>`
133133
--> $DIR/enum-variant-generic-args.rs:7:6
134134
|
135135
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
136136
| ^^^^ `Self` corresponds to this type
137137
...
138138
LL | impl<T> Enum<T> {
139-
| --------------- `Self` is `Enum` in this `impl`
139+
| --------------- `Self` is or type `Enum` in this `impl`
140140
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
141141
|
142142
LL | Enum::<()>::SVariant { v: () };
@@ -160,14 +160,14 @@ error[E0109]: type arguments are not allowed for this type
160160
LL | Self::<()>::SVariant::<()> { v: () };
161161
| ^^ type argument not allowed
162162
|
163-
note: the `Self` type is `Enum<T>`
163+
note: `Self` is of type `Enum<T>`
164164
--> $DIR/enum-variant-generic-args.rs:7:6
165165
|
166166
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
167167
| ^^^^ `Self` corresponds to this type
168168
...
169169
LL | impl<T> Enum<T> {
170-
| --------------- `Self` is `Enum` in this `impl`
170+
| --------------- `Self` is or type `Enum` in this `impl`
171171
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
172172
|
173173
LL | Enum::<()>::SVariant::<()> { v: () };
@@ -210,14 +210,14 @@ error[E0109]: type arguments are not allowed for this type
210210
LL | Self::<()>::UVariant;
211211
| ^^ type argument not allowed
212212
|
213-
note: the `Self` type is `Enum<T>`
213+
note: `Self` is of type `Enum<T>`
214214
--> $DIR/enum-variant-generic-args.rs:7:6
215215
|
216216
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
217217
| ^^^^ `Self` corresponds to this type
218218
...
219219
LL | impl<T> Enum<T> {
220-
| --------------- `Self` is `Enum` in this `impl`
220+
| --------------- `Self` is or type `Enum` in this `impl`
221221
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
222222
|
223223
LL | Enum::<()>::UVariant;
@@ -229,14 +229,14 @@ error[E0109]: type arguments are not allowed for this type
229229
LL | Self::<()>::UVariant::<()>;
230230
| ^^ type argument not allowed
231231
|
232-
note: the `Self` type is `Enum<T>`
232+
note: `Self` is of type `Enum<T>`
233233
--> $DIR/enum-variant-generic-args.rs:7:6
234234
|
235235
LL | enum Enum<T> { TSVariant(T), SVariant { v: T }, UVariant }
236236
| ^^^^ `Self` corresponds to this type
237237
...
238238
LL | impl<T> Enum<T> {
239-
| --------------- `Self` is `Enum` in this `impl`
239+
| --------------- `Self` is or type `Enum` in this `impl`
240240
help: the `Self` type doesn't accept type parameters, use the concrete type's name `Enum` instead if you want to specify its type parameters
241241
|
242242
LL | Enum::<()>::UVariant::<()>;

0 commit comments

Comments
 (0)