Merge pull request #6 from pettermahlen/mixed-generics

ensure generics use doesn't lead to warnings in generated code

Author: pettermahlen

dataenum-processor/src/main/java/com/spotify/dataenum/processor/generator/spec/SpecTypeFactory.java

@@ -56,7 +56,7 @@ public static TypeSpec create(OutputSpec spec) throws ParserException {
       ValueMethods valueMethods = new ValueMethods(value);
       factoryMethods.add(valueMethods.createFactoryMethod(spec));
       isMethods.add(valueMethods.createIsMethod());
-      asMethods.add(valueMethods.createAsMethod());
+      asMethods.add(valueMethods.createAsMethod(spec));
     }
 
     TypeSpec.Builder enumBuilder =

dataenum-processor/src/main/java/com/spotify/dataenum/processor/generator/value/ValueMethods.java

@@ -23,6 +23,7 @@
 import com.spotify.dataenum.processor.data.OutputValue;
 import com.spotify.dataenum.processor.data.Parameter;
 import com.squareup.javapoet.MethodSpec;
+import com.squareup.javapoet.MethodSpec.Builder;
 import com.squareup.javapoet.ParameterSpec;
 import java.util.ArrayList;
 import java.util.List;
@@ -95,12 +96,19 @@ public MethodSpec createIsMethod() {
         .build();
   }
 
-  public MethodSpec createAsMethod() {
-    return MethodSpec.methodBuilder("as" + value.name())
-        .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
-        .returns(value.parameterizedOutputClass())
-        .addStatement("return ($T) this", value.parameterizedOutputClass())
-        .build();
+  public MethodSpec createAsMethod(OutputSpec spec) {
+    Builder builder =
+        MethodSpec.methodBuilder("as" + value.name())
+            .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
+            .returns(value.parameterizedOutputClass())
+            .addStatement("return ($T) this", value.parameterizedOutputClass());
+
+    if (!ValueTypeFactory.extractMissingTypeVariablesForValue(value, spec).isEmpty()
+        && value.hasTypeVariables()) {
+      builder.addAnnotation(ValueTypeFactory.SUPPRESS_UNCHECKED_WARNINGS);
+    }
+
+    return builder.build();
   }
 
   private static String asCamelCase(String text) {

dataenum-processor/src/main/java/com/spotify/dataenum/processor/generator/value/ValueTypeFactory.java

@@ -26,20 +26,27 @@
 import com.spotify.dataenum.processor.generator.match.MatchMethods;
 import com.spotify.dataenum.processor.parser.ParserException;
 import com.spotify.dataenum.processor.util.Iterables;
+import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.FieldSpec;
 import com.squareup.javapoet.MethodSpec;
+import com.squareup.javapoet.MethodSpec.Builder;
 import com.squareup.javapoet.ParameterizedTypeName;
 import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 import com.squareup.javapoet.TypeVariableName;
+import com.squareup.javapoet.WildcardTypeName;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.lang.model.element.Modifier;
 
 public class ValueTypeFactory {
 
+  static final AnnotationSpec SUPPRESS_UNCHECKED_WARNINGS =
+      AnnotationSpec.builder(SuppressWarnings.class).addMember("value", "$S", "unchecked").build();
+
   private ValueTypeFactory() {}
 
   public static TypeSpec create(
@@ -163,7 +170,8 @@ private static MethodSpec createEquals(OutputValue value) throws ParserException
     result.addStatement("if (other == this) return true");
     result.addStatement("if (!(other instanceof $T)) return false", value.outputClass());
 
-    result.addStatement("$1T o = ($1T) other", value.outputClass());
+    TypeName wildCardTypeName = withWildCardTypeParameters(value);
+    result.addStatement("$1T o = ($1T) other", wildCardTypeName);
     result.addCode("$[return ");
     boolean first = true;
     for (Parameter parameter : value.parameters()) {
@@ -189,6 +197,18 @@ private static MethodSpec createEquals(OutputValue value) throws ParserException
     return result.build();
   }
 
+  private static TypeName withWildCardTypeParameters(OutputValue value) {
+    if (!value.hasTypeVariables()) {
+      return value.outputClass();
+    }
+
+    TypeName[] wildCards = new TypeName[Iterables.sizeOf(value.typeVariables())];
+
+    Arrays.fill(wildCards, WildcardTypeName.subtypeOf(TypeName.OBJECT));
+
+    return ParameterizedTypeName.get(value.outputClass(), wildCards);
+  }
+
   private static MethodSpec createHashCode(OutputValue value) {
     MethodSpec.Builder result =
         MethodSpec.methodBuilder("hashCode")
@@ -252,18 +272,34 @@ private static MethodSpec createToString(OutputValue value) {
   }
 
   private static MethodSpec createAsSpecMethod(OutputValue value, OutputSpec spec) {
+    List<TypeVariableName> missingTypeVariables = extractMissingTypeVariablesForValue(value, spec);
+
+    Builder builder =
+        MethodSpec.methodBuilder("as" + spec.outputClass().simpleName())
+            .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
+            .returns(spec.parameterizedOutputClass())
+            .addTypeVariables(missingTypeVariables)
+            .addStatement("return ($T) this", spec.parameterizedOutputClass());
+
+    // if there are type variables that this sub-type doesn't use, they will lead to 'unchecked
+    // cast'
+    // warnings when compiling the generated code. These warnings are safe to suppress, since this
+    // sub type will never use those type variables.
+    if (!missingTypeVariables.isEmpty()) {
+      builder.addAnnotation(SUPPRESS_UNCHECKED_WARNINGS);
+    }
+
+    return builder.build();
+  }
+
+  static List<TypeVariableName> extractMissingTypeVariablesForValue(
+      OutputValue value, OutputSpec spec) {
     List<TypeVariableName> missingTypeVariables = new ArrayList<>();
     for (TypeVariableName typeVariableName : spec.typeVariables()) {
       if (!Iterables.contains(value.typeVariables(), typeVariableName)) {
         missingTypeVariables.add(typeVariableName);
       }
     }
-
-    return MethodSpec.methodBuilder("as" + spec.outputClass().simpleName())
-        .addModifiers(Modifier.PUBLIC, Modifier.FINAL)
-        .returns(spec.parameterizedOutputClass())
-        .addTypeVariables(missingTypeVariables)
-        .addStatement("return ($T) this", spec.parameterizedOutputClass())
-        .build();
+    return missingTypeVariables;
   }
 }

dataenum-processor/src/main/java/com/spotify/dataenum/processor/util/Iterables.java

@@ -43,4 +43,14 @@ public static <T> T[] toArray(Iterable<T> iterable, Class<T> tClass) {
     }
     return out.toArray((T[]) Array.newInstance(tClass, out.size()));
   }
+
+  public static <T> int sizeOf(Iterable<T> iterable) {
+    int size = 0;
+
+    for (T entry : iterable) {
+      size++;
+    }
+
+    return size;
+  }
 }

dataenum-processor/src/test/java/com/spotify/dataenum/processor/IntegrationTest.java

@@ -76,6 +76,16 @@ public void genericValuesEnum() throws Exception {
     assertThatEnumGeneratedMatchingFile("GenericValues");
   }
 
+  @Test
+  public void genericValuesGeneratedCodeCompiles() throws Exception {
+    Compilation compilation =
+        javac()
+            .withOptions("-Xlint:all")
+            .compile(JavaFileObjects.forResource("GenericValues.java"));
+
+    assertThat(compilation).succeededWithoutWarnings();
+  }
+
   @Test
   public void recursiveGenericValueEnum() throws Exception {
     assertThatEnumGeneratedMatchingFile("RecursiveGenericValue");