Disable derive clone on owned pattern

derive_builder/CHANGELOG.md

@@ -2,6 +2,12 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [Unreleased]
+
+### Changed
+- `Clone` is no longer derived on a builder using the owned pattern unless it
+  has a field override that uses the mutable/immutable pattern. #97
+
 ## [0.5.1] - 2017-12-16
 
 ### Changed

derive_builder/src/lib.rs

@@ -110,6 +110,8 @@
 //! ## Owned, aka Consuming
 //!
 //! Precede your struct (or field) with `#[builder(pattern = "owned")]` to opt into this pattern.
+//! Builders generated with this pattern do not automatically derive `Clone`, which allows builders
+//! to be generated for structs with fields that do not derive `Clone`.
 //!
 //! * Setters take and return `self`.
 //! * PRO: Setter calls and final build method can be chained.
@@ -584,6 +586,9 @@ fn builder_for_struct(ast: syn::MacroInput) -> quote::Tokens {
     for f in fields {
         let f_opts = field_options_from(f, &field_defaults);
 
+        if f_opts.builder_pattern.requires_clone() {
+            builder.mark_initializer_requires_clone();
+        }
         builder.push_field(f_opts.as_builder_field());
         builder.push_setter_fn(f_opts.as_setter());
         build_fn.push_initializer(f_opts.as_initializer());

derive_builder/src/options/struct_options.rs

@@ -47,6 +47,7 @@ impl StructOptions {
             visibility: &self.builder_visibility,
             fields: Vec::with_capacity(self.struct_size_hint),
             functions: Vec::with_capacity(self.struct_size_hint),
+            initializer_requires_clone: false,
             doc_comment: None,
             deprecation_notes: self.deprecation_notes.clone(),
             bindings: self.bindings,

derive_builder_core/src/builder.rs

@@ -61,6 +61,11 @@ pub struct Builder<'a> {
     pub fields: Vec<Tokens>,
     /// Functions of the builder struct, e.g. `fn bar() -> { unimplemented!() }`
     pub functions: Vec<Tokens>,
+    /// Whether this builder must derive `Clone`.
+    ///
+    /// This is true even for a builder using the `owned` pattern if there is a field whose setter
+    /// uses a different pattern.
+    pub initializer_requires_clone: bool,
     /// Doc-comment of the builder struct.
     pub doc_comment: Option<syn::Attribute>,
     /// Emit deprecation notes to the user.
@@ -76,6 +81,11 @@ impl<'a> ToTokens for Builder<'a> {
             let builder_vis = self.visibility;
             let builder_ident = self.ident;
             let derives = self.derives;
+            let clone_derive = if self.pattern.requires_clone() || self.initializer_requires_clone {
+                Some(syn::Ident::from("Clone"))
+            } else {
+                None
+            };
             let bounded_generics = self.compute_impl_bounds();
             let (impl_generics, _, _) = bounded_generics.split_for_impl();
             let (struct_generics, ty_generics, where_clause) = self.generics
@@ -95,7 +105,7 @@ impl<'a> ToTokens for Builder<'a> {
             );
 
             tokens.append(quote!(
-                #[derive(Default, Clone #( , #derives)* )]
+                #[derive(Default#( , #clone_derive)* #( , #derives)* )]
                 #builder_doc_comment
                 #builder_vis struct #builder_ident #struct_generics #where_clause {
                     #(#builder_fields)*
@@ -138,6 +148,15 @@ impl<'a> Builder<'a> {
         self
     }
 
+    /// Marks that this builder has to derive `Clone`.
+    ///
+    /// If the builder or any of its fields are not using the `Owned` pattern, then it must derive
+    /// `Clone`.
+    pub fn mark_initializer_requires_clone(&mut self) -> &mut Self {
+        self.initializer_requires_clone = true;
+        self
+    }
+
     /// Add `Clone` trait bound to generic types for non-owned builders.
     /// This enables target types to declare generics without requiring a
     /// `Clone` impl. This is the same as how the built-in derives for
@@ -184,6 +203,7 @@ macro_rules! default_builder {
             visibility: &syn::Visibility::Public,
             fields: vec![quote!(foo: u32,)],
             functions: vec![quote!(fn bar() -> { unimplemented!() })],
+            initializer_requires_clone: false,
             doc_comment: None,
             deprecation_notes: DeprecationNotes::default(),
             bindings: Default::default(),
@@ -286,7 +306,7 @@ mod tests {
         assert_eq!(
             quote!(#builder),
             quote!(
-            #[derive(Default, Clone)]
+            #[derive(Default)]
             pub struct FooBuilder<'a, T: Debug> where T: PartialEq {
                 foo: u32,
             }

dev/README.md

@@ -29,7 +29,7 @@ II: Running tests on stable... ✓
 II: Running tests on beta... ✓
 II: Running tests on nightly... ✓
 II: Running dev/compiletests.sh... ✓
-II: Running dev/checkfeatures.sh... ✓
+II: Running dev/checkminimumrust.sh... ✓
 OK: All checks passed!
 ```
 
@@ -39,6 +39,15 @@ OK: All checks passed!
   e.g. `git push --no-verify`.
 * You can also run `dev/githook.sh` manually at any time, without
   registering it as a git hook. But you still have to do the configuration.
+* If the nightly tests fail with any of the following errors, your target directory may contain
+  stale artifacts:
+
+    ```
+    error[E0463]: can't find crate for `derive_builder`
+    error[E0464]: multiple matching crates for `derive_builder`
+    ```
+
+    Running `cargo clean` should remedy the issue.
 
 ## Configuration
 
@@ -53,5 +62,5 @@ git config hooks.checkstable true
 git config hooks.checkbeta true
 git config hooks.checknightly true
 git config hooks.compiletests true
-git config hooks.checkfeatures true
+git config hooks.checkminimumrust true
 ```

dev/checkfeatures.sh → dev/checkminimumrust.sh

@@ -1,10 +1,10 @@
 #!/bin/bash
 
 function main {
-  export CARGO_TARGET_DIR="../target/__checkfeatures"
+  export CARGO_TARGET_DIR="../target/__checkminimumrust"
 
   commands=(
-    "cd derive_builder && rustup run 1.15.0 cargo test --all --color always --features \"skeptic_tests\""
+    "cd derive_builder && rustup run 1.15.0 cargo test --all --color always"
   )
 
   dev/travis-run-all.sh "${commands[@]}"

dev/githook.sh

@@ -19,7 +19,7 @@
 #       git config hooks.checkbeta true
 #       git config hooks.checknightly true
 #       git config hooks.nightlytests true
-#       git config hooks.checkfeatures true
+#       git config hooks.checkminimumrust true
 #
 # Note this will stash all local changes.
 set -u
@@ -36,7 +36,7 @@ function main {
 	[ "$checkbeta" == true ] && run_tests_on "beta"
 	[ "$checknightly" == true ] && run_tests_on "nightly"
 	[ "$nightlytests" == true ] && run_script "dev/nightlytests.sh"
-	[ "$checkfeatures" == true ] && run_script "dev/checkfeatures.sh"
+	[ "$checkminimumrust" == true ] && run_script "dev/checkminimumrust.sh"
 
 	if [ "$errors" != 0 ]; then
 		echo -e "${FMT_ERR}EE${FMT_RESET}: Some checks failed!"
@@ -100,7 +100,8 @@ function load_config {
 	lookup_git_flag checkbeta
 	lookup_git_flag checknightly
 	lookup_git_flag nightlytests
-	lookup_git_flag checkfeatures
+	lookup_git_flag checkminimumrust
+	obsolete_git_flag checkfeatures '`hooks.checkfeatures` has been replaced by `hooks.checkminimumrust`'
 
   if [ $config_status -ne 0 ]; then
 		echo -e "${FMT_ERR}EE${FMT_RESET}: Invalid git configuration. Aborting checks."
@@ -143,6 +144,19 @@ function lookup_git_flag {
 	fi
 }
 
+function obsolete_git_flag {
+	# lookup the boolean value of 'hooks.$1' (i.e. true or false)
+	# if it is set, then we inform the user that the flag is no longer in use
+	local flag="$(git config --bool hooks.$1)"
+	if [ "$flag" == true ] || [ "$flag" == false ]; then
+		[ -n "$2" ] && >&2 echo -e "${FMT_INFO}II${FMT_RESET}: $2"
+		>&2 echo -e "${FMT_INFO}II${FMT_RESET}: obsolete git flag found: hooks.$1"
+		>&2 echo "    you can remove it like"
+		>&2 echo "  $ git config --unset hooks.$1"
+		>&2 echo
+	fi
+}
+
 function echo_begin {
 	# suppress line-feeds, so next echo will be on the same line
 	echo -en "${FMT_INFO}II${FMT_RESET}: "