Skip to content

Commit

Permalink
Auto merge of #108402 - clubby789:diag-bool-not-unit, r=davidtwco
Browse files Browse the repository at this point in the history
Allow using `bool` instead of `Option<()>` in diagnostics

~~Disallow the unit type for `#[help]`, `#[note]` etc, instead using `bool` to express optional annotations without a span which I believe is more intuitive.~~

~~Test output ordering has changed in a few places, where a field was of type `()` and the annotation has been moved to the struct itself. If any of these changes are an issue, this can be restricted to allowing specifically `()`, and not `Option<()>`~~

~~Actual changes here: https://github.com/rust-lang/rust/pull/108402/files#diff-815b1d8debfc564112bd51093791d7c3f2ee288a37a8f5c0e89c11d1f609b4c0~~

Allows using `bool` in derive diagnostics to indicate an optional subdiagnostic without a span, where previously `Option<()>` had to be used

`@rustbot` label +A-diagnostics
  • Loading branch information
bors committed Mar 6, 2023
2 parents ac4379f + ef434f0 commit f63ccaf
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 45 deletions.
13 changes: 8 additions & 5 deletions compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::diagnostics::error::{
};
use crate::diagnostics::utils::{
build_field_mapping, is_doc_comment, report_error_if_not_applied_to_span, report_type_error,
should_generate_set_arg, type_is_unit, type_matches_path, FieldInfo, FieldInnerTy, FieldMap,
HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind,
should_generate_set_arg, type_is_bool, type_is_unit, type_matches_path, FieldInfo,
FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind,
};
use proc_macro2::{Ident, Span, TokenStream};
use quote::{format_ident, quote};
Expand Down Expand Up @@ -414,12 +414,15 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> {
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
}
SubdiagnosticKind::Note | SubdiagnosticKind::Help | SubdiagnosticKind::Warn => {
if type_matches_path(info.ty.inner_type(), &["rustc_span", "Span"]) {
let inner = info.ty.inner_type();
if type_matches_path(inner, &["rustc_span", "Span"]) {
Ok(self.add_spanned_subdiagnostic(binding, &fn_ident, slug))
} else if type_is_unit(info.ty.inner_type()) {
} else if type_is_unit(inner)
|| (matches!(info.ty, FieldInnerTy::Plain(_)) && type_is_bool(inner))
{
Ok(self.add_subdiagnostic(&fn_ident, slug))
} else {
report_type_error(attr, "`Span` or `()`")?
report_type_error(attr, "`Span`, `bool` or `()`")?
}
}
SubdiagnosticKind::Suggestion {
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_macros/src/diagnostics/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ pub(crate) fn type_is_unit(ty: &Type) -> bool {
if let Type::Tuple(TypeTuple { elems, .. }) = ty { elems.is_empty() } else { false }
}

/// Checks whether the type `ty` is `bool`.
pub(crate) fn type_is_bool(ty: &Type) -> bool {
type_matches_path(ty, &["bool"])
}

/// Reports a type error for field with `attr`.
pub(crate) fn report_type_error(
attr: &Attribute,
Expand Down Expand Up @@ -192,6 +197,11 @@ impl<'ty> FieldInnerTy<'ty> {
#inner
}
},
FieldInnerTy::Plain(t) if type_is_bool(t) => quote! {
if #binding {
#inner
}
},
FieldInnerTy::Plain(..) => quote! { #inner },
}
}
Expand Down
13 changes: 13 additions & 0 deletions tests/ui-fulldeps/session-diagnostic/diagnostic-derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,19 @@ struct OptUnitField {
bar: Option<()>,
}

#[derive(Diagnostic)]
#[diag(no_crate_example)]
struct BoolField {
#[primary_span]
spans: Span,
#[help]
foo: bool,
#[help(no_crate_help)]
//~^ ERROR the `#[help(...)]` attribute can only be applied to fields of type `Span`, `bool` or `()`
// only allow plain 'bool' fields
bar: Option<bool>,
}

#[derive(Diagnostic)]
#[diag(no_crate_example, code = "E0123")]
struct LabelWithTrailingPath {
Expand Down
Loading

0 comments on commit f63ccaf

Please sign in to comment.