#872 alternative/principled way to handle nullable from supertype

value-fixture/src/org/immutables/fixture/nullable/FromSupertypeNullable.java

@@ -0,0 +1,26 @@
+package org.immutables.fixture.nullable;
+
+import javax.annotation.Nullable;
+import org.immutables.value.Value.Immutable;
+
+interface FromSupertypeNullable {
+
+  interface SuperA {
+    @Nullable
+    String a();
+  }
+
+  interface SuperB {
+    @Nullable
+    String b();
+  }
+
+  @Immutable
+  interface Subtype extends SuperA, SuperB {
+    @Override
+    String a();
+
+    @Override
+    String b();
+  }
+}

value-fixture/test/org/immutables/fixture/nullable/NullableAttributesTest.java

@@ -17,6 +17,7 @@
 
 import java.util.Arrays;
 import java.util.Collections;
+import javax.annotation.Nullable;
 import org.junit.Test;
 import static org.immutables.check.Checkers.check;
 
@@ -115,8 +116,9 @@ public void nonnullDefault() {
         .arr()
         .addAx("a")
         .addAx("b", "c")
-        .build().ax())
-        .isOf("a", "b", "c");
+        .build()
+        .ax())
+            .isOf("a", "b", "c");
   }
 
   @Test
@@ -257,4 +259,41 @@ public void typeUseNullable() {
     check(r.i1()).isNull();
     check(r.l2()).isNull();
   }
+
+  @Test
+  public void nullableFromSupertypes() {
+    ImmutableSubtype s = ImmutableSubtype.builder()
+        // default a, b
+        .from(new FromSupertypeNullable.Subtype() {
+          @Override
+          public String a() {
+            return "a";
+          }
+
+          @Override
+          public String b() {
+            return "b";
+          }
+        })
+        // would not trip on NPE from supertype which declares it nullable
+        .from(new FromSupertypeNullable.SuperA() {
+          @Override
+          @Nullable
+          public String a() {
+            return null;
+          }
+        })
+        // would not trip on NPE from supertype which declares it nullable
+        .from(new FromSupertypeNullable.SuperB() {
+          @Override
+          @Nullable
+          public String b() {
+            return null;
+          }
+        })
+        .build();
+
+    check(s.a()).is("a");
+    check(s.b()).is("b");
+  }
 }

value-processor/src/org/immutables/value/processor/Immutables.generator

@@ -1317,7 +1317,13 @@ public interface BuildFinal[type.generics] {
       [for v in s.attributes, BitPosition pos = bs.positions v.name]
       [if pos]
       if ((bits[emptyIfZero pos.index] & [literal.hex pos.mask]) == 0) {
+        [if v.nullableInSupertypes]
+        if (instance.[v.names.get]() != null) {
+          [buildFromAttribute v]
+        }
+        [else]
         [buildFromAttribute v]
+        [/if]
         bits[emptyIfZero pos.index] |= [literal.hex pos.mask];
       }
       [else]

value-processor/src/org/immutables/value/processor/meta/FromSupertypesModel.java

@@ -140,6 +140,11 @@ private boolean isEligibleFromType(TypeElement typeElement, ValueAttribute attr)
       Type.Parser parser = new Type.Parser(tf, tf.parameters());
 
       if (parser.parse(ownType).equals(parser.parse(inheritedType))) {
+        if (!attr.isPrimitive()
+            && !attr.isNullable()
+            && attr.isNullableAccessor(accessor)) {
+          attr.nullableInSupertypes = true;
+        }
         return true;
       }
     } catch (Exception typeParseException) {

value-processor/src/org/immutables/value/processor/meta/ValueAttribute.java

@@ -19,12 +19,14 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import java.lang.annotation.ElementType;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -1519,6 +1521,18 @@ private boolean hasNakedWildcardArguments() {
     return false;
   }
 
+  boolean isNullableAccessor(Element element) {
+    for (AnnotationMirror annotation : element.getAnnotationMirrors()) {
+      TypeElement annotationElement = (TypeElement) annotation.getAnnotationType().asElement();
+      Name simpleName = annotationElement.getSimpleName();
+      Name qualifiedName = annotationElement.getQualifiedName();
+      if (isNullableAnnotation(simpleName, qualifiedName)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   private void initSpecialAnnotations() {
     Environment environment = containingType.constitution.protoclass().environment();
     ImmutableList.Builder<AnnotationInjection> annotationInjections = null;
@@ -1537,9 +1551,7 @@ private void initSpecialAnnotations() {
       TypeElement annotationElement = (TypeElement) annotation.getAnnotationType().asElement();
       Name simpleName = annotationElement.getSimpleName();
       Name qualifiedName = annotationElement.getQualifiedName();
-      if (qualifiedName.contentEquals(Annotations.JAVAX_CHECK_FOR_NULL)
-          || qualifiedName.contentEquals(Annotations.JAVAX_NULLABLE)
-          || simpleName.contentEquals(containingType.names().nullableAnnotation)) {
+      if (isNullableAnnotation(simpleName, qualifiedName)) {
         nullability = ImmutableNullabilityAnnotationInfo.of(annotationElement);
       } else if (simpleName.contentEquals(TypeStringProvider.EPHEMERAL_ANNOTATION_ALLOW_NULLS)) {
         nullElements = NullElements.ALLOW;
@@ -1567,6 +1579,12 @@ && protoclass().styles().style().validationMethod() == ValidationMethod.NONE) {
     }
   }
 
+  private boolean isNullableAnnotation(Name simpleName, Name qualifiedName) {
+    return qualifiedName.contentEquals(Annotations.JAVAX_CHECK_FOR_NULL)
+        || qualifiedName.contentEquals(Annotations.JAVAX_NULLABLE)
+        || simpleName.contentEquals(containingType.names().nullableAnnotation);
+  }
+
   public boolean isNullableCollector() {
     return typeKind.isCollectionOrMapping()
         && (isNullable() || containingType.isDeferCollectionAllocation());
@@ -1659,6 +1677,7 @@ public boolean requiresTrackIsSet() {
   }
 
   public boolean isGenerateImmutableCopyOf;
+  public boolean nullableInSupertypes;
 
   public Collection<TypeElement> getEnumElements() {
     if (isEnumType()) {