Skip to content

Commit d30fe8b

Browse files
matthiaskrgrmatthiaskrgr
authored
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.
2 parents 824dbb5 + 3d03a8a commit d30fe8b

6 files changed

+139
-54
lines changed

‎compiler/rustc_trait_selection/messages.ftl

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ trait_selection_invalid_on_clause_in_rustc_on_unimplemented = invalid `on`-claus
2828
.label = invalid on-clause here
2929
3030
trait_selection_malformed_on_unimplemented_attr = malformed `on_unimplemented` attribute
31+
.help = only `message`, `note` and `label` are allowed as options
32+
.label = invalid option found here
33+
34+
trait_selection_missing_options_for_on_unimplemented_attr = missing options for `on_unimplemented` attribute
35+
.help = at least one of the `message`, `note` and `label` options are expected
3136
3237
trait_selection_negative_positive_conflict = found both positive and negative implementation of trait `{$trait_desc}`{$self_desc ->
3338
[none] {""}

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

+47-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
use super::{ObligationCauseCode, PredicateObligation};
22
use crate::infer::error_reporting::TypeErrCtxt;
3+
use rustc_ast::AttrArgs;
4+
use rustc_ast::AttrArgsEq;
5+
use rustc_ast::AttrKind;
36
use rustc_ast::{Attribute, MetaItem, NestedMetaItem};
47
use rustc_attr as attr;
58
use rustc_data_structures::fx::FxHashMap;
@@ -342,7 +345,22 @@ pub enum AppendConstMessage {
342345

343346
#[derive(LintDiagnostic)]
344347
#[diag(trait_selection_malformed_on_unimplemented_attr)]
345-
pub struct NoValueInOnUnimplementedLint;
348+
#[help]
349+
pub struct MalformedOnUnimplementedAttrLint {
350+
#[label]
351+
pub span: Span,
352+
}
353+
354+
impl MalformedOnUnimplementedAttrLint {
355+
fn new(span: Span) -> Self {
356+
Self { span }
357+
}
358+
}
359+
360+
#[derive(LintDiagnostic)]
361+
#[diag(trait_selection_missing_options_for_on_unimplemented_attr)]
362+
#[help]
363+
pub struct MissingOptionsForOnUnimplementedAttr;
346364

347365
impl<'tcx> OnUnimplementedDirective {
348366
fn parse(
@@ -453,7 +471,7 @@ impl<'tcx> OnUnimplementedDirective {
453471
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
454472
tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
455473
vec![item.span()],
456-
NoValueInOnUnimplementedLint,
474+
MalformedOnUnimplementedAttrLint::new(item.span()),
457475
);
458476
} else {
459477
// nothing found
@@ -530,21 +548,40 @@ impl<'tcx> OnUnimplementedDirective {
530548
append_const_msg: None,
531549
}))
532550
} else {
551+
let item = attr.get_normal_item();
552+
let report_span = match &item.args {
553+
AttrArgs::Empty => item.path.span,
554+
AttrArgs::Delimited(args) => args.dspan.entire(),
555+
AttrArgs::Eq(eq_span, AttrArgsEq::Ast(expr)) => eq_span.to(expr.span),
556+
AttrArgs::Eq(span, AttrArgsEq::Hir(expr)) => span.to(expr.span),
557+
};
558+
533559
tcx.emit_spanned_lint(
534560
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
535561
tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
536-
attr.span,
537-
NoValueInOnUnimplementedLint,
562+
report_span,
563+
MalformedOnUnimplementedAttrLint::new(report_span),
538564
);
539565
Ok(None)
540566
}
541567
} else if is_diagnostic_namespace_variant {
542-
tcx.emit_spanned_lint(
543-
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
544-
tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
545-
attr.span,
546-
NoValueInOnUnimplementedLint,
547-
);
568+
match &attr.kind {
569+
AttrKind::Normal(p) if !matches!(p.item.args, AttrArgs::Empty) => {
570+
tcx.emit_spanned_lint(
571+
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
572+
tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
573+
attr.span,
574+
MalformedOnUnimplementedAttrLint::new(attr.span),
575+
);
576+
}
577+
_ => tcx.emit_spanned_lint(
578+
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
579+
tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
580+
attr.span,
581+
MissingOptionsForOnUnimplementedAttr,
582+
),
583+
};
584+
548585
Ok(None)
549586
} else {
550587
let reported =

‎tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.rs

+8
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,15 @@ trait Boom {}
2323
//~^WARN malformed `on_unimplemented` attribute
2424
trait Doom {}
2525

26+
#[diagnostic::on_unimplemented]
27+
//~^WARN missing options for `on_unimplemented` attribute
28+
//~|WARN missing options for `on_unimplemented` attribute
29+
trait Whatever {}
30+
2631
fn take_foo(_: impl Foo) {}
2732
fn take_baz(_: impl Baz) {}
2833
fn take_boom(_: impl Boom) {}
34+
fn take_whatever(_: impl Whatever) {}
2935

3036
fn main() {
3137
take_foo(1_i32);
@@ -34,4 +40,6 @@ fn main() {
3440
//~^ERROR Boom
3541
take_boom(1_i32);
3642
//~^ERROR Boom
43+
take_whatever(1_i32);
44+
//~^ERROR the trait bound `i32: Whatever` is not satisfied
3745
}

‎tests/ui/diagnostic_namespace/on_unimplemented/do_not_fail_parsing_on_invalid_options_1.stderr

+62-15
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,53 @@ warning: malformed `on_unimplemented` attribute
1010
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:3:32
1111
|
1212
LL | #[diagnostic::on_unimplemented(unsupported = "foo")]
13-
| ^^^^^^^^^^^^^^^^^^^
13+
| ^^^^^^^^^^^^^^^^^^^ invalid option found here
14+
|
15+
= help: only `message`, `note` and `label` are allowed as options
1416

1517
warning: malformed `on_unimplemented` attribute
1618
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:12:50
1719
|
1820
LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")]
19-
| ^^^^^^^^^^^^^^^^^^^
21+
| ^^^^^^^^^^^^^^^^^^^ invalid option found here
22+
|
23+
= help: only `message`, `note` and `label` are allowed as options
2024

2125
warning: malformed `on_unimplemented` attribute
2226
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50
2327
|
2428
LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))]
25-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here
30+
|
31+
= help: only `message`, `note` and `label` are allowed as options
2632

2733
warning: malformed `on_unimplemented` attribute
28-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:1
34+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:22:32
2935
|
3036
LL | #[diagnostic::on_unimplemented = "boom"]
31-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
37+
| ^^^^^^^^ invalid option found here
38+
|
39+
= help: only `message`, `note` and `label` are allowed as options
40+
41+
warning: missing options for `on_unimplemented` attribute
42+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1
43+
|
44+
LL | #[diagnostic::on_unimplemented]
45+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
46+
|
47+
= help: at least one of the `message`, `note` and `label` options are expected
3248

3349
warning: malformed `on_unimplemented` attribute
3450
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:3:32
3551
|
3652
LL | #[diagnostic::on_unimplemented(unsupported = "foo")]
37-
| ^^^^^^^^^^^^^^^^^^^
53+
| ^^^^^^^^^^^^^^^^^^^ invalid option found here
3854
|
55+
= help: only `message`, `note` and `label` are allowed as options
3956
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
4057

4158
error[E0277]: the trait bound `i32: Foo` is not satisfied
42-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:14
59+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:37:14
4360
|
4461
LL | take_foo(1_i32);
4562
| -------- ^^^^^ the trait `Foo` is not implemented for `i32`
@@ -52,7 +69,7 @@ help: this trait has no implementations, consider adding one
5269
LL | trait Foo {}
5370
| ^^^^^^^^^
5471
note: required by a bound in `take_foo`
55-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:21
72+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:31:21
5673
|
5774
LL | fn take_foo(_: impl Foo) {}
5875
| ^^^ required by this bound in `take_foo`
@@ -61,12 +78,13 @@ warning: malformed `on_unimplemented` attribute
6178
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:12:50
6279
|
6380
LL | #[diagnostic::on_unimplemented(message = "Boom", unsupported = "Bar")]
64-
| ^^^^^^^^^^^^^^^^^^^
81+
| ^^^^^^^^^^^^^^^^^^^ invalid option found here
6582
|
83+
= help: only `message`, `note` and `label` are allowed as options
6684
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
6785

6886
error[E0277]: Boom
69-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:33:14
87+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:39:14
7088
|
7189
LL | take_baz(1_i32);
7290
| -------- ^^^^^ the trait `Baz` is not implemented for `i32`
@@ -79,7 +97,7 @@ help: this trait has no implementations, consider adding one
7997
LL | trait Baz {}
8098
| ^^^^^^^^^
8199
note: required by a bound in `take_baz`
82-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:27:21
100+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:32:21
83101
|
84102
LL | fn take_baz(_: impl Baz) {}
85103
| ^^^ required by this bound in `take_baz`
@@ -88,12 +106,13 @@ warning: malformed `on_unimplemented` attribute
88106
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:17:50
89107
|
90108
LL | #[diagnostic::on_unimplemented(message = "Boom", on(_Self = "i32", message = "whatever"))]
91-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
109+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid option found here
92110
|
111+
= help: only `message`, `note` and `label` are allowed as options
93112
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
94113

95114
error[E0277]: Boom
96-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:35:15
115+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:41:15
97116
|
98117
LL | take_boom(1_i32);
99118
| --------- ^^^^^ the trait `Boom` is not implemented for `i32`
@@ -106,11 +125,39 @@ help: this trait has no implementations, consider adding one
106125
LL | trait Boom {}
107126
| ^^^^^^^^^^
108127
note: required by a bound in `take_boom`
109-
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:28:22
128+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:33:22
110129
|
111130
LL | fn take_boom(_: impl Boom) {}
112131
| ^^^^ required by this bound in `take_boom`
113132

114-
error: aborting due to 3 previous errors; 8 warnings emitted
133+
warning: missing options for `on_unimplemented` attribute
134+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:26:1
135+
|
136+
LL | #[diagnostic::on_unimplemented]
137+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
138+
|
139+
= help: at least one of the `message`, `note` and `label` options are expected
140+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
141+
142+
error[E0277]: the trait bound `i32: Whatever` is not satisfied
143+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:43:19
144+
|
145+
LL | take_whatever(1_i32);
146+
| ------------- ^^^^^ the trait `Whatever` is not implemented for `i32`
147+
| |
148+
| required by a bound introduced by this call
149+
|
150+
help: this trait has no implementations, consider adding one
151+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:29:1
152+
|
153+
LL | trait Whatever {}
154+
| ^^^^^^^^^^^^^^
155+
note: required by a bound in `take_whatever`
156+
--> $DIR/do_not_fail_parsing_on_invalid_options_1.rs:34:26
157+
|
158+
LL | fn take_whatever(_: impl Whatever) {}
159+
| ^^^^^^^^ required by this bound in `take_whatever`
160+
161+
error: aborting due to 4 previous errors; 10 warnings emitted
115162

116163
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

+4-6
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,20 @@
11
#![feature(diagnostic_namespace)]
22

33
#[diagnostic::on_unimplemented(
4+
if(Self = "()"),
45
//~^WARN malformed `on_unimplemented` attribute
56
//~|WARN malformed `on_unimplemented` attribute
6-
if(Self = ()),
7-
message = "not used yet",
8-
label = "not used yet",
9-
note = "not used yet"
7+
message = "custom message",
8+
note = "custom note"
109
)]
1110
#[diagnostic::on_unimplemented(message = "fallback!!")]
1211
#[diagnostic::on_unimplemented(label = "fallback label")]
1312
#[diagnostic::on_unimplemented(note = "fallback note")]
14-
#[diagnostic::on_unimplemented(message = "fallback2!!")]
1513
trait Foo {}
1614

1715
fn takes_foo(_: impl Foo) {}
1816

1917
fn main() {
2018
takes_foo(());
21-
//~^ERROR fallback!!
19+
//~^ERROR custom message
2220
}

‎tests/ui/diagnostic_namespace/on_unimplemented/ignore_unsupported_options_and_continue_to_use_fallback.stderr

+13-23
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,38 @@
11
warning: malformed `on_unimplemented` attribute
2-
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:3:1
2+
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:4:5
33
|
4-
LL | / #[diagnostic::on_unimplemented(
5-
LL | |
6-
LL | |
7-
LL | | if(Self = ()),
8-
... |
9-
LL | | note = "not used yet"
10-
LL | | )]
11-
| |__^
4+
LL | if(Self = "()"),
5+
| ^^^^^^^^^^^^^^^ invalid option found here
126
|
7+
= help: only `message`, `note` and `label` are allowed as options
138
= note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default
149

1510
warning: malformed `on_unimplemented` attribute
16-
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:3:1
11+
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:4:5
1712
|
18-
LL | / #[diagnostic::on_unimplemented(
19-
LL | |
20-
LL | |
21-
LL | | if(Self = ()),
22-
... |
23-
LL | | note = "not used yet"
24-
LL | | )]
25-
| |__^
13+
LL | if(Self = "()"),
14+
| ^^^^^^^^^^^^^^^ invalid option found here
2615
|
16+
= help: only `message`, `note` and `label` are allowed as options
2717
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
2818

29-
error[E0277]: fallback!!
30-
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:20:15
19+
error[E0277]: custom message
20+
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:18:15
3121
|
3222
LL | takes_foo(());
3323
| --------- ^^ fallback label
3424
| |
3525
| required by a bound introduced by this call
3626
|
3727
= help: the trait `Foo` is not implemented for `()`
38-
= note: fallback note
28+
= note: custom note
3929
help: this trait has no implementations, consider adding one
40-
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:1
30+
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:13:1
4131
|
4232
LL | trait Foo {}
4333
| ^^^^^^^^^
4434
note: required by a bound in `takes_foo`
45-
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:17:22
35+
--> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:22
4636
|
4737
LL | fn takes_foo(_: impl Foo) {}
4838
| ^^^ required by this bound in `takes_foo`

0 commit comments

Comments
 (0)