Rollup merge of #116931 - weiznich:improve_diagnostic_on_unimplemented_warnings, r=compiler-errors

Improve the warning messages for the `#[diagnostic::on_unimplemented]`

This commit improves warnings emitted for malformed on unimplemented attributes by:

* Improving the span of the warnings
* Adding a label message to them
* Separating the messages for missing and unexpected options
* Adding a help message that says which options are supported

r? `@compiler-errors`

I'm happy to work on further improvements, so feel free to make suggestions.

compiler/rustc_trait_selection/messages.ftl

@@ -28,6 +28,11 @@ trait_selection_invalid_on_clause_in_rustc_on_unimplemented = invalid `on`-claus
     .label = invalid on-clause here
 
 trait_selection_malformed_on_unimplemented_attr = malformed `on_unimplemented` attribute
+    .help = only `message`, `note` and `label` are allowed as options
+    .label = invalid option found here
+
+trait_selection_missing_options_for_on_unimplemented_attr = missing options for `on_unimplemented` attribute
+    .help = at least one of the `message`, `note` and `label` options are expected
 
 trait_selection_negative_positive_conflict = found both positive and negative implementation of trait `{$trait_desc}`{$self_desc ->
         [none] {""}

compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs

@@ -1,5 +1,8 @@
 use super::{ObligationCauseCode, PredicateObligation};
 use crate::infer::error_reporting::TypeErrCtxt;
+use rustc_ast::AttrArgs;
+use rustc_ast::AttrArgsEq;
+use rustc_ast::AttrKind;
 use rustc_ast::{Attribute, MetaItem, NestedMetaItem};
 use rustc_attr as attr;
 use rustc_data_structures::fx::FxHashMap;
@@ -342,7 +345,22 @@ pub enum AppendConstMessage {
 
 #[derive(LintDiagnostic)]
 #[diag(trait_selection_malformed_on_unimplemented_attr)]
-pub struct NoValueInOnUnimplementedLint;
+#[help]
+pub struct MalformedOnUnimplementedAttrLint {
+    #[label]
+    pub span: Span,
+}
+
+impl MalformedOnUnimplementedAttrLint {
+    fn new(span: Span) -> Self {
+        Self { span }
+    }
+}
+
+#[derive(LintDiagnostic)]
+#[diag(trait_selection_missing_options_for_on_unimplemented_attr)]
+#[help]
+pub struct MissingOptionsForOnUnimplementedAttr;
 
 impl<'tcx> OnUnimplementedDirective {
     fn parse(
@@ -453,7 +471,7 @@ impl<'tcx> OnUnimplementedDirective {
                     UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
                     tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
                     vec![item.span()],
-                    NoValueInOnUnimplementedLint,
+                    MalformedOnUnimplementedAttrLint::new(item.span()),
                 );
             } else {
                 // nothing found
@@ -530,21 +548,40 @@ impl<'tcx> OnUnimplementedDirective {
                     append_const_msg: None,
                 }))
             } else {
+                let item = attr.get_normal_item();
+                let report_span = match &item.args {
+                    AttrArgs::Empty => item.path.span,
+                    AttrArgs::Delimited(args) => args.dspan.entire(),
+                    AttrArgs::Eq(eq_span, AttrArgsEq::Ast(expr)) => eq_span.to(expr.span),
+                    AttrArgs::Eq(span, AttrArgsEq::Hir(expr)) => span.to(expr.span),
+                };
+
                 tcx.emit_spanned_lint(
                     UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
                     tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
-                    attr.span,
+                    report_span,
-                    NoValueInOnUnimplementedLint,
+                    MalformedOnUnimplementedAttrLint::new(report_span),
                 );
                 Ok(None)
             }
         } else if is_diagnostic_namespace_variant {
-            tcx.emit_spanned_lint(
+            match &attr.kind {
-                UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                AttrKind::Normal(p) if !matches!(p.item.args, AttrArgs::Empty) => {
-                tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
+                    tcx.emit_spanned_lint(
-                attr.span,
+                        UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                NoValueInOnUnimplementedLint,
+                        tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
-            );
+                        attr.span,
+                        MalformedOnUnimplementedAttrLint::new(attr.span),
+                    );
+                }
+                _ => tcx.emit_spanned_lint(
+                    UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                    tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
+                    attr.span,
+                    MissingOptionsForOnUnimplementedAttr,
+                ),
+            };
+
             Ok(None)
         } else {
             let reported =

tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs

@@ -23,9 +23,15 @@ trait Boom {}
 //~^WARN malformed `on_unimplemented` attribute
 trait Doom {}
 
+#[diagnostic::on_unimplemented]
+//~^WARN missing options for `on_unimplemented` attribute
+//~|WARN missing options for `on_unimplemented` attribute
+trait Whatever {}
+
 fn take_foo(_: impl Foo) {}
 fn take_baz(_: impl Baz) {}
 fn take_boom(_: impl Boom) {}
+fn take_whatever(_: impl Whatever) {}
 
 fn main() {
     take_foo(1_i32);
@@ -34,4 +40,6 @@ fn main() {
     //~^ERROR Boom
     take_boom(1_i32);
     //~^ERROR Boom
+    take_whatever(1_i32);
+    //~^ERROR the trait bound `i32: Whatever` is not satisfied
 }

tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr

@@ -10,36 +10,53 @@ warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:3:32
    |
 LL | #[diagnostic::on_unimplemented(unsupported = "foo")]
-   |                                ^^^^^^^^^^^^^^^^^^^
+   |                                ^^^^^^^^^^^^^^^^^^^ invalid option found here
+   |
+   = help: only `message`, `note` and `label` are allowed as options
 
 warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:12:50
    |
 LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")]
-   |                                                  ^^^^^^^^^^^^^^^^^^^
+   |                                                  ^^^^^^^^^^^^^^^^^^^ invalid option found here
+   |
+   = help: only `message`, `note` and `label` are allowed as options
 
 warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50
    |
 LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))]
-   |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here
+   |
+   = help: only `message`, `note` and `label` are allowed as options
 
 warning: malformed `on_unimplemented` attribute
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:1
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:32
    |
 LL | #[diagnostic::on_unimplemented = "boom"]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                ^^^^^^^^ invalid option found here
+   |
+   = help: only `message`, `note` and `label` are allowed as options
+
+warning: missing options for `on_unimplemented` attribute
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1
+   |
+LL | #[diagnostic::on_unimplemented]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: at least one of the `message`, `note` and `label` options are expected
 
 warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:3:32
    |
 LL | #[diagnostic::on_unimplemented(unsupported = "foo")]
-   |                                ^^^^^^^^^^^^^^^^^^^
+   |                                ^^^^^^^^^^^^^^^^^^^ invalid option found here
    |
+   = help: only `message`, `note` and `label` are allowed as options
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error[E0277]: the trait bound `i32: Foo` is not satisfied
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:14
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:37:14
    |
 LL |     take_foo(1_i32);
    |     -------- ^^^^^ the trait `Foo` is not implemented for `i32`
@@ -52,7 +69,7 @@ help: this trait has no implementations, consider adding one
 LL | trait Foo {}
    | ^^^^^^^^^
 note: required by a bound in `take_foo`
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:21
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:21
    |
 LL | fn take_foo(_: impl Foo) {}
    |                     ^^^ required by this bound in `take_foo`
@@ -61,12 +78,13 @@ warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:12:50
    |
 LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")]
-   |                                                  ^^^^^^^^^^^^^^^^^^^
+   |                                                  ^^^^^^^^^^^^^^^^^^^ invalid option found here
    |
+   = help: only `message`, `note` and `label` are allowed as options
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error[E0277]: Boom
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:33:14
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:39:14
    |
 LL |     take_baz(1_i32);
    |     -------- ^^^^^ the trait `Baz` is not implemented for `i32`
@@ -79,7 +97,7 @@ help: this trait has no implementations, consider adding one
 LL | trait Baz {}
    | ^^^^^^^^^
 note: required by a bound in `take_baz`
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:27:21
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:32:21
    |
 LL | fn take_baz(_: impl Baz) {}
    |                     ^^^ required by this bound in `take_baz`
@@ -88,12 +106,13 @@ warning: malformed `on_unimplemented` attribute
   --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50
    |
 LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))]
-   |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here
    |
+   = help: only `message`, `note` and `label` are allowed as options
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
 error[E0277]: Boom
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:35:15
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:41:15
    |
 LL |     take_boom(1_i32);
    |     --------- ^^^^^ the trait `Boom` is not implemented for `i32`
@@ -106,11 +125,39 @@ help: this trait has no implementations, consider adding one
 LL | trait Boom {}
    | ^^^^^^^^^^
 note: required by a bound in `take_boom`
-  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:28:22
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:33:22
    |
 LL | fn take_boom(_: impl Boom) {}
    |                      ^^^^ required by this bound in `take_boom`
 
-error: aborting due to 3 previous errors; 8 warnings emitted
+warning: missing options for `on_unimplemented` attribute
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1
+   |
+LL | #[diagnostic::on_unimplemented]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: at least one of the `message`, `note` and `label` options are expected
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error[E0277]: the trait bound `i32: Whatever` is not satisfied
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:43:19
+   |
+LL |     take_whatever(1_i32);
+   |     ------------- ^^^^^ the trait `Whatever` is not implemented for `i32`
+   |     |
+   |     required by a bound introduced by this call
+   |
+help: this trait has no implementations, consider adding one
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:29:1
+   |
+LL | trait Whatever {}
+   | ^^^^^^^^^^^^^^
+note: required by a bound in `take_whatever`
+  --> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:34:26
+   |
+LL | fn take_whatever(_: impl Whatever) {}
+   |                          ^^^^^^^^ required by this bound in `take_whatever`
+
+error: aborting due to 4 previous errors; 10 warnings emitted
 
 For more information about this error, try `rustc --explain E0277`.

tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.rs

@@ -1,22 +1,20 @@
 #![feature(diagnostic_namespace)]
 
 #[diagnostic::on_unimplemented(
+    if(Self = "()"),
     //~^WARN malformed `on_unimplemented` attribute
     //~|WARN malformed `on_unimplemented` attribute
-    if(Self = ()),
+    message = "custom message",
-    message = "not used yet",
+    note = "custom note"
-    label = "not used yet",
-    note = "not used yet"
 )]
 #[diagnostic::on_unimplemented(message = "fallback!!")]
 #[diagnostic::on_unimplemented(label = "fallback label")]
 #[diagnostic::on_unimplemented(note = "fallback note")]
-#[diagnostic::on_unimplemented(message = "fallback2!!")]
 trait Foo {}
 
 fn takes_foo(_: impl Foo) {}
 
 fn main() {
     takes_foo(());
-    //~^ERROR fallback!!
+    //~^ERROR custom message
 }

tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr

@@ -1,33 +1,23 @@
 warning: malformed `on_unimplemented` attribute
-  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:3:1
+  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:4:5
    |
-LL | / #[diagnostic::on_unimplemented(
+LL |     if(Self = "()"),
-LL | |
+   |     ^^^^^^^^^^^^^^^ invalid option found here
-LL | |
-LL | |     if(Self = ()),
-...  |
-LL | |     note = "not used yet"
-LL | | )]
-   | |__^
    |
+   = help: only `message`, `note` and `label` are allowed as options
    = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default
 
 warning: malformed `on_unimplemented` attribute
-  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:3:1
+  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:4:5
    |
-LL | / #[diagnostic::on_unimplemented(
+LL |     if(Self = "()"),
-LL | |
+   |     ^^^^^^^^^^^^^^^ invalid option found here
-LL | |
-LL | |     if(Self = ()),
-...  |
-LL | |     note = "not used yet"
-LL | | )]
-   | |__^
    |
+   = help: only `message`, `note` and `label` are allowed as options
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
-error[E0277]: fallback!!
+error[E0277]: custom message
-  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:20:15
+  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:18:15
    |
 LL |     takes_foo(());
    |     --------- ^^ fallback label
@@ -35,14 +25,14 @@ LL |     takes_foo(());
    |     required by a bound introduced by this call
    |
    = help: the trait `Foo` is not implemented for `()`
-   = note: fallback note
+   = note: custom note
 help: this trait has no implementations, consider adding one
-  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:1
+  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:13:1
    |
 LL | trait Foo {}
    | ^^^^^^^^^
 note: required by a bound in `takes_foo`
-  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:17:22
+  --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:22
    |
 LL | fn takes_foo(_: impl Foo) {}
    |                      ^^^ required by this bound in `takes_foo`