SuperBuilder toBuilder: moved static helper method to builder impl

2 files changed, 29 insertions, 37 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
index fc532afb..640e5a71 100644
--- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
@@ -298,7 +298,9 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 
 		if (toBuilder) {
 			// Generate $fillValuesFrom() method in the abstract builder.
-			injectMethod(builderType, generateFillValuesMethod(tdParent, superclassBuilderClass != null, builderGenericName, classGenericName, builderImplClassName, typeParams));
+			injectMethod(builderType, generateFillValuesMethod(tdParent, superclassBuilderClass != null, builderGenericName, classGenericName, builderClassName, typeParams));
+			// Generate $fillValuesFromInstanceIntoBuilder() method in the builder implementation class.
+			injectMethod(builderType, generateStaticFillValuesMethod(tdParent, builderClassName, typeParams, builderFields, ast));
 		}
 
 		// Generate abstract self() and build() methods in the abstract builder.
@@ -327,9 +329,9 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 		
 		if (addCleaning) injectMethod(builderType, generateCleanMethod(builderFields, builderType, ast));
 		
-		if ((td.modifiers & ClassFileConstants.AccAbstract) != 0 && !toBuilder) {
+		boolean isAbstract = (td.modifiers & ClassFileConstants.AccAbstract) != 0;
+		if (isAbstract) {
 			// Only non-abstract classes get the Builder implementation.
-			// However, if we want to generate a toBuilder, we need a helper function in the builder implementation class.
 			return;
 		}
 		
@@ -343,9 +345,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 		}
 
 		if (toBuilder) {
-			// Generate $fillValuesFromInstanceIntoBuilder() method in the builder implementation class.
-			injectMethod(builderImplType, generateStaticFillValuesMethod(tdParent, builderClassName, typeParams, builderFields, ast));
-			
+			// Add the toBuilder() method to the annotated class.
 			switch (methodExists(TO_BUILDER_METHOD_NAME_STRING, tdParent, 0)) {
 			case EXISTS_BY_USER:
 				annotationNode.addWarning("Not generating toBuilder() as it already exists.");
@@ -355,10 +355,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 			default:
 				// Should not happen.
 			}
-			if ((td.modifiers & ClassFileConstants.AccAbstract) != 0) {
-				// The rest of the builder implementation class is not necessary for abstract classes.
-				return;
-			}
 		}
 
 		// Create the self() and build() methods in the BuilderImpl.
@@ -605,7 +601,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 	 * }
 	 * </pre>
 	 */
-	private MethodDeclaration generateFillValuesMethod(EclipseNode tdParent, boolean inherited, String builderGenericName, String classGenericName, String builderImplClassName, TypeParameter[] typeParams) {
+	private MethodDeclaration generateFillValuesMethod(EclipseNode tdParent, boolean inherited, String builderGenericName, String classGenericName, String builderClassName, TypeParameter[] typeParams) {
 		MethodDeclaration out = new MethodDeclaration(((CompilationUnitDeclaration) tdParent.top().get()).compilationResult);
 		out.selector = FILL_VALUES_METHOD_NAME;
 		out.bits |= ECLIPSE_DO_NOT_TOUCH_FLAG;
@@ -629,7 +625,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 
 		// Call the builder implemention's helper method that actually fills the values from the instance.
 		MessageSend callStaticFillValuesMethod = new MessageSend();
-		callStaticFillValuesMethod.receiver = new SingleNameReference(builderImplClassName.toCharArray(), 0);
+		callStaticFillValuesMethod.receiver = new SingleNameReference(builderClassName.toCharArray(), 0);
 		callStaticFillValuesMethod.selector = FILL_VALUES_STATIC_METHOD_NAME;
 		callStaticFillValuesMethod.arguments = new Expression[] {new SingleNameReference(INSTANCE_VARIABLE_NAME, 0), new ThisReference(0, 0)};
 		body.add(callStaticFillValuesMethod);
diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
index f858cd29..f9686cba 100644
--- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
@@ -242,7 +242,9 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		
 		if (toBuilder) {
 			// Generate $fillValuesFrom() method in the abstract builder.
-			injectMethod(builderType, generateFillValuesMethod(tdParent, superclassBuilderClassExpression != null, builderGenericName, classGenericName, builderImplClassName));
+			injectMethod(builderType, generateFillValuesMethod(tdParent, superclassBuilderClassExpression != null, builderGenericName, classGenericName, builderClassName));
+			// Generate $fillValuesFromInstanceIntoBuilder() method in the builder implementation class.
+			injectMethod(builderType, generateStaticFillValuesMethod(tdParent, builderClassName, typeParams, builderFields));
 		}
 
 		// Generate abstract self() and build() methods in the abstract builder.
@@ -272,9 +274,8 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		recursiveSetGeneratedBy(builderType.get(), ast, annotationNode.getContext());
 		
 		boolean isAbstract = (td.mods.flags & Flags.ABSTRACT) != 0;
-		if (!isAbstract || toBuilder) {
+		if (!isAbstract) {
 			// Only non-abstract classes get the Builder implementation.
-			// However, if we want to generate a toBuilder, we need a helper function in the builder implementation class.
 			
 			// Create the builder implementation class.
 			JavacNode builderImplType = findInnerClass(tdParent, builderImplClassName);
@@ -285,20 +286,13 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 				return;
 			}
 
-			if (toBuilder) {
-				// Generate $fillValuesFromInstanceIntoBuilder() method in the builder implementation class.
-				injectMethod(builderImplType, generateStaticFillValuesMethod(tdParent, builderClassName, typeParams, builderFields));
-			}
+			// Create a simple constructor for the BuilderImpl class.
+			JCMethodDecl cd = HandleConstructor.createConstructor(AccessLevel.PRIVATE, List.<JCAnnotation>nil(), builderImplType, List.<JavacNode>nil(), false, annotationNode);
+			if (cd != null) injectMethod(builderImplType, cd);
 			
-			if (!isAbstract) {
-				// Create a simple constructor for the BuilderImpl class.
-				JCMethodDecl cd = HandleConstructor.createConstructor(AccessLevel.PRIVATE, List.<JCAnnotation>nil(), builderImplType, List.<JavacNode>nil(), false, annotationNode);
-				if (cd != null) injectMethod(builderImplType, cd);
-				
-				// Create the self() and build() methods in the BuilderImpl.
-				injectMethod(builderImplType, generateSelfMethod(builderImplType, typeParams));
-				injectMethod(builderImplType, generateBuildMethod(buildMethodName, returnType, builderImplType, thrownExceptions));
-			}
+			// Create the self() and build() methods in the BuilderImpl.
+			injectMethod(builderImplType, generateSelfMethod(builderImplType, typeParams));
+			injectMethod(builderImplType, generateBuildMethod(buildMethodName, returnType, builderImplType, thrownExceptions));
 			
 			recursiveSetGeneratedBy(builderImplType.get(), ast, annotationNode.getContext());
 		}
@@ -307,18 +301,20 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		generateBuilderBasedConstructor(tdParent, typeParams, builderFields, annotationNode, builderClassName,
 				superclassBuilderClassExpression != null);
 		
-		if (!isAbstract) {
-			// Only non-abstract classes get the builder() method.
+		if (isAbstract) {
+			// Only non-abstract classes get the builder() and toBuilder() methods.
+			return;
+		}
 			
-			// Add the builder() method to the annotated class.
-			// Allow users to specify their own builder() methods, e.g., to provide default values.
-			if (methodExists(builderMethodName, tdParent, -1) == MemberExistsResult.NOT_EXISTS) {
-				JCMethodDecl builderMethod = generateBuilderMethod(builderMethodName, builderClassName, builderImplClassName, annotationNode, tdParent, typeParams);
-				recursiveSetGeneratedBy(builderMethod, ast, annotationNode.getContext());
-				if (builderMethod != null) injectMethod(tdParent, builderMethod);
-			}
+		// Add the builder() method to the annotated class.
+		// Allow users to specify their own builder() methods, e.g., to provide default values.
+		if (methodExists(builderMethodName, tdParent, -1) == MemberExistsResult.NOT_EXISTS) {
+			JCMethodDecl builderMethod = generateBuilderMethod(builderMethodName, builderClassName, builderImplClassName, annotationNode, tdParent, typeParams);
+			recursiveSetGeneratedBy(builderMethod, ast, annotationNode.getContext());
+			if (builderMethod != null) injectMethod(tdParent, builderMethod);
 		}
 
+		// Add the toBuilder() method to the annotated class.
 		if (toBuilder) {
 			switch (methodExists(TO_BUILDER_METHOD_NAME, tdParent, 0)) {
 			case EXISTS_BY_USER: