From dbc173ba5c575eea404d3a76b3bc24d35656767c Mon Sep 17 00:00:00 2001 From: Jan Rieke Date: Thu, 16 Aug 2018 14:38:44 +0200 Subject: @SuperBuilder adapts @Builder.Default behavior from @Builder as #1347 is fixed now (javac) --- src/core/lombok/javac/handlers/HandleBuilder.java | 2 +- .../lombok/javac/handlers/HandleSuperBuilder.java | 31 ++++++++++------------ .../after-delombok/SuperBuilderWithDefaults.java | 27 ++++++++++++++----- 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 { return maker.MethodDef(maker.Modifiers(Flags.PUBLIC), type.toName(buildName), returnType, List.nil(), List.nil(), thrownExceptions, body, null); } - public JCMethodDecl generateDefaultProvider(Name methodName, JavacNode fieldNode, List params) { + public static JCMethodDecl generateDefaultProvider(Name methodName, JavacNode fieldNode, List 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 { JCExpression type; Name rawName; Name name; + Name nameOfDefaultProvider; Name nameOfSetFlag; SingularData singularData; ObtainVia obtainVia; @@ -154,16 +155,12 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { } 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 { } 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.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 { - private long millis = System.currentTimeMillis(); - private N numberField = null; + private long millis; + private N numberField; + @java.lang.SuppressWarnings("all") + private static long $default$millis() { + return System.currentTimeMillis(); + } + @java.lang.SuppressWarnings("all") + private static N $default$numberField() { + return null; + } @java.lang.SuppressWarnings("all") public static abstract class ParentBuilder, B extends ParentBuilder> { @java.lang.SuppressWarnings("all") @@ -53,8 +61,10 @@ public class SuperBuilderWithDefaults { } @java.lang.SuppressWarnings("all") protected Parent(final ParentBuilder 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.$default$millis(); + this.numberField = b.numberField; + if (!b.numberField$set) this.numberField = Parent.$default$numberField(); } @java.lang.SuppressWarnings("all") public static ParentBuilder builder() { @@ -62,7 +72,11 @@ public class SuperBuilderWithDefaults { } } public static class Child extends Parent { - 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> extends Parent.ParentBuilder { @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() { -- cgit