Add Nullable Setters to AutoValueTest, fix some bugs

Author: octylFractal

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

@@ -1410,6 +1410,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/BuilderSpec.java

@@ -385,7 +385,8 @@ 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.
-      if (property.isNullable() || nullableAnnotation != null) {
+      // 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/autovalue.vm

@@ -254,7 +254,7 @@ $a
     ${setter.access}${builderTypeName}${builderActualTypes} ##
         ${setter.name}(${nullableAnnotation}$setter.parameterType $p) {
 
-      #if (!$setter.primitiveParameter && !$nullableAnnotation)
+      #if (!${setter.primitiveParameter} && ${nullableAnnotation.isEmpty()})
 
       if ($p == null) {
         throw new NullPointerException("Null $p.name");