1
Fork 0

Forbid #[suggestion_*(...)] on Vecs

It is ambiguous whether this should produce several `.span_suggestions()`
calls or one `.multipart_suggestions()` call.
This commit is contained in:
Xiretza 2022-10-16 16:15:39 +02:00
parent 11d96b5930
commit f2acbb9d1e
7 changed files with 117 additions and 45 deletions

View file

@ -322,11 +322,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
let generated_code = self let generated_code = self
.generate_inner_field_code( .generate_inner_field_code(
attr, attr,
FieldInfo { FieldInfo { binding: binding_info, ty: inner_ty, span: &field.span() },
binding: binding_info,
ty: inner_ty.inner_type().unwrap_or(&field.ty),
span: &field.span(),
},
binding, binding,
) )
.unwrap_or_else(|v| v.to_compile_error()); .unwrap_or_else(|v| v.to_compile_error());
@ -418,9 +414,9 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug)) Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
} }
SubdiagnosticKind::Note | SubdiagnosticKind::Help | SubdiagnosticKind::Warn => { SubdiagnosticKind::Note | SubdiagnosticKind::Help | SubdiagnosticKind::Warn => {
if type_matches_path(info.ty, &["rustc_span", "Span"]) { if type_matches_path(info.ty.inner_type(), &["rustc_span", "Span"]) {
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug)) Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
} else if type_is_unit(info.ty) { } else if type_is_unit(info.ty.inner_type()) {
Ok(self.add_subdiagnostic(&fn_ident, slug)) Ok(self.add_subdiagnostic(&fn_ident, slug))
} else { } else {
report_type_error(attr, "`Span` or `()`")? report_type_error(attr, "`Span` or `()`")?
@ -432,6 +428,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
code_field, code_field,
code_init, code_init,
} => { } => {
if let FieldInnerTy::Vec(_) = info.ty {
throw_invalid_attr!(attr, &meta, |diag| {
diag
.note("`#[suggestion(...)]` applied to `Vec` field is ambiguous")
.help("to show a suggestion consisting of multiple parts, use a `Subdiagnostic` annotated with `#[multipart_suggestion(...)]`")
.help("to show a variable set of suggestions, use a `Vec` of `Subdiagnostic`s annotated with `#[suggestion(...)]`")
});
}
let (span_field, mut applicability) = self.span_and_applicability_of_ty(info)?; let (span_field, mut applicability) = self.span_and_applicability_of_ty(info)?;
if let Some((static_applicability, span)) = static_applicability { if let Some((static_applicability, span)) = static_applicability {
@ -489,7 +494,7 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
&self, &self,
info: FieldInfo<'_>, info: FieldInfo<'_>,
) -> Result<(TokenStream, SpannedOption<TokenStream>), DiagnosticDeriveError> { ) -> Result<(TokenStream, SpannedOption<TokenStream>), DiagnosticDeriveError> {
match &info.ty { match &info.ty.inner_type() {
// If `ty` is `Span` w/out applicability, then use `Applicability::Unspecified`. // If `ty` is `Span` w/out applicability, then use `Applicability::Unspecified`.
ty @ Type::Path(..) if type_matches_path(ty, &["rustc_span", "Span"]) => { ty @ Type::Path(..) if type_matches_path(ty, &["rustc_span", "Span"]) => {
let binding = &info.binding.binding; let binding = &info.binding.binding;

View file

@ -247,11 +247,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
return quote! {}; return quote! {};
} }
let info = FieldInfo { let info = FieldInfo { binding, ty: inner_ty, span: &ast.span() };
binding,
ty: inner_ty.inner_type().unwrap_or(&ast.ty),
span: &ast.span(),
};
let generated = self let generated = self
.generate_field_code_inner(kind_stats, attr, info, inner_ty.will_iterate()) .generate_field_code_inner(kind_stats, attr, info, inner_ty.will_iterate())
@ -312,6 +308,21 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
let binding = info.binding.binding.clone(); let binding = info.binding.binding.clone();
// FIXME(#100717): support `Option<Span>` on `primary_span` like in the // FIXME(#100717): support `Option<Span>` on `primary_span` like in the
// diagnostic derive // diagnostic derive
if !matches!(info.ty, FieldInnerTy::Plain(_)) {
throw_invalid_attr!(attr, &Meta::Path(path), |diag| {
let diag = diag.note("there must be exactly one primary span");
if kind_stats.has_normal_suggestion {
diag.help(
"to create a suggestion with multiple spans, \
use `#[multipart_suggestion]` instead",
)
} else {
diag
}
});
}
self.span_field.set_once(binding, span); self.span_field.set_once(binding, span);
} }

View file

@ -80,7 +80,7 @@ fn report_error_if_not_applied_to_ty(
path: &[&str], path: &[&str],
ty_name: &str, ty_name: &str,
) -> Result<(), DiagnosticDeriveError> { ) -> Result<(), DiagnosticDeriveError> {
if !type_matches_path(info.ty, path) { if !type_matches_path(info.ty.inner_type(), path) {
report_type_error(attr, ty_name)?; report_type_error(attr, ty_name)?;
} }
@ -105,8 +105,8 @@ pub(crate) fn report_error_if_not_applied_to_span(
attr: &Attribute, attr: &Attribute,
info: &FieldInfo<'_>, info: &FieldInfo<'_>,
) -> Result<(), DiagnosticDeriveError> { ) -> Result<(), DiagnosticDeriveError> {
if !type_matches_path(info.ty, &["rustc_span", "Span"]) if !type_matches_path(info.ty.inner_type(), &["rustc_span", "Span"])
&& !type_matches_path(info.ty, &["rustc_errors", "MultiSpan"]) && !type_matches_path(info.ty.inner_type(), &["rustc_errors", "MultiSpan"])
{ {
report_type_error(attr, "`Span` or `MultiSpan`")?; report_type_error(attr, "`Span` or `MultiSpan`")?;
} }
@ -115,44 +115,50 @@ pub(crate) fn report_error_if_not_applied_to_span(
} }
/// Inner type of a field and type of wrapper. /// Inner type of a field and type of wrapper.
#[derive(Copy, Clone)]
pub(crate) enum FieldInnerTy<'ty> { pub(crate) enum FieldInnerTy<'ty> {
/// Field is wrapped in a `Option<$inner>`. /// Field is wrapped in a `Option<$inner>`.
Option(&'ty Type), Option(&'ty Type),
/// Field is wrapped in a `Vec<$inner>`. /// Field is wrapped in a `Vec<$inner>`.
Vec(&'ty Type), Vec(&'ty Type),
/// Field isn't wrapped in an outer type. /// Field isn't wrapped in an outer type.
None, Plain(&'ty Type),
} }
impl<'ty> FieldInnerTy<'ty> { impl<'ty> FieldInnerTy<'ty> {
/// Returns inner type for a field, if there is one. /// Returns inner type for a field, if there is one.
/// ///
/// - If `ty` is an `Option`, returns `FieldInnerTy::Option { inner: (inner type) }`. /// - If `ty` is an `Option<Inner>`, returns `FieldInnerTy::Option(Inner)`.
/// - If `ty` is a `Vec`, returns `FieldInnerTy::Vec { inner: (inner type) }`. /// - If `ty` is a `Vec<Inner>`, returns `FieldInnerTy::Vec(Inner)`.
/// - Otherwise returns `None`. /// - Otherwise returns `FieldInnerTy::Plain(ty)`.
pub(crate) fn from_type(ty: &'ty Type) -> Self { pub(crate) fn from_type(ty: &'ty Type) -> Self {
let variant: &dyn Fn(&'ty Type) -> FieldInnerTy<'ty> = fn single_generic_type(ty: &Type) -> &Type {
if type_matches_path(ty, &["std", "option", "Option"]) { let Type::Path(ty_path) = ty else {
&FieldInnerTy::Option panic!("expected path type");
} else if type_matches_path(ty, &["std", "vec", "Vec"]) {
&FieldInnerTy::Vec
} else {
return FieldInnerTy::None;
}; };
if let Type::Path(ty_path) = ty {
let path = &ty_path.path; let path = &ty_path.path;
let ty = path.segments.iter().last().unwrap(); let ty = path.segments.iter().last().unwrap();
if let syn::PathArguments::AngleBracketed(bracketed) = &ty.arguments { let syn::PathArguments::AngleBracketed(bracketed) = &ty.arguments else {
if bracketed.args.len() == 1 { panic!("expected bracketed generic arguments");
if let syn::GenericArgument::Type(ty) = &bracketed.args[0] { };
return variant(ty);
} assert_eq!(bracketed.args.len(), 1);
}
} let syn::GenericArgument::Type(ty) = &bracketed.args[0] else {
panic!("expected generic parameter to be a type generic");
};
ty
} }
unreachable!(); if type_matches_path(ty, &["std", "option", "Option"]) {
FieldInnerTy::Option(single_generic_type(ty))
} else if type_matches_path(ty, &["std", "vec", "Vec"]) {
FieldInnerTy::Vec(single_generic_type(ty))
} else {
FieldInnerTy::Plain(ty)
}
} }
/// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e. /// Returns `true` if `FieldInnerTy::with` will result in iteration for this inner type (i.e.
@ -160,15 +166,16 @@ impl<'ty> FieldInnerTy<'ty> {
pub(crate) fn will_iterate(&self) -> bool { pub(crate) fn will_iterate(&self) -> bool {
match self { match self {
FieldInnerTy::Vec(..) => true, FieldInnerTy::Vec(..) => true,
FieldInnerTy::Option(..) | FieldInnerTy::None => false, FieldInnerTy::Option(..) | FieldInnerTy::Plain(_) => false,
} }
} }
/// Returns `Option` containing inner type if there is one. /// Returns the inner type.
pub(crate) fn inner_type(&self) -> Option<&'ty Type> { pub(crate) fn inner_type(&self) -> &'ty Type {
match self { match self {
FieldInnerTy::Option(inner) | FieldInnerTy::Vec(inner) => Some(inner), FieldInnerTy::Option(inner) | FieldInnerTy::Vec(inner) | FieldInnerTy::Plain(inner) => {
FieldInnerTy::None => None, inner
}
} }
} }
@ -185,7 +192,7 @@ impl<'ty> FieldInnerTy<'ty> {
#inner #inner
} }
}, },
FieldInnerTy::None => quote! { #inner }, FieldInnerTy::Plain(..) => quote! { #inner },
} }
} }
} }
@ -194,7 +201,7 @@ impl<'ty> FieldInnerTy<'ty> {
/// `generate_*` methods from walking the attributes themselves. /// `generate_*` methods from walking the attributes themselves.
pub(crate) struct FieldInfo<'a> { pub(crate) struct FieldInfo<'a> {
pub(crate) binding: &'a BindingInfo<'a>, pub(crate) binding: &'a BindingInfo<'a>,
pub(crate) ty: &'a Type, pub(crate) ty: FieldInnerTy<'a>,
pub(crate) span: &'a proc_macro2::Span, pub(crate) span: &'a proc_macro2::Span,
} }

View file

@ -799,3 +799,11 @@ struct SuggestionStyleGood {
#[suggestion(code = "", style = "hidden")] #[suggestion(code = "", style = "hidden")]
sub: Span, sub: Span,
} }
#[derive(Diagnostic)]
#[diag(compiletest_example)]
struct SuggestionOnVec {
#[suggestion(suggestion, code = "")]
//~^ ERROR `#[suggestion(...)]` is not a valid attribute
sub: Vec<Span>,
}

View file

@ -589,6 +589,16 @@ error: `code = "..."`/`code(...)` must contain only string literals
LL | #[suggestion(code = 3)] LL | #[suggestion(code = 3)]
| ^^^^^^^^ | ^^^^^^^^
error: `#[suggestion(...)]` is not a valid attribute
--> $DIR/diagnostic-derive.rs:806:5
|
LL | #[suggestion(suggestion, code = "")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[suggestion(...)]` applied to `Vec` field is ambiguous
= help: to show a suggestion consisting of multiple parts, use a `Subdiagnostic` annotated with `#[multipart_suggestion(...)]`
= help: to show a variable set of suggestions, use a `Vec` of `Subdiagnostic`s annotated with `#[suggestion(...)]`
error: cannot find attribute `nonsense` in this scope error: cannot find attribute `nonsense` in this scope
--> $DIR/diagnostic-derive.rs:55:3 --> $DIR/diagnostic-derive.rs:55:3
| |
@ -660,7 +670,7 @@ note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg`
--> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:LL:CC --> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:LL:CC
= 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) = 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)
error: aborting due to 83 previous errors error: aborting due to 84 previous errors
Some errors have detailed explanations: E0277, E0425. Some errors have detailed explanations: E0277, E0425.
For more information about an error, try `rustc --explain E0277`. For more information about an error, try `rustc --explain E0277`.

View file

@ -798,3 +798,13 @@ struct SuggestionStyleInvalid4 {
#[primary_span] #[primary_span]
sub: Span, sub: Span,
} }
#[derive(Subdiagnostic)]
#[suggestion(parse_add_paren, code = "")]
//~^ ERROR suggestion without `#[primary_span]` field
struct PrimarySpanOnVec {
#[primary_span]
//~^ ERROR `#[primary_span]` is not a valid attribute
//~| NOTE there must be exactly one primary span
sub: Vec<Span>,
}

View file

@ -501,6 +501,27 @@ error: `#[suggestion(style(...))]` is not a valid attribute
LL | #[suggestion(parse_add_paren, code = "", style("foo"))] LL | #[suggestion(parse_add_paren, code = "", style("foo"))]
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^
error: `#[primary_span]` is not a valid attribute
--> $DIR/subdiagnostic-derive.rs:806:5
|
LL | #[primary_span]
| ^^^^^^^^^^^^^^^
|
= note: there must be exactly one primary span
= help: to create a suggestion with multiple spans, use `#[multipart_suggestion]` instead
error: suggestion without `#[primary_span]` field
--> $DIR/subdiagnostic-derive.rs:803:1
|
LL | / #[suggestion(parse_add_paren, code = "")]
LL | |
LL | | struct PrimarySpanOnVec {
LL | | #[primary_span]
... |
LL | | sub: Vec<Span>,
LL | | }
| |_^
error: cannot find attribute `foo` in this scope error: cannot find attribute `foo` in this scope
--> $DIR/subdiagnostic-derive.rs:63:3 --> $DIR/subdiagnostic-derive.rs:63:3
| |
@ -561,6 +582,6 @@ error[E0425]: cannot find value `slug` in module `rustc_errors::fluent`
LL | #[label(slug)] LL | #[label(slug)]
| ^^^^ not found in `rustc_errors::fluent` | ^^^^ not found in `rustc_errors::fluent`
error: aborting due to 79 previous errors error: aborting due to 81 previous errors
For more information about this error, try `rustc --explain E0425`. For more information about this error, try `rustc --explain E0425`.