From 7224c56780b81d72cafc70f56dc2a1fbf0ed8e0a Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 12 Mar 2017 18:25:48 +0100 Subject: [PATCH 1/3] Deny warnings and missing docs --- derive_builder_core/src/builder.rs | 3 +++ derive_builder_core/src/lib.rs | 2 ++ src/lib.rs | 1 + 3 files changed, 6 insertions(+) diff --git a/derive_builder_core/src/builder.rs b/derive_builder_core/src/builder.rs index 8b57aaa1..6100fc2e 100644 --- a/derive_builder_core/src/builder.rs +++ b/derive_builder_core/src/builder.rs @@ -102,16 +102,19 @@ impl<'a> Builder<'a> { self } + /// Add a field to the builder pub fn push_field(&mut self, f: BuilderField) -> &mut Self { self.fields.push(quote!(#f)); self } + /// Add a setter function to the builder pub fn push_setter_fn(&mut self, f: Setter) -> &mut Self { self.functions.push(quote!(#f)); self } + /// Add final build function to the builder pub fn push_build_fn(&mut self, f: BuildMethod) -> &mut Self { self.functions.push(quote!(#f)); self diff --git a/derive_builder_core/src/lib.rs b/derive_builder_core/src/lib.rs index 3bc8486e..5eedbf5d 100644 --- a/derive_builder_core/src/lib.rs +++ b/derive_builder_core/src/lib.rs @@ -21,6 +21,8 @@ //! [`derive_builder`]: https://!crates.io/crates/derive_builder //! [`derive_builder_core`]: https://!crates.io/crates/derive_builder_core +#![deny(warnings, missing_docs)] + extern crate proc_macro; extern crate syn; #[macro_use] diff --git a/src/lib.rs b/src/lib.rs index 08c5c4a6..19b070dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -269,6 +269,7 @@ //! [`derive_builder_core`]: https://crates.io/crates/derive_builder_core #![crate_type = "proc-macro"] +#![deny(warnings)] extern crate proc_macro; extern crate syn; From b2de75bb01b9313407fa82488ed32d36573282b1 Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 12 Mar 2017 18:26:09 +0100 Subject: [PATCH 2/3] Doc typos and smaller cleanups --- src/lib.rs | 23 ++++++++++++----------- src/options/mod.rs | 5 ++--- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 19b070dc..22301996 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,7 +57,7 @@ //! ``` //! //! By default all generated setter-methods take and return `&mut self` -//! (aka _non-conusuming_ builder pattern). Accordingly the build method also takes a +//! (aka _non-conusuming_ builder pattern). Accordingly, the build method also takes a //! reference by default. //! //! You can easily opt into different patterns and control many other aspects. @@ -96,14 +96,14 @@ //! # } fn main() { try_main().unwrap(); } //! ``` //! -//! Now it comes in handy that our setter methods takes and returns a mutable reference. Otherwise +//! Now it comes in handy that our setter methods take and return mutable references. Otherwise //! we would need to write something more clumsy like `builder = builder.ipsum("42")` to reassign //! the return value each time we have to call a setter conditionally. //! //! Setters with mutable references are therefore a convenient default for the builder //! pattern in Rust. //! -//! But this is a free world and the choice is still yours. +//! But this is a free world and the choice is still yours! //! //! ## Owned, aka Consuming //! @@ -163,9 +163,9 @@ //! } //! ``` //! -//! ## Doc-Comments and Attributes +//! ## Documentation Comments and Attributes //! -//! `#[derive(Builder)]` copies doc-comments and attributes `#[...]` from your fields +//! `#[derive(Builder)]` copies doc comments and attributes (`#[...]`) from your fields //! to the according builder fields and setter-methods, if it is one of the following: //! //! * `/// ...` @@ -173,7 +173,8 @@ //! * `#[cfg(...)]` //! * `#[allow(...)]` //! -//! The whitelisting minimizes interference with other custom attributes like Serde/Diesel etc. +//! The whitelisting minimizes interference with other custom attributes like +//! those used by Serde, Diesel, or others. //! //! ```rust //! # #[macro_use] @@ -217,7 +218,7 @@ //! # fn main() {} //! ``` //! -//! Alternatively you can use the more verbose form: +//! Alternatively, you can use the more verbose form: //! //! - `#[builder(setter(skip="true"))]` //! - `#[builder(setter(skip="false"))]` @@ -256,11 +257,11 @@ //! //! If you experience any problems during compilation, you can enable additional debug output //! by setting the environment variable `RUST_LOG=derive_builder=trace` before you call `cargo` -//! or `rustc`. Example: `RUST_LOG=derive_builder=trace cargo test`. +//! or `rustc`. Example: `env RUST_LOG=derive_builder=trace cargo test`. //! //! ## Report Issues and Ideas //! -//! https://github.com/colin-kiegel/rust-derive-builder/issues +//! [Open an issue on GitHub](https://github.com/colin-kiegel/rust-derive-builder/issues) //! //! If possible please try to provide the debugging info if you experience unexpected //! compilation errors (see above). @@ -286,7 +287,7 @@ use proc_macro::TokenStream; use std::sync::atomic::{AtomicBool, Ordering, ATOMIC_BOOL_INIT}; use options::{struct_options_from, field_options_from}; -// beware: static muts are not threadsafe. :-) +// Beware: static muts are not threadsafe. :-) static mut LOGGER_INITIALIZED: AtomicBool = ATOMIC_BOOL_INIT; // false #[doc(hidden)] @@ -312,7 +313,7 @@ fn builder_for_struct(ast: syn::MacroInput) -> quote::Tokens { let fields = match ast.body { syn::Body::Struct(syn::VariantData::Struct(fields)) => fields, - _ => panic!("#[derive(Builder)] can only be used with braced structs"), + _ => panic!("`#[derive(Builder)]` can only be used with braced structs"), }; let mut builder = opts.to_builder(); diff --git a/src/options/mod.rs b/src/options/mod.rs index 68d2ca12..18fdf10c 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -11,7 +11,7 @@ pub use self::field_options::FieldOptions; pub use self::struct_mode::StructMode; pub use self::struct_options::StructOptions; -/// Get the tuple of `StructOptions` and field defaults (`OptionsBuilder`) from the ast. +/// Get the tuple of `StructOptions` and field defaults (`OptionsBuilder`) from the AST. pub fn struct_options_from(ast: &syn::MacroInput) -> (StructOptions, OptionsBuilder) { OptionsBuilder::::parse(ast).into() } @@ -99,8 +99,7 @@ impl OptionsBuilder where match attr.value { // i.e. `#[builder(...)]` - syn::MetaItem::List(ref _ident, ref nested_attrs) - => { + syn::MetaItem::List(ref _ident, ref nested_attrs) => { self.setter_enabled(true); self.parse_builder_options(nested_attrs); return From 4e522a405261d199a38ca3d7f8510615741c0b66 Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 12 Mar 2017 18:26:20 +0100 Subject: [PATCH 3/3] Simplify code a bit --- derive_builder_core/src/lib.rs | 16 +++++++------- src/options/field_mode.rs | 39 ++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/derive_builder_core/src/lib.rs b/derive_builder_core/src/lib.rs index 5eedbf5d..2602c3e2 100644 --- a/derive_builder_core/src/lib.rs +++ b/derive_builder_core/src/lib.rs @@ -39,11 +39,11 @@ mod initializer; mod setter; mod options; -pub use self::build_method::BuildMethod; -pub use self::builder_field::BuilderField; -pub use self::builder::Builder; -pub use self::deprecation_notes::DeprecationNotes; -pub use self::initializer::Initializer; -pub use self::setter::Setter; -pub use self::doc_comment::doc_comment_from; -pub use self::options::BuilderPattern; +pub use build_method::BuildMethod; +pub use builder_field::BuilderField; +pub use builder::Builder; +pub use deprecation_notes::DeprecationNotes; +pub use initializer::Initializer; +pub use setter::Setter; +pub use doc_comment::doc_comment_from; +pub use options::BuilderPattern; diff --git a/src/options/field_mode.rs b/src/options/field_mode.rs index 11b90492..d1a7e67d 100644 --- a/src/options/field_mode.rs +++ b/src/options/field_mode.rs @@ -14,10 +14,10 @@ impl OptionsBuilder { pub fn parse(f: syn::Field) -> Self { let mut builder = Self::default(); - { - let ident = f.ident.as_ref().expect(&format!("Missing identifier for field of type `{:?}`.", f.ty)); - trace!("Parsing field `{}`.", ident); - } + trace!("Parsing field `{}`.", { + f.ident.as_ref() + .expect(&format!("Missing identifier for field of type `{:?}`.", f.ty)) + }); builder.parse_attributes(&f.attrs); builder.mode.field_ident = f.ident; @@ -28,10 +28,7 @@ impl OptionsBuilder { .iter() .filter(|a| { let keep = filter_attr(a); - match keep { - true => trace!("Keeping attribute `{:?}`", a), - false => trace!("Ignoring attribute `{:?}`", a) - } + trace!("{} attribute `{:?}`", if keep { "Keeping" } else { "Ignoring" }, a); keep }) .map(|x| x.clone()) @@ -45,18 +42,28 @@ impl OptionsBuilder { let mut deprecation_notes = self.mode.deprecation_notes; deprecation_notes.extend(&defaults.mode.deprecation_notes); + macro_rules! f { + ($field:ident) => { + self.$field.or_else(|| defaults.$field.clone()) + }; + (mode $field:ident) => { + self.mode.$field.or_else(|| defaults.mode.$field.clone()) + }; + } + let mode = FieldMode { - field_ident: self.mode.field_ident.or_else(|| defaults.mode.field_ident.clone()), - field_type: self.mode.field_type.or_else(|| defaults.mode.field_type.clone()), - setter_attrs: self.mode.setter_attrs.or_else(|| defaults.mode.setter_attrs.clone()), + field_ident: f!(mode field_ident), + field_type: f!(mode field_type), + setter_attrs: f!(mode setter_attrs), deprecation_notes: deprecation_notes, }; + OptionsBuilder:: { - setter_enabled: self.setter_enabled.or_else(|| defaults.setter_enabled), - builder_pattern: self.builder_pattern.or_else(|| defaults.builder_pattern), - setter_name: self.setter_name.or_else(|| defaults.setter_name.clone()), - setter_prefix: self.setter_prefix.or_else(|| defaults.setter_prefix.clone()), - setter_vis: self.setter_vis.or_else(|| defaults.setter_vis.clone()), + setter_enabled: f!(setter_enabled), + builder_pattern: f!(builder_pattern), + setter_name: f!(setter_name), + setter_prefix: f!(setter_prefix), + setter_vis: f!(setter_vis), mode: mode, } }