Rollup merge of #127591 - compiler-errors:label-after-primary, r=lcnr

Make sure that labels are defined after the primary span in diagnostics

Putting a `#[label]` before a `#[primary_span]` results in that label being overwritten, due to the semantics of `Diagnostic::span` and the fact that labels are stored in the `MultiSpan` of the diagnostic.

This isn't possible to fix in general, since a lot of code actually *relies* in this overwriting behavior (e.g. `rustc_on_unimplemented`). However, it's useful to enforce this for derive-diagnostics, since this is certainly never what you intend to do in a derived diagnostic, where all the fields are meaningful parts of the diagnostic being rendered.

This only matters for `#[label]`, since those are the ones stored in the `MultiSpan` of the error.

We could also make this "just work" by sorting the attrs or processing the primary span attr first, however I think it's kinda pointless to do.

There was 1 case where this mattered, but we literally didn't have a test exercising that diagnostic 🙃

4 files changed, 24 insertions, 1 deletions

diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
index 46bd80c2df6..f93d89d6c0f 100644
--- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
+++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs
@@ -269,6 +269,7 @@ impl DiagnosticDeriveVariantBuilder {
         let field_binding = &binding_info.binding;
 
         let inner_ty = FieldInnerTy::from_type(&field.ty);
+        let mut seen_label = false;
 
         field
             .attrs
@@ -280,6 +281,14 @@ impl DiagnosticDeriveVariantBuilder {
                 }
 
                 let name = attr.path().segments.last().unwrap().ident.to_string();
+
+                if name == "primary_span" && seen_label {
+                    span_err(attr.span().unwrap(), format!("`#[primary_span]` must be placed before labels, since it overwrites the span of the diagnostic")).emit();
+                }
+                if name == "label" {
+                    seen_label = true;
+                }
+
                 let needs_clone =
                     name == "primary_span" && matches!(inner_ty, FieldInnerTy::Vec(_));
                 let (binding, needs_destructure) = if needs_clone {
diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs
index 0620f3d709e..cd6503ae444 100644
--- a/compiler/rustc_resolve/src/errors.rs
+++ b/compiler/rustc_resolve/src/errors.rs
@@ -1089,8 +1089,8 @@ pub(crate) struct ToolWasAlreadyRegistered {
 #[derive(Diagnostic)]
 #[diag(resolve_tool_only_accepts_identifiers)]
 pub(crate) struct ToolOnlyAcceptsIdentifiers {
-    #[label]
     #[primary_span]
+    #[label]
     pub(crate) span: Span,
     pub(crate) tool: Symbol,
 }
diff --git a/tests/ui/tool-attributes/invalid-tool.rs b/tests/ui/tool-attributes/invalid-tool.rs
new file mode 100644
index 00000000000..125333231d2
--- /dev/null
+++ b/tests/ui/tool-attributes/invalid-tool.rs
@@ -0,0 +1,6 @@
+#![feature(register_tool)]
+
+#![register_tool(1)]
+//~^ ERROR `register_tool` only accepts identifiers
+
+fn main() {}
diff --git a/tests/ui/tool-attributes/invalid-tool.stderr b/tests/ui/tool-attributes/invalid-tool.stderr
new file mode 100644
index 00000000000..deafa6d167c
--- /dev/null
+++ b/tests/ui/tool-attributes/invalid-tool.stderr
@@ -0,0 +1,8 @@
+error: `register_tool` only accepts identifiers
+  --> $DIR/invalid-tool.rs:3:18
+   |
+LL | #![register_tool(1)]
+   |                  ^ not an identifier
+
+error: aborting due to 1 previous error
+