Generate an unwrapped setter for Optional properties.

Readme.md

@@ -125,6 +125,7 @@ public final class Person {
     private final boolean isAdult;
 }
 ```
+
 ##### @Builder on constructors
 
 You can also place the annotation on a constructor;
@@ -204,9 +205,9 @@ public class DateFactoryTest {
     @Test
     public void use_date_builder() {
         Date date = DateBuilder.date()
-                .month(12)
-                .year(23)
-                .build();
+            .month(12)
+            .year(23)
+            .build();
 
         Assert.assertEquals(11, date.getMonth());
         Assert.assertEquals(1, date.getDay());
@@ -289,9 +290,9 @@ public final class User {
         this.firstName = firstName;
         this.lastName = lastName;
         this.displayName = displayName == null
-           ? firstName + " " + lastName
-           : displayName;
-   }
+            ? firstName + " " + lastName
+            : displayName;
+    }
 }
 ```
 
@@ -311,11 +312,11 @@ User user = UserBuilder.user()
 The Staged Builder style has one downside:
 when evolving your API, you cannot change a required property to be optional
 (with the small exception of the last required property)
-without breaking existing code that uses the Builder generated for the required property,
+without breaking existing code that uses the Builder generated when the property was required -
 even though, purely from an API perspective, that should not be a breaking change for the clients of your classes.
 
 For example, if we take the above `User` class, but with `username` being required,
-the client code using the Builder will look as follows:
+the client code using that Builder looks something like this:
 
 ```java
 User user = UserBuilder.user()
@@ -326,7 +327,7 @@ User user = UserBuilder.user()
     .build();
 ```
 
-However, if we change `username` to be optional by annotating it with `@Opt`,
+However, if we then change `username` to be optional by annotating it with `@Opt`,
 the above code will stop compiling,
 because the `username()` call can no longer happen after the call to `email()`,
 but must instead be moved to after the call to `lastName()`.
@@ -357,9 +358,9 @@ public final class User {
         this.firstName = firstName;
         this.lastName = lastName;
         this.displayName = displayName == null
-           ? firstName + " " + lastName
-           : displayName;
-   }
+            ? firstName + " " + lastName
+            : displayName;
+    }
 }
 ```
 
@@ -378,10 +379,11 @@ User user = UserBuilder.user()
 Note that this style works best if either the class being built has a small number of properties,
 or if there is a natural order to those properties, like in the `User` example above.
 The reason why is that there is only a single spot where a given optional property can be set
-(for example, `username()` above can only be called right after calling `email()`).
-That is different from the `STAGED` style,
+(for example, `username()` above can only be called right after calling `email()`),
+which might make it difficult to find if the class has a large amount of properties without an obvious order to them.
+This is different from the `STAGED` style,
 where all optional properties can be set right before calling `build()`,
-and they can be set in any order, which makes them easier to find.
+and they can be set in any order, which makes them more easily discoverable.
 
 ##### Other @Builder attributes
 

src/main/java/org/jilt/internal/AbstractBuilderGenerator.java

@@ -10,6 +10,7 @@
 import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 import com.squareup.javapoet.TypeVariableName;
+import com.squareup.javapoet.WildcardTypeName;
 import org.jilt.Builder;
 import org.jilt.Opt;
 import org.jilt.utils.Utils;
@@ -25,7 +26,6 @@
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.util.Elements;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -139,23 +139,53 @@ private List<MethodSpec> generateBuilderSetterMethods(VariableElement attribute)
                 /* abstractMethod */ false);
     }
 
-    protected List<MethodSpec> generateSetterMethods(VariableElement attribute, boolean mangleTypeParameters,
+    protected final List<MethodSpec> generateSetterMethods(VariableElement attribute, boolean mangleTypeParameters,
             boolean abstractMethod) {
+        List<MethodSpec> ret = new ArrayList<MethodSpec>(2);
+
+        String fieldName = this.attributeSimpleName(attribute);
         TypeName parameterType = this.attributeType(attribute, mangleTypeParameters);
-        MethodSpec.Builder method = MethodSpec
+        MethodSpec.Builder setter = MethodSpec
                 .methodBuilder(this.setterMethodName(attribute))
                 .addModifiers(Modifier.PUBLIC)
                 .returns(this.returnTypeForSetterFor(attribute, mangleTypeParameters))
                 .addParameter(this.setterParameterSpec(attribute, parameterType));
         if (abstractMethod) {
-            method.addModifiers(Modifier.ABSTRACT);
+            setter.addModifiers(Modifier.ABSTRACT);
         } else {
-            String fieldName = this.attributeSimpleName(attribute);
-            method.addStatement("this.$1L = $1L", fieldName)
+            setter.addStatement("this.$1L = $1L", fieldName)
                     .addStatement("return this");
         }
+        ret.add(setter.build());
+
+        if (this.typeIsJavaUtilOptional(parameterType)) {
+            TypeName optionalValueType;
+            if (parameterType instanceof ParameterizedTypeName) {
+                ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) parameterType;
+                optionalValueType = parameterizedTypeName.typeArguments.get(0);
+                if (optionalValueType instanceof WildcardTypeName) {
+                    // for wildcards, we use java.lang.Object too
+                    optionalValueType = ClassName.OBJECT;
+                }
+            } else {
+                // if Optional is used as a raw type, use java.lang.Object as the type argument
+                optionalValueType = ClassName.OBJECT;
+            }
+            MethodSpec.Builder optionSetter = MethodSpec
+                    .methodBuilder(this.setterMethodName(attribute))
+                    .addModifiers(Modifier.PUBLIC)
+                    .returns(this.returnTypeForSetterFor(attribute, mangleTypeParameters))
+                    .addParameter(this.setterParameterSpec(attribute, optionalValueType));
+            if (abstractMethod) {
+                optionSetter.addModifiers(Modifier.ABSTRACT);
+            } else {
+                optionSetter.addStatement("this.$1L = $2T.ofNullable($1L)", fieldName, ClassName.get("java.util", "Optional"))
+                        .addStatement("return this");
+            }
+            ret.add(optionSetter.build());
+        }
 
-        return Collections.singletonList(method.build());
+        return ret;
     }
 
     protected abstract TypeName returnTypeForSetterFor(VariableElement attribute, boolean withMangledTypeParameters);
@@ -176,20 +206,20 @@ protected final TypeName mangleTypeName(TypeName ret) {
         if (ret instanceof ParameterizedTypeName) {
             ParameterizedTypeName parameterizedTypeName = (ParameterizedTypeName) ret;
             return ParameterizedTypeName.get(parameterizedTypeName.rawType,
-                    this.mangleTypeNameParameters(parameterizedTypeName.typeArguments).toArray(new TypeName[]{}));
+                    this.mangleTypeParameters(parameterizedTypeName.typeArguments).toArray(new TypeName[]{}));
         }
         return ret;
     }
 
-    private List<TypeName> mangleTypeNameParameters(List<TypeName> typeArguments) {
-        List<TypeName> ret = new ArrayList<TypeName>(typeArguments.size());
-        for (TypeName typeName : typeArguments) {
-            if (typeName instanceof TypeVariableName) {
+    private List<TypeName> mangleTypeParameters(List<TypeName> typeParameters) {
+        List<TypeName> ret = new ArrayList<TypeName>(typeParameters.size());
+        for (TypeName typeParameter : typeParameters) {
+            if (typeParameter instanceof TypeVariableName) {
                 // if this is a type variable, we need to mangle it
-                TypeVariableName typeVariableName = (TypeVariableName) typeName;
+                TypeVariableName typeVariableName = (TypeVariableName) typeParameter;
                 ret.add(this.mangleTypeParameter(typeVariableName));
             } else {
-                ret.add(typeName);
+                ret.add(typeParameter);
             }
         }
         return ret;
@@ -205,7 +235,7 @@ private ParameterSpec setterParameterSpec(VariableElement attribute, TypeName pa
         ParameterSpec.Builder ret = ParameterSpec.builder(parameterType,
                 this.attributeSimpleName(attribute));
         AnnotationMirror nullableAnnotation = this.firstAnnotationCalledNullable(attribute);
-        if (nullableAnnotation!= null) {
+        if (nullableAnnotation != null) {
             ret.addAnnotation(AnnotationSpec.get(nullableAnnotation));
         }
         return ret.build();

src/test/java/org/jilt/test/OptionalsValueTest.java

@@ -1,7 +1,11 @@
 package org.jilt.test;
 
+import org.jilt.test.data.optionals.OptionalsRawValue;
+import org.jilt.test.data.optionals.OptionalsRawValueBuilder;
 import org.jilt.test.data.optionals.OptionalsValue;
 import org.jilt.test.data.optionals.OptionalsValueBuilder;
+import org.jilt.test.data.optionals.OptionalsWildcardValue;
+import org.jilt.test.data.optionals.OptionalsWildcardValueBuilder;
 import org.junit.Test;
 
 import java.util.Optional;
@@ -10,13 +14,45 @@
 
 public class OptionalsValueTest {
     @Test
-    public void optional_property_is_always_optional() {
-        OptionalsValue<String, Integer> value = OptionalsValueBuilder.<String, Integer>optionalsValue()
-                .t2(33)
+    public void optional_type_property_is_implicitly_optional() {
+        OptionalsValue<String> value = OptionalsValueBuilder.<String>optionalsValue()
+                .v(null)
                 .optional(Optional.of("abc"))
                 .build();
 
         assertThat(value.optional).contains("abc");
-        assertThat(value.t2).isEqualTo(33);
+        assertThat(value.v).isNull();
+    }
+
+    @Test
+    public void optional_property_has_unwrapped_setter() {
+        OptionalsValue<Integer> value = OptionalsValueBuilder.<Integer>optionalsValue()
+                .v(null)
+                .optional(33)
+                .build();
+
+        assertThat(value.optional).contains(33);
+        assertThat(value.v).isNull();
+    }
+
+    @Test
+    public void raw_optional_uses_java_lang_object_in_unwrapped_setter() {
+        Object someObject = new Object();
+        OptionalsRawValue value = OptionalsRawValueBuilder.optionalsRawValue()
+                .rawOptional(someObject)
+                .build();
+
+        //noinspection unchecked
+        assertThat(value.rawOptional).contains(someObject);
+    }
+
+    @Test
+    public void wildcard_optional_uses_java_lang_object_in_unwrapped_setter() {
+        Object someObject = new Object();
+        OptionalsWildcardValue value = OptionalsWildcardValueBuilder.optionalsWildcardValue()
+                .wildcardOptional(someObject)
+                .build();
+
+        assertThat(value.wildcardOptional).isEqualTo(Optional.of(someObject));
     }
 }

src/test/java/org/jilt/test/data/optionals/OptionalsRawValue.java

@@ -0,0 +1,16 @@
+package org.jilt.test.data.optionals;
+
+import org.jilt.Builder;
+import org.jilt.BuilderStyle;
+
+import java.util.Optional;
+
+public final class OptionalsRawValue {
+    @SuppressWarnings("rawtypes")
+    public final Optional rawOptional;
+
+    @Builder(style = BuilderStyle.STAGED)
+    public OptionalsRawValue(@SuppressWarnings("rawtypes") Optional rawOptional) {
+        this.rawOptional = rawOptional;
+    }
+}

src/test/java/org/jilt/test/data/optionals/OptionalsValue.java

@@ -6,12 +6,12 @@
 import java.util.Optional;
 
 @Builder(style = BuilderStyle.STAGED)
-public final class OptionalsValue<T1, T2> {
-    public final Optional<T1> optional;
-    public final T2 t2;
+public final class OptionalsValue<T> {
+    public final Optional<T> optional;
+    public final Void v;
 
-    public OptionalsValue(Optional<T1> optional, T2 t2) {
+    public OptionalsValue(Optional<T> optional, Void v) {
         this.optional = optional;
-        this.t2 = t2;
+        this.v = v;
     }
 }

src/test/java/org/jilt/test/data/optionals/OptionalsWildcardValue.java

@@ -0,0 +1,15 @@
+package org.jilt.test.data.optionals;
+
+import org.jilt.Builder;
+import org.jilt.BuilderStyle;
+
+import java.util.Optional;
+
+public final class OptionalsWildcardValue {
+    public final Optional<?> wildcardOptional;
+
+    @Builder(style = BuilderStyle.STAGED)
+    public OptionalsWildcardValue(Optional<?> wildcardOptional) {
+        this.wildcardOptional = wildcardOptional;
+    }
+}