Skip to content

Commit 6b01b35

Browse files
XiretzaXiretza
committed
Forbid #[suggestion_*(...)] on Vecs
It is ambiguous whether this should produce several `.span_suggestions()` calls or one `.multipart_suggestions()` call.
1 parent 2afe585 commit 6b01b35

File tree

7 files changed

+117
-45
lines changed

7 files changed

+117
-45
lines changed

‎compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
322322
let generated_code = self
323323
.generate_inner_field_code(
324324
attr,
325-
FieldInfo {
326-
binding: binding_info,
327-
ty: inner_ty.inner_type().unwrap_or(&field.ty),
328-
span: &field.span(),
329-
},
325+
FieldInfo { binding: binding_info, ty: inner_ty, span: &field.span() },
330326
binding,
331327
)
332328
.unwrap_or_else(|v| v.to_compile_error());
@@ -402,9 +398,9 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
402398
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
403399
}
404400
SubdiagnosticKind::Note | SubdiagnosticKind::Help | SubdiagnosticKind::Warn => {
405-
if type_matches_path(info.ty, &["rustc_span", "Span"]) {
401+
if type_matches_path(info.ty.inner_type(), &["rustc_span", "Span"]) {
406402
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
407-
} else if type_is_unit(info.ty) {
403+
} else if type_is_unit(info.ty.inner_type()) {
408404
Ok(self.add_subdiagnostic(&fn_ident, slug))
409405
} else {
410406
report_type_error(attr, "`Span` or `()`")?
@@ -416,6 +412,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
416412
code_field,
417413
code_init,
418414
} => {
415+
if let FieldInnerTy::Vec(_) = info.ty {
416+
throw_invalid_attr!(attr, &meta, |diag| {
417+
diag
418+
.note("`#[suggestion(...)]` applied to `Vec` field is ambiguous")
419+
.help("to show a suggestion consisting of multiple parts, use a `Subdiagnostic` annotated with `#[multipart_suggestion(...)]`")
420+
.help("to show a variable set of suggestions, use a `Vec` of `Subdiagnostic`s annotated with `#[suggestion(...)]`")
421+
});
422+
}
423+
419424
let (span_field, mut applicability) = self.span_and_applicability_of_ty(info)?;
420425

421426
if let Some((static_applicability, span)) = static_applicability {
@@ -473,7 +478,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
473478
&self,
474479
info: FieldInfo<'_>,
475480
) -> Result<(TokenStream, SpannedOption<TokenStream>), DiagnosticDeriveError> {
476-
match &info.ty {
481+
match &info.ty.inner_type() {
477482
// If `ty` is `Span` w/out applicability, then use `Applicability::Unspecified`.
478483
ty @ Type::Path(..) if type_matches_path(ty, &["rustc_span", "Span"]) => {
479484
let binding = &info.binding.binding;

‎compiler/rustc_macros/src/diagnostics/subdiagnostic.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,11 +247,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
247247
return quote! {};
248248
}
249249

250-
let info = FieldInfo {
251-
binding,
252-
ty: inner_ty.inner_type().unwrap_or(&ast.ty),
253-
span: &ast.span(),
254-
};
250+
let info = FieldInfo { binding, ty: inner_ty, span: &ast.span() };
255251

256252
let generated = self
257253
.generate_field_code_inner(kind_stats, attr, info, inner_ty.will_iterate())
@@ -312,6 +308,21 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
312308
let binding = info.binding.binding.clone();
313309
// FIXME(#100717): support `Option<Span>` on `primary_span` like in the
314310
// diagnostic derive
311+
if !matches!(info.ty, FieldInnerTy::Plain(_)) {
312+
throw_invalid_attr!(attr, &Meta::Path(path), |diag| {
313+
let diag = diag.note("there must be exactly one primary span");
314+
315+
if kind_stats.has_normal_suggestion {
316+
diag.help(
317+
"to create a suggestion with multiple spans, \
318+
use `#[multipart_suggestion]` instead",
319+
)
320+
} else {
321+
diag
322+
}
323+
});
324+
}
325+
315326
self.span_field.set_once(binding, span);
316327
}
317328

‎compiler/rustc_macros/src/diagnostics/utils.rs

Lines changed: 37 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ fn report_error_if_not_applied_to_ty(
8080
path: &[&str],
8181
ty_name: &str,
8282
) -> Result<(), DiagnosticDeriveError> {
83-
if !type_matches_path(info.ty, path) {
83+
if !type_matches_path(info.ty.inner_type(), path) {
8484
report_type_error(attr, ty_name)?;
8585
}
8686

@@ -105,8 +105,8 @@ pub(crate) fn report_error_if_not_applied_to_span(
105105
attr: &Attribute,
106106
info: &FieldInfo<'_>,
107107
) -> Result<(), DiagnosticDeriveError> {
108-
if !type_matches_path(info.ty, &["rustc_span", "Span"])
109-
&& !type_matches_path(info.ty, &["rustc_errors", "MultiSpan"])
108+
if !type_matches_path(info.ty.inner_type(), &["rustc_span", "Span"])
109+
&& !type_matches_path(info.ty.inner_type(), &["rustc_errors", "MultiSpan"])
110110
{
111111
report_type_error(attr, "`Span` or `MultiSpan`")?;
112112
}
@@ -115,60 +115,67 @@ pub(crate) fn report_error_if_not_applied_to_span(
115115
}
116116

117117
/// Inner type of a field and type of wrapper.
118+
#[derive(Copy, Clone)]
118119
pub(crate) enum FieldInnerTy<'ty> {
119120
/// Field is wrapped in a `Option<$inner>`.
120121
Option(&'ty Type),
121122
/// Field is wrapped in a `Vec<$inner>`.
122123
Vec(&'ty Type),
123124
/// Field isn't wrapped in an outer type.
124-
None,
125+
Plain(&'ty Type),
125126
}
126127

127128
impl<'ty> FieldInnerTy<'ty> {
128129
/// Returns inner type for a field, if there is one.
129130
///
130-
/// - If `ty` is an `Option`, returns `FieldInnerTy::Option { inner: (inner type) }`.
131-
/// - If `ty` is a `Vec`, returns `FieldInnerTy::Vec { inner: (inner type) }`.
132-
/// - Otherwise returns `None`.
131+
/// - If `ty` is an `Option<Inner>`, returns `FieldInnerTy::Option(Inner)`.
132+
/// - If `ty` is a `Vec<Inner>`, returns `FieldInnerTy::Vec(Inner)`.
133+
/// - Otherwise returns `FieldInnerTy::Plain(ty)`.
133134
pub(crate) fn from_type(ty: &'ty Type) -> Self {
134-
let variant: &dyn Fn(&'ty Type) -> FieldInnerTy<'ty> =
135-
if type_matches_path(ty, &["std", "option", "Option"]) {
136-
&FieldInnerTy::Option
137-
} else if type_matches_path(ty, &["std", "vec", "Vec"]) {
138-
&FieldInnerTy::Vec
139-
} else {
140-
return FieldInnerTy::None;
135+
fn single_generic_type(ty: &Type) -> &Type {
136+
let Type::Path(ty_path) = ty else {
137+
panic!("expected path type");
141138
};
142139

143-
if let Type::Path(ty_path) = ty {
144140
let path = &ty_path.path;
145141
let ty = path.segments.iter().last().unwrap();
146-
if let syn::PathArguments::AngleBracketed(bracketed) = &ty.arguments {
147-
if bracketed.args.len() == 1 {
148-
if let syn::GenericArgument::Type(ty) = &bracketed.args[0] {
149-
return variant(ty);
150-
}
151-
}
152-
}
142+
let syn::PathArguments::AngleBracketed(bracketed) = &ty.arguments else {
143+
panic!("expected bracketed generic arguments");
144+
};
145+
146+
assert_eq!(bracketed.args.len(), 1);
147+
148+
let syn::GenericArgument::Type(ty) = &bracketed.args[0] else {
149+
panic!("expected generic parameter to be a type generic");
150+
};
151+
152+
ty
153153
}
154154

155-
unreachable!();
155+
if type_matches_path(ty, &["std", "option", "Option"]) {
156+
FieldInnerTy::Option(single_generic_type(ty))
157+
} else if type_matches_path(ty, &["std", "vec", "Vec"]) {
158+
FieldInnerTy::Vec(single_generic_type(ty))
159+
} else {
160+
FieldInnerTy::Plain(ty)
161+
}
156162
}
157163

158164
/// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e.
159165
/// that cloning might be required for values moved in the loop body).
160166
pub(crate) fn will_iterate(&self) -> bool {
161167
match self {
162168
FieldInnerTy::Vec(..) => true,
163-
FieldInnerTy::Option(..) | FieldInnerTy::None => false,
169+
FieldInnerTy::Option(..) | FieldInnerTy::Plain(_) => false,
164170
}
165171
}
166172

167-
/// Returns `Option` containing inner type if there is one.
168-
pub(crate) fn inner_type(&self) -> Option<&'ty Type> {
173+
/// Returns the inner type.
174+
pub(crate) fn inner_type(&self) -> &'ty Type {
169175
match self {
170-
FieldInnerTy::Option(inner) | FieldInnerTy::Vec(inner) => Some(inner),
171-
FieldInnerTy::None => None,
176+
FieldInnerTy::Option(inner) | FieldInnerTy::Vec(inner) | FieldInnerTy::Plain(inner) => {
177+
inner
178+
}
172179
}
173180
}
174181

@@ -185,7 +192,7 @@ impl<'ty> FieldInnerTy<'ty> {
185192
#inner
186193
}
187194
},
188-
FieldInnerTy::None => quote! { #inner },
195+
FieldInnerTy::Plain(..) => quote! { #inner },
189196
}
190197
}
191198
}
@@ -194,7 +201,7 @@ impl<'ty> FieldInnerTy<'ty> {
194201
/// `generate_*` methods from walking the attributes themselves.
195202
pub(crate) struct FieldInfo<'a> {
196203
pub(crate) binding: &'a BindingInfo<'a>,
197-
pub(crate) ty: &'a Type,
204+
pub(crate) ty: FieldInnerTy<'a>,
198205
pub(crate) span: &'a proc_macro2::Span,
199206
}
200207

‎src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,3 +801,11 @@ struct SuggestionStyleGood {
801801
#[suggestion(code = "", style = "hidden")]
802802
sub: Span,
803803
}
804+
805+
#[derive(Diagnostic)]
806+
#[diag(compiletest_example)]
807+
struct SuggestionOnVec {
808+
#[suggestion(suggestion, code = "")]
809+
//~^ ERROR `#[suggestion(...)]` is not a valid attribute
810+
sub: Vec<Span>,
811+
}

‎src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,16 @@ error: `code = "..."`/`code(...)` must contain only string literals
605605
LL | #[suggestion(code = 3)]
606606
| ^^^^^^^^
607607

608+
error: `#[suggestion(...)]` is not a valid attribute
609+
--> $DIR/diagnostic-derive.rs:808:5
610+
|
611+
LL | #[suggestion(suggestion, code = "")]
612+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
613+
|
614+
= note: `#[suggestion(...)]` applied to `Vec` field is ambiguous
615+
= help: to show a suggestion consisting of multiple parts, use a `Subdiagnostic` annotated with `#[multipart_suggestion(...)]`
616+
= help: to show a variable set of suggestions, use a `Vec` of `Subdiagnostic`s annotated with `#[suggestion(...)]`
617+
608618
error: cannot find attribute `nonsense` in this scope
609619
--> $DIR/diagnostic-derive.rs:55:3
610620
|
@@ -676,7 +686,7 @@ note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg`
676686
--> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:LL:CC
677687
= note: this error originates in the derive macro `Diagnostic` which comes from the expansion of the macro `forward` (in Nightly builds, run with -Z macro-backtrace for more info)
678688

679-
error: aborting due to 85 previous errors
689+
error: aborting due to 86 previous errors
680690

681691
Some errors have detailed explanations: E0277, E0425.
682692
For more information about an error, try `rustc --explain E0277`.

‎src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,3 +798,13 @@ struct SuggestionStyleInvalid4 {
798798
#[primary_span]
799799
sub: Span,
800800
}
801+
802+
#[derive(Subdiagnostic)]
803+
#[suggestion(parse_add_paren, code = "")]
804+
//~^ ERROR suggestion without `#[primary_span]` field
805+
struct PrimarySpanOnVec {
806+
#[primary_span]
807+
//~^ ERROR `#[primary_span]` is not a valid attribute
808+
//~| NOTE there must be exactly one primary span
809+
sub: Vec<Span>,
810+
}

‎src/test/ui-fulldeps/session-diagnostic/subdiagnostic-derive.stderr

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,27 @@ error: `#[suggestion(style(...))]` is not a valid attribute
501501
LL | #[suggestion(parse_add_paren, code = "", style("foo"))]
502502
| ^^^^^^^^^^^^
503503

504+
error: `#[primary_span]` is not a valid attribute
505+
--> $DIR/subdiagnostic-derive.rs:806:5
506+
|
507+
LL | #[primary_span]
508+
| ^^^^^^^^^^^^^^^
509+
|
510+
= note: there must be exactly one primary span
511+
= help: to create a suggestion with multiple spans, use `#[multipart_suggestion]` instead
512+
513+
error: suggestion without `#[primary_span]` field
514+
--> $DIR/subdiagnostic-derive.rs:803:1
515+
|
516+
LL | / #[suggestion(parse_add_paren, code = "")]
517+
LL | |
518+
LL | | struct PrimarySpanOnVec {
519+
LL | | #[primary_span]
520+
... |
521+
LL | | sub: Vec<Span>,
522+
LL | | }
523+
| |_^
524+
504525
error: cannot find attribute `foo` in this scope
505526
--> $DIR/subdiagnostic-derive.rs:63:3
506527
|
@@ -561,6 +582,6 @@ error[E0425]: cannot find value `slug` in module `rustc_errors::fluent`
561582
LL | #[label(slug)]
562583
| ^^^^ not found in `rustc_errors::fluent`
563584

564-
error: aborting due to 79 previous errors
585+
error: aborting due to 81 previous errors
565586

566587
For more information about this error, try `rustc --explain E0425`.

0 commit comments

Comments
 (0)