From 10538d4d2b9c0403573211cb4881547880b3b913 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Thu, 9 Nov 2023 08:33:21 +0100 Subject: [PATCH] Add some additional warnings for duplicated diagnostic items This commit adds warnings if a user supplies several diagnostic options where we can only apply one of them. We explicitly warn about ignored options here. In addition a small test for these warnings is added. --- compiler/rustc_trait_selection/messages.ftl | 4 + .../error_reporting/on_unimplemented.rs | 108 ++++++++++++++++-- .../error_reporting/type_err_ctxt_ext.rs | 2 +- ...ed_options_and_continue_to_use_fallback.rs | 2 + ...ptions_and_continue_to_use_fallback.stderr | 28 ++++- .../report_warning_on_duplicated_options.rs | 25 ++++ ...eport_warning_on_duplicated_options.stderr | 67 +++++++++++ 7 files changed, 219 insertions(+), 17 deletions(-) create mode 100644 tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.rs create mode 100644 tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.stderr diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl index a9792ca2795..d753aa8618e 100644 --- a/compiler/rustc_trait_selection/messages.ftl +++ b/compiler/rustc_trait_selection/messages.ftl @@ -22,6 +22,10 @@ trait_selection_dump_vtable_entries = vtable entries for `{$trait_ref}`: {$entri trait_selection_empty_on_clause_in_rustc_on_unimplemented = empty `on`-clause in `#[rustc_on_unimplemented]` .label = empty on-clause here +trait_selection_ignored_diagnostic_option = `{$option_name}` is ignored due to previous definition of `{$option_name}` + .other_label = `{$option_name}` is first declared here + .label = `{$option_name}` is already declared here + trait_selection_inherent_projection_normalization_overflow = overflow evaluating associated type `{$ty}` trait_selection_invalid_on_clause_in_rustc_on_unimplemented = invalid `on`-clause in `#[rustc_on_unimplemented]` 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 60da6a58920..da2d5dfbfb5 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 @@ -321,7 +321,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } #[derive(Clone, Debug)] -pub struct OnUnimplementedFormatString(Symbol); +pub struct OnUnimplementedFormatString(Symbol, Span); #[derive(Debug)] pub struct OnUnimplementedDirective { @@ -350,7 +350,7 @@ pub struct OnUnimplementedNote { pub enum AppendConstMessage { #[default] Default, - Custom(Symbol), + Custom(Symbol, Span), } #[derive(LintDiagnostic)] @@ -372,6 +372,35 @@ impl MalformedOnUnimplementedAttrLint { #[help] pub struct MissingOptionsForOnUnimplementedAttr; +#[derive(LintDiagnostic)] +#[diag(trait_selection_ignored_diagnostic_option)] +pub struct IgnoredDiagnosticOption { + pub option_name: &'static str, + #[label] + pub span: Span, + #[label(trait_selection_other_label)] + pub prev_span: Span, +} + +impl IgnoredDiagnosticOption { + fn maybe_emit_warning<'tcx>( + tcx: TyCtxt<'tcx>, + item_def_id: DefId, + new: Option, + old: Option, + option_name: &'static str, + ) { + if let (Some(new_item), Some(old_item)) = (new, old) { + tcx.emit_spanned_lint( + UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, + tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()), + new_item, + IgnoredDiagnosticOption { span: new_item, prev_span: old_item, option_name }, + ); + } + } +} + impl<'tcx> OnUnimplementedDirective { fn parse( tcx: TyCtxt<'tcx>, @@ -384,8 +413,9 @@ impl<'tcx> OnUnimplementedDirective { let mut errored = None; let mut item_iter = items.iter(); - let parse_value = |value_str| { - OnUnimplementedFormatString::try_parse(tcx, item_def_id, value_str, span).map(Some) + let parse_value = |value_str, value_span| { + OnUnimplementedFormatString::try_parse(tcx, item_def_id, value_str, span, value_span) + .map(Some) }; let condition = if is_root { @@ -398,7 +428,7 @@ impl<'tcx> OnUnimplementedDirective { .ok_or_else(|| tcx.sess.emit_err(InvalidOnClauseInOnUnimplemented { span }))?; attr::eval_condition(cond, &tcx.sess.parse_sess, Some(tcx.features()), &mut |cfg| { if let Some(value) = cfg.value - && let Err(guar) = parse_value(value) + && let Err(guar) = parse_value(value, cfg.span) { errored = Some(guar); } @@ -417,17 +447,17 @@ impl<'tcx> OnUnimplementedDirective { for item in item_iter { if item.has_name(sym::message) && message.is_none() { if let Some(message_) = item.value_str() { - message = parse_value(message_)?; + message = parse_value(message_, item.span())?; continue; } } else if item.has_name(sym::label) && label.is_none() { if let Some(label_) = item.value_str() { - label = parse_value(label_)?; + label = parse_value(label_, item.span())?; continue; } } else if item.has_name(sym::note) { if let Some(note_) = item.value_str() { - if let Some(note) = parse_value(note_)? { + if let Some(note) = parse_value(note_, item.span())? { notes.push(note); continue; } @@ -437,7 +467,7 @@ impl<'tcx> OnUnimplementedDirective { && !is_diagnostic_namespace_variant { if let Some(parent_label_) = item.value_str() { - parent_label = parse_value(parent_label_)?; + parent_label = parse_value(parent_label_, item.span())?; continue; } } else if item.has_name(sym::on) @@ -470,7 +500,7 @@ impl<'tcx> OnUnimplementedDirective { && !is_diagnostic_namespace_variant { if let Some(msg) = item.value_str() { - append_const_msg = Some(AppendConstMessage::Custom(msg)); + append_const_msg = Some(AppendConstMessage::Custom(msg, item.span())); continue; } else if item.is_word() { append_const_msg = Some(AppendConstMessage::Default); @@ -519,6 +549,54 @@ impl<'tcx> OnUnimplementedDirective { subcommands.extend(directive.subcommands); let mut notes = aggr.notes; notes.extend(directive.notes); + IgnoredDiagnosticOption::maybe_emit_warning( + tcx, + item_def_id, + directive.message.as_ref().map(|f| f.1), + aggr.message.as_ref().map(|f| f.1), + "message", + ); + IgnoredDiagnosticOption::maybe_emit_warning( + tcx, + item_def_id, + directive.label.as_ref().map(|f| f.1), + aggr.label.as_ref().map(|f| f.1), + "label", + ); + IgnoredDiagnosticOption::maybe_emit_warning( + tcx, + item_def_id, + directive.condition.as_ref().map(|i| i.span), + aggr.condition.as_ref().map(|i| i.span), + "condition", + ); + IgnoredDiagnosticOption::maybe_emit_warning( + tcx, + item_def_id, + directive.parent_label.as_ref().map(|f| f.1), + aggr.parent_label.as_ref().map(|f| f.1), + "parent_label", + ); + IgnoredDiagnosticOption::maybe_emit_warning( + tcx, + item_def_id, + directive.append_const_msg.as_ref().and_then(|c| { + if let AppendConstMessage::Custom(_, s) = c { + Some(*s) + } else { + None + } + }), + aggr.append_const_msg.as_ref().and_then(|c| { + if let AppendConstMessage::Custom(_, s) = c { + Some(*s) + } else { + None + } + }), + "append_const_msg", + ); + Ok(Some(Self { condition: aggr.condition.or(directive.condition), subcommands, @@ -556,6 +634,7 @@ impl<'tcx> OnUnimplementedDirective { item_def_id, value, attr.span, + attr.span, )?), notes: Vec::new(), parent_label: None, @@ -633,7 +712,11 @@ impl<'tcx> OnUnimplementedDirective { // `with_no_visible_paths` is also used when generating the options, // so we need to match it here. ty::print::with_no_visible_paths!( - OnUnimplementedFormatString(v).format(tcx, trait_ref, &options_map) + OnUnimplementedFormatString(v, cfg.span).format( + tcx, + trait_ref, + &options_map + ) ) }); @@ -678,8 +761,9 @@ impl<'tcx> OnUnimplementedFormatString { item_def_id: DefId, from: Symbol, err_sp: Span, + value_span: Span, ) -> Result { - let result = OnUnimplementedFormatString(from); + let result = OnUnimplementedFormatString(from, value_span); result.verify(tcx, item_def_id, err_sp)?; Ok(result) } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 78c9ac157c0..e0366529f33 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -2729,7 +2729,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { Some(format!("{cannot_do_this} in const contexts")) } // overridden post message - (true, Some(AppendConstMessage::Custom(custom_msg))) => { + (true, Some(AppendConstMessage::Custom(custom_msg, _))) => { Some(format!("{cannot_do_this}{custom_msg}")) } // fallback to generic message 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 8410b3eb105..0893f29c4a3 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 @@ -8,6 +8,8 @@ note = "custom note" )] #[diagnostic::on_unimplemented(message = "fallback!!")] +//~^ `message` is ignored due to previous definition of `message` +//~| `message` is ignored due to previous definition of `message` #[diagnostic::on_unimplemented(label = "fallback label")] #[diagnostic::on_unimplemented(note = "fallback note")] trait Foo {} 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 906472beb49..fc4aa8ef7d8 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 @@ -7,6 +7,15 @@ LL | if(Self = "()"), = help: only `message`, `note` and `label` are allowed as options = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default +warning: `message` is ignored due to previous definition of `message` + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:10:32 + | +LL | message = "custom message", + | -------------------------- `message` is first declared here +... +LL | #[diagnostic::on_unimplemented(message = "fallback!!")] + | ^^^^^^^^^^^^^^^^^^^^^^ `message` is already declared here + warning: malformed `on_unimplemented` attribute --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:4:5 | @@ -16,8 +25,19 @@ LL | if(Self = "()"), = help: only `message`, `note` and `label` are allowed as options = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` +warning: `message` is ignored due to previous definition of `message` + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:10:32 + | +LL | message = "custom message", + | -------------------------- `message` is first declared here +... +LL | #[diagnostic::on_unimplemented(message = "fallback!!")] + | ^^^^^^^^^^^^^^^^^^^^^^ `message` is already declared here + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + error[E0277]: custom message - --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:18:15 + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:20:15 | LL | takes_foo(()); | --------- ^^ fallback label @@ -28,16 +48,16 @@ LL | takes_foo(()); = note: custom note = note: fallback note help: this trait has no implementations, consider adding one - --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:13:1 + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:1 | LL | trait Foo {} | ^^^^^^^^^ note: required by a bound in `takes_foo` - --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:15:22 + --> $DIR/ignore_unsupported_options_and_continue_to_use_fallback.rs:17:22 | LL | fn takes_foo(_: impl Foo) {} | ^^^ required by this bound in `takes_foo` -error: aborting due to previous error; 2 warnings emitted +error: aborting due to previous error; 4 warnings emitted For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.rs b/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.rs new file mode 100644 index 00000000000..a7becd2f88f --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.rs @@ -0,0 +1,25 @@ +#![feature(diagnostic_namespace)] + +#[diagnostic::on_unimplemented( + message = "first message", + label = "first label", + note = "custom note" +)] +#[diagnostic::on_unimplemented( + message = "second message", + //~^WARN `message` is ignored due to previous definition of `message` + //~|WARN `message` is ignored due to previous definition of `message` + label = "second label", + //~^WARN `label` is ignored due to previous definition of `label` + //~|WARN `label` is ignored due to previous definition of `label` + note = "second note" +)] +trait Foo {} + + +fn takes_foo(_: impl Foo) {} + +fn main() { + takes_foo(()); + //~^ERROR first message +} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.stderr new file mode 100644 index 00000000000..f9395ae85bc --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/report_warning_on_duplicated_options.stderr @@ -0,0 +1,67 @@ +warning: `message` is ignored due to previous definition of `message` + --> $DIR/report_warning_on_duplicated_options.rs:9:5 + | +LL | message = "first message", + | ------------------------- `message` is first declared here +... +LL | message = "second message", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ `message` is already declared here + | + = note: `#[warn(unknown_or_malformed_diagnostic_attributes)]` on by default + +warning: `label` is ignored due to previous definition of `label` + --> $DIR/report_warning_on_duplicated_options.rs:12:5 + | +LL | label = "first label", + | --------------------- `label` is first declared here +... +LL | label = "second label", + | ^^^^^^^^^^^^^^^^^^^^^^ `label` is already declared here + +warning: `message` is ignored due to previous definition of `message` + --> $DIR/report_warning_on_duplicated_options.rs:9:5 + | +LL | message = "first message", + | ------------------------- `message` is first declared here +... +LL | message = "second message", + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ `message` is already declared here + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +warning: `label` is ignored due to previous definition of `label` + --> $DIR/report_warning_on_duplicated_options.rs:12:5 + | +LL | label = "first label", + | --------------------- `label` is first declared here +... +LL | label = "second label", + | ^^^^^^^^^^^^^^^^^^^^^^ `label` is already declared here + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0277]: first message + --> $DIR/report_warning_on_duplicated_options.rs:23:15 + | +LL | takes_foo(()); + | --------- ^^ first label + | | + | required by a bound introduced by this call + | + = help: the trait `Foo` is not implemented for `()` + = note: custom note + = note: second note +help: this trait has no implementations, consider adding one + --> $DIR/report_warning_on_duplicated_options.rs:17:1 + | +LL | trait Foo {} + | ^^^^^^^^^ +note: required by a bound in `takes_foo` + --> $DIR/report_warning_on_duplicated_options.rs:20:22 + | +LL | fn takes_foo(_: impl Foo) {} + | ^^^ required by this bound in `takes_foo` + +error: aborting due to previous error; 4 warnings emitted + +For more information about this error, try `rustc --explain E0277`.