Simplify darling_opts::BuildFnError

Avoid custom parsing of meta items.
colin-kiegel · Jan 20, 2024 · d91b966 · d91b966
1 parent 4c008cb
commit d91b966
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 57 deletions.
diff --git a/derive_builder/tests/compile-fail/build_fn_error.stderr b/derive_builder/tests/compile-fail/build_fn_error.stderr
@@ -1,8 +1,8 @@
-error: Too many items: Expected no more than 1
- --> tests/compile-fail/build_fn_error.rs:4:20
+error: Unknown field: `path`
+ --> tests/compile-fail/build_fn_error.rs:4:52
   |
 4 | #[builder(build_fn(error(validation_error = false, path = "hello")))]
-  |                    ^^^^^
+  |                                                    ^^^^
 
 error: Cannot set `error(validation_error = false)` when using `validate`
   --> tests/compile-fail/build_fn_error.rs:10:26
@@ -16,7 +16,15 @@ error: Unknown field: `path`
 16 | #[builder(build_fn(error(path = "hello")))]
    |                          ^^^^
 
-error: Too few items: Expected at least 1
+error: Missing field `validation_error` at build_fn/error
+  --> tests/compile-fail/build_fn_error.rs:15:10
+   |
+15 | #[derive(Builder)]
+   |          ^^^^^^^
+   |
+   = note: this error originates in the derive macro `Builder` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: Missing field `validation_error`
   --> tests/compile-fail/build_fn_error.rs:22:20
    |
 22 | #[builder(build_fn(error()))]

diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs
@@ -2,7 +2,7 @@ use std::{borrow::Cow, vec::IntoIter};
 
 use crate::BuildMethod;
 
-use darling::util::{Flag, PathList};
+use darling::util::{Flag, PathList, SpannedValue};
 use darling::{self, Error, FromMeta};
 use proc_macro2::{Span, TokenStream};
 use syn::parse::{ParseStream, Parser};
@@ -69,61 +69,41 @@ fn no_visibility_conflict<T: Visibility>(v: &T) -> darling::Result<()> {
     }
 }
 
+#[derive(Debug, Clone, FromMeta)]
+struct BuildFnErrorGenerated {
+    /// Indicates whether or not the generated error should have
+    /// a validation variant that takes a `String` as its contents.
+    validation_error: SpannedValue<bool>,
+}
+
 #[derive(Debug, Clone)]
 enum BuildFnError {
-    Path(Path),
-    ValidationError(bool, Span),
+    Existing(Path),
+    Generated(BuildFnErrorGenerated),
 }
 
 impl BuildFnError {
-    fn as_path(&self) -> Option<&Path> {
+    fn as_existing(&self) -> Option<&Path> {
         match self {
-            BuildFnError::Path(p) => Some(p),
-            BuildFnError::ValidationError(_, _) => None,
+            BuildFnError::Existing(p) => Some(p),
+            BuildFnError::Generated(_) => None,
         }
     }
 
-    fn as_validation_error(&self) -> Option<(bool, &Span)> {
+    fn as_generated(&self) -> Option<&BuildFnErrorGenerated> {
         match self {
-            BuildFnError::ValidationError(b, p) => Some((*b, p)),
-            BuildFnError::Path(_) => None,
+            BuildFnError::Generated(e) => Some(e),
+            BuildFnError::Existing(_) => None,
         }
     }
 }
 
 impl FromMeta for BuildFnError {
-    fn from_list(items: &[syn::NestedMeta]) -> darling::Result<Self> {
-        let item = match items {
-            [] => Err(Error::too_few_items(1)),
-            [item] => Ok(item),
-            _ => Err(Error::too_many_items(1)),
-        }?;
-        let nested = match &item {
-            syn::NestedMeta::Meta(nested) => Ok(nested),
-            syn::NestedMeta::Lit(_) => Err(Error::unsupported_format("literal")),
-        }?;
-        match darling::util::path_to_string(nested.path()).as_ref() {
-            "validation_error" => {
-                let span = nested.span();
-                let boolean = FromMeta::from_meta(nested).map_err(|e| e.at("validation_error"))?;
-                Ok(BuildFnError::ValidationError(boolean, span))
-            }
-            other => {
-                Err(Error::unknown_field_with_alts(other, &["validation_error"]).with_span(nested))
-            }
-        }
-    }
-
-    fn from_value(value: &syn::Lit) -> darling::Result<Self> {
-        if let syn::Lit::Str(s) = value {
-            let contents: Path = s.parse()?;
-            if contents.segments.is_empty() {
-                Err(Error::unknown_value("").with_span(s))
-            } else {
-                Ok(Self::Path(contents))
-            }
-        } else {
-            Err(Error::unexpected_lit_type(value))
+    fn from_meta(item: &Meta) -> darling::Result<Self> {
+        match item {
+            Meta::Path(_) => Err(Error::unsupported_format("word").with_span(item)),
+            Meta::List(_) => BuildFnErrorGenerated::from_meta(item).map(Self::Generated),
+            Meta::NameValue(i) => Path::from_value(&i.lit).map(Self::Existing),
         }
     }
 }
@@ -168,14 +148,19 @@ pub struct BuildFn {
 impl BuildFn {
     fn validation_needs_error(self) -> darling::Result<Self> {
         let mut acc = Error::accumulator();
-        if let (true, Some(BuildFnError::ValidationError(false, s))) =
-            (self.validate.is_some(), &self.error)
-        {
-            acc.push(
-                Error::custom("Cannot set `error(validation_error = false)` when using `validate`")
-                    .with_span(s),
-            );
+        if self.validate.is_some() {
+            if let Some(BuildFnError::Generated(e)) = &self.error {
+                if !*e.validation_error {
+                    acc.push(
+                        Error::custom(
+                            "Cannot set `error(validation_error = false)` when using `validate`",
+                        )
+                        .with_span(&e.validation_error),
+                    )
+                }
+            }
         }
+
         acc.finish_with(self)
     }
 }
@@ -724,7 +709,7 @@ impl Options {
     }
 
     pub fn builder_error_ident(&self) -> Path {
-        if let Some(BuildFnError::Path(existing)) = self.build_fn.error.as_ref() {
+        if let Some(BuildFnError::Existing(existing)) = self.build_fn.error.as_ref() {
             existing.clone()
         } else if let Some(ref custom) = self.name {
             format_ident!("{}Error", custom).into()
@@ -795,14 +780,15 @@ impl Options {
                 .build_fn
                 .error
                 .as_ref()
-                .and_then(BuildFnError::as_path)
+                .and_then(BuildFnError::as_existing)
                 .is_none(),
             generate_validation_error: self
                 .build_fn
                 .error
                 .as_ref()
-                .and_then(BuildFnError::as_validation_error)
-                .map_or(true, |(b, _)| b),
+                .and_then(BuildFnError::as_generated)
+                .map(|e| *e.validation_error)
+                .unwrap_or(true),
             no_alloc: cfg!(not(any(feature = "alloc", feature = "lib_has_std"))),
             must_derive_clone: self.requires_clone(),
             doc_comment: None,
@@ -1013,7 +999,7 @@ impl<'a> FieldWithDefaults<'a> {
             conversion: self.conversion(),
             custom_error_type_span: self.parent.build_fn.error.as_ref().and_then(|err_ty| {
                 match err_ty {
-                    BuildFnError::Path(p) => Some(p.span()),
+                    BuildFnError::Existing(p) => Some(p.span()),
                     _ => None,
                 }
             }),