make error codes reflect reality better

Author: jdonszelmann

compiler/rustc_attr_parsing/messages.ftl

@@ -121,10 +121,6 @@ attr_parsing_unsupported_literal_cfg_boolean =
     literal in `cfg` predicate value must be a boolean
 attr_parsing_unsupported_literal_cfg_string =
     literal in `cfg` predicate value must be a string
-attr_parsing_unsupported_literal_deprecated_kv_pair =
-    item in `deprecated` must be a key/value pair
-attr_parsing_unsupported_literal_deprecated_string =
-    literal in `deprecated` value must be a string
 attr_parsing_unsupported_literal_generic =
     unsupported literal
 attr_parsing_unsupported_literal_suggestion =

compiler/rustc_attr_parsing/src/attributes/confusables.rs

@@ -30,12 +30,12 @@ impl<S: Stage> AttributeParser<S> for ConfusablesParser {
             for param in list.mixed() {
                 let span = param.span();
 
-                let Some(lit) = param.lit() else {
-                    cx.expected_string_literal(span);
+                let Some(lit) = param.lit().and_then(|i| i.value_str()) else {
+                    cx.expected_string_literal(span, param.lit());
                     continue;
                 };
 
-                this.confusables.push(lit.symbol);
+                this.confusables.push(lit);
             }
 
             this.first_span.get_or_insert(cx.attr_span);

compiler/rustc_attr_parsing/src/attributes/deprecation.rs

@@ -7,7 +7,6 @@ use super::{AttributeOrder, OnDuplicate, SingleAttributeParser};
 use crate::context::{AcceptContext, Stage};
 use crate::parser::ArgParser;
 use crate::session_diagnostics;
-use crate::session_diagnostics::UnsupportedLiteralReason;
 
 pub(crate) struct DeprecationParser;
 
@@ -26,13 +25,7 @@ fn get<S: Stage>(
         if let Some(value_str) = v.value_as_str() {
             Some(value_str)
         } else {
-            let lit = v.value_as_lit();
-            cx.emit_err(session_diagnostics::UnsupportedLiteral {
-                span: v.value_span,
-                reason: UnsupportedLiteralReason::DeprecatedString,
-                is_bytestr: lit.kind.is_bytestr(),
-                start_point_span: cx.sess().source_map().start_point(lit.span),
-            });
+            cx.expected_string_literal(v.value_span, Some(&v.value_as_lit()));
             None
         }
     } else {
@@ -60,57 +53,60 @@ impl<S: Stage> SingleAttributeParser<S> for DeprecationParser {
 
         let is_rustc = features.staged_api();
 
-        if let Some(value) = args.name_value()
-            && let Some(value_str) = value.value_as_str()
-        {
-            note = Some(value_str)
-        } else if let Some(list) = args.list() {
-            for param in list.mixed() {
-                let param_span = param.span();
-                let Some(param) = param.meta_item() else {
-                    cx.emit_err(session_diagnostics::UnsupportedLiteral {
-                        span: param_span,
-                        reason: UnsupportedLiteralReason::DeprecatedKvPair,
-                        is_bytestr: false,
-                        start_point_span: cx.sess().source_map().start_point(param_span),
-                    });
-                    return None;
-                };
+        match args {
+            ArgParser::NoArgs => {
+                // ok
+            }
+            ArgParser::List(list) => {
+                for param in list.mixed() {
+                    let Some(param) = param.meta_item() else {
+                        cx.unexpected_literal(param.span());
+                        return None;
+                    };
 
-                let ident_name = param.path().word_sym();
+                    let ident_name = param.path().word_sym();
 
-                match ident_name {
-                    Some(name @ sym::since) => {
-                        since = Some(get(cx, name, param_span, param.args(), &since)?);
-                    }
-                    Some(name @ sym::note) => {
-                        note = Some(get(cx, name, param_span, param.args(), &note)?);
-                    }
-                    Some(name @ sym::suggestion) => {
-                        if !features.deprecated_suggestion() {
-                            cx.emit_err(session_diagnostics::DeprecatedItemSuggestion {
-                                span: param_span,
-                                is_nightly: cx.sess().is_nightly_build(),
-                                details: (),
-                            });
+                    match ident_name {
+                        Some(name @ sym::since) => {
+                            since = Some(get(cx, name, param.span(), param.args(), &since)?);
+                        }
+                        Some(name @ sym::note) => {
+                            note = Some(get(cx, name, param.span(), param.args(), &note)?);
                         }
+                        Some(name @ sym::suggestion) => {
+                            if !features.deprecated_suggestion() {
+                                cx.emit_err(session_diagnostics::DeprecatedItemSuggestion {
+                                    span: param.span(),
+                                    is_nightly: cx.sess().is_nightly_build(),
+                                    details: (),
+                                });
+                            }
 
-                        suggestion = Some(get(cx, name, param_span, param.args(), &suggestion)?);
-                    }
-                    _ => {
-                        cx.unknown_key(
-                            param_span,
-                            param.path().to_string(),
-                            if features.deprecated_suggestion() {
-                                &["since", "note", "suggestion"]
-                            } else {
-                                &["since", "note"]
-                            },
-                        );
-                        return None;
+                            suggestion =
+                                Some(get(cx, name, param.span(), param.args(), &suggestion)?);
+                        }
+                        _ => {
+                            cx.unknown_key(
+                                param.span(),
+                                param.path().to_string(),
+                                if features.deprecated_suggestion() {
+                                    &["since", "note", "suggestion"]
+                                } else {
+                                    &["since", "note"]
+                                },
+                            );
+                            return None;
+                        }
                     }
                 }
             }
+            ArgParser::NameValue(v) => {
+                let Some(value) = v.value_as_str() else {
+                    cx.expected_string_literal(v.value_span, Some(v.value_as_lit()));
+                    return None;
+                };
+                note = Some(value);
+            }
         }
 
         let since = if let Some(since) = since {

compiler/rustc_attr_parsing/src/attributes/inline.rs

@@ -73,15 +73,15 @@ impl<S: Stage> SingleAttributeParser<S> for RustcForceInlineParser {
                 };
 
                 let Some(reason) = l.lit().and_then(|i| i.kind.str()) else {
-                    cx.expected_string_literal(l.span());
+                    cx.expected_string_literal(l.span(), l.lit());
                     return None;
                 };
 
                 Some(reason)
             }
             ArgParser::NameValue(v) => {
                 let Some(reason) = v.value_as_str() else {
-                    cx.expected_string_literal(v.value_span);
+                    cx.expected_string_literal(v.value_span, Some(v.value_as_lit()));
                     return None;
                 };
 

compiler/rustc_attr_parsing/src/attributes/repr.rs

@@ -34,6 +34,7 @@ impl<S: Stage> CombineAttributeParser<S> for ReprParser {
         let mut reprs = Vec::new();
 
         let Some(list) = args.list() else {
+            cx.expected_list(cx.attr_span);
             return reprs;
         };
 

compiler/rustc_attr_parsing/src/context.rs

@@ -4,8 +4,7 @@ use std::ops::{Deref, DerefMut};
 use std::sync::LazyLock;
 
 use private::Sealed;
-use rustc_ast as ast;
-use rustc_ast::NodeId;
+use rustc_ast::{self as ast, MetaItemLit, NodeId};
 use rustc_attr_data_structures::AttributeKind;
 use rustc_attr_data_structures::lints::{AttributeLint, AttributeLintKind};
 use rustc_errors::{DiagCtxtHandle, Diagnostic};
@@ -217,13 +216,25 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         self.emit_err(UnknownMetaItem { span, item: found, expected: options })
     }
 
-    pub(crate) fn expected_string_literal(&self, span: Span) -> ErrorGuaranteed {
+    /// error that a string literal was expected.
+    /// You can optionally give the literal you did find (which you found not to be a string literal)
+    /// which can make better errors. For example, if the literal was a byte string it will suggest
+    /// removing the `b` prefix.
+    pub(crate) fn expected_string_literal(
+        &self,
+        span: Span,
+        actual_literal: Option<&MetaItemLit>,
+    ) -> ErrorGuaranteed {
         self.emit_err(AttributeParseError {
             span,
             attr_span: self.attr_span,
             template: self.template.clone(),
             attribute: self.attr_path.clone(),
-            reason: AttributeParseErrorReason::ExpectedStringLiteral,
+            reason: AttributeParseErrorReason::ExpectedStringLiteral {
+                byte_string: actual_literal.and_then(|i| {
+                    i.kind.is_bytestr().then(|| self.sess().source_map().start_point(i.span))
+                }),
+            },
         })
     }
 
@@ -260,6 +271,18 @@ impl<'f, 'sess: 'f, S: Stage> AcceptContext<'f, 'sess, S> {
         })
     }
 
+    /// an error that should be emitted when a [`MetaItemOrLitParser`](crate::parser::MetaItemOrLitParser)
+    /// was expected *not* to be a literal, but instead a meta item.
+    pub(crate) fn unexpected_literal(&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::UnexpectedLiteral,
+        })
+    }
+
     pub(crate) fn expected_single_argument(&self, span: Span) -> ErrorGuaranteed {
         self.emit_err(AttributeParseError {
             span,

compiler/rustc_attr_parsing/src/session_diagnostics.rs

@@ -16,8 +16,6 @@ pub(crate) enum UnsupportedLiteralReason {
     Generic,
     CfgString,
     CfgBoolean,
-    DeprecatedString,
-    DeprecatedKvPair,
 }
 
 #[derive(Diagnostic)]
@@ -190,6 +188,7 @@ pub(crate) struct InvalidReprHintNoValue {
 }
 
 /// Error code: E0565
+// FIXME(jdonszelmann): slowly phased out
 pub(crate) struct UnsupportedLiteral {
     pub span: Span,
     pub reason: UnsupportedLiteralReason,
@@ -212,12 +211,6 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for UnsupportedLiteral {
                 UnsupportedLiteralReason::CfgBoolean => {
                     fluent::attr_parsing_unsupported_literal_cfg_boolean
                 }
-                UnsupportedLiteralReason::DeprecatedString => {
-                    fluent::attr_parsing_unsupported_literal_deprecated_string
-                }
-                UnsupportedLiteralReason::DeprecatedKvPair => {
-                    fluent::attr_parsing_unsupported_literal_deprecated_kv_pair
-                }
             },
         );
         diag.span(self.span);
@@ -473,9 +466,10 @@ pub(crate) struct UnrecognizedReprHint {
 }
 
 pub(crate) enum AttributeParseErrorReason {
-    ExpectedStringLiteral,
+    ExpectedStringLiteral { byte_string: Option<Span> },
     ExpectedSingleArgument,
     ExpectedList,
+    UnexpectedLiteral,
     ExpectedNameValue(Option<Symbol>),
     DuplicateKey(Symbol),
     ExpectedSpecificArgument { possibilities: Vec<&'static str>, strings: bool },
@@ -497,27 +491,44 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError {
         diag.span(self.attr_span);
         diag.code(E0539);
         match self.reason {
-            AttributeParseErrorReason::ExpectedStringLiteral => {
-                diag.span_note(self.span, "expected a string literal here");
+            AttributeParseErrorReason::ExpectedStringLiteral { byte_string } => {
+                if let Some(start_point_span) = byte_string {
+                    diag.span_suggestion(
+                        start_point_span,
+                        fluent::attr_parsing_unsupported_literal_suggestion,
+                        "",
+                        Applicability::MaybeIncorrect,
+                    );
+                    diag.note("expected a normal string literal, not a byte string literal");
+
+                    return diag;
+                } else {
+                    diag.span_label(self.span, "expected a string literal here");
+                }
             }
             AttributeParseErrorReason::ExpectedSingleArgument => {
-                diag.span_note(self.span, "expected a single argument here");
+                diag.span_label(self.span, "expected a single argument here");
+                diag.code(E0540);
             }
             AttributeParseErrorReason::ExpectedList => {
-                diag.span_note(self.span, "expected this to be a list");
+                diag.span_label(self.span, "expected this to be a list");
             }
             AttributeParseErrorReason::DuplicateKey(key) => {
-                diag.span_note(self.span, format!("found `{key}` used as a key more than once"));
+                diag.span_label(self.span, format!("found `{key}` used as a key more than once"));
                 diag.code(E0538);
             }
+            AttributeParseErrorReason::UnexpectedLiteral => {
+                diag.span_label(self.span, format!("didn't expect a literal here"));
+                diag.code(E0565);
+            }
             AttributeParseErrorReason::ExpectedNameValue(None) => {
-                diag.span_note(
+                diag.span_label(
                     self.span,
                     format!("expected this to be of the form `{name} = \"...\"`"),
                 );
             }
             AttributeParseErrorReason::ExpectedNameValue(Some(name)) => {
-                diag.span_note(
+                diag.span_label(
                     self.span,
                     format!("expected this to be of the form `{name} = \"...\"`"),
                 );
@@ -527,13 +538,13 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError {
                 match possibilities.as_slice() {
                     &[] => {}
                     &[x] => {
-                        diag.span_note(
+                        diag.span_label(
                             self.span,
                             format!("the only valid argument here is {quote}{x}{quote}"),
                         );
                     }
                     [first, second] => {
-                        diag.span_note(self.span, format!("valid arguments are {quote}{first}{quote} or {quote}{second}{quote}"));
+                        diag.span_label(self.span, format!("valid arguments are {quote}{first}{quote} or {quote}{second}{quote}"));
                     }
                     [first @ .., second_to_last, last] => {
                         let mut res = String::new();
@@ -544,7 +555,7 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for AttributeParseError {
                             "{quote}{second_to_last}{quote} or {quote}{last}{quote}"
                         ));
 
-                        diag.span_note(self.span, format!("valid arguments are {res}"));
+                        diag.span_label(self.span, format!("valid arguments are {res}"));
                     }
                 }
             }

compiler/rustc_error_codes/src/error_codes/E0534.md

@@ -1,8 +1,14 @@
+#### Note: this error code is no longer emitted by the compiler
+
+This is because it was too specific to the `inline` attribute.
+Similar diagnostics occur for other attributes too.
+The example here will now emit `E0540`
+
 The `inline` attribute was malformed.
 
 Erroneous code example:
 
-```compile_fail,E0534
+```compile_fail,E0540
 #[inline()] // error: expected one argument
 pub fn something() {}
 

compiler/rustc_error_codes/src/error_codes/E0535.md

@@ -1,8 +1,13 @@
-An unknown argument was given to the `inline` attribute.
+#### Note: this error code is no longer emitted by the compiler
+
+This is because it was too specific to the `inline` attribute.
+Similar diagnostics occur for other attributes too.
+The example here will now emit `E0539`
+
 
 Erroneous code example:
 
-```compile_fail,E0535
+```compile_fail,E0539
 #[inline(unknown)] // error: invalid argument
 pub fn something() {}