@SuperBuilder adapts @Builder.Default behavior from @Builder as #1347 is

fixed now (javac)

3 files changed, 36 insertions, 24 deletions

diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java
index f04ea8b1..a6ba5d0a 100644
--- a/src/core/lombok/javac/handlers/HandleBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleBuilder.java
@@ -595,7 +595,7 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 		return maker.MethodDef(maker.Modifiers(Flags.PUBLIC), type.toName(buildName), returnType, List.<JCTypeParameter>nil(), List.<JCVariableDecl>nil(), thrownExceptions, body, null);
 	}
 	
-	public JCMethodDecl generateDefaultProvider(Name methodName, JavacNode fieldNode, List<JCTypeParameter> params) {
+	public static JCMethodDecl generateDefaultProvider(Name methodName, JavacNode fieldNode, List<JCTypeParameter> params) {
 		JavacTreeMaker maker = fieldNode.getTreeMaker();
 		JCVariableDecl field = (JCVariableDecl) fieldNode.get();
 		
diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
index 38fec2f6..c00bd66d 100644
--- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
@@ -33,12 +33,12 @@ import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCAnnotation;
+import com.sun.tools.javac.tree.JCTree.JCAssign;
 import com.sun.tools.javac.tree.JCTree.JCBlock;
 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;
@@ -83,6 +83,7 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		JCExpression type;
 		Name rawName;
 		Name name;
+		Name nameOfDefaultProvider;
 		Name nameOfSetFlag;
 		SingularData singularData;
 		ObtainVia obtainVia;
@@ -154,16 +155,12 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 			}
 			
 			if (isDefault != null) {
+				bfd.nameOfDefaultProvider = tdParent.toName("$default$" + bfd.name);
 				bfd.nameOfSetFlag = tdParent.toName(bfd.name + "$set");
-				// 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. 
+				bfd.nameOfSetFlag = tdParent.toName(bfd.name + "$set");
+				JCMethodDecl md = HandleBuilder.generateDefaultProvider(bfd.nameOfDefaultProvider, fieldNode, td.typarams);
+				recursiveSetGeneratedBy(md, ast, annotationNode.getContext());
+				if (md != null) injectMethod(tdParent, md);
 			}
 			addObtainVia(bfd, fieldNode);
 			builderFields.add(bfd);
@@ -427,17 +424,17 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 			} else {
 				rhs = maker.Select(maker.Ident(builderVariableName), bfd.rawName);
 			}
-			JCFieldAccess thisX = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
+			JCFieldAccess fieldInThis = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
 			
-			JCStatement assign = maker.Exec(maker.Assign(thisX, rhs));
+			JCStatement assign = maker.Exec(maker.Assign(fieldInThis, rhs));
+			statements.append(assign);
 			
-			// In case of @Builder.Default, only set the value if it really was set in the builder.
+			// In case of @Builder.Default, set the value to the default if it was NOT 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);
+				fieldInThis = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
+				JCAssign assignDefault = maker.Assign(fieldInThis, maker.Apply(typeParameterNames(maker, ((JCClassDecl) typeNode.get()).typarams), maker.Select(maker.Ident(((JCClassDecl) typeNode.get()).name), bfd.nameOfDefaultProvider), List.<JCExpression>nil()));
+				statements.append(maker.If(maker.Unary(CTC_NOT, setField), maker.Exec(assignDefault), null));
 			}
 		}
 		
diff --git a/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java b/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
index 7b6b4578..06d88f4d 100644
--- a/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
+++ b/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
@@ -1,8 +1,16 @@
 import java.util.List;

 public class SuperBuilderWithDefaults {

 	public static class Parent<N extends Number> {

-		private long millis = System.currentTimeMillis();

-		private N numberField = null;

+		private long millis;

+		private N numberField;

+		@java.lang.SuppressWarnings("all")

+		private static <N extends Number> long $default$millis() {

+			return System.currentTimeMillis();

+		}

+		@java.lang.SuppressWarnings("all")

+		private static <N extends Number> N $default$numberField() {

+			return null;

+		}

 		@java.lang.SuppressWarnings("all")

 		public static abstract class ParentBuilder<N extends Number, C extends Parent<N>, B extends ParentBuilder<N, C, B>> {

 			@java.lang.SuppressWarnings("all")

@@ -53,8 +61,10 @@ public class SuperBuilderWithDefaults {
 		}

 		@java.lang.SuppressWarnings("all")

 		protected Parent(final ParentBuilder<N, ?, ?> b) {

-			if (b.millis$set) this.millis = b.millis;

-			if (b.numberField$set) this.numberField = b.numberField;

+			this.millis = b.millis;

+			if (!b.millis$set) this.millis = Parent.<N>$default$millis();

+			this.numberField = b.numberField;

+			if (!b.numberField$set) this.numberField = Parent.<N>$default$numberField();

 		}

 		@java.lang.SuppressWarnings("all")

 		public static <N extends Number> ParentBuilder<N, ?, ?> builder() {

@@ -62,7 +72,11 @@ public class SuperBuilderWithDefaults {
 		}

 	}

 	public static class Child extends Parent<Integer> {

-		private double doubleField = Math.PI;

+		private double doubleField;

+		@java.lang.SuppressWarnings("all")

+		private static double $default$doubleField() {

+			return Math.PI;

+		}

 		@java.lang.SuppressWarnings("all")

 		public static abstract class ChildBuilder<C extends Child, B extends ChildBuilder<C, B>> extends Parent.ParentBuilder<Integer, C, B> {

 			@java.lang.SuppressWarnings("all")

@@ -106,7 +120,8 @@ public class SuperBuilderWithDefaults {
 		@java.lang.SuppressWarnings("all")

 		protected Child(final ChildBuilder<?, ?> b) {

 			super(b);

-			if (b.doubleField$set) this.doubleField = b.doubleField;

+			this.doubleField = b.doubleField;

+			if (!b.doubleField$set) this.doubleField = Child.$default$doubleField();

 		}

 		@java.lang.SuppressWarnings("all")

 		public static ChildBuilder<?, ?> builder() {