#[diagnostic::on_unimplemented] without filters

This commit adds support for a `#[diagnostic::on_unimplemented]` attribute with the following options: * `message` to customize the primary error message * `note` to add a customized note message to an error message * `label` to customize the label part of the error message Co-authored-by: León Orell Valerian Liehr <[email protected]> Co-authored-by: Michael Goulet <[email protected]>
rust-lang · Sep 12, 2023 · 5b8a7a0 · 5b8a7a0
1 parent e5fedce
commit 5b8a7a0
Show file tree

Hide file tree

Showing 18 changed files with 360 additions and 39 deletions.
diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs
@@ -99,6 +99,22 @@ impl Attribute {
         }
     }
 
+    pub fn path_matches(&self, name: &[Symbol]) -> bool {
+        match &self.kind {
+            AttrKind::Normal(normal) => {
+                normal.item.path.segments.len() == name.len()
+                    && normal
+                        .item
+                        .path
+                        .segments
+                        .iter()
+                        .zip(name)
+                        .all(|(s, n)| s.args.is_none() && s.ident.name == *n)
+            }
+            AttrKind::DocComment(..) => false,
+        }
+    }
+
     pub fn is_word(&self) -> bool {
         if let AttrKind::Normal(normal) = &self.kind {
             matches!(normal.item.args, AttrArgs::Empty)

diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs
@@ -411,7 +411,7 @@ declare_features! (
     /// Allows having using `suggestion` in the `#[deprecated]` attribute.
     (active, deprecated_suggestion, "1.61.0", Some(94785), None),
     /// Allows using the `#[diagnostic]` attribute tool namespace
-    (active, diagnostic_namespace, "1.73.0", Some(94785), None),
+    (active, diagnostic_namespace, "1.73.0", Some(111996), None),
     /// Controls errors in trait implementations.
     (active, do_not_recommend, "1.67.0", Some(51992), None),
     /// Tells rustdoc to automatically generate `#[doc(cfg(...))]`.

diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -3404,8 +3404,8 @@ declare_lint_pass! {
         UNFULFILLED_LINT_EXPECTATIONS,
         UNINHABITED_STATIC,
         UNKNOWN_CRATE_TYPES,
-        UNKNOWN_DIAGNOSTIC_ATTRIBUTES,
         UNKNOWN_LINTS,
+        UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
         UNNAMEABLE_TEST_ITEMS,
         UNNAMEABLE_TYPES,
         UNREACHABLE_CODE,
@@ -4419,7 +4419,8 @@ declare_lint! {
 }
 
 declare_lint! {
-    /// The `unknown_diagnostic_attributes` lint detects unrecognized diagnostic attributes.
+    /// The `unknown_or_malformed_diagnostic_attributes` lint detects unrecognized or otherwise malformed
+    /// diagnostic attributes.
     ///
     /// ### Example
     ///
@@ -4431,15 +4432,17 @@ declare_lint! {
     ///
     /// {{produces}}
     ///
+    ///
     /// ### Explanation
     ///
     /// It is usually a mistake to specify a diagnostic attribute that does not exist. Check
     /// the spelling, and check the diagnostic attribute listing for the correct name. Also
     /// consider if you are using an old version of the compiler, and the attribute
     /// is only available in a newer version.
-    pub UNKNOWN_DIAGNOSTIC_ATTRIBUTES,
+    pub UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
     Warn,
-    "unrecognized diagnostic attribute"
+    "unrecognized or malformed diagnostic attribute",
+    @feature_gate = sym::diagnostic_namespace;
 }
 
 declare_lint! {

diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -2408,6 +2408,22 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
+    pub fn get_attrs_by_path<'attr>(
+        self,
+        did: DefId,
+        attr: &'attr [Symbol],
+    ) -> impl Iterator<Item = &'tcx ast::Attribute> + 'attr
+    where
+        'tcx: 'attr,
+    {
+        let filter_fn = move |a: &&ast::Attribute| a.path_matches(&attr);
+        if let Some(did) = did.as_local() {
+            self.hir().attrs(self.hir().local_def_id_to_hir_id(did)).iter().filter(filter_fn)
+        } else {
+            self.item_attrs(did).iter().filter(filter_fn)
+        }
+    }
+
     pub fn get_attr(self, did: impl Into<DefId>, attr: Symbol) -> Option<&'tcx ast::Attribute> {
         if cfg!(debug_assertions) && !rustc_feature::is_valid_for_get_attr(attr) {
             let did: DefId = did.into();

diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl
@@ -140,6 +140,9 @@ passes_deprecated_annotation_has_no_effect =
 passes_deprecated_attribute =
     deprecated attribute must be paired with either stable or unstable attribute
 
+passes_diagnostic_diagnostic_on_unimplemented_only_for_traits =
+    `#[diagnostic::on_unimplemented]` can only be applied to trait definitions
+
 passes_diagnostic_item_first_defined =
     the diagnostic item is first defined here
 

diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
@@ -16,6 +16,7 @@ use rustc_hir::{
     self, FnSig, ForeignItem, HirId, Item, ItemKind, TraitItem, CRATE_HIR_ID, CRATE_OWNER_ID,
 };
 use rustc_hir::{MethodKind, Target, Unsafety};
+use rustc_macros::LintDiagnostic;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault;
 use rustc_middle::query::Providers;
@@ -24,7 +25,7 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::lint::builtin::{
     CONFLICTING_REPR_HINTS, INVALID_DOC_ATTRIBUTES, INVALID_MACRO_EXPORT_ARGUMENTS,
-    UNUSED_ATTRIBUTES,
+    UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, UNUSED_ATTRIBUTES,
 };
 use rustc_session::parse::feature_err;
 use rustc_span::symbol::{kw, sym, Symbol};
@@ -36,6 +37,10 @@ use rustc_trait_selection::traits::ObligationCtxt;
 use std::cell::Cell;
 use std::collections::hash_map::Entry;
 
+#[derive(LintDiagnostic)]
+#[diag(passes_diagnostic_diagnostic_on_unimplemented_only_for_traits)]
+pub struct DiagnosticOnUnimplementedOnlyForTraits;
+
 pub(crate) fn target_from_impl_item<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_item: &hir::ImplItem<'_>,
@@ -104,6 +109,9 @@ impl CheckAttrVisitor<'_> {
         let mut seen = FxHashMap::default();
         let attrs = self.tcx.hir().attrs(hir_id);
         for attr in attrs {
+            if attr.path_matches(&[sym::diagnostic, sym::on_unimplemented]) {
+                self.check_diagnostic_on_unimplemented(attr.span, hir_id, target);
+            }
             match attr.name_or_empty() {
                 sym::do_not_recommend => self.check_do_not_recommend(attr.span, target),
                 sym::inline => self.check_inline(hir_id, attr, span, target),
@@ -284,6 +292,18 @@ impl CheckAttrVisitor<'_> {
         }
     }
 
+    /// Checks if `#[diagnostic::on_unimplemented]` is applied to a trait definition
+    fn check_diagnostic_on_unimplemented(&self, attr_span: Span, hir_id: HirId, target: Target) {
+        if !matches!(target, Target::Trait) {
+            self.tcx.emit_spanned_lint(
+                UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                hir_id,
+                attr_span,
+                DiagnosticOnUnimplementedOnlyForTraits,
+            );
+        }
+    }
+
     /// Checks if an `#[inline]` is applied to a function or a closure. Returns `true` if valid.
     fn check_inline(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool {
         match target {

diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
@@ -26,7 +26,7 @@ use rustc_middle::middle::stability;
 use rustc_middle::ty::RegisteredTools;
 use rustc_middle::ty::{TyCtxt, Visibility};
 use rustc_session::lint::builtin::{
-    LEGACY_DERIVE_HELPERS, SOFT_UNSTABLE, UNKNOWN_DIAGNOSTIC_ATTRIBUTES,
+    LEGACY_DERIVE_HELPERS, SOFT_UNSTABLE, UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
 };
 use rustc_session::lint::builtin::{UNUSED_MACROS, UNUSED_MACRO_RULES};
 use rustc_session::lint::BuiltinLintDiagnostics;
@@ -610,9 +610,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         if res == Res::NonMacroAttr(NonMacroAttrKind::Tool)
             && path.segments.len() >= 2
             && path.segments[0].ident.name == sym::diagnostic
+            && path.segments[1].ident.name != sym::on_unimplemented
         {
             self.tcx.sess.parse_sess.buffer_lint(
-                UNKNOWN_DIAGNOSTIC_ATTRIBUTES,
+                UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
                 path.segments[1].span(),
                 node_id,
                 "unknown diagnostic attribute",

diff --git a/compiler/rustc_trait_selection/messages.ftl b/compiler/rustc_trait_selection/messages.ftl
@@ -27,6 +27,8 @@ trait_selection_inherent_projection_normalization_overflow = overflow evaluating
 trait_selection_invalid_on_clause_in_rustc_on_unimplemented = invalid `on`-clause in `#[rustc_on_unimplemented]`
     .label = invalid on-clause here
 
+trait_selection_malformed_on_unimplemented_attr = malformed `on_unimplemented` attribute
+
 trait_selection_negative_positive_conflict = found both positive and negative implementation of trait `{$trait_desc}`{$self_desc ->
         [none] {""}
        *[default] {" "}for type `{$self_desc}`

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
@@ -9,6 +9,7 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::ty::GenericArgsRef;
 use rustc_middle::ty::{self, GenericParamDefKind, TyCtxt};
 use rustc_parse_format::{ParseMode, Parser, Piece, Position};
+use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::{Span, DUMMY_SP};
 use std::iter;
@@ -336,14 +337,19 @@ pub enum AppendConstMessage {
     Custom(Symbol),
 }
 
+#[derive(LintDiagnostic)]
+#[diag(trait_selection_malformed_on_unimplemented_attr)]
+pub struct NoValueInOnUnimplementedLint;
+
 impl<'tcx> OnUnimplementedDirective {
     fn parse(
         tcx: TyCtxt<'tcx>,
         item_def_id: DefId,
         items: &[NestedMetaItem],
         span: Span,
         is_root: bool,
-    ) -> Result<Self, ErrorGuaranteed> {
+        is_diagnostic_namespace_variant: bool,
+    ) -> Result<Option<Self>, ErrorGuaranteed> {
         let mut errored = None;
         let mut item_iter = items.iter();
 
@@ -391,7 +397,10 @@ impl<'tcx> OnUnimplementedDirective {
                     note = parse_value(note_)?;
                     continue;
                 }
-            } else if item.has_name(sym::parent_label) && parent_label.is_none() {
+            } else if item.has_name(sym::parent_label)
+                && parent_label.is_none()
+                && !is_diagnostic_namespace_variant
+            {
                 if let Some(parent_label_) = item.value_str() {
                     parent_label = parse_value(parent_label_)?;
                     continue;
@@ -401,15 +410,30 @@ impl<'tcx> OnUnimplementedDirective {
                 && message.is_none()
                 && label.is_none()
                 && note.is_none()
+                && !is_diagnostic_namespace_variant
+            // FIXME(diagnostic_namespace): disallow filters for now
             {
                 if let Some(items) = item.meta_item_list() {
-                    match Self::parse(tcx, item_def_id, &items, item.span(), false) {
-                        Ok(subcommand) => subcommands.push(subcommand),
+                    match Self::parse(
+                        tcx,
+                        item_def_id,
+                        &items,
+                        item.span(),
+                        false,
+                        is_diagnostic_namespace_variant,
+                    ) {
+                        Ok(Some(subcommand)) => subcommands.push(subcommand),
+                        Ok(None) => bug!(
+                            "This cannot happen for now as we only reach that if `is_diagnostic_namespace_variant` is false"
+                        ),
                         Err(reported) => errored = Some(reported),
                     };
                     continue;
                 }
-            } else if item.has_name(sym::append_const_msg) && append_const_msg.is_none() {
+            } else if item.has_name(sym::append_const_msg)
+                && append_const_msg.is_none()
+                && !is_diagnostic_namespace_variant
+            {
                 if let Some(msg) = item.value_str() {
                     append_const_msg = Some(AppendConstMessage::Custom(msg));
                     continue;
@@ -419,47 +443,82 @@ impl<'tcx> OnUnimplementedDirective {
                 }
             }
 
-            // nothing found
-            tcx.sess.emit_err(NoValueInOnUnimplemented { span: item.span() });
+            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()),
+                    vec![item.span()],
+                    NoValueInOnUnimplementedLint,
+                );
+            } else {
+                // nothing found
+                tcx.sess.emit_err(NoValueInOnUnimplemented { span: item.span() });
+            }
         }
 
         if let Some(reported) = errored {
-            Err(reported)
+            if is_diagnostic_namespace_variant { Ok(None) } else { Err(reported) }
         } else {
-            Ok(OnUnimplementedDirective {
+            Ok(Some(OnUnimplementedDirective {
                 condition,
                 subcommands,
                 message,
                 label,
                 note,
                 parent_label,
                 append_const_msg,
-            })
+            }))
         }
     }
 
     pub fn of_item(tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<Option<Self>, ErrorGuaranteed> {
-        let Some(attr) = tcx.get_attr(item_def_id, sym::rustc_on_unimplemented) else {
+        let mut is_diagnostic_namespace_variant = false;
+        let Some(attr) = tcx.get_attr(item_def_id, sym::rustc_on_unimplemented).or_else(|| {
+            if tcx.features().diagnostic_namespace {
+                is_diagnostic_namespace_variant = true;
+                tcx.get_attrs_by_path(item_def_id, &[sym::diagnostic, sym::on_unimplemented]).next()
+            } else {
+                None
+            }
+        }) else {
             return Ok(None);
         };
 
         let result = if let Some(items) = attr.meta_item_list() {
-            Self::parse(tcx, item_def_id, &items, attr.span, true).map(Some)
+            Self::parse(tcx, item_def_id, &items, attr.span, true, is_diagnostic_namespace_variant)
         } else if let Some(value) = attr.value_str() {
-            Ok(Some(OnUnimplementedDirective {
-                condition: None,
-                message: None,
-                subcommands: vec![],
-                label: Some(OnUnimplementedFormatString::try_parse(
-                    tcx,
-                    item_def_id,
-                    value,
+            if !is_diagnostic_namespace_variant {
+                Ok(Some(OnUnimplementedDirective {
+                    condition: None,
+                    message: None,
+                    subcommands: vec![],
+                    label: Some(OnUnimplementedFormatString::try_parse(
+                        tcx,
+                        item_def_id,
+                        value,
+                        attr.span,
+                    )?),
+                    note: None,
+                    parent_label: None,
+                    append_const_msg: None,
+                }))
+            } else {
+                tcx.emit_spanned_lint(
+                    UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
+                    tcx.hir().local_def_id_to_hir_id(item_def_id.expect_local()),
                     attr.span,
-                )?),
-                note: None,
-                parent_label: None,
-                append_const_msg: None,
-            }))
+                    NoValueInOnUnimplementedLint,
+                );
+                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,
+            );
+            Ok(None)
         } else {
             let reported =
                 tcx.sess.delay_span_bug(DUMMY_SP, "of_item: neither meta_item_list nor value_str");

diff --git a/tests/ui/diagnostic_namespace/feature-gate-diagnostic_namespace.rs b/tests/ui/diagnostic_namespace/feature-gate-diagnostic_namespace.rs
@@ -1,12 +1,12 @@
 #[diagnostic::non_existing_attribute]
 //~^ERROR `#[diagnostic]` attribute name space is experimental [E0658]
-//~|WARNING unknown diagnostic attribute [unknown_diagnostic_attributes]
+//~|WARNING unknown diagnostic attribute [unknown_or_malformed_diagnostic_attributes]
 pub trait Bar {
 }
 
 #[diagnostic::non_existing_attribute(with_option = "foo")]
 //~^ERROR `#[diagnostic]` attribute name space is experimental [E0658]
-//~|WARNING unknown diagnostic attribute [unknown_diagnostic_attributes]
+//~|WARNING unknown diagnostic attribute [unknown_or_malformed_diagnostic_attributes]
 struct Foo;
 
 fn main() {