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

Add try_setter option and generate appropriate methods #72

Merged
merged 17 commits into from
Apr 12, 2017
Merged
Show file tree
Hide file tree
Changes from 11 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 .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ matrix:
- rust: 1.15.0
env: JOB=build CARGO_FEATURES="struct_default"
- rust: nightly
env: JOB=build CARGO_FEATURES="compiletests logging"
env: JOB=build CARGO_FEATURES="nightlytests logging"
- rust: nightly
env: JOB=style_check
allow_failures:
Expand Down
10 changes: 10 additions & 0 deletions derive_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## [0.4.3] - 2017-04-13

### Added
- try_setters, e.g. `#[builder(try_setter)]`. These setters are exposed alongside the
normal field setters and allow callers to pass in values which have
fallible conversions to the needed type through `TryInto`. This attribute
can only be used on nightly when until `#![feature(try_from)]` is declared
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when until -> when

in the consuming crate's root; this will change when Rust issue
[#33417](https://github.com/rust-lang/rust/issues/33417) is resolved.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice summary! 👍


## [0.4.2] - 2017-04-10

### Fixed
Expand Down
54 changes: 54 additions & 0 deletions derive_builder/examples/try_setter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//! This example illustrates the use of try-setters.
//! Tests are suppressed so that this doesn't break the build on stable.
#![cfg(not(test))]
#![feature(try_from)]

#[macro_use]
extern crate derive_builder;

use std::convert::TryFrom;
use std::net::{IpAddr, AddrParseError};
use std::str::FromStr;
use std::string::ToString;

/// Temporary newtype hack around lack of TryFrom implementations
/// in std. The rust-lang issue on the subject says that there will be a
/// blanket impl for everything that currently implements FromStr, which
/// will make this feature much more useful for input validation.
#[derive(Debug, Clone, PartialEq)]
pub struct MyAddr(IpAddr);

impl From<IpAddr> for MyAddr {
fn from(v: IpAddr) -> Self {
MyAddr(v)
}
}

impl<'a> TryFrom<&'a str> for MyAddr {
type Err = AddrParseError;

fn try_from(v: &str) -> Result<Self, AddrParseError> {
Ok(MyAddr(IpAddr::from_str(v)?))
}
}

#[derive(Builder, Debug, PartialEq)]
#[builder(try_setter, setter(into))]
struct Lorem {
pub name: String,
pub addr: MyAddr,
}

fn main() {
create("Jane", "1.2.3.4").unwrap();
create("Bobby", "").unwrap_err();
}

fn create(name: &str, addr: &str) -> Result<Lorem, String> {
// Fallible and infallible setters can be mixed freely when using
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colin-kiegel can you please try running cargo expand --example try_setter on this and watch what happens to the comment? Something weird is happening here, but I'm not sure what.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me guess, does the comment appear at random places in the generated output? :-)

I noticed very strange distributions of comments before. I could imagine that this is caused by the syn crate, which we use to parse the input into an ast. But didn't investigate it, because it didn't cause any trouble.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to consistently get moved into the build method just before the uninitialized field error string for "name", even when I move it around in the raw source.

// the mutable builder pattern.
LoremBuilder::default()
.name(name)
.try_addr(addr).map_err(|e| e.to_string())?
.build()
}
49 changes: 49 additions & 0 deletions derive_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,52 @@
//! }
//! ```
//!
//! ## Fallible Setters
//!
//! Alongside the normal setter methods, you can expose fallible setters which are generic over
//! the `TryInto` trait. TryInto is a not-yet-stable trait
//! (see rust-lang issue [#33417](https://github.com/rust-lang/rust/issues/33417)) similar to
//! `Into` with the key distinction that the conversion can fail, and therefore produces a `Result`.
//!
//! You can only declare the `try_setter` attribute today if you're targeting nightly, and you have
//! to add `#![feature(try_from)]` to your crate to use it.
//!
//!
//! ```rust,ignore
//! #![feature(try_from)]
//! # #[macro_use]
//! # extern crate derive_builder;
//! #
//! #[derive(Builder, Debug, PartialEq)]
//! #[builder(try_setter, setter(into))]
//! struct Lorem {
//! pub name: String,
//! pub ipsum: u8,
//! }
//!
//! #[derive(Builder, Debug, PartialEq)]
//! struct Ipsum {
//! #[builder(try_setter, setter(into, name = "foo"))]
//! pub dolor: u8,
//! }
//!
//! fn main() {
//! LoremBuilder::default()
//! .try_ipsum(1u16).unwrap()
//! .name("hello")
//! .build()
//! .expect("1 fits into a u8");
//!
//! IpsumBuilder::default()
//! .try_foo(1u16)
//! .unwrap()
//! .build()
//! .expect("1 fits into a u8");
//! }
//! ```
//!
//! A complete example of using fallible setters is available in `examples/try_setter.rs`.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a note that the name and visibility are inherited from the setter settings.

You could change the example into something like this, to better explain it?

//! # #[macro_use]
//! # extern crate derive_builder;
//! #
//! #[derive(Builder, Debug, PartialEq)]
//! struct Lorem {
//!     #[builder(try_setter, setter(name="foo"))]
//!     pub ipsum: u8,
//! }

//!
//! ## Default Values
//!
//! You can define default values for each field via annotation by `#[builder(default="...")]`,
Expand Down Expand Up @@ -366,6 +412,9 @@
//! - If derive_builder depends on your crate, and vice versa, then a cyclic
//! dependency would occur. To break it you could try to depend on the
//! [`derive_builder_core`] crate instead.
//! - The `try_setter` attribute and `owned` builder pattern are not compatible in practice;
//! an error during building will consume the builder, making it impossible to continue
//! construction.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch :-)

//!
//! ## Debugging Info
//!
Expand Down
2 changes: 2 additions & 0 deletions derive_builder/src/options/field_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl OptionsBuilder<FieldMode> {
setter_vis: f!(setter_vis),
default_expression: f!(default_expression),
setter_into: f!(setter_into),
try_setter: f!(try_setter),
no_std: f!(no_std),
mode: mode,
}
Expand Down Expand Up @@ -133,6 +134,7 @@ impl From<OptionsBuilder<FieldMode>> for FieldOptions {
field_ident: field_ident,
field_type: field_type,
setter_into: b.setter_into.unwrap_or(false),
try_setter: b.try_setter.unwrap_or(false),
deprecation_notes: b.mode.deprecation_notes.clone(),
default_expression: b.default_expression.clone(),
use_default_struct: b.mode.use_default_struct,
Expand Down
3 changes: 3 additions & 0 deletions derive_builder/src/options/field_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub struct FieldOptions {
pub attrs: Vec<syn::Attribute>,
/// Bindings to libstd or libcore.
pub bindings: Bindings,
/// Enables code generation for the TryInto setter.
pub try_setter: bool,
}

impl DefaultExpression {
Expand Down Expand Up @@ -58,6 +60,7 @@ impl FieldOptions {
pub fn as_setter<'a>(&'a self) -> Setter<'a> {
Setter {
enabled: self.setter_enabled,
try_setter: self.try_setter,
visibility: &self.setter_visibility,
pattern: self.builder_pattern,
attrs: &self.attrs,
Expand Down
11 changes: 11 additions & 0 deletions derive_builder/src/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub struct OptionsBuilder<Mode> {
setter_vis: Option<syn::Visibility>,
default_expression: Option<DefaultExpression>,
setter_into: Option<bool>,
try_setter: Option<bool>,
no_std: Option<bool>,
mode: Mode,
}
Expand All @@ -66,6 +67,7 @@ impl<Mode> From<Mode> for OptionsBuilder<Mode> {
setter_prefix: None,
setter_name: None,
setter_vis: None,
try_setter: None,
default_expression: None,
setter_into: None,
no_std: None,
Expand Down Expand Up @@ -100,6 +102,12 @@ impl<Mode> OptionsBuilder<Mode> where
desc: "setter type conversion",
map: |x: bool| { x },
}

impl_setter!{
ident: try_setter,
desc: "try_setter activation",
map: |x: bool| { x },
}

impl_setter!{
ident: default_expression,
Expand Down Expand Up @@ -198,6 +206,9 @@ impl<Mode> OptionsBuilder<Mode> where
// setter implicitly enabled
self.setter_enabled(true)
},
"try_setter" => {
self.try_setter(true)
}
"default" => {
if !cfg!(feature = "struct_default") && self.mode.struct_mode() {
let where_info = self.where_diagnostics();
Expand Down
1 change: 1 addition & 0 deletions derive_builder/src/options/struct_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ impl From<OptionsBuilder<StructMode>> for (StructOptions, OptionsBuilder<FieldMo
setter_prefix: b.setter_prefix,
setter_vis: b.setter_vis,
setter_into: b.setter_into,
try_setter: b.try_setter,
default_expression: field_default_expression,
no_std: b.no_std,
mode: {
Expand Down
9 changes: 9 additions & 0 deletions derive_builder_core/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ impl Bindings {
":: std :: convert :: Into"
})
}

/// TryInto trait.
pub fn try_into_trait(&self) -> RawTokens<&'static str> {
RawTokens(if self.no_std {
":: core :: convert :: TryInto"
} else {
":: std :: convert :: TryInto"
})
}
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions derive_builder_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
//! [`derive_builder_core`]: https://!crates.io/crates/derive_builder_core

#![deny(warnings, missing_docs)]
#![cfg_attr(test, recursion_limit = "100")]

extern crate proc_macro;
extern crate syn;
Expand Down
58 changes: 57 additions & 1 deletion derive_builder_core/src/setter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ use Bindings;
pub struct Setter<'a> {
/// Enables code generation for this setter fn.
pub enabled: bool,
/// Enables code generation for the `try_` variant of this setter fn.
pub try_setter: bool,
/// Visibility of the setter, e.g. `syn::Visibility::Public`.
pub visibility: &'a syn::Visibility,
/// How the setter method takes and returns `self` (e.g. mutably).
Expand Down Expand Up @@ -117,6 +119,26 @@ impl<'a> ToTokens for Setter<'a> {
new.#field_ident = #option::Some(#into_value);
new
}));

if self.try_setter {
let try_into = self.bindings.try_into_trait();
let try_ty_params = quote!(<VALUE: #try_into<#ty>>);
let try_ident = syn::Ident::new(format!("try_{}", ident));
let result = self.bindings.result_ty();

tokens.append(quote!(
#(#attrs)*
#vis fn #try_ident #try_ty_params (#self_param, value: VALUE)
-> #result<#return_ty, VALUE::Err>
{
let converted : #ty = value.try_into()?;
let mut new = #self_into_return_ty;
new.#field_ident = #option::Some(converted);
Ok(new)
}));
} else {
trace!("Skipping try_setter for `{}`.", self.field_ident);
}
} else {
trace!("Skipping setter for `{}`.", self.field_ident);
}
Expand All @@ -131,6 +153,7 @@ macro_rules! default_setter {
() => {
Setter {
enabled: true,
try_setter: false,
visibility: &syn::Visibility::Public,
pattern: BuilderPattern::Mutable,
attrs: &vec![],
Expand Down Expand Up @@ -221,7 +244,7 @@ mod tests {
));
}

// including
// including try_setter
#[test]
fn full() {
let attrs = vec![syn::parse_outer_attr("#[some_attr]").unwrap()];
Expand All @@ -233,6 +256,7 @@ mod tests {
setter.attrs = attrs.as_slice();
setter.generic_into = true;
setter.deprecation_notes = &deprecated;
setter.try_setter = true;

assert_eq!(quote!(#setter), quote!(
#[some_attr]
Expand All @@ -242,6 +266,15 @@ mod tests {
new.foo = ::std::option::Option::Some(value.into());
new
}

#[some_attr]
pub fn try_foo<VALUE: ::std::convert::TryInto<Foo>>(&mut self, value: VALUE)
-> ::std::result::Result<&mut Self, VALUE::Err> {
let converted : Foo = value.try_into()?;
let mut new = self;
new.foo = ::std::option::Option::Some(converted);
Ok(new)
}
));
}

Expand Down Expand Up @@ -282,4 +315,27 @@ mod tests {

assert_eq!(quote!(#setter), quote!());
}

#[test]
fn try_setter() {
let mut setter : Setter = default_setter!();
setter.pattern = BuilderPattern::Mutable;
setter.try_setter = true;

assert_eq!(quote!(#setter), quote!(
pub fn foo(&mut self, value: Foo) -> &mut Self {
let mut new = self;
new.foo = ::std::option::Option::Some(value);
new
}

pub fn try_foo<VALUE: ::std::convert::TryInto<Foo>>(&mut self, value: VALUE)
-> ::std::result::Result<&mut Self, VALUE::Err> {
let converted : Foo = value.try_into()?;
let mut new = self;
new.foo = ::std::option::Option::Some(converted);
Ok(new)
}
));
}
}
2 changes: 1 addition & 1 deletion derive_builder_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ publish = false
path = "src/lib.rs"

[features]
compiletests = ["compiletest_rs"]
nightlytests = ["compiletest_rs"]
logging = [ "derive_builder/logging" ]
struct_default = [ "derive_builder/struct_default" ]

Expand Down
2 changes: 1 addition & 1 deletion derive_builder_test/tests/compiletests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(feature = "compiletests")]
#![cfg(feature = "nightlytests")]
extern crate compiletest_rs as compiletest;

// note:
Expand Down
23 changes: 23 additions & 0 deletions derive_builder_test/tests/try_setter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![cfg_attr(feature = "nightlytests", feature(try_from))]

use std::net::IpAddr;

#[macro_use]
extern crate derive_builder;

#[derive(Debug, Clone, Builder)]
#[builder(try_setter)]
pub struct Lorem {
source: IpAddr,
dest: IpAddr,
name: String,
}

#[derive(Default, Debug, Clone, Builder)]
#[builder(default, setter(prefix = "set", into), try_setter)]
pub struct Ipsum {
source: Option<IpAddr>,
name: String,
}

fn main() { }