Add support for Nullable on Optional property builders

value/src/it/functional/src/test/java/com/google/auto/value/AutoValueTest.java

@@ -1510,6 +1510,54 @@ public void testOmitOptionalWithBuilder() {
     assertThat(suppliedDirectly.optionalString()).hasValue("foo");
     assertThat(suppliedDirectly.optionalInteger()).hasValue(23);
   }
+
+  @AutoValue
+  public abstract static class OptionalPropertyWithNullableBuilder {
+    public abstract String notOptional();
+    public abstract com.google.common.base.Optional<String> optional();
+
+    public static Builder builder() {
+      return new AutoValue_AutoValueTest_OptionalPropertyWithNullableBuilder.Builder();
+    }
+
+    @AutoValue.Builder
+    public interface Builder {
+      Builder notOptional(String s);
+      Builder optional(@Nullable String s);
+      OptionalPropertyWithNullableBuilder build();
+    }
+  }
+
+  @Test
+  public void testOmitOptionalWithNullableBuilder() {
+      OptionalPropertyWithNullableBuilder instance1 = OptionalPropertyWithNullableBuilder.builder()
+        .notOptional("hello")
+        .build();
+    assertThat(instance1.notOptional()).isEqualTo("hello");
+    assertThat(instance1.optional()).isAbsent();
+
+    OptionalPropertyWithNullableBuilder instance2 = OptionalPropertyWithNullableBuilder.builder()
+        .notOptional("hello")
+        .optional(null)
+        .build();
+    assertThat(instance2.notOptional()).isEqualTo("hello");
+    assertThat(instance2.optional()).isAbsent();
+    assertThat(instance1).isEqualTo(instance2);
+
+    OptionalPropertyWithNullableBuilder instance3 = OptionalPropertyWithNullableBuilder.builder()
+        .notOptional("hello")
+        .optional("world")
+        .build();
+    assertThat(instance3.notOptional()).isEqualTo("hello");
+    assertThat(instance3.optional()).hasValue("world");
+
+    try {
+        OptionalPropertyWithNullableBuilder.builder().build();
+      fail("Expected IllegalStateException for unset non-Optional property");
+    } catch (IllegalStateException e) {
+      assertThat(e.getMessage()).contains("notOptional");
+    }
+  }
 
   @AutoValue
   public abstract static class NullableOptionalPropertiesWithBuilder {

value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java

@@ -371,10 +371,9 @@ private void defineVarsForType(
   @Override
   Optional<String> nullableAnnotationForMethod(
       ExecutableElement propertyMethod, ImmutableList<AnnotationMirror> methodAnnotations) {
-    OptionalInt nullableAnnotationIndex = nullableAnnotationIndex(methodAnnotations);
-    if (nullableAnnotationIndex.isPresent()) {
-      ImmutableList<String> annotations = annotationStrings(methodAnnotations);
-      return Optional.of(annotations.get(nullableAnnotationIndex.getAsInt()));
+    Optional<String> nullableAnnotationList = nullableAnnotationIfInList(methodAnnotations);
+    if (nullableAnnotationList.isPresent()) {
+      return nullableAnnotationList;
     } else {
       List<? extends AnnotationMirror> typeAnnotations =
           propertyMethod.getReturnType().getAnnotationMirrors();
@@ -385,6 +384,15 @@ Optional<String> nullableAnnotationForMethod(
     }
   }
 
+  public static Optional<String> nullableAnnotationIfInList(ImmutableList<AnnotationMirror> methodAnnotations) {
+    OptionalInt nullableAnnotationIndex = nullableAnnotationIndex(methodAnnotations);
+    if (nullableAnnotationIndex.isPresent()) {
+      ImmutableList<String> annotations = annotationStrings(methodAnnotations);
+      return Optional.of(annotations.get(nullableAnnotationIndex.getAsInt()));
+    }
+    return Optional.empty();
+  }
+
   private static OptionalInt nullableAnnotationIndex(List<? extends AnnotationMirror> annotations) {
     for (int i = 0; i < annotations.size(); i++) {
       if (isNullable(annotations.get(i))) {

value/src/main/java/com/google/auto/value/processor/BuilderSpec.java

@@ -24,6 +24,7 @@
 import com.google.auto.common.MoreTypes;
 import com.google.auto.value.processor.AutoValueOrOneOfProcessor.Property;
 import com.google.common.collect.ImmutableBiMap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
@@ -34,12 +35,14 @@
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.processing.ProcessingEnvironment;
+import javax.lang.model.element.AnnotationMirror;
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.TypeParameterElement;
+import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.DeclaredType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
@@ -288,22 +291,40 @@ public class PropertySetter {
     private final String parameterTypeString;
     private final boolean primitiveParameter;
     private final String copyOf;
+    private final String nullableAnnotation;
 
     public PropertySetter(ExecutableElement setter, TypeMirror propertyType) {
       this.access = SimpleMethod.access(setter);
       this.name = setter.getSimpleName().toString();
-      TypeMirror parameterType = Iterables.getOnlyElement(setter.getParameters()).asType();
+      VariableElement parameterElement = Iterables.getOnlyElement(setter.getParameters());
+      TypeMirror parameterType = parameterElement.asType();
       primitiveParameter = parameterType.getKind().isPrimitive();
       this.parameterTypeString = parameterTypeString(setter, parameterType);
       Types typeUtils = processingEnv.getTypeUtils();
       TypeMirror erasedPropertyType = typeUtils.erasure(propertyType);
       boolean sameType = typeUtils.isSameType(typeUtils.erasure(parameterType), erasedPropertyType);
       if (sameType) {
         this.copyOf = null;
+        this.nullableAnnotation = "";
       } else {
         String rawTarget = TypeEncoder.encodeRaw(erasedPropertyType);
-        String of = Optionalish.isOptional(propertyType) ? "of" : "copyOf";
+        Optionalish optional = Optionalish.createIfOptional(propertyType);
+        String nullableAnnotation = "";
+        String of;
+        if (optional != null) {
+          ImmutableList<AnnotationMirror> annotationMirrors = ImmutableList.copyOf(parameterElement.getAnnotationMirrors());
+          Optional<String> nullableAnnotationFromParam = AutoValueProcessor.nullableAnnotationIfInList(annotationMirrors);
+          if (nullableAnnotationFromParam.isPresent()) {
+            of = optional.getNullable();
+            nullableAnnotation = nullableAnnotationFromParam.get() + " ";
+          } else {
+            of = "of";
+          }
+        } else {
+          of = "copyOf";
+        }
         this.copyOf = rawTarget + "." + of + "(%s)";
+        this.nullableAnnotation = nullableAnnotation;
       }
     }
 
@@ -337,6 +358,10 @@ public boolean getPrimitiveParameter() {
       return primitiveParameter;
     }
 
+    public String getNullableAnnotation() {
+      return nullableAnnotation;
+    }
+
     public String copy(AutoValueProcessor.Property property) {
       if (copyOf == null) {
         return property.toString();
@@ -345,6 +370,7 @@ public String copy(AutoValueProcessor.Property property) {
       String copy = String.format(copyOf, property);
 
       // Add a null guard only in cases where we are using copyOf and the property is @Nullable.
+      // No guard for the @Nullable annotation case because from/ofNullable doesn't need it
       if (property.isNullable()) {
         copy = String.format("(%s == null ? null : %s)", property, copy);
       }

value/src/main/java/com/google/auto/value/processor/Optionalish.java

@@ -96,6 +96,24 @@ public String getEmpty() {
     return TypeEncoder.encodeRaw(optionalType) + empty;
   }
 
+  /**
+   * Returns a string representing the method name to call to obtain the nullable version of
+   * this Optional. This will be something like {@code "fromNullable"} or {@code "ofNullable"}.
+   *
+   * <p>This method is public so that it can be referenced as {@code p.optional.nullable} from
+   * templates.
+   */
+  public String getNullable() {
+    if (optionalType.getTypeArguments().isEmpty()) {
+      // No typeArguments means a primitive wrapper -- it has no nullable input
+      return "of";
+    }
+    TypeElement typeElement = MoreElements.asType(optionalType.asElement());
+    return typeElement.getQualifiedName().toString().startsWith("java.util.")
+        ? "ofNullable"
+        : "fromNullable";
+  }
+
   TypeMirror getContainedType(Types typeUtils) {
     List<? extends TypeMirror> typeArguments = optionalType.getTypeArguments();
     switch (typeArguments.size()) {

value/src/main/java/com/google/auto/value/processor/autovalue.vm

@@ -240,11 +240,17 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes {
 
     #foreach ($setter in $builderSetters[$p.name])
 
+    #if ($p.nullable)
+      #set ($nullableAnnotation = $p.nullableAnnotation)
+    #else
+      #set ($nullableAnnotation = $setter.nullableAnnotation)
+    #end
+
     @Override
     ${setter.access}${builderTypeName}${builderActualTypes} ##
-        ${setter.name}(${p.nullableAnnotation}$setter.parameterType $p) {
+        ${setter.name}(${nullableAnnotation}$setter.parameterType $p) {
 
-      #if (!$setter.primitiveParameter && !$p.nullable)
+      #if (!${setter.primitiveParameter} && ${nullableAnnotation.isEmpty()})
 
         #if ($identifiers)
 

value/src/test/java/com/google/auto/value/processor/CompilationTest.java

@@ -776,6 +776,7 @@ public void correctBuilder() throws Exception {
         "  public abstract ImmutableList<T> anImmutableList();",
         "  public abstract Optional<String> anOptionalString();",
         "  public abstract NestedAutoValue<T> aNestedAutoValue();",
+        "  public abstract Optional<Object> anOptionalObject();",
         "",
         "  public abstract Builder<T> toBuilder();",
         "",
@@ -790,6 +791,7 @@ public void correctBuilder() throws Exception {
         "    public abstract Builder<T> anOptionalString(Optional<String> s);",
         "    public abstract Builder<T> anOptionalString(String s);",
         "    public abstract NestedAutoValue.Builder<T> aNestedAutoValueBuilder();",
+        "    public abstract Builder<T> anOptionalObject(@Nullable Object s);",
         "",
         "    public Builder<T> aList(ArrayList<T> x) {",
              // ArrayList should not be imported in the generated class.
@@ -851,6 +853,7 @@ public void correctBuilder() throws Exception {
             "  private final ImmutableList<T> anImmutableList;",
             "  private final Optional<String> anOptionalString;",
             "  private final NestedAutoValue<T> aNestedAutoValue;",
+            "  private final Optional<Object> anOptionalObject;",
             "",
             "  private AutoValue_Baz(",
             "      int anInt,",
@@ -859,14 +862,16 @@ public void correctBuilder() throws Exception {
             "      List<T> aList,",
             "      ImmutableList<T> anImmutableList,",
             "      Optional<String> anOptionalString,",
-            "      NestedAutoValue<T> aNestedAutoValue) {",
+            "      NestedAutoValue<T> aNestedAutoValue,",
+            "      Optional<Object> anOptionalObject) {",
             "    this.anInt = anInt;",
             "    this.aByteArray = aByteArray;",
             "    this.aNullableIntArray = aNullableIntArray;",
             "    this.aList = aList;",
             "    this.anImmutableList = anImmutableList;",
             "    this.anOptionalString = anOptionalString;",
             "    this.aNestedAutoValue = aNestedAutoValue;",
+            "    this.anOptionalObject = anOptionalObject;",
             "  }",
             "",
             "  @Override public int anInt() {",
@@ -900,6 +905,10 @@ public void correctBuilder() throws Exception {
             "    return aNestedAutoValue;",
             "  }",
             "",
+            "  @Override public Optional<Object> anOptionalObject() {",
+            "    return anOptionalObject;",
+            "  }",
+            "",
             "  @Override public String toString() {",
             "    return \"Baz{\"",
             "        + \"anInt=\" + anInt + \", \"",
@@ -908,7 +917,8 @@ public void correctBuilder() throws Exception {
             "        + \"aList=\" + aList + \", \"",
             "        + \"anImmutableList=\" + anImmutableList + \", \"",
             "        + \"anOptionalString=\" + anOptionalString + \", \"",
-            "        + \"aNestedAutoValue=\" + aNestedAutoValue",
+            "        + \"aNestedAutoValue=\" + aNestedAutoValue + \", \"",
+            "        + \"anOptionalObject=\" + anOptionalObject",
             "        + \"}\";",
             "  }",
             "",
@@ -928,7 +938,8 @@ public void correctBuilder() throws Exception {
             "          && (this.aList.equals(that.aList()))",
             "          && (this.anImmutableList.equals(that.anImmutableList()))",
             "          && (this.anOptionalString.equals(that.anOptionalString()))",
-            "          && (this.aNestedAutoValue.equals(that.aNestedAutoValue()));",
+            "          && (this.aNestedAutoValue.equals(that.aNestedAutoValue()))",
+            "          && (this.anOptionalObject.equals(that.anOptionalObject()));",
             "    }",
             "    return false;",
             "  }",
@@ -949,6 +960,8 @@ public void correctBuilder() throws Exception {
             "    h$ ^= anOptionalString.hashCode();",
             "    h$ *= 1000003;",
             "    h$ ^= aNestedAutoValue.hashCode();",
+            "    h$ *= 1000003;",
+            "    h$ ^= anOptionalObject.hashCode();",
             "    return h$;",
             "  }",
             "",
@@ -966,6 +979,7 @@ public void correctBuilder() throws Exception {
             "    private Optional<String> anOptionalString = Optional.absent();",
             "    private NestedAutoValue.Builder<T> aNestedAutoValueBuilder$;",
             "    private NestedAutoValue<T> aNestedAutoValue;",
+            "    private Optional<Object> anOptionalObject = Optional.absent();",
             "",
             "    Builder() {",
             "    }",
@@ -978,6 +992,7 @@ public void correctBuilder() throws Exception {
             "      this.anImmutableList = source.anImmutableList();",
             "      this.anOptionalString = source.anOptionalString();",
             "      this.aNestedAutoValue = source.aNestedAutoValue();",
+            "      this.anOptionalObject = source.anOptionalObject();",
             "    }",
             "",
             "    @Override",
@@ -1097,6 +1112,12 @@ public void correctBuilder() throws Exception {
             "    }",
             "",
             "    @Override",
+            "    public Baz.Builder<T> anOptionalObject(@Nullable Object anOptionalObject) {",
+            "      this.anOptionalObject = Optional.fromNullable(anOptionalObject);",
+            "      return this;",
+            "    }",
+            "",
+            "    @Override",
             "    public Baz<T> build() {",
             "      if (anImmutableListBuilder$ != null) {",
             "        this.anImmutableList = anImmutableListBuilder$.build();",
@@ -1130,7 +1151,8 @@ public void correctBuilder() throws Exception {
             "          this.aList,",
             "          this.anImmutableList,",
             "          this.anOptionalString,",
-            "          this.aNestedAutoValue);",
+            "          this.aNestedAutoValue,",
+            "          this.anOptionalObject);",
             "    }",
             "  }",
             "}");