Skip to content

Commit

Permalink
Safely support custom errors (#194)
Browse files Browse the repository at this point in the history
* Add UninitializedFieldError

Structured errors need a safe way to act on uninitialized fields
that doesn't conflate with validation errors. This commit adds that by
providing a struct to represent an uninitialized field and generating
conversions from that into the auto-generated error type.

* Attach validation-related errors to better span

If the build() method cannot convert the validation error to the
generated builder error, this is a problem with the validation function.
The error span should point there, not at the macro call-site.

* Allow specification of preexisting error

Using `#[builder(build_fn(error = "..."))]` allows passing an error type 
to use instead of generating one. This makes it possible to integrate with
a crate's existing error.

* Add run-pass custom error test

This test checks that we don't require unnecessary impls
for custom errors when uninitialized fields are impossible.

Fixes #181 
Fixes #191
  • Loading branch information
TedDriggs committed Jan 19, 2021
1 parent ee13c51 commit c72cd24
Show file tree
Hide file tree
Showing 15 changed files with 463 additions and 235 deletions.
54 changes: 54 additions & 0 deletions derive_builder/examples/custom_error.rs
@@ -0,0 +1,54 @@
//! This example shows using custom validation with a non-string error type.
//!
//! This relies on how the generated build function is constructed; the validator
//! is invoked in conjunction with the `?` operator, so anything that converts to
//! the generated `FooBuilderError` type is valid.

#[macro_use]
extern crate derive_builder;

use derive_builder::UninitializedFieldError;
use std::fmt;

fn validate_age(builder: &ExampleBuilder) -> Result<(), Error> {
match builder.age {
Some(age) if age > 150 => Err(Error::UnrealisticAge(age)),
_ => Ok(()),
}
}

#[derive(Debug, Builder)]
#[builder(setter(into), build_fn(validate = "validate_age", error = "Error"))]
struct Example {
name: String,
age: usize,
}

enum Error {
UninitializedField(&'static str),
UnrealisticAge(usize),
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::UnrealisticAge(age) => write!(f, "Nobody is {} years old", age),
Self::UninitializedField(field) => write!(f, "Required field '{}' not set", field),
}
}
}

impl From<UninitializedFieldError> for Error {
fn from(e: UninitializedFieldError) -> Self {
Self::UninitializedField(e.field_name())
}
}

fn main() {
let person_err = ExampleBuilder::default()
.name("Jane Doe")
.age(200usize)
.build()
.unwrap_err();
println!("{}", person_err);
}
55 changes: 55 additions & 0 deletions derive_builder/examples/custom_error_generic.rs
@@ -0,0 +1,55 @@
//! This example shows combining generics with custom errors and validation.
//!
//! Note the use of the type parameter in the `#[builder(...)]` attribute.

#[macro_use]
extern crate derive_builder;

use derive_builder::UninitializedFieldError;

trait Popular {
fn is_popular(&self) -> bool;
}

impl<'a> Popular for &'a str {
fn is_popular(&self) -> bool {
!self.starts_with('b')
}
}

#[derive(Debug, Builder)]
#[builder(build_fn(validate = "check_person", error = "Error<N>"))]
struct Person<N: Popular + Clone> {
name: N,
age: u16,
}

#[derive(Debug)]
enum Error<N> {
UninitializedField(&'static str),
UnpopularName(N),
}

impl<N> From<UninitializedFieldError> for Error<N> {
fn from(error: UninitializedFieldError) -> Self {
Self::UninitializedField(error.field_name())
}
}

fn check_person<N: Popular + Clone>(builder: &PersonBuilder<N>) -> Result<(), Error<N>> {
if let Some(name) = &builder.name {
if !name.is_popular() {
return Err(Error::UnpopularName(name.clone()));
}
}

Ok(())
}

fn main() {
dbg!(PersonBuilder::default()
.name("bill")
.age(71)
.build()
.unwrap_err());
}
4 changes: 4 additions & 0 deletions derive_builder/src/lib.rs
Expand Up @@ -541,10 +541,14 @@
#![deny(warnings)]
#![cfg_attr(not(feature = "std"), no_std)]

extern crate derive_builder_core;
extern crate derive_builder_macro;

pub use derive_builder_macro::Builder;

#[doc(inline)]
pub use derive_builder_core::UninitializedFieldError;

#[doc(hidden)]
pub mod export {
pub mod core {
Expand Down
@@ -0,0 +1,44 @@
#[macro_use]
extern crate derive_builder;

use derive_builder::UninitializedFieldError;

trait Popular {
fn is_popular(&self) -> bool;
}

impl<'a> Popular for &'a str {
fn is_popular(&self) -> bool {
!self.starts_with('b')
}
}

#[derive(Debug, Builder)]
#[builder(build_fn(validate = "check_person", error = "Error<N>"))]
struct Person<N> {
name: N,
age: u16,
}

enum Error<N> {
UninitializedField(&'static str),
UnpopularName(N),
}

impl<N> From<UninitializedFieldError> for Error<N> {
fn from(error: UninitializedFieldError) -> Self {
Self::UninitializedField(error.field_name())
}
}

fn check_person<N: Popular + Clone>(builder: &PersonBuilder<N>) -> Result<(), Error<N>> {
if let Some(name) = &builder.name {
if !name.is_popular() {
return Err(Error::UnpopularName(name.clone()));
}
}

Ok(())
}

fn main() {}
@@ -0,0 +1,10 @@
error[E0277]: the trait bound `N: Popular` is not satisfied
--> $DIR/custom_error_generic_missing_bound.rs:17:31
|
17 | #[builder(build_fn(validate = "check_person", error = "Error<N>"))]
| ^^^^^^^^^^^^^^ the trait `Popular` is not implemented for `N`
18 | struct Person<N> {
| - consider adding a `where N: Popular` bound
...
34 | fn check_person<N: Popular + Clone>(builder: &PersonBuilder<N>) -> Result<(), Error<N>> {
| ------------ ------- required by this bound in `check_person`
37 changes: 37 additions & 0 deletions derive_builder/tests/compile-fail/custom_error_no_from.rs
@@ -0,0 +1,37 @@
#[macro_use]
extern crate derive_builder;

fn validate_age(age: usize) -> Result<(), Error> {
if age > 200 {
Err(Error::UnrealisticAge(age))
} else {
Ok(())
}
}

fn check_person(builder: &PersonBuilder) -> Result<(), Error> {
if let Some(age) = builder.age {
validate_age(age)
} else {
Ok(())
}
}

#[derive(Builder)]
#[builder(build_fn(validate = "check_person", error = "Error"))]
struct Person {
name: String,
age: usize,
}

// NOTE: This enum has a variant for the uninitialized field case (called MissingData)
// but has forgotten `impl From<derive_builder::UninitializedFieldError>`, which is a
// compile-blocking mistake.
#[derive(Debug)]
enum Error {
/// A required field is not filled out.
MissingData(&'static str),
UnrealisticAge(usize),
}

fn main() {}
8 changes: 8 additions & 0 deletions derive_builder/tests/compile-fail/custom_error_no_from.stderr
@@ -0,0 +1,8 @@
error[E0277]: the trait bound `Error: std::convert::From<derive_builder::UninitializedFieldError>` is not satisfied
--> $DIR/custom_error_no_from.rs:21:55
|
21 | #[builder(build_fn(validate = "check_person", error = "Error"))]
| ^^^^^^^ the trait `std::convert::From<derive_builder::UninitializedFieldError>` is not implemented for `Error`
|
= note: required because of the requirements on the impl of `std::convert::Into<Error>` for `derive_builder::UninitializedFieldError`
= note: required by `std::convert::Into::into`
3 changes: 2 additions & 1 deletion derive_builder/tests/custom_default.rs
Expand Up @@ -4,6 +4,7 @@ extern crate pretty_assertions;
extern crate derive_builder;

mod field_level {
use derive_builder::UninitializedFieldError;
#[derive(Debug, PartialEq, Default, Builder, Clone)]
struct Lorem {
required: String,
Expand All @@ -16,7 +17,7 @@ mod field_level {
#[builder(default = r#"format!("{}-{}-{}-{}",
Clone::clone(self.required
.as_ref()
.ok_or("required must be initialized")?),
.ok_or_else(|| UninitializedFieldError::new("required"))?),
match self.explicit_default { Some(ref x) => x, None => "EMPTY" },
self.escaped_default.as_ref().map(|x| x.as_ref()).unwrap_or("EMPTY"),
if let Some(ref x) = self.raw_default { x } else { "EMPTY" })"#)]
Expand Down
49 changes: 49 additions & 0 deletions derive_builder/tests/run-pass/custom_error_default.rs
@@ -0,0 +1,49 @@
//! This test ensures custom errors don't need a conversion from `UninitializedFieldError`
//! if uninitialized fields are impossible.

#[macro_use]
extern crate derive_builder;

#[derive(Default, Builder)]
#[builder(default, build_fn(validate = "check_person", error = "Error"))]
struct Person {
name: String,
age: u16,
}

/// An error that deliberately doesn't have `impl From<UninitializedFieldError>`; as long
/// as `PersonBuilder` uses `Person::default` then missing field errors are never possible.
enum Error {
UnpopularName(String),
UnrealisticAge(u16),
}

fn check_age_realistic(age: u16) -> Result<(), Error> {
if age > 150 {
Err(Error::UnrealisticAge(age))
} else {
Ok(())
}
}

fn check_name_popular(name: &str) -> Result<(), Error> {
if name.starts_with('B') {
Err(Error::UnpopularName(name.to_string()))
} else {
Ok(())
}
}

fn check_person(builder: &PersonBuilder) -> Result<(), Error> {
if let Some(age) = &builder.age {
check_age_realistic(*age)?;
}

if let Some(name) = &builder.name {
check_name_popular(name)?;
}

Ok(())
}

fn main() {}
15 changes: 12 additions & 3 deletions derive_builder_core/src/build_method.rs
Expand Up @@ -2,6 +2,7 @@ use doc_comment_from;
use proc_macro2::{Span, TokenStream};
use quote::{ToTokens, TokenStreamExt};
use syn;
use syn::spanned::Spanned;
use Block;
use BuilderPattern;
use Initializer;
Expand Down Expand Up @@ -51,7 +52,7 @@ pub struct BuildMethod<'a> {
/// Type parameters and lifetimes attached to this builder struct.
pub target_ty_generics: Option<syn::TypeGenerics<'a>>,
/// Type of error.
pub error_ty: syn::Ident,
pub error_ty: syn::Path,
/// Field initializers for the target type.
pub initializers: Vec<TokenStream>,
/// Doc-comment of the builder struct.
Expand Down Expand Up @@ -81,7 +82,10 @@ impl<'a> ToTokens for BuildMethod<'a> {
let ident = syn::Ident::new(DEFAULT_STRUCT_NAME, Span::call_site());
quote!(let #ident: #target_ty #target_ty_generics = #default_expr;)
});
let validate_fn = self.validate_fn.as_ref().map(|vfn| quote!(#vfn(&self)?;));
let validate_fn = self
.validate_fn
.as_ref()
.map(|vfn| quote_spanned!(vfn.span() => #vfn(&self)?;));
let error_ty = &self.error_ty;

if self.enabled {
Expand Down Expand Up @@ -119,6 +123,11 @@ impl<'a> BuildMethod<'a> {
}
}

// pub struct BuildMethodError {
// is_generated: bool,
// ident: syn::Ident,
// }

/// Helper macro for unit tests. This is _only_ public in order to be accessible
/// from doc-tests too.
#[doc(hidden)]
Expand All @@ -132,7 +141,7 @@ macro_rules! default_build_method {
pattern: BuilderPattern::Mutable,
target_ty: &syn::Ident::new("Foo", ::proc_macro2::Span::call_site()),
target_ty_generics: None,
error_ty: syn::Ident::new("FooBuilderError", ::proc_macro2::Span::call_site()),
error_ty: syn::parse_quote!(FooBuilderError),
initializers: vec![quote!(foo: self.foo,)],
doc_comment: None,
default_struct: None,
Expand Down

0 comments on commit c72cd24

Please sign in to comment.