diff options
7 files changed, 207 insertions, 34 deletions
diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java index 3f711a19..bb031ebc 100755 --- a/src/core/lombok/eclipse/handlers/HandleBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java @@ -565,7 +565,9 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> { invoke.type = namePlusTypeParamsToTypeReference(type, builderClassName.toCharArray(), !isStatic, typeParams, p); Expression receiver = invoke; - List<Statement> statements = null; + List<Statement> preStatements = null; + List<Statement> postStatements = null; + for (BuilderFieldData bfd : builderFields) { String setterName = new String(bfd.name); String setterPrefix = !prefix.isEmpty() ? prefix : fluent ? "" : "set"; @@ -584,23 +586,31 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> { } else { String obtainName = bfd.obtainVia.method(); boolean obtainIsStatic = bfd.obtainVia.isStatic(); - for (int i = 0; i < tgt.length; i++) { - MessageSend obtainExpr = new MessageSend(); - if (obtainIsStatic) { - if (typeParams != null && typeParams.length > 0) { - obtainExpr.typeArguments = new TypeReference[typeParams.length]; - for (int j = 0; j<typeParams.length; j++) { - obtainExpr.typeArguments[j] = new SingleTypeReference(typeParams[j].name, 0); - } + MessageSend obtainExpr = new MessageSend(); + if (obtainIsStatic) { + if (typeParams != null && typeParams.length > 0) { + obtainExpr.typeArguments = new TypeReference[typeParams.length]; + for (int j = 0; j<typeParams.length; j++) { + obtainExpr.typeArguments[j] = new SingleTypeReference(typeParams[j].name, 0); } - obtainExpr.receiver = generateNameReference(type, 0); - } else { - obtainExpr.receiver = new ThisReference(0, 0); } - obtainExpr.selector = obtainName.toCharArray(); - if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)}; - tgt[i] = obtainExpr; + obtainExpr.receiver = generateNameReference(type, 0); + } else { + obtainExpr.receiver = new ThisReference(0, 0); } + obtainExpr.selector = obtainName.toCharArray(); + if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)}; + for (int i = 0; i < tgt.length; i++) tgt[i] = new SingleNameReference(bfd.name, 0L); + + // javac appears to cache the type of JCMethodInvocation expressions based on position, meaning, if you have 2 ObtainVia-based method invokes on different types, you get bizarre type mismatch errors. + // going via a local variable declaration solves the problem. We copy this behaviour + // for ecj so we match what javac's handler does. + LocalDeclaration ld = new LocalDeclaration(bfd.name, 0, 0); + ld.modifiers = ClassFileConstants.AccFinal; + ld.type = EclipseHandlerUtil.copyType(bfd.type, source); + ld.initialization = obtainExpr; + if (preStatements == null) preStatements = new ArrayList<Statement>(); + preStatements.add(ld); } ms.selector = setterName.toCharArray(); @@ -612,23 +622,28 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> { ms.arguments = new Expression[] {tgt[1]}; ms.receiver = new SingleNameReference(BUILDER_TEMP_VAR, p); EqualExpression isNotNull = new EqualExpression(tgt[0], new NullLiteral(pS, pE), OperatorIds.NOT_EQUAL); - if (statements == null) statements = new ArrayList<Statement>(); - statements.add(new IfStatement(isNotNull, ms, pS, pE)); + if (postStatements == null) postStatements = new ArrayList<Statement>(); + postStatements.add(new IfStatement(isNotNull, ms, pS, pE)); } } - if (statements != null) { - out.statements = new Statement[statements.size() + 2]; - for (int i = 0; i < statements.size(); i++) out.statements[i + 1] = statements.get(i); + int preSs = preStatements == null ? 0 : preStatements.size(); + int postSs = postStatements == null ? 0 : postStatements.size(); + if (postSs > 0) { + out.statements = new Statement[preSs + postSs + 2]; + for (int i = 0; i < preSs; i++) out.statements[i] = preStatements.get(i); + for (int i = 0; i < postSs; i++) out.statements[preSs + 1 + i] = postStatements.get(i); LocalDeclaration b = new LocalDeclaration(BUILDER_TEMP_VAR, pS, pE); - out.statements[0] = b; - b.modifiers |= Modifier.FINAL; + out.statements[preSs] = b; + b.modifiers |= ClassFileConstants.AccFinal; b.type = namePlusTypeParamsToTypeReference(type, builderClassName.toCharArray(), !isStatic, typeParams, p); b.type.sourceStart = pS; b.type.sourceEnd = pE; b.initialization = receiver; - out.statements[out.statements.length - 1] = new ReturnStatement(new SingleNameReference(BUILDER_TEMP_VAR, p), pS, pE); + out.statements[preSs + postSs + 1] = new ReturnStatement(new SingleNameReference(BUILDER_TEMP_VAR, p), pS, pE); } else { - out.statements = new Statement[] {new ReturnStatement(receiver, pS, pE)}; + out.statements = new Statement[preSs + 1]; + for (int i = 0; i < preSs; i++) out.statements[i] = preStatements.get(i); + out.statements[preSs] = new ReturnStatement(receiver, pS, pE); } if (cfv.generateUnique()) { diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index 1e8b8a22..0bd43ba4 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -45,6 +45,7 @@ import com.sun.tools.javac.tree.JCTree.JCIdent; import com.sun.tools.javac.tree.JCTree.JCIf; import com.sun.tools.javac.tree.JCTree.JCLiteral; import com.sun.tools.javac.tree.JCTree.JCMethodDecl; +import com.sun.tools.javac.tree.JCTree.JCMethodInvocation; import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.tree.JCTree.JCNewClass; import com.sun.tools.javac.tree.JCTree.JCPrimitiveTypeTree; @@ -553,7 +554,9 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> { JCExpression call = maker.NewClass(null, List.<JCExpression>nil(), namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams), List.<JCExpression>nil(), null); JCExpression invoke = call; + ListBuffer<JCStatement> preStatements = null; ListBuffer<JCStatement> statements = new ListBuffer<JCStatement>(); + for (BuilderFieldData bfd : builderFields) { String setterPrefix = !prefix.isEmpty() ? prefix : fluent ? "" : "set"; String prefixedSetterName = bfd.name.toString(); @@ -566,17 +569,22 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> { tgt[i] = maker.Select(maker.Ident(type.toName("this")), bfd.obtainVia == null ? bfd.rawName : type.toName(bfd.obtainVia.field())); } } else { + String name = bfd.obtainVia.method(); + JCMethodInvocation inv; if (bfd.obtainVia.isStatic()) { - for (int i = 0; i < tgt.length; i++) { - JCExpression c = maker.Select(maker.Ident(type.toName(type.getName())), type.toName(bfd.obtainVia.method())); - tgt[i] = maker.Apply(typeParameterNames(maker, typeParams), c, List.<JCExpression>of(maker.Ident(type.toName("this")))); - } + JCExpression c = maker.Select(maker.Ident(type.toName(type.getName())), type.toName(name)); + inv = maker.Apply(typeParameterNames(maker, typeParams), c, List.<JCExpression>of(maker.Ident(type.toName("this")))); } else { - for (int i = 0; i < tgt.length; i++) { - JCExpression c = maker.Select(maker.Ident(type.toName("this")), type.toName(bfd.obtainVia.method())); - tgt[i] = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>nil()); - } + JCExpression c = maker.Select(maker.Ident(type.toName("this")), type.toName(name)); + inv = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>nil()); } + for (int i = 0; i < tgt.length; i++) tgt[i] = maker.Ident(bfd.name); + + // javac appears to cache the type of JCMethodInvocation expressions based on position, meaning, if you have 2 ObtainVia-based method invokes on different types, you get bizarre type mismatch errors. + // going via a local variable declaration solves the problem. + JCExpression varType = JavacHandlerUtil.cloneType(maker, bfd.type, ast, type.getContext()); + if (preStatements == null) preStatements = new ListBuffer<JCStatement>(); + preStatements.append(maker.VarDef(maker.Modifiers(Flags.FINAL), bfd.name, varType, inv)); } JCExpression arg; @@ -589,6 +597,7 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> { statements.append(maker.If(isNotNull, maker.Exec(invokeBuilder), null)); } } + if (!statements.isEmpty()) { JCExpression tempVarType = namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams); statements.prepend(maker.VarDef(maker.Modifiers(Flags.FINAL), type.toName(BUILDER_TEMP_VAR), tempVarType, invoke)); @@ -596,6 +605,11 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> { } else { statements.append(maker.Return(invoke)); } + + if (preStatements != null) { + preStatements.appendList(statements); + statements = preStatements; + } JCBlock body = maker.Block(0, statements.toList()); List<JCAnnotation> annsOnMethod = cfv.generateUnique() ? List.of(maker.Annotation(genTypeRef(type, CheckerFrameworkVersion.NAME__UNIQUE), List.<JCExpression>nil())) : List.<JCAnnotation>nil(); return maker.MethodDef(maker.Modifiers(toJavacModifier(access), annsOnMethod), type.toName(toBuilderMethodName), namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams), List.<JCTypeParameter>nil(), List.<JCVariableDecl>nil(), List.<JCExpression>nil(), body, null); diff --git a/test/transform/resource/after-delombok/BuilderWithToBuilder.java b/test/transform/resource/after-delombok/BuilderWithToBuilder.java index 2c4b0531..aa47c430 100644 --- a/test/transform/resource/after-delombok/BuilderWithToBuilder.java +++ b/test/transform/resource/after-delombok/BuilderWithToBuilder.java @@ -86,7 +86,8 @@ class BuilderWithToBuilder<T> { } @java.lang.SuppressWarnings("all") public BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> toBuilder() { - final BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> builder = new BuilderWithToBuilder.BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.<T>rrr(this)); + final T foo = BuilderWithToBuilder.<T>rrr(this); + final BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> builder = new BuilderWithToBuilder.BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(foo); if (this.bars != null) builder.bars(this.bars); return builder; } diff --git a/test/transform/resource/after-delombok/I2335_BuilderMultipleObtainVia.java b/test/transform/resource/after-delombok/I2335_BuilderMultipleObtainVia.java new file mode 100644 index 00000000..f9cd424d --- /dev/null +++ b/test/transform/resource/after-delombok/I2335_BuilderMultipleObtainVia.java @@ -0,0 +1,59 @@ +public class I2335_BuilderMultipleObtainVia { + private String theString; + private Long theLong; + public I2335_BuilderMultipleObtainVia(String theString, Long theLong) { + setTheString(theString); + setTheLong(theLong); + } + public String getTheString() { + return theString; + } + public Long getTheLong() { + return theLong; + } + public void setTheString(String theString) { + this.theString = theString; + } + public void setTheLong(Long theLong) { + this.theLong = theLong; + } + @java.lang.SuppressWarnings("all") + public static class I2335_BuilderMultipleObtainViaBuilder { + @java.lang.SuppressWarnings("all") + private String theString; + @java.lang.SuppressWarnings("all") + private Long theLong; + @java.lang.SuppressWarnings("all") + I2335_BuilderMultipleObtainViaBuilder() { + } + @java.lang.SuppressWarnings("all") + public I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder theString(final String theString) { + this.theString = theString; + return this; + } + @java.lang.SuppressWarnings("all") + public I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder theLong(final Long theLong) { + this.theLong = theLong; + return this; + } + @java.lang.SuppressWarnings("all") + public I2335_BuilderMultipleObtainVia build() { + return new I2335_BuilderMultipleObtainVia(this.theString, this.theLong); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder(theString=" + this.theString + ", theLong=" + this.theLong + ")"; + } + } + @java.lang.SuppressWarnings("all") + public static I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder builder() { + return new I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder(); + } + @java.lang.SuppressWarnings("all") + public I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder toBuilder() { + final String theString = this.getTheString(); + final Long theLong = this.getTheLong(); + return new I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder().theString(theString).theLong(theLong); + } +} diff --git a/test/transform/resource/after-ecj/BuilderWithToBuilder.java b/test/transform/resource/after-ecj/BuilderWithToBuilder.java index 131b0ca0..6d935736 100644 --- a/test/transform/resource/after-ecj/BuilderWithToBuilder.java +++ b/test/transform/resource/after-ecj/BuilderWithToBuilder.java @@ -74,7 +74,8 @@ import lombok.Builder; return new BuilderWithToBuilder.BuilderWithToBuilderBuilder<T>(); } public @java.lang.SuppressWarnings("all") BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> toBuilder() { - final BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> builder = new BuilderWithToBuilder.BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.<T>rrr(this)); + final T foo = BuilderWithToBuilder.<T>rrr(this); + final BuilderWithToBuilder.BuilderWithToBuilderBuilder<T> builder = new BuilderWithToBuilder.BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(foo); if ((this.bars != null)) builder.bars(this.bars); return builder; diff --git a/test/transform/resource/after-ecj/I2335_BuilderMultipleObtainVia.java b/test/transform/resource/after-ecj/I2335_BuilderMultipleObtainVia.java new file mode 100644 index 00000000..7d8b8953 --- /dev/null +++ b/test/transform/resource/after-ecj/I2335_BuilderMultipleObtainVia.java @@ -0,0 +1,51 @@ +import lombok.Builder; +public @Builder class I2335_BuilderMultipleObtainVia { + public static @java.lang.SuppressWarnings("all") class I2335_BuilderMultipleObtainViaBuilder { + private @java.lang.SuppressWarnings("all") String theString; + private @java.lang.SuppressWarnings("all") Long theLong; + @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainViaBuilder() { + super(); + } + public @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder theString(final String theString) { + this.theString = theString; + return this; + } + public @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder theLong(final Long theLong) { + this.theLong = theLong; + return this; + } + public @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainVia build() { + return new I2335_BuilderMultipleObtainVia(this.theString, this.theLong); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (((("I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder(theString=" + this.theString) + ", theLong=") + this.theLong) + ")"); + } + } + private String theString; + private Long theLong; + public @Builder(toBuilder = true) I2335_BuilderMultipleObtainVia(@Builder.ObtainVia(method = "getTheString") String theString, @Builder.ObtainVia(method = "getTheLong") Long theLong) { + super(); + setTheString(theString); + setTheLong(theLong); + } + public String getTheString() { + return theString; + } + public Long getTheLong() { + return theLong; + } + public void setTheString(String theString) { + this.theString = theString; + } + public void setTheLong(Long theLong) { + this.theLong = theLong; + } + public static @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder builder() { + return new I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder(); + } + public @java.lang.SuppressWarnings("all") I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder toBuilder() { + String theString = this.getTheString(); + Long theLong = this.getTheLong(); + return new I2335_BuilderMultipleObtainVia.I2335_BuilderMultipleObtainViaBuilder().theString(theString).theLong(theLong); + } +} diff --git a/test/transform/resource/before/I2335_BuilderMultipleObtainVia.java b/test/transform/resource/before/I2335_BuilderMultipleObtainVia.java new file mode 100644 index 00000000..ef3723c0 --- /dev/null +++ b/test/transform/resource/before/I2335_BuilderMultipleObtainVia.java @@ -0,0 +1,32 @@ +import lombok.Builder; + +@Builder +public class I2335_BuilderMultipleObtainVia { + private String theString; + private Long theLong; + + @Builder(toBuilder = true) + public I2335_BuilderMultipleObtainVia( + @Builder.ObtainVia(method = "getTheString") String theString, + @Builder.ObtainVia(method = "getTheLong") Long theLong + ) { + setTheString(theString); + setTheLong(theLong); + } + + public String getTheString() { + return theString; + } + + public Long getTheLong() { + return theLong; + } + + public void setTheString(String theString) { + this.theString = theString; + } + + public void setTheLong(Long theLong) { + this.theLong = theLong; + } +} |