From 9017b974eef3b43cb4a0c781cc24404b896b0ce5 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Thu, 19 Oct 2023 08:44:15 +0200 Subject: [PATCH 1/2] 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 --- compiler/rustc_trait_selection/messages.ftl | 5 ++ .../error_reporting/on_unimplemented.rs | 57 +++++++++++--- ...o_not_fail_parsing_on_invalid_options_1.rs | 8 ++ ...t_fail_parsing_on_invalid_options_1.stderr | 77 +++++++++++++++---- ...ptions_and_continue_to_use_fallback.stderr | 6 +- 5 files changed, 126 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index 20253b32add8c..a9792ca2795c9 100644 --- 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 index 9c9b78f415238..6a17dea02b4f2 100644 --- 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 index 00fb59d14d76a..346d8373f7398 100644 --- 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/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr index bd39c91ffe88f..162ddd79fbbd9 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr +++ b/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`. diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr index 6a83d8e39c656..ba14c9c84b809 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr +++ b/tests/ui/diagnostic_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!! From 3d03a8a6537ae944d38fb22b2edd1df0dcbaeeb1 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 20 Oct 2023 13:24:58 +0200 Subject: [PATCH 2/2] Fix a span for one of the test cases --- ...ed_options_and_continue_to_use_fallback.rs | 10 +++--- ...ptions_and_continue_to_use_fallback.stderr | 34 ++++++------------- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.rs b/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.rs index 353075863918c..8410b3eb10513 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.rs +++ b/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 = "not used yet", - label = "not used yet", - note = "not used yet" + message = "custom message", + note = "custom note" )] #[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 } diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr index ba14c9c84b809..7860e540589db 100644 --- a/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr +++ b/tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr @@ -1,35 +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 | | -LL | | -LL | | if(Self = ()), -... | -LL | | note = "not used yet" -LL | | )] - | |__^ invalid option found here +LL | if(Self = "()"), + | ^^^^^^^^^^^^^^^ 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 - --> $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 | | -LL | | -LL | | if(Self = ()), -... | -LL | | note = "not used yet" -LL | | )] - | |__^ invalid option found here +LL | if(Self = "()"), + | ^^^^^^^^^^^^^^^ 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!! - --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:20:15 +error[E0277]: custom message + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:18:15 | LL | takes_foo(()); | --------- ^^ fallback label @@ -37,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`