Skip to content

Commit

Permalink
macros: use typed identifiers in subdiag derive
Browse files Browse the repository at this point in the history
As in the diagnostic derive, using typed identifiers in the
subdiagnostic derive improves the diagnostics of using the subdiagnostic
derive as Fluent messages will be confirmed to exist at compile-time.

Signed-off-by: David Wood <david.wood@huawei.com>
  • Loading branch information
davidtwco committed Jun 24, 2022
1 parent 99bc979 commit abd3467
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 197 deletions.
21 changes: 21 additions & 0 deletions compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,27 @@ impl<S: Into<String>> From<S> for DiagnosticMessage {
}
}

/// Translating *into* a subdiagnostic message from a diagnostic message is a little strange - but
/// the subdiagnostic functions (e.g. `span_label`) take a `SubdiagnosticMessage` and the
/// subdiagnostic derive refers to typed identifiers that are `DiagnosticMessage`s, so need to be
/// able to convert between these, as much as they'll be converted back into `DiagnosticMessage`
/// using `with_subdiagnostic_message` eventually. Don't use this other than for the derive.
impl Into<SubdiagnosticMessage> for DiagnosticMessage {
fn into(self) -> SubdiagnosticMessage {
match self {
DiagnosticMessage::Str(s) => SubdiagnosticMessage::Str(s),
DiagnosticMessage::FluentIdentifier(id, None) => {
SubdiagnosticMessage::FluentIdentifier(id)
}
// There isn't really a sensible behaviour for this because it loses information but
// this is the most sensible of the behaviours.
DiagnosticMessage::FluentIdentifier(_, Some(attr)) => {
SubdiagnosticMessage::FluentAttr(attr)
}
}
}
}

/// A span together with some additional data.
#[derive(Clone, Debug)]
pub struct SpanLabel {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_macros/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream {
/// ```ignore (rust)
/// #[derive(SessionSubdiagnostic)]
/// pub enum ExpectedIdentifierLabel<'tcx> {
/// #[label(slug = "parser-expected-identifier")]
/// #[label(parser::expected_identifier)]
/// WithoutFound {
/// #[primary_span]
/// span: Span,
/// }
/// #[label(slug = "parser-expected-identifier-found")]
/// #[label(parser::expected_identifier_found)]
/// WithFound {
/// #[primary_span]
/// span: Span,
Expand All @@ -86,7 +86,7 @@ pub fn session_diagnostic_derive(s: Structure<'_>) -> TokenStream {
/// }
///
/// #[derive(SessionSubdiagnostic)]
/// #[suggestion_verbose(slug = "parser-raw-identifier")]
/// #[suggestion_verbose(parser::raw_identifier)]
/// pub struct RawIdentifierSuggestion<'tcx> {
/// #[primary_span]
/// span: Span,
Expand Down
84 changes: 72 additions & 12 deletions compiler/rustc_macros/src/diagnostics/subdiagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use quote::{format_ident, quote};
use std::collections::HashMap;
use std::fmt;
use std::str::FromStr;
use syn::{spanned::Spanned, Meta, MetaList, MetaNameValue};
use syn::{parse_quote, spanned::Spanned, Meta, MetaList, MetaNameValue, NestedMeta, Path};
use synstructure::{BindingInfo, Structure, VariantInfo};

/// Which kind of suggestion is being created?
Expand Down Expand Up @@ -194,8 +194,8 @@ struct SessionSubdiagnosticDeriveBuilder<'a> {
kind: Option<(SubdiagnosticKind, proc_macro::Span)>,

/// Slug of the subdiagnostic - corresponds to the Fluent identifier for the message - from the
/// `#[kind(slug = "...")]` attribute on the type or variant.
slug: Option<(String, proc_macro::Span)>,
/// `#[kind(slug)]` attribute on the type or variant.
slug: Option<(Path, proc_macro::Span)>,
/// If a suggestion, the code to suggest as a replacement - from the `#[kind(code = "...")]`
/// attribute on the type or variant.
code: Option<(TokenStream, proc_macro::Span)>,
Expand Down Expand Up @@ -224,9 +224,34 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
let meta = attr.parse_meta()?;
let kind = match meta {
Meta::List(MetaList { ref nested, .. }) => {
for nested_attr in nested {
let mut nested_iter = nested.into_iter();
if let Some(nested_attr) = nested_iter.next() {
match nested_attr {
NestedMeta::Meta(Meta::Path(path)) => {
self.slug.set_once((path.clone(), span));
}
NestedMeta::Meta(meta @ Meta::NameValue(_))
if matches!(
meta.path().segments.last().unwrap().ident.to_string().as_str(),
"code" | "applicability"
) =>
{
// don't error for valid follow-up attributes
}
nested_attr => {
throw_invalid_nested_attr!(attr, &nested_attr, |diag| {
diag.help(
"first argument of the attribute should be the diagnostic \
slug",
)
})
}
};
}

for nested_attr in nested_iter {
let meta = match nested_attr {
syn::NestedMeta::Meta(ref meta) => meta,
NestedMeta::Meta(ref meta) => meta,
_ => throw_invalid_nested_attr!(attr, &nested_attr),
};

Expand All @@ -241,7 +266,6 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
let formatted_str = self.build_format(&s.value(), s.span());
self.code.set_once((formatted_str, span));
}
"slug" => self.slug.set_once((s.value(), span)),
"applicability" => {
let value = match Applicability::from_str(&s.value()) {
Ok(v) => v,
Expand All @@ -253,11 +277,23 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
self.applicability.set_once((quote! { #value }, span));
}
_ => throw_invalid_nested_attr!(attr, &nested_attr, |diag| {
diag.help("only `code`, `slug` and `applicability` are valid nested attributes")
diag.help(
"only `code` and `applicability` are valid nested \
attributes",
)
}),
}
}
_ => throw_invalid_nested_attr!(attr, &nested_attr),
_ => throw_invalid_nested_attr!(attr, &nested_attr, |diag| {
if matches!(meta, Meta::Path(_)) {
diag.help(
"a diagnostic slug must be the first argument to the \
attribute",
)
} else {
diag
}
}),
}
}

Expand All @@ -281,10 +317,27 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
);
}

if matches!(
kind,
SubdiagnosticKind::Label | SubdiagnosticKind::Help | SubdiagnosticKind::Note
) && self.applicability.is_some()
{
throw_span_err!(
span,
&format!(
"`applicability` is not a valid nested attribute of a `{}` attribute",
name
)
);
}

if self.slug.is_none() {
throw_span_err!(
span,
&format!("`slug` must be set in a `#[{}(...)]` attribute", name)
&format!(
"diagnostic slug must be first argument of a `#[{}(...)]` attribute",
name
)
);
}

Expand Down Expand Up @@ -335,7 +388,10 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
return Ok(quote! {});
}
_ => throw_invalid_attr!(attr, &meta, |diag| {
diag.help("only `primary_span`, `applicability` and `skip_arg` are valid field attributes")
diag.help(
"only `primary_span`, `applicability` and `skip_arg` are valid field \
attributes",
)
}),
},
_ => throw_invalid_attr!(attr, &meta),
Expand Down Expand Up @@ -375,7 +431,11 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
}

// Missing slug errors will already have been reported.
let slug = self.slug.as_ref().map(|(slug, _)| &**slug).unwrap_or("missing-slug");
let slug = self
.slug
.as_ref()
.map(|(slug, _)| slug.clone())
.unwrap_or_else(|| parse_quote! { you::need::to::specify::a::slug });
let code = match self.code.as_ref() {
Some((code, _)) => Some(quote! { #code }),
None if is_suggestion => {
Expand All @@ -397,7 +457,7 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {

let diag = &self.diag;
let name = format_ident!("{}{}", if span_field.is_some() { "span_" } else { "" }, kind);
let message = quote! { rustc_errors::SubdiagnosticMessage::message(#slug) };
let message = quote! { rustc_errors::fluent::#slug };
let call = if matches!(kind, SubdiagnosticKind::Suggestion(..)) {
if let Some(span) = span_field {
quote! { #diag.#name(#span, #message, #code, #applicability); }
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ struct BadTypePlus {
#[derive(SessionSubdiagnostic)]
pub enum BadTypePlusSub {
#[suggestion(
slug = "parser-add-paren",
parser::add_paren,
code = "{sum_with_parens}",
applicability = "machine-applicable"
)]
Expand All @@ -274,12 +274,12 @@ pub enum BadTypePlusSub {
#[primary_span]
span: Span,
},
#[label(slug = "parser-forgot-paren")]
#[label(parser::forgot_paren)]
ForgotParen {
#[primary_span]
span: Span,
},
#[label(slug = "parser-expect-path")]
#[label(parser::expect_path)]
ExpectPath {
#[primary_span]
span: Span,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ pub struct AddressOfTemporaryTaken {
#[derive(SessionSubdiagnostic)]
pub enum AddReturnTypeSuggestion<'tcx> {
#[suggestion(
slug = "typeck-add-return-type-add",
typeck::add_return_type_add,
code = "-> {found} ",
applicability = "machine-applicable"
)]
Expand All @@ -207,7 +207,7 @@ pub enum AddReturnTypeSuggestion<'tcx> {
found: Ty<'tcx>,
},
#[suggestion(
slug = "typeck-add-return-type-missing-here",
typeck::add_return_type_missing_here,
code = "-> _ ",
applicability = "has-placeholders"
)]
Expand All @@ -219,12 +219,12 @@ pub enum AddReturnTypeSuggestion<'tcx> {

#[derive(SessionSubdiagnostic)]
pub enum ExpectedReturnTypeLabel<'tcx> {
#[label(slug = "typeck-expected-default-return-type")]
#[label(typeck::expected_default_return_type)]
Unit {
#[primary_span]
span: Span,
},
#[label(slug = "typeck-expected-return-type")]
#[label(typeck::expected_return_type)]
Other {
#[primary_span]
span: Span,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui-fulldeps/internal-lints/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct DeriveSessionDiagnostic {
}

#[derive(SessionSubdiagnostic)]
#[note(slug = "note")]
#[note(parser::add_paren)]
struct Note {
#[primary_span]
span: Span,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ struct NoApplicability {
}

#[derive(SessionSubdiagnostic)]
#[note(slug = "note")]
#[note(parser::add_paren)]
struct Note;

#[derive(SessionDiagnostic)]
Expand Down
Loading

0 comments on commit abd3467

Please sign in to comment.