introduce duplicate attribute diagnostic logic

Author: jdonszelmann

compiler/rustc_attr_parsing/src/attributes/deprecation.rs

@@ -2,8 +2,8 @@ use rustc_attr_data_structures::{AttributeKind, DeprecatedSince, Deprecation};
 use rustc_span::symbol::Ident;
 use rustc_span::{Span, Symbol, sym};
 
-use super::SingleAttributeParser;
 use super::util::parse_version;
+use super::{AttributeDuplicates, SingleAttributeParser};
 use crate::context::AcceptContext;
 use crate::parser::ArgParser;
 use crate::session_diagnostics;
@@ -47,12 +47,13 @@ fn get(
 
 impl SingleAttributeParser for DeprecationParser {
     const PATH: &'static [rustc_span::Symbol] = &[sym::deprecated];
+    const ON_DUPLICATE_STRATEGY: AttributeDuplicates = AttributeDuplicates::ErrorFollowing;
 
-    fn on_duplicate(cx: &AcceptContext<'_>, first_span: rustc_span::Span) {
+    fn on_duplicate(cx: &AcceptContext<'_>, used: Span, unused: Span) {
         // FIXME(jdonszelmann): merge with errors from check_attrs.rs
         cx.emit_err(session_diagnostics::UnusedMultiple {
-            this: cx.attr_span,
-            other: first_span,
+            this: used,
+            other: unused,
             name: sym::deprecated,
         });
     }

compiler/rustc_attr_parsing/src/attributes/mod.rs

@@ -17,11 +17,12 @@
 use std::marker::PhantomData;
 
 use rustc_attr_data_structures::AttributeKind;
-use rustc_span::Span;
+use rustc_span::{Span, Symbol};
 use thin_vec::ThinVec;
 
 use crate::context::{AcceptContext, FinalizeContext};
 use crate::parser::ArgParser;
+use crate::session_diagnostics::UnusedMultiple;
 
 pub(crate) mod allow_unstable;
 pub(crate) mod cfg;
@@ -74,11 +75,23 @@ pub(crate) trait AttributeParser: Default + 'static {
 pub(crate) trait SingleAttributeParser: 'static {
     const PATH: &'static [rustc_span::Symbol];
 
+    const ON_DUPLICATE_STRATEGY: AttributeDuplicates;
+
     /// Caled when a duplicate attribute is found.
     ///
-    /// `first_span` is the span of the first occurrence of this attribute.
+    /// - `unused` is the span of the attribute that was unused or bad because of some
+    ///   duplicate reason (see [`AttributeDuplicates`])
+    /// - `used` is the span of the attribute that was used in favor of the unused attribute
     // FIXME(jdonszelmann): default error
-    fn on_duplicate(cx: &AcceptContext<'_>, first_span: Span);
+    fn on_duplicate(cx: &AcceptContext<'_>, used: Span, unused: Span) {
+        cx.emit_err(UnusedMultiple {
+            this: used,
+            other: unused,
+            name: Symbol::intern(
+                &Self::PATH.into_iter().map(|i| i.to_string()).collect::<Vec<_>>().join(".."),
+            ),
+        });
+    }
 
     /// Converts a single syntactical attribute to a single semantic attribute, or [`AttributeKind`]
     fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind>;
@@ -94,12 +107,24 @@ impl<T: SingleAttributeParser> Default for Single<T> {
 
 impl<T: SingleAttributeParser> AttributeParser for Single<T> {
     const ATTRIBUTES: AcceptMapping<Self> = &[(T::PATH, |group: &mut Single<T>, cx, args| {
-        if let Some((_, s)) = group.1 {
-            T::on_duplicate(cx, s);
-            return;
-        }
-
         if let Some(pa) = T::convert(cx, args) {
+            match T::ON_DUPLICATE_STRATEGY {
+                // keep the first and error
+                AttributeDuplicates::ErrorFollowing => {
+                    if let Some((_, unused)) = group.1 {
+                        T::on_duplicate(cx, cx.attr_span, unused);
+                        return;
+                    }
+                }
+                // keep the new one and warn about the previous,
+                // then replace
+                AttributeDuplicates::FutureWarnPreceding => {
+                    if let Some((_, used)) = group.1 {
+                        T::on_duplicate(cx, used, cx.attr_span);
+                    }
+                }
+            }
+
             group.1 = Some((pa, cx.attr_span));
         }
     })];
@@ -109,6 +134,68 @@ impl<T: SingleAttributeParser> AttributeParser for Single<T> {
     }
 }
 
+pub(crate) enum OnDuplicate {
+    /// Give a default warning
+    Warn,
+
+    /// Duplicates will be a warning, with a note that this will be an error in the future.
+    WarnButFutureError,
+
+    /// Give a default error
+    Error,
+
+    /// Ignore duplicates
+    Ignore,
+
+    /// Custom function called when a duplicate attribute is found.
+    ///
+    /// - `unused` is the span of the attribute that was unused or bad because of some
+    ///   duplicate reason (see [`AttributeDuplicates`])
+    /// - `used` is the span of the attribute that was used in favor of the unused attribute
+    Custom(fn(cx: &AcceptContext<'_>, used: Span, unused: Span)),
+}
+
+impl OnDuplicate {
+    fn exec<P: SingleAttributeParser>(&self, cx: &AcceptContext<'_>, used: Span, unused: Span) {
+        match self {
+            OnDuplicate::Warn => {
+                todo!()
+            }
+            OnDuplicate::WarnButFutureError => {
+                todo!()
+            }
+            OnDuplicate::Error => {
+                cx.emit_err(UnusedMultiple {
+                    this: used,
+                    other: unused,
+                    name: Symbol::intern(
+                        &P::PATH.into_iter().map(|i| i.to_string()).collect::<Vec<_>>().join(".."),
+                    ),
+                });
+            }
+            OnDuplicate::Ignore => {}
+            OnDuplicate::Custom(f) => f(cx, used, unused),
+        }
+    }
+}
+
+pub(crate) enum AttributeDuplicates {
+    /// Duplicates after the first attribute will be an error.
+    ///
+    /// This should be used where duplicates would be ignored, but carry extra
+    /// meaning that could cause confusion. For example, `#[stable(since="1.0")]
+    /// #[stable(since="2.0")]`, which version should be used for `stable`?
+    ErrorFollowing,
+
+    /// Duplicates preceding the last instance of the attribute will be a
+    /// warning, with a note that this will be an error in the future.
+    ///
+    /// This is the same as `FutureWarnFollowing`, except the last attribute is
+    /// the one that is "used". Ideally these can eventually migrate to
+    /// `ErrorPreceding`.
+    FutureWarnPreceding,
+}
+
 type ConvertFn<E> = fn(ThinVec<E>) -> AttributeKind;
 
 /// Alternative to [`AttributeParser`] that automatically handles state management.

compiler/rustc_attr_parsing/src/attributes/stability.rs

@@ -8,7 +8,7 @@ use rustc_errors::ErrorGuaranteed;
 use rustc_span::{Span, Symbol, kw, sym};
 
 use super::util::parse_version;
-use super::{AcceptMapping, AttributeParser, SingleAttributeParser};
+use super::{AcceptMapping, AttributeDuplicates, AttributeParser, SingleAttributeParser};
 use crate::context::{AcceptContext, FinalizeContext};
 use crate::parser::{ArgParser, MetaItemParser};
 use crate::session_diagnostics::{self, UnsupportedLiteralReason};
@@ -122,9 +122,10 @@ pub(crate) struct ConstStabilityIndirectParser;
 // FIXME(jdonszelmann): single word attribute group when we have these
 impl SingleAttributeParser for ConstStabilityIndirectParser {
     const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_const_stable_indirect];
+    const ON_DUPLICATE_STRATEGY: AttributeDuplicates = AttributeDuplicates::ErrorFollowing;
 
     // ignore
-    fn on_duplicate(_cx: &AcceptContext<'_>, _first_span: Span) {}
+    fn on_duplicate(_cx: &AcceptContext<'_>, _used: Span, _unused: Span) {}
 
     fn convert(_cx: &AcceptContext<'_>, _args: &ArgParser<'_>) -> Option<AttributeKind> {
         Some(AttributeKind::ConstStabilityIndirect)

compiler/rustc_attr_parsing/src/attributes/transparency.rs

@@ -1,8 +1,8 @@
 use rustc_attr_data_structures::AttributeKind;
 use rustc_span::hygiene::Transparency;
-use rustc_span::sym;
+use rustc_span::{Span, sym};
 
-use super::{AcceptContext, SingleAttributeParser};
+use super::{AcceptContext, AttributeDuplicates, SingleAttributeParser};
 use crate::parser::ArgParser;
 
 pub(crate) struct TransparencyParser;
@@ -12,9 +12,10 @@ pub(crate) struct TransparencyParser;
 #[allow(rustc::diagnostic_outside_of_impl)]
 impl SingleAttributeParser for TransparencyParser {
     const PATH: &'static [rustc_span::Symbol] = &[sym::rustc_macro_transparency];
+    const ON_DUPLICATE_STRATEGY: AttributeDuplicates = AttributeDuplicates::ErrorFollowing;
 
-    fn on_duplicate(cx: &crate::context::AcceptContext<'_>, first_span: rustc_span::Span) {
-        cx.dcx().span_err(vec![first_span, cx.attr_span], "multiple macro transparency attributes");
+    fn on_duplicate(cx: &crate::context::AcceptContext<'_>, used: Span, unused: Span) {
+        cx.dcx().span_err(vec![used, unused], "multiple macro transparency attributes");
     }
 
     fn convert(cx: &AcceptContext<'_>, args: &ArgParser<'_>) -> Option<AttributeKind> {