Merge pull request #1101 from ddcruver/bugfix/fix-raw-type-builders

joelittlejohn · web-flow · commit e5850b9c0523 · 2020-02-25T14:27:40.000Z
Fix raw type warnings when extending a builder
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/AdditionalPropertiesRule.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/AdditionalPropertiesRule.java
@@ -23,7 +23,7 @@
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.stream.StreamSupport;
-import org.apache.commons.collections15.CollectionUtils;
+
 import org.jsonschema2pojo.Schema;
 
 import com.fasterxml.jackson.databind.JsonNode;
@@ -213,7 +213,7 @@ private JMethod addInnerBuilder(JDefinedClass jclass, JType propertyType, JField
     }
 
     private String getBuilderClassName(JDefinedClass c) {
-        return ruleFactory.getNameHelper().getBuilderClassName(c);
+        return ruleFactory.getNameHelper().getBaseBuilderClassName(c);
     }
 
 }
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/BuilderRule.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/BuilderRule.java
@@ -48,24 +48,33 @@ public class BuilderRule implements Rule<JDefinedClass, JDefinedClass> {
   public JDefinedClass apply(String nodeName, JsonNode node, JsonNode parent, JDefinedClass instanceClass, Schema currentSchema) {
 
     // Create the inner class for the builder
+    JDefinedClass concreteBuilderClass;
     JDefinedClass builderClass;
 
     try {
-      String builderName = ruleFactory.getNameHelper().getBuilderClassName(instanceClass);
-      builderClass = instanceClass._class(JMod.PUBLIC + JMod.STATIC, builderName);
+
+      String concreteBuilderClassName = ruleFactory.getNameHelper().getBuilderClassName(instanceClass);
+      String builderClassName = ruleFactory.getNameHelper().getBaseBuilderClassName(instanceClass);
+
+      builderClass = instanceClass._class(JMod.ABSTRACT + JMod.PUBLIC + JMod.STATIC, builderClassName);
+
+      concreteBuilderClass = instanceClass._class(JMod.PUBLIC + JMod.STATIC, concreteBuilderClassName);
+      concreteBuilderClass._extends(builderClass.narrow(instanceClass));
+
     } catch (JClassAlreadyExistsException e) {
       return e.getExistingClass();
     }
 
-    // Determine which builder (if any) this builder should inherit from
+    // Determine which base builder (if any) this builder should inherit from
     JClass parentBuilderClass = null;
     JClass parentClass = instanceClass._extends();
     if (!(parentClass.isPrimitive() || reflectionHelper.isFinal(parentClass) || Objects.equals(parentClass.fullName(), "java.lang.Object"))) {
-      parentBuilderClass = reflectionHelper.getBuilderClass(parentClass);
+      parentBuilderClass = reflectionHelper.getBaseBuilderClass(parentClass);
     }
 
-    // Determine the generic type 'T' that the builder will create instances of
-    JTypeVar instanceType = builderClass.generify("T", instanceClass);
+    // Determine the generic type name and that the builder will create instances of
+    String builderTypeParameterName = ruleFactory.getNameHelper().getBuilderTypeParameterName(instanceClass);
+    JTypeVar instanceType = builderClass.generify(builderTypeParameterName, instanceClass);
 
     // For new builders we need to create an instance variable and 'build' method
     // for inheriting builders we'll receive these from the superType
@@ -83,31 +92,50 @@ public JDefinedClass apply(String nodeName, JsonNode node, JsonNode parent, JDef
       body.assign(JExpr._this().ref(instanceField), JExpr._null());
       body._return(result);
 
-      // Create the noargs builder constructor
-      generateNoArgsBuilderConstructor(instanceClass, builderClass);
+      // Create the noargs builder constructors
+      generateNoArgsBuilderConstructors(instanceClass, builderClass, concreteBuilderClass);
     } else {
       // Declare the inheritance
-      builderClass._extends(parentBuilderClass);
-      
+      builderClass._extends(parentBuilderClass.narrow(parentBuilderClass.owner().ref(builderTypeParameterName)));
+
       JMethod buildMethod = builderClass.method(JMod.PUBLIC, instanceType, "build");
       buildMethod.annotate(Override.class);
 
       JBlock body = buildMethod.body();
-      body._return(JExpr.cast(instanceType, JExpr._super().invoke("build")));
+      body._return(JExpr._super().invoke("build"));
 
-      // Create the noargs builder constructor
-      generateNoArgsBuilderConstructor(instanceClass, builderClass);
+      // Create the noargs builder constructors
+      generateNoArgsBuilderConstructors(instanceClass, builderClass, concreteBuilderClass);
     }
 
     return builderClass;
   }
 
-  private void generateNoArgsBuilderConstructor(JDefinedClass instanceClass, JDefinedClass builderClass) {
-    JMethod noargsConstructor = builderClass.constructor(JMod.PUBLIC);
-    JAnnotationUse warningSuppression = noargsConstructor.annotate(SuppressWarnings.class);
+  private void generateNoArgsBuilderConstructors(JDefinedClass instanceClass, JDefinedClass baseBuilderClass, JDefinedClass builderClass) {
+
+    generateNoArgsBaseBuilderConstructor(instanceClass, baseBuilderClass, builderClass);
+    generateNoArgsBuilderConstructor(instanceClass, baseBuilderClass, builderClass);
+  }
+
+  private void generateNoArgsBuilderConstructor(JDefinedClass instanceClass, JDefinedClass baseBuilderClass, JDefinedClass builderClass) {
+
+    // Add the constructor to builder.
+    JMethod noArgsConstructor = builderClass.constructor(JMod.PUBLIC);
+    JBlock constructorBlock = noArgsConstructor.body();
+
+    // Determine if we need to invoke the super() method for our parent builder
+    if (!(baseBuilderClass.isPrimitive() || reflectionHelper.isFinal(baseBuilderClass) || Objects.equals(baseBuilderClass.fullName(), "java.lang.Object"))) {
+      constructorBlock.invoke("super");
+    }
+  }
+
+  private void generateNoArgsBaseBuilderConstructor(JDefinedClass instanceClass, JDefinedClass builderClass, JDefinedClass concreteBuilderClass) {
+
+    JMethod noArgsConstructor = builderClass.constructor(JMod.PUBLIC);
+    JAnnotationUse warningSuppression = noArgsConstructor.annotate(SuppressWarnings.class);
     warningSuppression.param("value", "unchecked");
 
-    JBlock constructorBlock = noargsConstructor.body();
+    JBlock constructorBlock = noArgsConstructor.body();
 
     JFieldVar instanceField = reflectionHelper.searchClassAndSuperClassesForField("instance", builderClass);
 
@@ -120,9 +148,8 @@ private void generateNoArgsBuilderConstructor(JDefinedClass instanceClass, JDefi
     // Only initialize the instance if the object being constructed is actually this class
     // if it's a subtype then ignore the instance initialization since the subclass will initialize it
     constructorBlock.directStatement("// Skip initialization when called from subclass");
-    JInvocation comparison = JExpr._this().invoke("getClass").invoke("equals").arg(JExpr.dotclass(builderClass));
+    JInvocation comparison = JExpr._this().invoke("getClass").invoke("equals").arg(JExpr.dotclass(concreteBuilderClass));
     JConditional ifNotSubclass = constructorBlock._if(comparison);
     ifNotSubclass._then().assign(JExpr._this().ref(instanceField), JExpr.cast(instanceField.type(), JExpr._new(instanceClass)));
   }
-
 }
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/ConstructorRule.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/ConstructorRule.java
@@ -147,9 +147,10 @@ private void addFieldsConstructor(JDefinedClass instanceClass, Set<String> class
 
     // If we're using InnerClassBuilder implementations then we also need to generate those
     if (generationConfig.isGenerateBuilders() && generationConfig.isUseInnerClassBuilders()) {
-      JDefinedClass builderClass = ruleFactory.getReflectionHelper()
-          .getBuilderClass(instanceClass);
-      generateFieldsBuilderConstructor(builderClass, instanceClass, instanceConstructor);
+      JDefinedClass baseBuilderClass = ruleFactory.getReflectionHelper().getBaseBuilderClass(instanceClass);
+      JDefinedClass concreteBuilderClass = ruleFactory.getReflectionHelper().getConcreteBuilderClass(instanceClass);
+
+      generateFieldsBuilderConstructor(baseBuilderClass, concreteBuilderClass, instanceClass, instanceConstructor);
     }
   }
 
@@ -161,9 +162,10 @@ private void addCopyConstructor(JDefinedClass instanceClass, Set<String> classPr
 
     // If we're using InnerClassBuilder implementations then we also need to generate those
     if (generationConfig.isGenerateBuilders() && generationConfig.isUseInnerClassBuilders()) {
-      JDefinedClass builderClass = ruleFactory.getReflectionHelper()
-          .getBuilderClass(instanceClass);
-      generateFieldsBuilderConstructor(builderClass, instanceClass, instanceConstructor);
+      JDefinedClass baseBuilderClass = ruleFactory.getReflectionHelper().getBaseBuilderClass(instanceClass);
+      JDefinedClass concreteBuilderClass = ruleFactory.getReflectionHelper().getConcreteBuilderClass(instanceClass);
+
+      generateFieldsBuilderConstructor(baseBuilderClass, concreteBuilderClass, instanceClass, instanceConstructor);
     }
   }
 
@@ -234,7 +236,8 @@ private LinkedHashSet<String> getSuperTypeConstructorPropertiesRecursive(JsonNod
     return rtn;
   }
 
-  private void generateFieldsBuilderConstructor(JDefinedClass builderClass, JDefinedClass instanceClass, JMethod instanceConstructor) {
+  private void generateFieldsBuilderConstructor(JDefinedClass builderClass, JDefinedClass concreteBuilderClass, JDefinedClass instanceClass, JMethod instanceConstructor) {
+
     // Locate the instance field since we'll need it to assign a value
     JFieldVar instanceField = reflectionHelper.searchClassAndSuperClassesForField("instance", builderClass);
 
@@ -268,11 +271,34 @@ private void generateFieldsBuilderConstructor(JDefinedClass builderClass, JDefin
     JInvocation comparison = JExpr._this()
         .invoke("getClass")
         .invoke("equals")
-        .arg(JExpr.dotclass(builderClass));
+        .arg(JExpr.dotclass(concreteBuilderClass));
     JConditional ifNotSubclass = constructorBlock._if(comparison);
     ifNotSubclass._then()
         .assign(JExpr._this()
             .ref(instanceField), JExpr.cast(instanceField.type(), instanceConstructorInvocation));
+
+    generateFieldsConcreteBuilderConstructor(builderClass, concreteBuilderClass, instanceConstructor);
+
+  }
+
+  private void generateFieldsConcreteBuilderConstructor(JDefinedClass baseBuilderClass, JDefinedClass builderClass, JMethod instanceConstructor) {
+
+    // Create Typed Builder Constructor
+    JMethod builderConstructor = builderClass.constructor(JMod.PUBLIC);
+    JBlock builderConstructorBlock = builderConstructor.body();
+
+    // The typed builder constructor should have the exact same parameters as the inheritable builder.
+    for (JVar param : instanceConstructor.params()) {
+      builderConstructor.param(param.type(), param.name());
+    }
+
+    if (!(baseBuilderClass.isPrimitive() || reflectionHelper.isFinal(baseBuilderClass) || Objects.equals(baseBuilderClass.fullName(), "java.lang.Object"))) {
+      JInvocation superMethod = builderConstructorBlock.invoke("super");
+
+      for (JVar param : builderConstructor.params()) {
+        superMethod.arg(param);
+      }
+    }
   }
 
   private JMethod generateCopyConstructor(JDefinedClass jclass, Set<String> classProperties, Set<String> combinedSuperProperties) {
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/PropertyRule.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/rules/PropertyRule.java
@@ -262,7 +262,7 @@ private JMethod addLegacyBuilder(JDefinedClass c, JFieldVar field, String jsonPr
   }
 
   private JMethod addInnerBuilderMethod(JDefinedClass c, JFieldVar field, String jsonPropertyName, JsonNode node)    {
-    JDefinedClass builderClass = ruleFactory.getReflectionHelper().getBuilderClass(c);
+    JDefinedClass builderClass = ruleFactory.getReflectionHelper().getBaseBuilderClass(c);
 
     JMethod builderMethod = builderClass.method(JMod.PUBLIC, builderClass, getBuilderName(jsonPropertyName, node));
 
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/util/NameHelper.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/util/NameHelper.java
@@ -42,6 +42,10 @@ public NameHelper(GenerationConfig generationConfig) {
         this.generationConfig = generationConfig;
     }
 
+    public String getBuilderTypeParameterName(JDefinedClass instanceClass) {
+        return "T";
+    }
+
     public String replaceIllegalCharacters(String name) {
         return name.replaceAll(ILLEGAL_CHARACTER_REGEX, "_");
     }
@@ -220,8 +224,20 @@ private String getPropertyNameForAccessor(String jsonPropertyName, JsonNode node
         return jsonPropertyName;
     }
 
-    public String getBuilderClassName(JClass outterClass) {
-        return outterClass.name() + "Builder";
+    public String getBaseBuilderClassName(JClass outerClass) {
+        return outerClass.name() + getBuilderClassNameSuffix(outerClass) + getBaseBuilderClassNameSuffix(outerClass);
+    }
+
+    public String getBaseBuilderClassNameSuffix(JClass outerClass) {
+        return "Base";
+    }
+
+    public String getBuilderClassName(JClass outerClass) {
+        return outerClass.name() + getBuilderClassNameSuffix(outerClass);
+    }
+
+    public String getBuilderClassNameSuffix(JClass outerClass) {
+        return "Builder";
     }
 
     public String getUniqueClassName(String nodeName, JsonNode node, JPackage _package) {
diff --git a/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/util/ReflectionHelper.java b/jsonschema2pojo-core/src/main/java/org/jsonschema2pojo/util/ReflectionHelper.java
@@ -28,6 +28,7 @@
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Spliterator;
 import java.util.Spliterators;
 import java.util.stream.StreamSupport;
@@ -90,15 +91,28 @@ public JFieldVar searchSuperClassesForField(String property, JDefinedClass jclas
     return searchClassAndSuperClassesForField(property, definedSuperClass);
   }
 
-  public JDefinedClass getBuilderClass(JDefinedClass target) {
+  public JDefinedClass getConcreteBuilderClass(JDefinedClass instanceClass) {
+    String builderClassname = ruleFactory.getNameHelper().getBuilderClassName(instanceClass);
+
+    return StreamSupport.stream(Spliterators.spliteratorUnknownSize(instanceClass.classes(), Spliterator.ORDERED), false)
+            .filter(definedClass -> definedClass.name().equals(builderClassname)).findFirst().orElse(null);
+  }
+
+  public JDefinedClass getConcreteBuilderClass(JClass target) {
     String builderClassname = ruleFactory.getNameHelper().getBuilderClassName(target);
+    return getAllPackageClasses(target._package()).stream().filter(definedClass -> definedClass.name().equals(builderClassname)).findFirst()
+            .orElse(null);
+  }
+
+  public JDefinedClass getBaseBuilderClass(JDefinedClass target) {
+    String builderClassname = ruleFactory.getNameHelper().getBaseBuilderClassName(target);
 
     return StreamSupport.stream(Spliterators.spliteratorUnknownSize(target.classes(), Spliterator.ORDERED), false)
         .filter(definedClass -> definedClass.name().equals(builderClassname)).findFirst().orElse(null);
   }
 
-  public JDefinedClass getBuilderClass(JClass target) {
-    String builderClassname = ruleFactory.getNameHelper().getBuilderClassName(target);
+  public JDefinedClass getBaseBuilderClass(JClass target) {
+    String builderClassname = ruleFactory.getNameHelper().getBaseBuilderClassName(target);
     return getAllPackageClasses(target._package()).stream().filter(definedClass -> definedClass.name().equals(builderClassname)).findFirst()
         .orElse(null);
   }
@@ -127,7 +141,20 @@ private JDefinedClass definedClassOrNullFromType(JType type) {
     }
     JClass fieldClass = type.boxify();
     JPackage jPackage = fieldClass._package();
-    return this._getClass(fieldClass.name(), jPackage);
+    try
+    {
+      return this._getClass(fieldClass.name(), jPackage);
+    } catch (NoClassDefFoundError error) {
+      String name = fieldClass.name();
+      String erasureName = fieldClass.erasure().name();
+
+      if(!Objects.equals(name, erasureName)) {
+        ruleFactory.getLogger().debug("Could not get class for type with name: " + name + " trying " + erasureName + " instead.");
+        return this._getClass(erasureName, jPackage);
+      } else {
+        throw error;
+      }
+    }
   }
 
   private JDefinedClass _getClass(String name, JPackage _package) {

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@`
`23`	`23`	`import java.util.Spliterator;`
`24`	`24`	`import java.util.Spliterators;`
`25`	`25`	`import java.util.stream.StreamSupport;`
`26`		`-import org.apache.commons.collections15.CollectionUtils;`
	`26`	`+`
`27`	`27`	`import org.jsonschema2pojo.Schema;`
`28`	`28`
`29`	`29`	`import com.fasterxml.jackson.databind.JsonNode;`
`@@ -213,7 +213,7 @@ private JMethod addInnerBuilder(JDefinedClass jclass, JType propertyType, JField`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`private String getBuilderClassName(JDefinedClass c) {`
`216`		`- return ruleFactory.getNameHelper().getBuilderClassName(c);`
	`216`	`+ return ruleFactory.getNameHelper().getBaseBuilderClassName(c);`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	`}`
Original file line number	Diff line number	Diff line change
`@@ -262,7 +262,7 @@ private JMethod addLegacyBuilder(JDefinedClass c, JFieldVar field, String jsonPr`
`262`	`262`	`}`
`263`	`263`
`264`	`264`	`private JMethod addInnerBuilderMethod(JDefinedClass c, JFieldVar field, String jsonPropertyName, JsonNode node) {`
`265`		`- JDefinedClass builderClass = ruleFactory.getReflectionHelper().getBuilderClass(c);`
	`265`	`+ JDefinedClass builderClass = ruleFactory.getReflectionHelper().getBaseBuilderClass(c);`
`266`	`266`
`267`	`267`	`JMethod builderMethod = builderClass.method(JMod.PUBLIC, builderClass, getBuilderName(jsonPropertyName, node));`
`268`	`268`