Skip to content

Commit

Permalink
Improve the warning messages for the #[diagnostic::on_unimplemented]
Browse files Browse the repository at this point in the history
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
  • Loading branch information
weiznich committed Oct 19, 2023
1 parent 020d008 commit 9017b97
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 27 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_trait_selection/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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] {""}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`
Expand All @@ -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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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!!
Expand Down

0 comments on commit 9017b97

Please sign in to comment.