From 2b9279f3131056a1a1dd5de7513de4eb98987770 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 3 May 2023 23:22:57 +0000 Subject: [PATCH 1/2] Diagnostic args are still args if they're documented --- .../src/diagnostics/subdiagnostic.rs | 12 ++--- .../rustc_macros/src/diagnostics/utils.rs | 3 +- .../diagnostic-derive-doc-comment-field.rs | 47 +++++++++++++++++++ ...diagnostic-derive-doc-comment-field.stderr | 43 +++++++++++++++++ 4 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs create mode 100644 tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 62d49c1c64e27..83b47f8acfbd6 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -4,17 +4,16 @@ use crate::diagnostics::error::{ invalid_attr, span_err, throw_invalid_attr, throw_span_err, DiagnosticDeriveError, }; use crate::diagnostics::utils::{ - build_field_mapping, is_doc_comment, new_code_ident, - report_error_if_not_applied_to_applicability, report_error_if_not_applied_to_span, FieldInfo, - FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, + build_field_mapping, build_suggestion_code, is_doc_comment, new_code_ident, + report_error_if_not_applied_to_applicability, report_error_if_not_applied_to_span, + should_generate_set_arg, AllowMultipleAlternatives, FieldInfo, FieldInnerTy, FieldMap, + HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; use proc_macro2::TokenStream; use quote::{format_ident, quote}; use syn::{spanned::Spanned, Attribute, Meta, MetaList, Path}; use synstructure::{BindingInfo, Structure, VariantInfo}; -use super::utils::{build_suggestion_code, AllowMultipleAlternatives}; - /// The central struct for constructing the `add_to_diagnostic` method from an annotated struct. pub(crate) struct SubdiagnosticDeriveBuilder { diag: syn::Ident, @@ -212,7 +211,6 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { /// Generates the code for a field with no attributes. fn generate_field_set_arg(&mut self, binding: &BindingInfo<'_>) -> TokenStream { let ast = binding.ast(); - assert_eq!(ast.attrs.len(), 0, "field with attribute used as diagnostic arg"); let diag = &self.parent.diag; let ident = ast.ident.as_ref().unwrap(); @@ -580,7 +578,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { .variant .bindings() .iter() - .filter(|binding| binding.ast().attrs.is_empty()) + .filter(|binding| should_generate_set_arg(binding.ast())) .map(|binding| self.generate_field_set_arg(binding)) .collect(); diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index b9b09c66230b8..bc97e39bebd8a 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -851,7 +851,8 @@ impl quote::IdentFragment for SubdiagnosticKind { /// Returns `true` if `field` should generate a `set_arg` call rather than any other diagnostic /// call (like `span_label`). pub(super) fn should_generate_set_arg(field: &Field) -> bool { - field.attrs.is_empty() + // Perhaps this should be an exhaustive list... + field.attrs.iter().all(|attr| is_doc_comment(attr)) } pub(super) fn is_doc_comment(attr: &Attribute) -> bool { diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs new file mode 100644 index 0000000000000..7e1f7b1c5c10d --- /dev/null +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs @@ -0,0 +1,47 @@ +// check-fail +// Tests that a doc comment will not preclude a field from being considered a diagnostic argument + +// The proc_macro2 crate handles spans differently when on beta/stable release rather than nightly, +// changing the output of this test. Since Subdiagnostic is strictly internal to the compiler +// the test is just ignored on stable and beta: +// ignore-stage1 +// ignore-beta +// ignore-stable + +#![feature(rustc_private)] +#![crate_type = "lib"] + +extern crate rustc_errors; +extern crate rustc_fluent_macro; +extern crate rustc_macros; +extern crate rustc_session; +extern crate rustc_span; + +use rustc_errors::{Applicability, DiagnosticMessage, SubdiagnosticMessage}; +use rustc_fluent_macro::fluent_messages; +use rustc_macros::{Diagnostic, Subdiagnostic}; +use rustc_span::Span; + +fluent_messages! { "./example.ftl" } + +struct NotIntoDiagnosticArg; + +#[derive(Diagnostic)] +//~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied +#[diag(no_crate_example)] +struct Test { + #[primary_span] + span: Span, + /// A doc comment + arg: NotIntoDiagnosticArg, +} + +#[derive(Subdiagnostic)] +//~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied +#[label(no_crate_example)] +struct SubTest { + #[primary_span] + span: Span, + /// A doc comment + arg: NotIntoDiagnosticArg, +} diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr new file mode 100644 index 0000000000000..27044748d0803 --- /dev/null +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr @@ -0,0 +1,43 @@ +error[E0277]: the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied + --> $DIR/diagnostic-derive-doc-comment-field.rs:29:10 + | +LL | #[derive(Diagnostic)] + | ^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` + | + = help: the following other types implement trait `IntoDiagnosticArg`: + &'a T + &'a std::path::Path + &'a str + &rustc_target::spec::TargetTriple + Box<(dyn std::error::Error + 'static)> + CString + CguReuse + Cow<'a, str> + and 42 others +note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg` + --> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:747:5 + = 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[E0277]: the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied + --> $DIR/diagnostic-derive-doc-comment-field.rs:39:10 + | +LL | #[derive(Subdiagnostic)] + | ^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` + | + = help: the following other types implement trait `IntoDiagnosticArg`: + &'a T + &'a std::path::Path + &'a str + &rustc_target::spec::TargetTriple + Box<(dyn std::error::Error + 'static)> + CString + CguReuse + Cow<'a, str> + and 42 others +note: required by a bound in `Diagnostic::set_arg` + --> $COMPILER_DIR/rustc_errors/src/diagnostic.rs:964:5 + = note: this error originates in the derive macro `Subdiagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From a156bd771457110415b1eec74cf52c9502d461a3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 3 May 2023 23:53:44 +0000 Subject: [PATCH 2/2] Make spans a bit better --- .../src/diagnostics/diagnostic_builder.rs | 9 ++-- .../src/diagnostics/subdiagnostic.rs | 21 ++++++---- .../rustc_macros/src/diagnostics/utils.rs | 6 +++ .../diagnostic-derive-doc-comment-field.rs | 6 ++- ...diagnostic-derive-doc-comment-field.stderr | 41 +++++++------------ .../session-diagnostic/diagnostic-derive.rs | 2 +- .../diagnostic-derive.stderr | 9 ++-- 7 files changed, 48 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index 427c82c410b91..cd6e36874603f 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -9,7 +9,7 @@ use crate::diagnostics::utils::{ FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind, }; use proc_macro2::{Ident, Span, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, quote_spanned}; use syn::Token; use syn::{parse_quote, spanned::Spanned, Attribute, Meta, Path, Type}; use synstructure::{BindingInfo, Structure, VariantInfo}; @@ -251,7 +251,8 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { let diag = &self.parent.diag; let field = binding_info.ast(); - let field_binding = &binding_info.binding; + let mut field_binding = binding_info.binding.clone(); + field_binding.set_span(field.ty.span()); let ident = field.ident.as_ref().unwrap(); let ident = format_ident!("{}", ident); // strip `r#` prefix, if present @@ -284,9 +285,9 @@ impl<'a> DiagnosticDeriveVariantBuilder<'a> { name == "primary_span" && matches!(inner_ty, FieldInnerTy::Vec(_)); let (binding, needs_destructure) = if needs_clone { // `primary_span` can accept a `Vec` so don't destructure that. - (quote! { #field_binding.clone() }, false) + (quote_spanned! {inner_ty.span()=> #field_binding.clone() }, false) } else { - (quote! { #field_binding }, true) + (quote_spanned! {inner_ty.span()=> #field_binding }, true) }; let generated_code = self diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 83b47f8acfbd6..374ba1a45c06e 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -209,18 +209,20 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { } /// Generates the code for a field with no attributes. - fn generate_field_set_arg(&mut self, binding: &BindingInfo<'_>) -> TokenStream { - let ast = binding.ast(); - + fn generate_field_set_arg(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream { let diag = &self.parent.diag; - let ident = ast.ident.as_ref().unwrap(); - // strip `r#` prefix, if present - let ident = format_ident!("{}", ident); + + let field = binding_info.ast(); + let mut field_binding = binding_info.binding.clone(); + field_binding.set_span(field.ty.span()); + + let ident = field.ident.as_ref().unwrap(); + let ident = format_ident!("{}", ident); // strip `r#` prefix, if present quote! { #diag.set_arg( stringify!(#ident), - #binding + #field_binding ); } } @@ -397,7 +399,8 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { clone_suggestion_code: bool, ) -> Result { let span = attr.span().unwrap(); - let ident = &list.path.segments.last().unwrap().ident; + let mut ident = list.path.segments.last().unwrap().ident.clone(); + ident.set_span(info.ty.span()); let name = ident.to_string(); let name = name.as_str(); @@ -496,7 +499,7 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> { .variant .bindings() .iter() - .filter(|binding| !binding.ast().attrs.is_empty()) + .filter(|binding| !should_generate_set_arg(binding.ast())) .map(|binding| self.generate_field_attr_code(binding, kind_stats)) .collect(); diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index bc97e39bebd8a..e2434981f8d17 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -207,6 +207,12 @@ impl<'ty> FieldInnerTy<'ty> { FieldInnerTy::Plain(..) => quote! { #inner }, } } + + pub fn span(&self) -> proc_macro2::Span { + match self { + FieldInnerTy::Option(ty) | FieldInnerTy::Vec(ty) | FieldInnerTy::Plain(ty) => ty.span(), + } + } } /// Field information passed to the builder. Deliberately omits attrs to discourage the diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs index 7e1f7b1c5c10d..642b58b0753fd 100644 --- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs @@ -1,5 +1,7 @@ // check-fail // Tests that a doc comment will not preclude a field from being considered a diagnostic argument +// normalize-stderr-test "the following other types implement trait `IntoDiagnosticArg`:(?:.*\n){0,9}\s+and \d+ others" -> "normalized in stderr" +// normalize-stderr-test "diagnostic_builder\.rs:[0-9]+:[0-9]+" -> "diagnostic_builder.rs:LL:CC" // The proc_macro2 crate handles spans differently when on beta/stable release rather than nightly, // changing the output of this test. Since Subdiagnostic is strictly internal to the compiler @@ -27,21 +29,21 @@ fluent_messages! { "./example.ftl" } struct NotIntoDiagnosticArg; #[derive(Diagnostic)] -//~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied #[diag(no_crate_example)] struct Test { #[primary_span] span: Span, /// A doc comment arg: NotIntoDiagnosticArg, + //~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied } #[derive(Subdiagnostic)] -//~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied #[label(no_crate_example)] struct SubTest { #[primary_span] span: Span, /// A doc comment arg: NotIntoDiagnosticArg, + //~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied } diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr index 27044748d0803..e4b8958b4fae8 100644 --- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr @@ -1,42 +1,29 @@ error[E0277]: the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied - --> $DIR/diagnostic-derive-doc-comment-field.rs:29:10 + --> $DIR/diagnostic-derive-doc-comment-field.rs:37:10 | LL | #[derive(Diagnostic)] - | ^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` + | ---------- required by a bound introduced by this call +... +LL | arg: NotIntoDiagnosticArg, + | ^^^^^^^^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` | - = help: the following other types implement trait `IntoDiagnosticArg`: - &'a T - &'a std::path::Path - &'a str - &rustc_target::spec::TargetTriple - Box<(dyn std::error::Error + 'static)> - CString - CguReuse - Cow<'a, str> - and 42 others + = help: normalized in stderr note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg` - --> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:747:5 - = 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) + --> $COMPILER_DIR/rustc_errors/src/diagnostic_builder.rs:LL:CC + = note: this error originates in the macro `forward` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied - --> $DIR/diagnostic-derive-doc-comment-field.rs:39:10 + --> $DIR/diagnostic-derive-doc-comment-field.rs:47:10 | LL | #[derive(Subdiagnostic)] - | ^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` + | ------------- required by a bound introduced by this call +... +LL | arg: NotIntoDiagnosticArg, + | ^^^^^^^^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg` | - = help: the following other types implement trait `IntoDiagnosticArg`: - &'a T - &'a std::path::Path - &'a str - &rustc_target::spec::TargetTriple - Box<(dyn std::error::Error + 'static)> - CString - CguReuse - Cow<'a, str> - and 42 others + = help: normalized in stderr note: required by a bound in `Diagnostic::set_arg` --> $COMPILER_DIR/rustc_errors/src/diagnostic.rs:964:5 - = note: this error originates in the derive macro `Subdiagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 2 previous errors diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index 80ab03ab24c86..39e34d73f9a43 100644 --- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -339,12 +339,12 @@ struct ErrorWithDefaultLabelAttr<'a> { } #[derive(Diagnostic)] -//~^ ERROR the trait bound `Hello: IntoDiagnosticArg` is not satisfied #[diag(no_crate_example, code = "E0123")] struct ArgFieldWithoutSkip { #[primary_span] span: Span, other: Hello, + //~^ ERROR the trait bound `Hello: IntoDiagnosticArg` is not satisfied } #[derive(Diagnostic)] diff --git a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr index 5e1bea4e38cc3..801e4b5793cc1 100644 --- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr +++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive.stderr @@ -641,15 +641,18 @@ LL | #[derive(Diagnostic)] = note: this error originates in the derive macro `Diagnostic` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Hello: IntoDiagnosticArg` is not satisfied - --> $DIR/diagnostic-derive.rs:341:10 + --> $DIR/diagnostic-derive.rs:346:12 | LL | #[derive(Diagnostic)] - | ^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `Hello` + | ---------- required by a bound introduced by this call +... +LL | other: Hello, + | ^^^^^ the trait `IntoDiagnosticArg` is not implemented for `Hello` | = help: normalized in stderr note: required by a bound in `DiagnosticBuilder::<'a, G>::set_arg` --> $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 macro `forward` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 84 previous errors