use consistent attr errors in all attribute parsers

Author: jdonszelmann

compiler/rustc_attr_parsing/messages.ftl

@@ -27,8 +27,6 @@ attr_parsing_ill_formed_attribute_input = {$num_suggestions ->
         [1] attribute must be of the form {$suggestions}
         *[other] valid forms for the attribute are {$suggestions}
     }
-attr_parsing_incorrect_meta_item = expected a quoted string literal
-attr_parsing_incorrect_meta_item_suggestion = consider surrounding this with quotes
 
 attr_parsing_incorrect_repr_format_align_one_arg =
     incorrect `repr(align)` attribute format: `align` takes exactly one argument in parentheses
@@ -85,9 +83,6 @@ attr_parsing_missing_note =
 attr_parsing_missing_since =
     missing 'since'
 
-attr_parsing_multiple_item =
-    multiple '{$item}' items
-
 attr_parsing_multiple_stability_levels =
     multiple stability levels
 

compiler/rustc_attr_parsing/src/attributes/confusables.rs

@@ -19,9 +19,7 @@ impl<S: Stage> AttributeParser<S> for ConfusablesParser {
         template!(List: r#""name1", "name2", ..."#),
         |this, cx, args| {
             let Some(list) = args.list() else {
-                // FIXME(jdonszelmann): error when not a list? Bring validation code here.
-                //       NOTE: currently subsequent attributes are silently ignored using
-                //       tcx.get_attr().
+                cx.expected_list(cx.attr_span);
                 return;
             };
 
@@ -33,13 +31,7 @@ impl<S: Stage> AttributeParser<S> for ConfusablesParser {
                 let span = param.span();
 
                 let Some(lit) = param.lit() else {
-                    cx.emit_err(session_diagnostics::IncorrectMetaItem {
-                        span,
-                        suggestion: Some(session_diagnostics::IncorrectMetaItemSuggestion {
-                            lo: span.shrink_to_lo(),
-                            hi: span.shrink_to_hi(),
-                        }),
-                    });
+                    cx.expected_string_literal(span);
                     continue;
                 };
 

compiler/rustc_attr_parsing/src/attributes/deprecation.rs

@@ -20,10 +20,7 @@ fn get<S: Stage>(
     item: &Option<Symbol>,
 ) -> Option<Symbol> {
     if item.is_some() {
-        cx.emit_err(session_diagnostics::MultipleItem {
-            span: param_span,
-            item: ident.to_string(),
-        });
+        cx.duplicate_key(ident.span, ident.name);
         return None;
     }
     if let Some(v) = arg.name_value() {
@@ -40,8 +37,7 @@ fn get<S: Stage>(
             None
         }
     } else {
-        // FIXME(jdonszelmann): suggestion?
-        cx.emit_err(session_diagnostics::IncorrectMetaItem { span: param_span, suggestion: None });
+        cx.expected_name_value(param_span, Some(ident.name));
         None
     }
 }
@@ -103,15 +99,15 @@ impl<S: Stage> SingleAttributeParser<S> for DeprecationParser {
                         suggestion = Some(get(cx, ident, param_span, arg, &suggestion)?);
                     }
                     _ => {
-                        cx.emit_err(session_diagnostics::UnknownMetaItem {
-                            span: param_span,
-                            item: ident.to_string(),
-                            expected: if features.deprecated_suggestion() {
+                        cx.unknown_key(
+                            param_span,
+                            ident.to_string(),
+                            if features.deprecated_suggestion() {
                                 &["since", "note", "suggestion"]
                             } else {
                                 &["since", "note"]
                             },
-                        });
+                        );
                         return None;
                     }
                 }

compiler/rustc_attr_parsing/src/attributes/stability.rs

@@ -6,7 +6,7 @@ use rustc_attr_data_structures::{
 };
 use rustc_errors::ErrorGuaranteed;
 use rustc_feature::{AttributeTemplate, template};
-use rustc_span::{Span, Symbol, kw, sym};
+use rustc_span::{Ident, Span, Symbol, sym};
 
 use super::util::parse_version;
 use super::{AcceptMapping, AttributeOrder, AttributeParser, OnDuplicate, SingleAttributeParser};
@@ -76,7 +76,10 @@ impl<S: Stage> AttributeParser<S> for StabilityParser {
                 this.allowed_through_unstable_modules =
                     Some(match args.name_value().and_then(|i| i.value_as_str()) {
                         Some(msg) => msg,
-                        None => kw::Empty,
+                        None => {
+                            cx.expected_name_value(cx.attr_span, None);
+                            return;
+                        }
                     });
             },
         ),
@@ -220,23 +223,18 @@ fn insert_value_into_option_or_error<S: Stage>(
     cx: &AcceptContext<'_, '_, S>,
     param: &MetaItemParser<'_>,
     item: &mut Option<Symbol>,
+    name: Ident,
 ) -> Option<()> {
     if item.is_some() {
-        cx.emit_err(session_diagnostics::MultipleItem {
-            span: param.span(),
-            item: param.path_without_args().to_string(),
-        });
+        cx.duplicate_key(name.span, name.name);
         None
     } else if let Some(v) = param.args().name_value()
         && let Some(s) = v.value_as_str()
     {
         *item = Some(s);
         Some(())
     } else {
-        cx.emit_err(session_diagnostics::IncorrectMetaItem {
-            span: param.span(),
-            suggestion: None,
-        });
+        cx.expected_name_value(param.span(), Some(name.name));
         None
     }
 }
@@ -262,9 +260,10 @@ pub(crate) fn parse_stability<S: Stage>(
             return None;
         };
 
-        match param.word_or_empty_without_args().name {
-            sym::feature => insert_value_into_option_or_error(cx, &param, &mut feature)?,
-            sym::since => insert_value_into_option_or_error(cx, &param, &mut since)?,
+        let name = param.word_or_empty_without_args();
+        match name.name {
+            sym::feature => insert_value_into_option_or_error(cx, &param, &mut feature, name)?,
+            sym::since => insert_value_into_option_or_error(cx, &param, &mut since, name)?,
             _ => {
                 cx.emit_err(session_diagnostics::UnknownMetaItem {
                     span: param_span,
@@ -332,10 +331,10 @@ pub(crate) fn parse_unstability<S: Stage>(
 
         let (word, args) = param.word_or_empty();
         match word.name {
-            sym::feature => insert_value_into_option_or_error(cx, &param, &mut feature)?,
-            sym::reason => insert_value_into_option_or_error(cx, &param, &mut reason)?,
+            sym::feature => insert_value_into_option_or_error(cx, &param, &mut feature, word)?,
+            sym::reason => insert_value_into_option_or_error(cx, &param, &mut reason, word)?,
             sym::issue => {
-                insert_value_into_option_or_error(cx, &param, &mut issue)?;
+                insert_value_into_option_or_error(cx, &param, &mut issue, word)?;
 
                 // These unwraps are safe because `insert_value_into_option_or_error` ensures the meta item
                 // is a name/value pair string literal.
@@ -364,7 +363,9 @@ pub(crate) fn parse_unstability<S: Stage>(
                 }
                 is_soft = true;
             }
-            sym::implied_by => insert_value_into_option_or_error(cx, &param, &mut implied_by)?,
+            sym::implied_by => {
+                insert_value_into_option_or_error(cx, &param, &mut implied_by, word)?
+            }
             _ => {
                 cx.emit_err(session_diagnostics::UnknownMetaItem {
                     span: param.span(),

compiler/rustc_attr_parsing/src/attributes/transparency.rs

@@ -22,12 +22,19 @@ impl<S: Stage> SingleAttributeParser<S> for TransparencyParser {
         template!(NameValueStr: "transparent|semitransparent|opaque");
 
     fn convert(cx: &AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
-        match args.name_value().and_then(|nv| nv.value_as_str()) {
+        let Some(nv) = args.name_value() else {
+            cx.expected_name_value(cx.attr_span, None);
+            return None;
+        };
+        match nv.value_as_str() {
             Some(sym::transparent) => Some(Transparency::Transparent),
             Some(sym::semitransparent) => Some(Transparency::SemiTransparent),
             Some(sym::opaque) => Some(Transparency::Opaque),
-            Some(other) => {
-                cx.dcx().span_err(cx.attr_span, format!("unknown macro transparency: `{other}`"));
+            Some(_) => {
+                cx.expected_specific_argument_strings(
+                    nv.value_span,
+                    vec!["transparent", "semitransparent", "opaque"],
+                );
                 None
             }
             None => None,

compiler/rustc_attr_parsing/src/context.rs

@@ -27,7 +27,7 @@ use crate::attributes::stability::{
 use crate::attributes::transparency::TransparencyParser;
 use crate::attributes::{AttributeParser as _, Combine, Single};
 use crate::parser::{ArgParser, MetaItemParser};
-use crate::session_diagnostics::{AttributeParseError, AttributeParseErrorReason};
+use crate::session_diagnostics::{AttributeParseError, AttributeParseErrorReason, UnknownMetaItem};
 
 macro_rules! group_type {
     ($stage: ty) => {
@@ -241,8 +241,16 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         S::emit_lint(self.dcx(), lint, self.target_id, span, diag)
     }
 
+    pub(crate) fn unknown_key(
+        &self,
+        span: Span,
+        found: String,
+        options: &'static [&'static str],
+    ) -> ErrorGuaranteed {
+        self.emit_err(UnknownMetaItem { span, item: found, expected: options })
+    }
+
     pub(crate) fn expected_string_literal(&self, span: Span) -> ErrorGuaranteed {
-        // 539?
         self.emit_err(AttributeParseError {
             span,
             attr_span: self.attr_span,
@@ -252,12 +260,40 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         })
     }
 
-    // pub(crate) fn expected_any_arguments(&self, span: Span) -> ErrorGuaranteed {
-    //
-    // }
+    pub(crate) fn expected_list(&self, span: Span) -> ErrorGuaranteed {
+        self.emit_err(AttributeParseError {
+            span,
+            attr_span: self.attr_span,
+            template: self.template.clone(),
+            attribute: self.attr_path.clone(),
+            reason: AttributeParseErrorReason::ExpectedList,
+        })
+    }
+
+    /// emit an error that a `name = value` pair was expected at this span. The symbol can be given for
+    /// a nicer error message talking about the specific name that was found lacking a value.
+    pub(crate) fn expected_name_value(&self, span: Span, name: Option<Symbol>) -> ErrorGuaranteed {
+        self.emit_err(AttributeParseError {
+            span,
+            attr_span: self.attr_span,
+            template: self.template.clone(),
+            attribute: self.attr_path.clone(),
+            reason: AttributeParseErrorReason::ExpectedNameValue(name),
+        })
+    }
+
+    /// emit an error that a `name = value` pair was found where that name was already seen.
+    pub(crate) fn duplicate_key(&self, span: Span, key: Symbol) -> ErrorGuaranteed {
+        self.emit_err(AttributeParseError {
+            span,
+            attr_span: self.attr_span,
+            template: self.template.clone(),
+            attribute: self.attr_path.clone(),
+            reason: AttributeParseErrorReason::DuplicateKey(key),
+        })
+    }
 
     pub(crate) fn expected_single_argument(&self, span: Span) -> ErrorGuaranteed {
-        // E534?
         self.emit_err(AttributeParseError {
             span,
             attr_span: self.attr_span,
@@ -270,15 +306,34 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
     pub(crate) fn expected_specific_argument(
         &self,
         span: Span,
-        options: Vec<&'static str>,
+        possibilities: Vec<&'static str>,
+    ) -> ErrorGuaranteed {
+        self.emit_err(AttributeParseError {
+            span,
+            attr_span: self.attr_span,
+            template: self.template.clone(),
+            attribute: self.attr_path.clone(),
+            reason: AttributeParseErrorReason::ExpectedSpecificArgument {
+                possibilities,
+                strings: false,
+            },
+        })
+    }
+
+    pub(crate) fn expected_specific_argument_strings(
+        &self,
+        span: Span,
+        possibilities: Vec<&'static str>,
     ) -> ErrorGuaranteed {
-        // E535?
         self.emit_err(AttributeParseError {
             span,
             attr_span: self.attr_span,
             template: self.template.clone(),
             attribute: self.attr_path.clone(),
-            reason: AttributeParseErrorReason::ExpectedSpecificArgument(options),
+            reason: AttributeParseErrorReason::ExpectedSpecificArgument {
+                possibilities,
+                strings: true,
+            },
         })
     }
 }