Rework util::Flag

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+
+- **BREAKING CHANGE:** Remove many trait impls from `util::Flag`. 
+  This type had a number of deref and operator impls that made it usable as sort-of-a-boolean.
+  Real-world usage showed this type is more useful if it's able to carry a span for good errors,
+  and that most of those impls were unnecessary.
+
 ## v0.13.4 (April 6, 2022)
 
 - Impl `FromMeta` for `syn::Visibility` [#173](https://github.com/TedDriggs/darling/pull/173)

core/src/options/core.rs

@@ -4,12 +4,11 @@ use crate::ast::{Data, Fields, Style};
 use crate::codegen;
 use crate::codegen::PostfixTransform;
 use crate::options::{DefaultExpression, InputField, InputVariant, ParseAttribute, ParseData};
-use crate::util::Flag;
 use crate::{Error, FromMeta, Result};
 
 /// A struct or enum which should have `FromMeta` or `FromDeriveInput` implementations
 /// generated.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone)]
 pub struct Core {
     /// The type identifier.
     pub ident: syn::Ident,
@@ -40,7 +39,7 @@ pub struct Core {
     pub bound: Option<Vec<syn::WherePredicate>>,
 
     /// Whether or not unknown fields should produce an error at compilation time.
-    pub allow_unknown_fields: Flag,
+    pub allow_unknown_fields: Option<bool>,
 }
 
 impl Core {
@@ -164,7 +163,7 @@ impl<'a> From<&'a Core> for codegen::TraitImpl<'a> {
             default: v.as_codegen_default(),
             post_transform: v.post_transform.as_ref(),
             bound: v.bound.as_ref().map(|i| i.as_slice()),
-            allow_unknown_fields: v.allow_unknown_fields.into(),
+            allow_unknown_fields: v.allow_unknown_fields.unwrap_or_default(),
         }
     }
 }

core/src/options/from_variant.rs

@@ -6,7 +6,7 @@ use crate::codegen::FromVariantImpl;
 use crate::options::{DataShape, OuterFrom, ParseAttribute, ParseData};
 use crate::{FromMeta, Result};
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone)]
 pub struct FromVariantOptions {
     pub base: OuterFrom,
     /// The field on the deriving struct into which the discriminant expression

core/src/options/input_variant.rs

@@ -73,7 +73,7 @@ impl InputVariant {
         }
 
         if self.allow_unknown_fields.is_none() {
-            self.allow_unknown_fields = Some(parent.allow_unknown_fields.is_some());
+            self.allow_unknown_fields = Some(parent.allow_unknown_fields.unwrap_or_default());
         }
 
         self

core/src/options/outer_from.rs

@@ -6,7 +6,7 @@ use crate::{FromMeta, Result};
 
 /// Reusable base for `FromDeriveInput`, `FromVariant`, `FromField`, and other top-level
 /// `From*` traits.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone)]
 pub struct OuterFrom {
     /// The field on the target struct which should receive the type identifier, if any.
     pub ident: Option<Ident>,

core/src/util/flag.rs

@@ -0,0 +1,100 @@
+use proc_macro2::Span;
+use syn::{spanned::Spanned, Meta};
+
+use crate::{FromMeta, Result};
+
+/// A meta-item that can be present as a word - with no value - or absent.
+///
+/// # Defaulting
+/// Like `Option`, `Flag` does not require `#[darling(default)]` to be optional.
+/// If the caller does not include the property, then an absent `Flag` will be included
+/// in the receiver struct.
+///
+/// # Spans
+/// `Flag` keeps the span where its word was seen.
+/// This enables attaching custom error messages to the word, such as in the case of two
+/// conflicting flags being present.
+///
+/// # Example
+/// ```ignore
+/// #[derive(FromMeta)]
+/// #[darling(and_then = "Self::not_both")]
+/// struct Demo {
+///     flag_a: Flag,
+///     flag_b: Flag,
+/// }
+///
+/// impl Demo {
+///     fn not_both(self) -> Result<Self> {
+///         if self.flag_a.is_present() && self.flag_b.is_present() {
+///             Err(Error::custom("Cannot set flag_a and flag_b").with_span(self.flag_b))
+///         } else {
+///             Ok(self)
+///         }
+///     }
+/// }
+/// ```
+///
+/// The above struct would then produce the following error.
+///
+/// ```ignore
+/// #[example(flag_a, flag_b)]
+/// //                ^^^^^^ Cannot set flag_a and flag_b
+/// ```
+#[derive(Debug, Clone, Copy, Default)]
+pub struct Flag(Option<Span>);
+
+impl Flag {
+    /// Creates a new `Flag` which corresponds to the presence of a value.
+    pub fn present() -> Self {
+        Flag(Some(Span::call_site()))
+    }
+
+    /// Check if the flag is present.
+    pub fn is_present(&self) -> bool {
+        self.0.is_some()
+    }
+
+    #[deprecated(since = "0.14.0", note = "Use Flag::is_present")]
+    pub fn is_some(&self) -> bool {
+        self.is_present()
+    }
+}
+
+impl FromMeta for Flag {
+    fn from_none() -> Option<Self> {
+        Some(Flag(None))
+    }
+
+    fn from_meta(mi: &syn::Meta) -> Result<Self> {
+        if let Meta::Path(p) = mi {
+            Ok(Flag(Some(p.span())))
+        } else {
+            // The implementation for () will produce an error for all non-path meta items;
+            // call it to make sure the span behaviors and error messages are the same.
+            Err(<()>::from_meta(mi).unwrap_err())
+        }
+    }
+}
+
+impl Spanned for Flag {
+    fn span(&self) -> Span {
+        self.0.unwrap_or_else(Span::call_site)
+    }
+}
+
+impl From<Flag> for bool {
+    fn from(flag: Flag) -> Self {
+        flag.is_present()
+    }
+}
+
+impl From<bool> for Flag {
+    fn from(v: bool) -> Self {
+        if v {
+            Flag::present()
+        } else {
+            Flag(None)
+        }
+    }
+}

core/src/util/mod.rs

@@ -1,9 +1,6 @@
 //! Utility types for attribute parsing.
 
-use std::ops::{BitAnd, BitOr, Deref, Not};
-
-use crate::{FromMeta, Result};
-
+mod flag;
 mod ident_string;
 mod ignored;
 mod over_ride;
@@ -13,6 +10,7 @@ mod path_to_string;
 mod spanned_value;
 mod with_original;
 
+pub use self::flag::Flag;
 pub use self::ident_string::IdentString;
 pub use self::ignored::Ignored;
 pub use self::over_ride::Override;
@@ -21,101 +19,3 @@ pub use self::path_list::PathList;
 pub use self::path_to_string::path_to_string;
 pub use self::spanned_value::SpannedValue;
 pub use self::with_original::WithOriginal;
-
-/// Marker type equivalent to `Option<()>` for use in attribute parsing.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
-pub struct Flag(Option<()>);
-
-impl Flag {
-    /// Creates a new `Flag` which corresponds to the presence of a value.
-    pub fn present() -> Self {
-        Flag(Some(()))
-    }
-}
-
-impl Deref for Flag {
-    type Target = Option<()>;
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl FromMeta for Flag {
-    fn from_none() -> Option<Self> {
-        Some(Flag(None))
-    }
-
-    fn from_meta(mi: &syn::Meta) -> Result<Self> {
-        FromMeta::from_meta(mi).map(Flag)
-    }
-}
-
-impl From<Flag> for bool {
-    fn from(flag: Flag) -> Self {
-        flag.is_some()
-    }
-}
-
-impl From<bool> for Flag {
-    fn from(v: bool) -> Self {
-        if v {
-            Flag::present()
-        } else {
-            Flag(None)
-        }
-    }
-}
-
-impl From<Option<()>> for Flag {
-    fn from(v: Option<()>) -> Self {
-        Flag::from(v.is_some())
-    }
-}
-
-impl PartialEq<Option<()>> for Flag {
-    fn eq(&self, rhs: &Option<()>) -> bool {
-        self.0 == *rhs
-    }
-}
-
-impl PartialEq<Flag> for Option<()> {
-    fn eq(&self, rhs: &Flag) -> bool {
-        *self == rhs.0
-    }
-}
-
-impl PartialEq<bool> for Flag {
-    fn eq(&self, rhs: &bool) -> bool {
-        self.is_some() == *rhs
-    }
-}
-
-impl Not for Flag {
-    type Output = Self;
-
-    fn not(self) -> Self {
-        if self.is_some() {
-            Flag(None)
-        } else {
-            Flag::present()
-        }
-    }
-}
-
-impl BitAnd for Flag {
-    type Output = Self;
-
-    fn bitand(self, rhs: Self) -> Self {
-        #[allow(clippy::suspicious_arithmetic_impl)]
-        (self.into() && rhs.into()).into()
-    }
-}
-
-impl BitOr for Flag {
-    type Output = Self;
-
-    fn bitor(self, rhs: Self) -> Self {
-        #[allow(clippy::suspicious_arithmetic_impl)]
-        (self.into() || rhs.into()).into()
-    }
-}

tests/defaults.rs

@@ -146,7 +146,7 @@ mod implicit_default {
 
         assert_eq!(person.first_name, "James");
         assert_eq!(person.last_name, None);
-        assert_eq!(person.lefty, false);
+        assert!(!person.lefty.is_present());
     }
 }
 
@@ -184,6 +184,6 @@ mod overridden_implicit_default {
 
         assert_eq!(person.first_name, "Jane");
         assert_eq!(person.last_name, Some("Archer".into()));
-        assert_eq!(person.lefty, false);
+        assert!(!person.lefty.is_present());
     }
 }