Things I noticed while reviewing

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

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]
@@ -37,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;

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,17 +163,18 @@
 //! }
 //! ```
 //!
-//! ## 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:
 //!
 //! * `/// ...`
 //! * `#[doc = ...]`
 //! * `#[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).
@@ -269,6 +270,7 @@
 //! [`derive_builder_core`]: https://crates.io/crates/derive_builder_core
 
 #![crate_type = "proc-macro"]
+#![deny(warnings)]
 
 extern crate proc_macro;
 extern crate syn;
@@ -285,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)]
@@ -311,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();

src/options/field_mode.rs

@@ -14,10 +14,10 @@ impl OptionsBuilder<FieldMode> {
     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<FieldMode> {
             .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<FieldMode> {
         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::<FieldMode> {
-            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,
         }
     }

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<FieldMode>`) from the ast.
+/// Get the tuple of `StructOptions` and field defaults (`OptionsBuilder<FieldMode>`) from the AST.
 pub fn struct_options_from(ast: &syn::MacroInput) -> (StructOptions, OptionsBuilder<FieldMode>) {
     OptionsBuilder::<StructMode>::parse(ast).into()
 }
@@ -99,8 +99,7 @@ impl<Mode> OptionsBuilder<Mode> 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