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
rust-lang · Oct 19, 2023 · 6dfbcee · 6dfbcee
1 parent 020d008
commit 6dfbcee
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 27 deletions.
diff --git a/compiler/rustc_trait_selection/messages.ftl b/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] {""}

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/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,
-                    NoValueInOnUnimplementedLint,
+                    report_span,
+                    MalformedOnUnimplementedAttrLint::new(report_span),
                 );
                 Ok(None)
             }
         } else if is_diagnostic_namespace_variant {
-            tcx.emit_spanned_lint(
-                UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
-                attr.span,
-                NoValueInOnUnimplementedLint,
-            );
+            match &attr.kind {
+                AttrKind::Normal(p) if !matches!(p.item.args, AttrArgs::Empty) => {
+                    tcx.emit_spanned_lint(
+                        UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                        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 =

diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs b/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
 }
diff --git a/.../ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr b/.../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`.
diff --git a/...namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr b/...namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr
@@ -8,8 +8,9 @@ LL | |     if(Self = ()),
 ...  |
 LL | |     note = "not used yet"
 LL | | )]
-   | |__^
+   | |__^ invalid option found here
    |
+   = 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
@@ -22,8 +23,9 @@ LL | |     if(Self = ()),
 ...  |
 LL | |     note = "not used yet"
 LL | | )]
-   | |__^
+   | |__^ 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]: fallback!!