diff --git a/derive_builder/CHANGELOG.md b/derive_builder/CHANGELOG.md index 1efef791..fe14837b 100644 --- a/derive_builder/CHANGELOG.md +++ b/derive_builder/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased +- Only generate a validation error variant when necessary #201 + ## [0.10.0] - 2021-03-31 - Requires Rust 1.40.0 or newer (was 1.37.0) #169 - Logging feature is removed #177 diff --git a/derive_builder/examples/custom_defaults.rs b/derive_builder/examples/custom_defaults.rs index bf493d9e..c5d524c3 100644 --- a/derive_builder/examples/custom_defaults.rs +++ b/derive_builder/examples/custom_defaults.rs @@ -1,7 +1,19 @@ +use derive_builder::UninitializedFieldError; + #[macro_use] extern crate derive_builder; +/// Marker function that tells `derive_builder` to emit a `ValidationError` variant. +/// +/// Without this, `derive_builder` would believe that only uninitialized field errors +/// are possible in the `build()` method, and there would be a cryptic error message +/// about a missing conversion from `String`. +fn phantom_validate(_: T) -> Result<(), String> { + Ok(()) +} + #[derive(Builder, PartialEq, Debug)] +#[builder(build_fn(validate = "phantom_validate"))] struct Lorem { ipsum: String, #[builder(default = "self.default_dolor()?")] @@ -13,10 +25,10 @@ struct Lorem { } impl LoremBuilder { - fn default_dolor(&self) -> Result { + fn default_dolor(&self) -> Result { self.ipsum .clone() - .ok_or_else(|| "ipsum must be initialized to build dolor".to_string()) + .ok_or_else(|| UninitializedFieldError::new("ipsum")) } fn default_sit(&self) -> Result { diff --git a/derive_builder/src/lib.rs b/derive_builder/src/lib.rs index ab48ecb2..6f45840d 100644 --- a/derive_builder/src/lib.rs +++ b/derive_builder/src/lib.rs @@ -342,22 +342,26 @@ //! ```rust //! # #[macro_use] //! # extern crate derive_builder; +//! # use derive_builder::UninitializedFieldError; //! # //! # #[derive(Builder, PartialEq, Debug)] //! struct Lorem { //! ipsum: String, //! // Custom defaults can delegate to helper methods //! // and pass errors to the enclosing `build()` method via `?`. +//! // +//! // When doing this, it is recommended that you use a custom error type +//! // unless you are only returning UninitializedFieldError. //! #[builder(default = "self.default_dolor()?")] //! dolor: String, //! } //! //! impl LoremBuilder { //! // Private helper method with access to the builder struct. -//! fn default_dolor(&self) -> Result { +//! fn default_dolor(&self) -> Result { //! match self.ipsum { -//! Some(ref x) if x.chars().count() > 3 => Ok(format!("dolor {}", x)), -//! _ => Err("ipsum must at least 3 chars to build dolor".to_string()), +//! Some(ref x) => Ok(format!("dolor {}", x)), +//! _ => Err(UninitializedFieldError::new("ipsum")), //! } //! } //! } diff --git a/derive_builder/tests/try_setter.rs b/derive_builder/tests/try_setter.rs index 28f6a01c..038ee042 100644 --- a/derive_builder/tests/try_setter.rs +++ b/derive_builder/tests/try_setter.rs @@ -4,7 +4,8 @@ extern crate derive_builder; use std::convert::TryFrom; use std::net::{AddrParseError, IpAddr}; use std::str::FromStr; -use std::string::ToString; + +use derive_builder::UninitializedFieldError; #[derive(Debug, Clone, PartialEq)] pub struct MyAddr(IpAddr); @@ -23,8 +24,29 @@ impl<'a> TryFrom<&'a str> for MyAddr { } } +#[derive(Debug)] +enum LoremBuilderError { + UninitializedField(&'static str), + InvalidValue { field: &'static str, error: String }, +} + +impl LoremBuilderError { + fn invalid_value(field: &'static str, error: impl std::fmt::Display) -> Self { + Self::InvalidValue { + field, + error: format!("{}", error), + } + } +} + +impl From for LoremBuilderError { + fn from(e: UninitializedFieldError) -> Self { + Self::UninitializedField(e.field_name()) + } +} + #[derive(Debug, PartialEq, Builder)] -#[builder(try_setter, setter(into))] +#[builder(try_setter, setter(into), build_fn(error = "LoremBuilderError"))] struct Lorem { pub source: MyAddr, pub dest: MyAddr, @@ -46,9 +68,9 @@ fn exact_helper() -> Result { fn try_helper() -> Result { LoremBuilder::default() .try_source("1.2.3.4") - .map_err(|e| e.to_string())? + .map_err(|e| LoremBuilderError::invalid_value("source", e))? .try_dest("0.0.0.0") - .map_err(|e| e.to_string())? + .map_err(|e| LoremBuilderError::invalid_value("dest", e))? .build() } diff --git a/derive_builder_core/src/block.rs b/derive_builder_core/src/block.rs index a1ce0576..a288a748 100644 --- a/derive_builder_core/src/block.rs +++ b/derive_builder_core/src/block.rs @@ -67,7 +67,7 @@ mod test { use super::*; #[test] - #[should_panic(expected = r#"LexError"#)] + #[should_panic] fn block_invalid_token_trees() { Block::from_str("let x = 2; { x+1").unwrap(); } diff --git a/derive_builder_core/src/builder.rs b/derive_builder_core/src/builder.rs index cbabb946..7d18f33a 100644 --- a/derive_builder_core/src/builder.rs +++ b/derive_builder_core/src/builder.rs @@ -10,6 +10,13 @@ use BuilderPattern; use DeprecationNotes; use Setter; +#[derive(Debug)] +pub struct GeneratedError { + /// Whether the generated error needs a variant for errors produced by a caller-provided + /// validation function. + pub validation_error: bool, +} + /// Builder, implementing `quote::ToTokens`. /// /// # Examples @@ -126,10 +133,10 @@ pub struct Builder<'a> { pub field_initializers: Vec, /// Functions of the builder struct, e.g. `fn bar() -> { unimplemented!() }` pub functions: Vec, - /// Whether or not a generated error type is required. + /// The required generated error type. /// - /// This would be `false` in the case where an already-existing error is to be used. - pub generate_error: bool, + /// This would be `None` in the case where an already-existing error is to be used. + pub generated_error: Option, /// Whether this builder must derive `Clone`. /// /// This is true even for a builder using the `owned` pattern if there is a field whose setter @@ -207,10 +214,26 @@ impl<'a> ToTokens for Builder<'a> { } )); - if self.generate_error { + if let Some(generated_error) = &self.generated_error { + let validate = generated_error.validation_error; let builder_error_ident = format_ident!("{}Error", builder_ident); let builder_error_doc = format!("Error type for {}", builder_ident); + let validation_variant = if validate { + quote! { + /// Custom validation error + ValidationError(::std::string::String), + } + } else { + quote!() + }; + + let validation_display = if validate { + quote!(Self::ValidationError(ref error) => write!(f, "{}", error),) + } else { + quote!() + }; + tokens.append_all(quote!( #[doc=#builder_error_doc] #[derive(Debug)] @@ -218,8 +241,7 @@ impl<'a> ToTokens for Builder<'a> { #builder_vis enum #builder_error_ident { /// Uninitialized field UninitializedField(&'static str), - /// Custom validation error - ValidationError(String), + #validation_variant } impl ::derive_builder::export::core::convert::From<::derive_builder::UninitializedFieldError> for #builder_error_ident { @@ -228,24 +250,28 @@ impl<'a> ToTokens for Builder<'a> { } } - impl ::derive_builder::export::core::convert::From for #builder_error_ident { - fn from(s: String) -> Self { - Self::ValidationError(s) - } - } - impl ::derive_builder::export::core::fmt::Display for #builder_error_ident { fn fmt(&self, f: &mut ::derive_builder::export::core::fmt::Formatter) -> ::derive_builder::export::core::fmt::Result { match self { Self::UninitializedField(ref field) => write!(f, "`{}` must be initialized", field), - Self::ValidationError(ref error) => write!(f, "{}", error), + #validation_display } } } #[cfg(not(no_std))] - impl std::error::Error for #builder_error_ident {} + impl ::std::error::Error for #builder_error_ident {} )); + + if validate { + tokens.append_all(quote!( + impl ::derive_builder::export::core::convert::From<::std::string::String> for #builder_error_ident { + fn from(s: ::std::string::String) -> Self { + Self::ValidationError(s) + } + } + )); + } } } } @@ -323,7 +349,7 @@ macro_rules! default_builder { fields: vec![quote!(foo: u32,)], field_initializers: vec![quote!(foo: ::derive_builder::export::core::default::Default::default(), )], functions: vec![quote!(fn bar() -> { unimplemented!() })], - generate_error: true, + generated_error: Some(GeneratedError { validation_error: true }), must_derive_clone: true, doc_comment: None, deprecation_notes: DeprecationNotes::default(), @@ -337,42 +363,71 @@ mod tests { use super::*; use proc_macro2::TokenStream; - fn add_generated_error(result: &mut TokenStream) { - result.append_all(quote!( - #[doc="Error type for FooBuilder"] - #[derive(Debug)] - #[non_exhaustive] - pub enum FooBuilderError { - /// Uninitialized field - UninitializedField(&'static str), - /// Custom validation error - ValidationError(String), - } + fn add_generated_error(result: &mut TokenStream, generated_error: GeneratedError) { + if generated_error.validation_error { + result.append_all(quote!( + #[doc="Error type for FooBuilder"] + #[derive(Debug)] + #[non_exhaustive] + pub enum FooBuilderError { + /// Uninitialized field + UninitializedField(&'static str), + /// Custom validation error + ValidationError(::std::string::String), + } - impl ::derive_builder::export::core::convert::From<::derive_builder::UninitializedFieldError> for FooBuilderError { - fn from(s: ::derive_builder::UninitializedFieldError) -> Self { - Self::UninitializedField(s.field_name()) + impl ::derive_builder::export::core::convert::From<::derive_builder::UninitializedFieldError> for FooBuilderError { + fn from(s: ::derive_builder::UninitializedFieldError) -> Self { + Self::UninitializedField(s.field_name()) + } } - } - impl ::derive_builder::export::core::convert::From for FooBuilderError { - fn from(s: String) -> Self { - Self::ValidationError(s) + impl ::derive_builder::export::core::fmt::Display for FooBuilderError { + fn fmt(&self, f: &mut ::derive_builder::export::core::fmt::Formatter) -> ::derive_builder::export::core::fmt::Result { + match self { + Self::UninitializedField(ref field) => write!(f, "`{}` must be initialized", field), + Self::ValidationError(ref error) => write!(f, "{}", error), + } + } } - } - impl ::derive_builder::export::core::fmt::Display for FooBuilderError { - fn fmt(&self, f: &mut ::derive_builder::export::core::fmt::Formatter) -> ::derive_builder::export::core::fmt::Result { - match self { - Self::UninitializedField(ref field) => write!(f, "`{}` must be initialized", field), - Self::ValidationError(ref error) => write!(f, "{}", error), + #[cfg(not(no_std))] + impl ::std::error::Error for FooBuilderError {} + + impl ::derive_builder::export::core::convert::From<::std::string::String> for FooBuilderError { + fn from(s: ::std::string::String) -> Self { + Self::ValidationError(s) + } + } + )); + } else { + result.append_all(quote!( + #[doc="Error type for FooBuilder"] + #[derive(Debug)] + #[non_exhaustive] + pub enum FooBuilderError { + /// Uninitialized field + UninitializedField(&'static str), + } + + impl ::derive_builder::export::core::convert::From<::derive_builder::UninitializedFieldError> for FooBuilderError { + fn from(s: ::derive_builder::UninitializedFieldError) -> Self { + Self::UninitializedField(s.field_name()) + } + } + + impl ::derive_builder::export::core::fmt::Display for FooBuilderError { + fn fmt(&self, f: &mut ::derive_builder::export::core::fmt::Formatter) -> ::derive_builder::export::core::fmt::Result { + match self { + Self::UninitializedField(ref field) => write!(f, "`{}` must be initialized", field), + } } } - } - #[cfg(not(no_std))] - impl std::error::Error for FooBuilderError {} - )); + #[cfg(not(no_std))] + impl ::std::error::Error for FooBuilderError {} + )); + } } #[test] @@ -414,7 +469,12 @@ mod tests { } )); - add_generated_error(&mut result); + add_generated_error( + &mut result, + GeneratedError { + validation_error: true, + }, + ); result } @@ -469,7 +529,7 @@ mod tests { } )); - add_generated_error(&mut result); + add_generated_error(&mut result, GeneratedError { validation_error: true }); result }.to_string() @@ -527,7 +587,7 @@ mod tests { } )); - add_generated_error(&mut result); + add_generated_error(&mut result, GeneratedError { validation_error: true }); result }.to_string() @@ -583,7 +643,7 @@ mod tests { } )); - add_generated_error(&mut result); + add_generated_error(&mut result, GeneratedError { validation_error: true }); result }.to_string() @@ -639,7 +699,12 @@ mod tests { } )); - add_generated_error(&mut result); + add_generated_error( + &mut result, + GeneratedError { + validation_error: true, + }, + ); result } diff --git a/derive_builder_core/src/lib.rs b/derive_builder_core/src/lib.rs index 5294b203..4afd0843 100644 --- a/derive_builder_core/src/lib.rs +++ b/derive_builder_core/src/lib.rs @@ -47,7 +47,7 @@ pub use error::UninitializedFieldError; pub(crate) use block::Block; pub(crate) use build_method::BuildMethod; -pub(crate) use builder::Builder; +pub(crate) use builder::{Builder, GeneratedError}; pub(crate) use builder_field::BuilderField; use darling::FromDeriveInput; pub(crate) use deprecation_notes::DeprecationNotes; diff --git a/derive_builder_core/src/macro_options/darling_opts.rs b/derive_builder_core/src/macro_options/darling_opts.rs index 97e40efa..f9b6bc78 100644 --- a/derive_builder_core/src/macro_options/darling_opts.rs +++ b/derive_builder_core/src/macro_options/darling_opts.rs @@ -8,7 +8,9 @@ use proc_macro2::Span; use syn::{self, spanned::Spanned, Attribute, Generics, Ident, Path, Visibility}; use crate::macro_options::DefaultExpression; -use crate::{Builder, BuilderField, BuilderPattern, DeprecationNotes, Initializer, Setter}; +use crate::{ + Builder, BuilderField, BuilderPattern, DeprecationNotes, GeneratedError, Initializer, Setter, +}; /// `derive_builder` uses separate sibling keywords to represent /// mutually-exclusive visibility states. This trait requires implementers to @@ -60,6 +62,22 @@ pub struct BuildFn { error: Option, } +impl BuildFn { + /// Get the error type the build method needs generated. + fn generated_error(&self) -> Option { + // If the caller provided an error type, no generation is needed + if self.error.is_some() { + None + } else { + Some(GeneratedError { + // The generated error only needs to include a variant for validation failures + // if the caller has provided a custom validation function. + validation_error: self.validate.is_some(), + }) + } + } +} + impl Default for BuildFn { fn default() -> Self { BuildFn { @@ -397,7 +415,7 @@ 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(), + generated_error: self.build_fn.generated_error(), must_derive_clone: self.requires_clone(), doc_comment: None, deprecation_notes: Default::default(),