support @Builder.Default

4 files changed, 53 insertions, 41 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleBuilderDefault.java b/src/core/lombok/eclipse/handlers/HandleBuilderDefault.java
index be2b986d..d0c597fd 100644
--- a/src/core/lombok/eclipse/handlers/HandleBuilderDefault.java
+++ b/src/core/lombok/eclipse/handlers/HandleBuilderDefault.java
@@ -31,6 +31,7 @@ import lombok.core.AnnotationValues;
 import lombok.core.HandlerPriority;
 import lombok.eclipse.EclipseAnnotationHandler;
 import lombok.eclipse.EclipseNode;
+import lombok.experimental.SuperBuilder;
 
 @ProviderFor(EclipseAnnotationHandler.class)
 @HandlerPriority(-1025) //HandleBuilder's level, minus one.
@@ -39,8 +40,9 @@ public class HandleBuilderDefault extends EclipseAnnotationHandler<Builder.Defau
 		EclipseNode annotatedField = annotationNode.up();
 		if (annotatedField.getKind() != Kind.FIELD) return;
 		EclipseNode classWithAnnotatedField = annotatedField.up();
-		if (!hasAnnotation(Builder.class, classWithAnnotatedField) && !hasAnnotation("lombok.experimental.Builder", classWithAnnotatedField)) {
-			annotationNode.addWarning("@Builder.Default requires @Builder on the class for it to mean anything.");
+		if (!hasAnnotation(Builder.class, classWithAnnotatedField) && !hasAnnotation("lombok.experimental.Builder", classWithAnnotatedField)
+				&& !hasAnnotation(SuperBuilder.class, classWithAnnotatedField)) {
+			annotationNode.addWarning("@Builder.Default requires @Builder or @SuperBuilder on the class for it to mean anything.");
 		}
 	}
 }
diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
index 1b17e3df..3a92da4a 100644
--- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
@@ -46,6 +46,7 @@ import org.eclipse.jdt.internal.compiler.ast.Expression;
 import org.eclipse.jdt.internal.compiler.ast.FalseLiteral;
 import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.FieldReference;
+import org.eclipse.jdt.internal.compiler.ast.IfStatement;
 import org.eclipse.jdt.internal.compiler.ast.MessageSend;
 import org.eclipse.jdt.internal.compiler.ast.MethodDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.ParameterizedQualifiedTypeReference;
@@ -93,9 +94,8 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 
 	private static final char[] CLEAN_FIELD_NAME = "$lombokUnclean".toCharArray();
 	private static final char[] CLEAN_METHOD_NAME = "$lombokClean".toCharArray();
-	private static final char[] DEFAULT_PREFIX = {'$', 'd', 'e', 'f', 'a', 'u', 'l', 't', '$'};
-	private static final char[] SET_PREFIX = {'$', 's', 'e', 't'};
-	private static final String SELF_METHOD = "self";
+	private static final char[] SET_PREFIX = "$set".toCharArray();
+	private static final char[] SELF_METHOD_NAME = "self".toCharArray();
 
 	private static final AbstractMethodDeclaration[] EMPTY_METHODS = {};
 
@@ -103,7 +103,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 		TypeReference type;
 		char[] rawName;
 		char[] name;
-		char[] nameOfDefaultProvider;
 		char[] nameOfSetFlag;
 		SingularData singularData;
 		ObtainVia obtainVia;
@@ -185,13 +184,16 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 			}
 
 			if (isDefault != null) {
-				bfd.nameOfDefaultProvider = prefixWith(DEFAULT_PREFIX, bfd.name);
 				bfd.nameOfSetFlag = prefixWith(bfd.name, SET_PREFIX);
-
-				MethodDeclaration md = HandleBuilder.generateDefaultProvider(bfd.nameOfDefaultProvider, td.typeParameters, fieldNode, ast);
-				if (md != null) {
-					injectMethod(tdParent, md);
-				}
+				// The @Builder annotation removes the initializing expression on the field and moves
+				// it to a method called "$default$FIELDNAME". This method is then called upon building.
+				// We do NOT do this, because this is unexpected and may lead to bugs when using other 
+				// constructors (see, e.g., issue #1347).
+				// Instead, we keep the init expression and only set a new value in the builder-based
+				// constructor if it was set in the builder. Drawback is that the init expression is
+				// always executed, even if it was unnecessary because its value is overwritten by the 
+				// builder.
+				// TODO: Once the issue is resolved in @Builder, we can adapt the solution here. 
 			}
 			addObtainVia(bfd, fieldNode);
 			builderFields.add(bfd);
@@ -481,9 +483,17 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 				long[] positions = new long[] {p, p};
 				assignmentExpr = new QualifiedNameReference(variableInBuilder, positions, s, e);
 			}
-
-			Assignment assignment = new Assignment(thisX, assignmentExpr, (int) p);
+			Statement assignment = new Assignment(thisX, assignmentExpr, (int) p);
+			
+			// In case of @Builder.Default, only set the value if it really was set in the builder.
+			if (fieldNode.nameOfSetFlag != null) {
+				char[][] variableInBuilder = new char[][] {"b".toCharArray(), fieldNode.nameOfSetFlag};
+				long[] positions = new long[] {p, p};
+				QualifiedNameReference builderRef = new QualifiedNameReference(variableInBuilder, positions, s, e);
+				assignment = new IfStatement(builderRef, assignment, s, e);
+			}
 			statements.add(assignment);
+			
 			Annotation[] nonNulls = findAnnotations((FieldDeclaration)fieldNode.originalFieldNode.get(), NON_NULL_PATTERN);
 			if (nonNulls.length != 0) {
 				Statement nullCheck = generateNullCheck((FieldDeclaration)fieldNode.originalFieldNode.get(), sourceNode);
@@ -527,7 +537,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 
 	private MethodDeclaration generateAbstractSelfMethod(EclipseNode tdParent, boolean override, String builderGenericName) {
 		MethodDeclaration out = new MethodDeclaration(((CompilationUnitDeclaration) tdParent.top().get()).compilationResult);
-		out.selector = SELF_METHOD.toCharArray();
+		out.selector = SELF_METHOD_NAME;
 		out.bits |= ECLIPSE_DO_NOT_TOUCH_FLAG;
 		out.modifiers = ClassFileConstants.AccAbstract | ClassFileConstants.AccProtected | ExtraCompilerModifiers.AccSemicolonBody;
 		if (override) {
@@ -539,7 +549,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 
 	private MethodDeclaration generateSelfMethod(EclipseNode builderImplType) {
 		MethodDeclaration out = new MethodDeclaration(((CompilationUnitDeclaration) builderImplType.top().get()).compilationResult);
-		out.selector = SELF_METHOD.toCharArray();
+		out.selector = SELF_METHOD_NAME;
 		out.bits |= ECLIPSE_DO_NOT_TOUCH_FLAG;
 		out.modifiers = ClassFileConstants.AccProtected;
 		out.annotations = new Annotation[] {makeMarkerAnnotation(TypeConstants.JAVA_LANG_OVERRIDE, builderImplType.get())};
@@ -665,7 +675,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 			@Override public ReturnStatement get() {
 				MessageSend returnCall = new MessageSend();
 				returnCall.receiver = ThisReference.implicitThis();
-				returnCall.selector = SELF_METHOD.toCharArray();
+				returnCall.selector = SELF_METHOD_NAME;
 				return new ReturnStatement(returnCall, 0, 0);
 			}
 		};
diff --git a/src/core/lombok/javac/handlers/HandleBuilderDefault.java b/src/core/lombok/javac/handlers/HandleBuilderDefault.java
index 4c4ba0e8..af45a620 100644
--- a/src/core/lombok/javac/handlers/HandleBuilderDefault.java
+++ b/src/core/lombok/javac/handlers/HandleBuilderDefault.java
@@ -31,6 +31,7 @@ import lombok.Builder;
 import lombok.core.AST.Kind;
 import lombok.core.AnnotationValues;
 import lombok.core.HandlerPriority;
+import lombok.experimental.SuperBuilder;
 import lombok.javac.JavacAnnotationHandler;
 import lombok.javac.JavacNode;
 
@@ -41,8 +42,9 @@ public class HandleBuilderDefault extends JavacAnnotationHandler<Builder.Default
 		JavacNode annotatedField = annotationNode.up();
 		if (annotatedField.getKind() != Kind.FIELD) return;
 		JavacNode classWithAnnotatedField = annotatedField.up();
-		if (!hasAnnotation(Builder.class, classWithAnnotatedField) && !hasAnnotation("lombok.experimental.Builder", classWithAnnotatedField)) {
-			annotationNode.addWarning("@Builder.Default requires @Builder on the class for it to mean anything.");
+		if (!hasAnnotation(Builder.class, classWithAnnotatedField) && !hasAnnotation("lombok.experimental.Builder", classWithAnnotatedField)
+				&& !hasAnnotation(SuperBuilder.class, classWithAnnotatedField)) {
+			annotationNode.addWarning("@Builder.Default requires @Builder or @SuperBuilder on the class for it to mean anything.");
 			deleteAnnotationIfNeccessary(annotationNode, Builder.Default.class);
 		}
 	}
diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
index c48e4761..beee47a9 100644
--- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
@@ -39,6 +39,7 @@ import com.sun.tools.javac.tree.JCTree.JCClassDecl;
 import com.sun.tools.javac.tree.JCTree.JCExpression;
 import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
 import com.sun.tools.javac.tree.JCTree.JCIdent;
+import com.sun.tools.javac.tree.JCTree.JCIf;
 import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
 import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
 import com.sun.tools.javac.tree.JCTree.JCModifiers;
@@ -82,7 +83,6 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		JCExpression type;
 		Name rawName;
 		Name name;
-		Name nameOfDefaultProvider;
 		Name nameOfSetFlag;
 		SingularData singularData;
 		ObtainVia obtainVia;
@@ -164,13 +164,16 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 			}
 
 			if (isDefault != null) {
-				bfd.nameOfDefaultProvider = tdParent.toName("$default$" + bfd.name);
 				bfd.nameOfSetFlag = tdParent.toName(bfd.name + "$set");
-				JCMethodDecl md = generateDefaultProvider(bfd.nameOfDefaultProvider, fieldNode, td.typarams);
-				recursiveSetGeneratedBy(md, ast, annotationNode.getContext());
-				if (md != null) {
-					injectMethod(tdParent, md);
-				}
+				// The @Builder annotation removes the initializing expression on the field and moves
+				// it to a method called "$default$FIELDNAME". This method is then called upon building.
+				// We do NOT do this, because this is unexpected and may lead to bugs when using other 
+				// constructors (see, e.g., issue #1347).
+				// Instead, we keep the init expression and only set a new value in the builder-based
+				// constructor if it was set in the builder. Drawback is that the init expression is
+				// always executed, even if it was unnecessary because its value is overwritten by the 
+				// builder.
+				// TODO: Once the issue is resolved in @Builder, we can adapt the solution here. 
 			}
 			addObtainVia(bfd, fieldNode);
 			builderFields.add(bfd);
@@ -435,9 +438,16 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 			}
 			JCFieldAccess thisX = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
 
-			JCExpression assign = maker.Assign(thisX, rhs);
-
-			statements.append(maker.Exec(assign));
+			JCStatement assign = maker.Exec(maker.Assign(thisX, rhs));
+			
+			// In case of @Builder.Default, only set the value if it really was set in the builder.
+			if (bfd.nameOfSetFlag != null) {
+				JCFieldAccess setField = maker.Select(maker.Ident(builderVariableName), bfd.nameOfSetFlag);
+				JCIf ifSet = maker.If(setField, assign, null);
+				statements.append(ifSet);
+			} else {
+				statements.append(assign);
+			}
 		}
 
 		JCModifiers mods = maker.Modifiers(toJavacModifier(level), List.<JCAnnotation>nil());
@@ -575,18 +585,6 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		return maker.MethodDef(maker.Modifiers(Flags.PUBLIC), type.toName("$lombokClean"), maker.Type(Javac.createVoidType(type.getSymbolTable(), CTC_VOID)), List.<JCTypeParameter>nil(), List.<JCVariableDecl>nil(), List.<JCExpression>nil(), body, null);
 	}
 
-	private JCMethodDecl generateDefaultProvider(Name methodName, JavacNode fieldNode, List<JCTypeParameter> params) {
-		JavacTreeMaker maker = fieldNode.getTreeMaker();
-		JCVariableDecl field = (JCVariableDecl) fieldNode.get();
-
-		JCStatement statement = maker.Return(field.init);
-		field.init = null;
-
-		JCBlock body = maker.Block(0, List.<JCStatement>of(statement));
-		int modifiers = Flags.PRIVATE | Flags.STATIC;
-		return maker.MethodDef(maker.Modifiers(modifiers), methodName, cloneType(maker, field.vartype, field, fieldNode.getContext()), copyTypeParams(fieldNode, params), List.<JCVariableDecl>nil(), List.<JCExpression>nil(), body, null);
-	}
-
 	private void generateBuilderFields(JavacNode builderType, java.util.List<BuilderFieldData> builderFields, JCTree source) {
 		int len = builderFields.size();
 		java.util.List<JavacNode> existing = new ArrayList<JavacNode>();