Rollup merge of #111170 - compiler-errors:diag-doc, r=petrochenkov

Diagnostic args are still args if they're documented Fixes https://rust-lang.zulipchat.com/#narrow/stream/336883-i18n/topic/.60.23.5Bderive.28Diagnostic.29.5D.60.20works.20badly.20with.20docs/near/355597997 There's a lot of really strange code incongruencies between `Diagnostic` and `Subdiagnostic` derive. Perhaps those macros need some more overhaul, but I didn't really want to do it today.
2023-05-08 09:30:23 -07:00 · 2023-05-08 09:30:23 -07:00 · 24ba82de4e
commit 24ba82de4e
parent beb49671c2 a156bd7714
7 changed files with 116 additions and 25 deletions
--- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
+++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
@ -9,7 +9,7 @@
    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 @@ fn generate_field_code(&mut self, binding_info: &BindingInfo<'_>) -> TokenStream
        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 @@ fn generate_field_attrs_code(&mut self, binding_info: &BindingInfo<'_>) -> Token
                    name == "primary_span" && matches!(inner_ty, FieldInnerTy::Vec(_));
                let (binding, needs_destructure) = if needs_clone {
                    // `primary_span` can accept a `Vec<Span>` 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
--- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs
+++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs
@ -4,17 +4,16 @@
    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,
@ -210,19 +209,20 @@ fn identify_kind(&mut self) -> Result<Vec<(SubdiagnosticKind, Path)>, Diagnostic
    }

    /// 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");
-
+    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
            );
        }
    }
@ -399,7 +399,8 @@ fn generate_field_code_inner_list(
        clone_suggestion_code: bool,
    ) -> Result<TokenStream, DiagnosticDeriveError> {
        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();

@ -498,7 +499,7 @@ pub fn into_tokens(&mut self) -> Result<TokenStream, DiagnosticDeriveError> {
            .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();

@ -580,7 +581,7 @@ pub fn into_tokens(&mut self) -> Result<TokenStream, DiagnosticDeriveError> {
            .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();

--- a/compiler/rustc_macros/src/diagnostics/utils.rs
+++ b/compiler/rustc_macros/src/diagnostics/utils.rs
@ -207,6 +207,12 @@ pub(crate) fn with(&self, binding: impl ToTokens, inner: impl ToTokens) -> Token
            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
@ -851,7 +857,8 @@ fn span(&self) -> Option<proc_macro2::Span> {
 /// 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 {
--- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs
+++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.rs
@ -0,0 +1,49 @@
+// 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
+// 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)]
+#[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)]
+#[label(no_crate_example)]
+struct SubTest {
+    #[primary_span]
+    span: Span,
+    /// A doc comment
+    arg: NotIntoDiagnosticArg,
+    //~^ ERROR the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied
+}
--- a/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr
+++ b/tests/ui-fulldeps/session-diagnostic/diagnostic-derive-doc-comment-field.stderr
@ -0,0 +1,30 @@
+error[E0277]: the trait bound `NotIntoDiagnosticArg: IntoDiagnosticArg` is not satisfied
+  --> $DIR/diagnostic-derive-doc-comment-field.rs:37:10
+   |
+LL | #[derive(Diagnostic)]
+   |          ---------- required by a bound introduced by this call
+...
+LL |     arg: NotIntoDiagnosticArg,
+   |          ^^^^^^^^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg`
+   |
+   = 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 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:47:10
+   |
+LL | #[derive(Subdiagnostic)]
+   |          ------------- required by a bound introduced by this call
+...
+LL |     arg: NotIntoDiagnosticArg,
+   |          ^^^^^^^^^^^^^^^^^^^^ the trait `IntoDiagnosticArg` is not implemented for `NotIntoDiagnosticArg`
+   |
+   = help: normalized in stderr
+note: required by a bound in `Diagnostic::set_arg`
+  --> $COMPILER_DIR/rustc_errors/src/diagnostic.rs:964:5
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.
--- 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)]
--- 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