Rollup merge of rust-lang#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.

Author: compiler-errors

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<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

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,
@@ -210,19 +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();
-        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 @@ impl<'parent, 'a> SubdiagnosticDeriveVariantBuilder<'parent, 'a> {
         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 @@ 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();
 
@@ -580,7 +581,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();
 

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
@@ -851,7 +857,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 {

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
+}

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`.

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)]

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