Addresses feedback to improve design.

derive_builder/CHANGELOG.md

@@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased] - 2023-12-21
-- Add `build_fn(validation_error = <bool>)` to disable generation of `ValidationError` within the builder's error so that `alloc::string` is avoided.
+- Add `build_fn(error(validation_error = <bool>))` to disable generation of `ValidationError` within the builder's error so that `alloc::string` is avoided.
 - Add feature `alloc` for controlling linking of `alloc` crate during `no_std`. This way users can use `no_std` without providing a `global_allocator`.
 
 ## [Unreleased] - 2023-07-25

derive_builder/README.md

@@ -130,7 +130,7 @@ It's as simple as three steps:
 - **Builder derivations**: You can use `#[builder(derive(Trait1, Trait2, ...))]` to have the builder derive additonal traits. All builders derive `Default` and `Clone`, so you should not declare those in this attribute.
 - **Pass-through attributes**: Use `#[builder_struct_attr(...)]`, `#[builder_impl_attr(...)]`, `#[builder_field_attr(...)]`, and `#[builder_setter_attr(...)]` to declare attributes that will be added to the relevant part of the generated builder.
 - **no_std support**: Just add `#[builder(no_std)]` to your struct, use feature `alloc`, and add `extern crate alloc` to your crate.
-- **No alloc no_std support**: Do not use `alloc` feature and then either add `#[builder(no_std, build_fn(validation_error = false))]` or `#[builder(no_std, build_fn(error = "path::to::Error"))]` to your struct.
+- **No alloc no_std support**: Do not use `alloc` feature and then either add `#[builder(no_std, build_fn(error(validation_error = false)))]` or `#[builder(no_std, build_fn(error = "path::to::Error"))]` to your struct.
 - **Renaming and re-export support**: Use `#[builder(crate = "...")]` to set the root for `derive_builder`. This is useful if you want to rename `derive_builder` in `Cargo.toml` or if your crate is re-exporting `derive_builder::Builder` and needs the generated code to not directly reference the `derive_builder` crate.
 
 For more information and examples please take a look at our [documentation][doc].

derive_builder/tests/compile-fail/build_fn_error.rs

@@ -0,0 +1,27 @@
+use derive_builder::Builder;
+
+#[derive(Builder)]
+#[builder(build_fn(error(validation_error = false, path = "hello")))]
+struct Fee {
+    hi: u32,
+}
+
+#[derive(Builder)]
+#[builder(build_fn(error(validation_error = false), validate = "hello"))]
+struct Foo {
+    hi: u32,
+}
+
+#[derive(Builder)]
+#[builder(build_fn(error(path = "hello")))]
+struct Fii {
+    hi: u32,
+}
+
+#[derive(Builder)]
+#[builder(build_fn(error()))]
+struct Fum {
+    hi: u32,
+}
+
+fn main() {}

derive_builder/tests/compile-fail/build_fn_error.stderr

@@ -0,0 +1,23 @@
+error: Too many items: Expected no more than 1
+ --> tests/compile-fail/build_fn_error.rs:4:20
+  |
+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
+   |
+10 | #[builder(build_fn(error(validation_error = false), validate = "hello"))]
+   |                          ^^^^^^^^^^^^^^^^
+
+error: Unknown field: `path`
+  --> tests/compile-fail/build_fn_error.rs:16:26
+   |
+16 | #[builder(build_fn(error(path = "hello")))]
+   |                          ^^^^
+
+error: Too few items: Expected at least 1
+  --> tests/compile-fail/build_fn_error.rs:22:20
+   |
+22 | #[builder(build_fn(error()))]
+   |                    ^^^^^

derive_builder_core/src/builder.rs

@@ -132,8 +132,8 @@ pub struct Builder<'a> {
     /// Whether to include `ValidationError` in the generated enum. Necessary to avoid dependency
     /// on `alloc::string`.
     ///
-    /// This would be `false` when `build_fn.validation_error == false`. This is ignored when
-    /// `generate_error` is `false`.
+    /// This would be `false` when `build_fn.error.as_validation_error() == Some((false, _))`. This
+    /// has no effect when `generate_error` is `false`.
     pub generate_validation_error: bool,
     /// Whether this builder must derive `Clone`.
     ///

derive_builder_core/src/macro_options/darling_opts.rs

@@ -69,6 +69,65 @@ fn no_visibility_conflict<T: Visibility>(v: &T) -> darling::Result<()> {
     }
 }
 
+#[derive(Debug, Clone)]
+enum BuildFnError {
+    Path(Path),
+    ValidationError(bool, Span),
+}
+
+impl BuildFnError {
+    fn as_path(&self) -> Option<&Path> {
+        match self {
+            BuildFnError::Path(p) => Some(p),
+            BuildFnError::ValidationError(_, _) => None,
+        }
+    }
+
+    fn as_validation_error(&self) -> Option<(bool, &Span)> {
+        match self {
+            BuildFnError::ValidationError(b, p) => Some((*b, p)),
+            BuildFnError::Path(_) => 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))
+        }
+    }
+}
+
 /// Options for the `build_fn` property in struct-level builder options.
 /// There is no inheritance for these settings from struct-level to field-level,
 /// so we don't bother using `Option` for values in this struct.
@@ -81,38 +140,43 @@ pub struct BuildFn {
     public: Flag,
     private: Flag,
     vis: Option<syn::Visibility>,
-    /// The path to an existing error type that the build method should return.
+    /// Either the path to an existing error type that the build method should return or a meta
+    /// list of options to modify the generated error.
+    ///
+    /// Setting this to a path will prevent `derive_builder` from generating an error type for the
+    /// build method.
     ///
-    /// Setting this will prevent `derive_builder` from generating an error type for the build
-    /// method.
+    /// This options supports to formats: path `error = "path::to::Error"` and meta list
+    /// `error(<options>)`. Supported mata list options are the following:
     ///
-    /// # Type Bounds
+    /// * `validation_error = bool` - Whether to generate `ValidationError(String)` as a variant
+    ///   of the build error type. Setting this to `false` will prevent `derive_builder` from
+    ///   using the `validate` function but this also means it does not generate any usage of the
+    ///  `alloc` crate (useful when disabling the `alloc` feature in `no_std`).
+    ///
+    /// # Type Bounds for Custom Error
     /// This type's bounds depend on other settings of the builder.
     ///
     /// * If uninitialized fields cause `build()` to fail, then this type
     ///   must `impl From<UninitializedFieldError>`. Uninitialized fields do not cause errors
     ///   when default values are provided for every field or at the struct level.
     /// * If `validate` is specified, then this type must provide a conversion from the specified
     ///   function's error type.
-    error: Option<Path>,
-    /// Whether to generate `ValidationError(String)` as a variant of the build error type.
-    ///
-    /// Setting this to `false` will prevent `derive_builder` from using the `validate` function
-    /// but this also means it does not generate any usage of the `alloc` crate (useful when
-    /// disabling the `alloc` feature in `no_std`).
-    validation_error: bool,
+    error: Option<BuildFnError>,
 }
 
 impl BuildFn {
     fn validation_needs_error(self) -> darling::Result<Self> {
-        if self.validate.is_some() && !self.validation_error {
-            Err(darling::Error::custom(
-                "Cannot set `validation_error = false` when using `validate`",
-            )
-            .with_span(&self.validate))
-        } else {
-            Ok(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),
+            );
         }
+        acc.finish_with(self)
     }
 }
 
@@ -126,7 +190,6 @@ impl Default for BuildFn {
             private: Default::default(),
             vis: None,
             error: None,
-            validation_error: true,
         }
     }
 }
@@ -661,7 +724,7 @@ impl Options {
     }
 
     pub fn builder_error_ident(&self) -> Path {
-        if let Some(existing) = self.build_fn.error.as_ref() {
+        if let Some(BuildFnError::Path(existing)) = self.build_fn.error.as_ref() {
             existing.clone()
         } else if let Some(ref custom) = self.name {
             format_ident!("{}Error", custom).into()
@@ -728,8 +791,18 @@ impl Options {
             fields: Vec::with_capacity(self.field_count()),
             field_initializers: Vec::with_capacity(self.field_count()),
             functions: Vec::with_capacity(self.field_count()),
-            generate_error: self.build_fn.error.is_none(),
-            generate_validation_error: self.build_fn.validation_error,
+            generate_error: self
+                .build_fn
+                .error
+                .as_ref()
+                .and_then(BuildFnError::as_path)
+                .is_none(),
+            generate_validation_error: self
+                .build_fn
+                .error
+                .as_ref()
+                .and_then(BuildFnError::as_validation_error)
+                .map_or(true, |(b, _)| b),
             must_derive_clone: self.requires_clone(),
             doc_comment: None,
             deprecation_notes: Default::default(),
@@ -937,12 +1010,12 @@ impl<'a> FieldWithDefaults<'a> {
             default_value: self.field.default.as_ref(),
             use_default_struct: self.use_parent_default(),
             conversion: self.conversion(),
-            custom_error_type_span: self
-                .parent
-                .build_fn
-                .error
-                .as_ref()
-                .map(|err_ty| err_ty.span()),
+            custom_error_type_span: self.parent.build_fn.error.as_ref().and_then(|err_ty| {
+                match err_ty {
+                    BuildFnError::Path(p) => Some(p.span()),
+                    _ => None,
+                }
+            }),
         }
     }
 

derive_builder_no_alloc_tests/src/lib.rs

@@ -19,7 +19,7 @@ pub struct Foo {
 }
 
 #[derive(Builder)]
-#[builder(no_std, build_fn(validation_error = false))]
+#[builder(no_std, build_fn(error(validation_error = false)))]
 pub struct Fee {
     pub bar: i32,
 }