Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Structural error #169

Merged
merged 5 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion derive_builder/build/skeptic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use std::io::{Write, Read};
const DOC_TPL_DIR: &'static str = "src/doc_tpl/";
const DOC_TPL_OUT_DIR: &'static str = "doc_tpl/";

fn generate_doc_tpl_tests() -> Result<Vec<String>, Box<Error>> {
fn generate_doc_tpl_tests() -> Result<Vec<String>, Box<dyn Error>> {
trace!("Generating doc template tests");
let root_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR")?);
let mut tpl_dir = root_dir;
Expand Down
2 changes: 1 addition & 1 deletion derive_builder/examples/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ fn main() {
let x = LoremBuilder::default().ipsum(120).build().unwrap_err();

// .. the build will fail:
assert_eq!(&x, "You'll tire yourself out");
assert_eq!(&x.to_string(), "You'll tire yourself out");
}
9 changes: 5 additions & 4 deletions derive_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
//!
//! You can easily opt into different patterns and control many other aspects.
//!
//! The build method returns `Result<T, String>`, where `T` is the struct you started with.
//! The build method returns `Result<T, E>`, where `T` is the struct you started with
//! and E is a generated builder error type.
//! It returns `Err` if you didn't initialize all fields and no default values were
//! provided.
//!
Expand All @@ -74,7 +75,7 @@
//! ```rust
//! # #[macro_use] extern crate derive_builder;
//! # #[derive(Builder)] struct Lorem { ipsum: u32 }
//! # fn try_main() -> Result<(), String> {
//! # fn try_main() -> Result<(), Box<dyn std::error::Error>> {
//! let x: Lorem = LoremBuilder::default().ipsum(42).build()?;
//! # Ok(())
//! # } fn main() { try_main().unwrap(); }
Expand All @@ -87,7 +88,7 @@
//! ```rust
//! # #[macro_use] extern crate derive_builder;
//! # #[derive(Builder)] struct Lorem { ipsum: u32 }
//! # fn try_main() -> Result<(), String> {
//! # fn try_main() -> Result<(), Box<dyn std::error::Error>> {
//! # let geek = true;
//! let mut builder = LoremBuilder::default();
//! if geek {
Expand Down Expand Up @@ -439,7 +440,7 @@
//! let x = LoremBuilder::default().ipsum(120).build().unwrap_err();
//!
//! // .. the build will fail:
//! assert_eq!(&x, "You'll tire yourself out");
//! assert_eq!(&x.to_string(), "You'll tire yourself out");
//! }
//! ```
//!
Expand Down
9 changes: 9 additions & 0 deletions derive_builder/src/options/darling_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,14 @@ impl Options {
.expect("Struct name with Builder suffix should be an ident")
}

pub fn builder_error_ident(&self) -> Ident {
if let Some(ref custom) = self.name {
format_ident!("{}Error", custom)
} else {
format_ident!("{}BuilderError", self.ident)
}
}

/// The visibility of the builder struct.
/// If a visibility was declared in attributes, that will be used;
/// otherwise the struct's own visibility will be used.
Expand Down Expand Up @@ -393,6 +401,7 @@ impl Options {
pattern: self.pattern,
target_ty: &self.ident,
target_ty_generics: Some(ty_generics),
error_ty: self.builder_error_ident(),
initializers: Vec::with_capacity(self.field_count()),
doc_comment: None,
bindings: self.bindings(),
Expand Down
6 changes: 3 additions & 3 deletions derive_builder/tests/builder_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ struct Lorem {
}

#[test]
#[should_panic(expected = "`ipsum` must be initialized")]
fn panic_if_uninitialized() {
MyBuilder::default().build().unwrap();
fn error_if_uninitialized() {
let error = MyBuilder::default().build().unwrap_err();
assert_eq!(&error.to_string(), "`ipsum` must be initialized");
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions derive_builder/tests/custom_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ mod field_level {
}

#[test]
#[should_panic(expected = "`required` must be initialized")]
fn panic_if_uninitialized() {
LoremBuilder::default().build().unwrap();
fn error_if_uninitialized() {
let error = LoremBuilder::default().build().unwrap_err();
assert_eq!(&error.to_string(), "`required` must be initialized");
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions derive_builder/tests/generic_structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ where
}

#[test]
#[should_panic(expected = "`ipsum` must be initialized")]
fn panic_if_uninitialized() {
GenericBuilder::<String>::default().build().unwrap();
fn error_if_uninitialized() {
let error = GenericBuilder::<String>::default().build().unwrap_err();
assert_eq!(&error.to_string(), "`ipsum` must be initialized");
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions derive_builder/tests/lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ struct Lorem<'a> {
}

#[test]
#[should_panic(expected = "`ipsum` must be initialized")]
fn panic_if_uninitialized() {
LoremBuilder::default().build().unwrap();
fn error_if_uninitialized() {
let error = LoremBuilder::default().build().unwrap_err();
assert_eq!(&error.to_string(), "`ipsum` must be initialized");
}

#[test]
Expand Down
26 changes: 20 additions & 6 deletions derive_builder/tests/run-pass/custom_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,26 @@
#[macro_use]
extern crate derive_builder;

type Clone = ();
type Into = ();
type Option = ();
type Result = ();
type Some = ();
type String = ();
struct Unit;

type Clone = Unit;
type Into = Unit;
type Option = Unit;
type Result = Unit;
type Some = Unit;
type String = Unit;

impl core::fmt::Debug for Unit {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "()")
}
}

impl core::fmt::Display for Unit {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
write!(f, "()")
}
}

#[derive(Builder)]
struct IgnoreEmptyStruct {}
Expand Down
7 changes: 3 additions & 4 deletions derive_builder/tests/setter_visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,11 @@ pub mod foo {
}

#[test]
#[should_panic(expected = "`private` must be initialized")]
fn public_setters_foreign_module() {
let y = foo::IpsumBuilder::default()
let error = foo::IpsumBuilder::default()
.public("Hello")
.build()
.unwrap();
.unwrap_err();

assert_eq!(y.public, "Hello".to_string());
assert_eq!(&error.to_string(), "`private` must be initialized");
}
4 changes: 2 additions & 2 deletions derive_builder/tests/try_setter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ struct Ipsum {
pub source: MyAddr,
}

fn exact_helper() -> Result<Lorem, String> {
fn exact_helper() -> Result<Lorem, LoremBuilderError> {
LoremBuilder::default()
.source(IpAddr::from_str("1.2.3.4").unwrap())
.dest(IpAddr::from_str("0.0.0.0").unwrap())
.build()
}

fn try_helper() -> Result<Lorem, String> {
fn try_helper() -> Result<Lorem, LoremBuilderError> {
LoremBuilder::default()
.try_source("1.2.3.4")
.map_err(|e| e.to_string())?
Expand Down
9 changes: 7 additions & 2 deletions derive_builder/tests/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,19 @@ impl LoremBuilder {
#[test]
fn out_of_bounds() {
assert_eq!(
&LoremBuilder::default().my_effort(120).build().unwrap_err(),
&LoremBuilder::default()
.my_effort(120)
.build()
.unwrap_err()
.to_string(),
"Don't wear yourself out"
);
assert_eq!(
&LoremBuilder::default()
.rivals_effort(120)
.build()
.unwrap_err(),
.unwrap_err()
.to_string(),
"Your rival is cheating"
);
}
Expand Down
19 changes: 11 additions & 8 deletions derive_builder_core/src/build_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use DEFAULT_STRUCT_NAME;
/// # let build_method = default_build_method!();
/// #
/// # assert_eq!(quote!(#build_method).to_string(), quote!(
/// pub fn build(&self) -> ::std::result::Result<Foo, ::std::string::String> {
/// pub fn build(&self) -> ::std::result::Result<Foo, FooBuilderError> {
/// Ok(Foo {
/// foo: self.foo,
/// })
Expand All @@ -51,6 +51,8 @@ pub struct BuildMethod<'a> {
pub target_ty: &'a syn::Ident,
/// 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,
/// Field initializers for the target type.
pub initializers: Vec<TokenStream>,
/// Doc-comment of the builder struct.
Expand Down Expand Up @@ -84,14 +86,14 @@ impl<'a> ToTokens for BuildMethod<'a> {
});
let validate_fn = self.validate_fn.as_ref().map(|vfn| quote!(#vfn(&self)?;));
let result = self.bindings.result_ty();
let string = self.bindings.string_ty();
let error_ty = &self.error_ty;

if self.enabled {
trace!("Deriving build method `{}`.", self.ident);
tokens.append_all(quote!(
#doc_comment
#vis fn #ident(#self_param)
-> #result<#target_ty #target_ty_generics, #string>
-> #result<#target_ty #target_ty_generics, #error_ty>
{
#validate_fn
#default_struct
Expand Down Expand Up @@ -137,6 +139,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()),
initializers: vec![quote!(foo: self.foo,)],
doc_comment: None,
bindings: Default::default(),
Expand All @@ -159,7 +162,7 @@ mod tests {
assert_eq!(
quote!(#build_method).to_string(),
quote!(
pub fn build(&self) -> ::std::result::Result<Foo, ::std::string::String> {
pub fn build(&self) -> ::std::result::Result<Foo, FooBuilderError> {
Ok(Foo {
foo: self.foo,
})
Expand All @@ -178,7 +181,7 @@ mod tests {
assert_eq!(
quote!(#build_method).to_string(),
quote!(
pub fn build(&self) -> ::core::result::Result<Foo, ::alloc::string::String> {
pub fn build(&self) -> ::core::result::Result<Foo, FooBuilderError> {
Ok(Foo {
foo: self.foo,
})
Expand All @@ -197,7 +200,7 @@ mod tests {
assert_eq!(
quote!(#build_method).to_string(),
quote!(
pub fn build(&self) -> ::std::result::Result<Foo, ::std::string::String> {
pub fn build(&self) -> ::std::result::Result<Foo, FooBuilderError> {
let __default: Foo = { Default::default() };
Ok(Foo {
foo: self.foo,
Expand Down Expand Up @@ -227,7 +230,7 @@ mod tests {
assert_eq!(
quote!(#build_method).to_string(),
quote!(
pub fn finish(&self) -> ::std::result::Result<Foo, ::std::string::String> {
pub fn finish(&self) -> ::std::result::Result<Foo, FooBuilderError> {
Ok(Foo {
foo: self.foo,
})
Expand All @@ -249,7 +252,7 @@ mod tests {
assert_eq!(
quote!(#build_method).to_string(),
quote!(
pub fn build(&self) -> ::std::result::Result<Foo, ::std::string::String> {
pub fn build(&self) -> ::std::result::Result<Foo, FooBuilderError> {
IpsumBuilder::validate(&self)?;

Ok(Foo {
Expand Down
Loading