From d9031506b5e7804c277d29db060973cd17033e9a Mon Sep 17 00:00:00 2001 From: Roel Spilker Date: Tue, 12 Dec 2017 02:42:14 +0100 Subject: support for @Builder on methods with a generified return type. Fixes #1420 --- doc/changelog.markdown | 1 + src/core/lombok/javac/handlers/HandleBuilder.java | 26 ++++++++++---- .../after-delombok/BuilderGenericMethod.java | 40 ++++++++++++++++++++++ .../BuilderWithExistingBuilderClass.java | 2 +- .../after-delombok/BuilderWithToBuilder.java | 2 +- .../resource/after-ecj/BuilderGenericMethod.java | 35 +++++++++++++++++++ .../resource/before/BuilderGenericMethod.java | 11 ++++++ 7 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 test/transform/resource/after-delombok/BuilderGenericMethod.java create mode 100644 test/transform/resource/after-ecj/BuilderGenericMethod.java create mode 100644 test/transform/resource/before/BuilderGenericMethod.java diff --git a/doc/changelog.markdown b/doc/changelog.markdown index fd4af685..51c1bf8d 100644 --- a/doc/changelog.markdown +++ b/doc/changelog.markdown @@ -10,6 +10,7 @@ Lombok Changelog * BUGFIX: The generated hashCode would break the contract if `callSuper=true,of={}`. [Issue #1505](https://github.com/rzwitserloot/lombok/issues/1505) * BUGFIX: `delombok` no longer prints the synthetic outer-class parameter. [Issue #1521](https://github.com/rzwitserloot/lombok/issues/1521) * BUGFIX: @Builder.Default now also works when type parameters are present. [Issue #1527](https://github.com/rzwitserloot/lombok/issues/1527) +* BUGFIX: @Builder now also works on method with a generified return type. [Issue #1420](https://github.com/rzwitserloot/lombok/issues/1420) * INSTALLER: By default, the lombok installer now inserts an absolute path in `eclipse.ini` and friends, instead of a relative path. If you want the old behavior, you can use `java -jar -Dlombok.installer.fullpath=false lombok.jar`. ### v1.16.18 (July 3rd, 2017) diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index ea436422..420d4b72 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -212,7 +212,7 @@ public class HandleBuilder extends JavacAnnotationHandler { thrownExceptions = jmd.thrown; nameOfBuilderMethod = jmd.name; if (returnType instanceof JCTypeApply) { - returnType = ((JCTypeApply) returnType).clazz; + returnType = cloneType(tdParent.getTreeMaker(), returnType, ast, annotationNode.getContext()); } if (builderClassName.isEmpty()) { if (returnType instanceof JCFieldAccess) { @@ -232,7 +232,16 @@ public class HandleBuilder extends JavacAnnotationHandler { if (Character.isLowerCase(builderClassName.charAt(0))) { builderClassName = Character.toTitleCase(builderClassName.charAt(0)) + builderClassName.substring(1); } - } else { + } else if (returnType instanceof JCTypeApply) { + JCExpression clazz = ((JCTypeApply) returnType).clazz; + if (clazz instanceof JCFieldAccess) { + builderClassName = ((JCFieldAccess) clazz).name + "Builder"; + } else if (clazz instanceof JCIdent) { + builderClassName = ((JCIdent) clazz).name + "Builder"; + } + } + + if (builderClassName.isEmpty()) { // This shouldn't happen. System.err.println("Lombok bug ID#20140614-1651: javac HandleBuilder: return type to name conversion failed: " + returnType.getClass()); builderClassName = td.name.toString() + "Builder"; @@ -253,11 +262,14 @@ public class HandleBuilder extends JavacAnnotationHandler { tpOnRet = ((JCTypeApply) fullReturnType).arguments; } - if (returnType instanceof JCIdent) { - simpleName = ((JCIdent) returnType).name; + JCExpression namingType = returnType; + if (returnType instanceof JCTypeApply) namingType = ((JCTypeApply) returnType).clazz; + + if (namingType instanceof JCIdent) { + simpleName = ((JCIdent) namingType).name; pkg = null; - } else if (returnType instanceof JCFieldAccess) { - JCFieldAccess jcfa = (JCFieldAccess) returnType; + } else if (namingType instanceof JCFieldAccess) { + JCFieldAccess jcfa = (JCFieldAccess) namingType; simpleName = jcfa.name; pkg = unpack(jcfa.selected); if (pkg.startsWith("ERR:")) { @@ -266,7 +278,7 @@ public class HandleBuilder extends JavacAnnotationHandler { return; } } else { - annotationNode.addError("Expected a (parameterized) type here instead of a " + returnType.getClass().getName()); + annotationNode.addError("Expected a (parameterized) type here instead of a " + namingType.getClass().getName()); return; } diff --git a/test/transform/resource/after-delombok/BuilderGenericMethod.java b/test/transform/resource/after-delombok/BuilderGenericMethod.java new file mode 100644 index 00000000..f70ae871 --- /dev/null +++ b/test/transform/resource/after-delombok/BuilderGenericMethod.java @@ -0,0 +1,40 @@ +import java.util.List; +import java.util.*; +class BuilderGenericMethod { + public Map foo(int a, long b) { + return null; + } + @java.lang.SuppressWarnings("all") + public class MapBuilder { + @java.lang.SuppressWarnings("all") + private int a; + @java.lang.SuppressWarnings("all") + private long b; + @java.lang.SuppressWarnings("all") + MapBuilder() { + } + @java.lang.SuppressWarnings("all") + public MapBuilder a(final int a) { + this.a = a; + return this; + } + @java.lang.SuppressWarnings("all") + public MapBuilder b(final long b) { + this.b = b; + return this; + } + @java.lang.SuppressWarnings("all") + public Map build() { + return BuilderGenericMethod.this.foo(a, b); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "BuilderGenericMethod.MapBuilder(a=" + this.a + ", b=" + this.b + ")"; + } + } + @java.lang.SuppressWarnings("all") + public MapBuilder builder() { + return new MapBuilder(); + } +} diff --git a/test/transform/resource/after-delombok/BuilderWithExistingBuilderClass.java b/test/transform/resource/after-delombok/BuilderWithExistingBuilderClass.java index 15293470..0a5edacd 100644 --- a/test/transform/resource/after-delombok/BuilderWithExistingBuilderClass.java +++ b/test/transform/resource/after-delombok/BuilderWithExistingBuilderClass.java @@ -24,7 +24,7 @@ class BuilderWithExistingBuilderClass { return this; } @java.lang.SuppressWarnings("all") - public BuilderWithExistingBuilderClass build() { + public BuilderWithExistingBuilderClass build() { return BuilderWithExistingBuilderClass.staticMethod(arg1, arg2, arg3); } @java.lang.Override diff --git a/test/transform/resource/after-delombok/BuilderWithToBuilder.java b/test/transform/resource/after-delombok/BuilderWithToBuilder.java index e2ce1966..46387f0f 100644 --- a/test/transform/resource/after-delombok/BuilderWithToBuilder.java +++ b/test/transform/resource/after-delombok/BuilderWithToBuilder.java @@ -165,7 +165,7 @@ class StaticWithToBuilder { return this; } @java.lang.SuppressWarnings("all") - public StaticWithToBuilder build() { + public StaticWithToBuilder build() { return StaticWithToBuilder.test(mOne, bar); } @java.lang.Override diff --git a/test/transform/resource/after-ecj/BuilderGenericMethod.java b/test/transform/resource/after-ecj/BuilderGenericMethod.java new file mode 100644 index 00000000..1b770654 --- /dev/null +++ b/test/transform/resource/after-ecj/BuilderGenericMethod.java @@ -0,0 +1,35 @@ +import java.util.List; +import lombok.Builder; +import java.util.*; +class BuilderGenericMethod { + public @java.lang.SuppressWarnings("all") class MapBuilder { + private @java.lang.SuppressWarnings("all") int a; + private @java.lang.SuppressWarnings("all") long b; + @java.lang.SuppressWarnings("all") MapBuilder() { + super(); + } + public @java.lang.SuppressWarnings("all") MapBuilder a(final int a) { + this.a = a; + return this; + } + public @java.lang.SuppressWarnings("all") MapBuilder b(final long b) { + this.b = b; + return this; + } + public @java.lang.SuppressWarnings("all") Map build() { + return BuilderGenericMethod.this.foo(a, b); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (((("BuilderGenericMethod.MapBuilder(a=" + this.a) + ", b=") + this.b) + ")"); + } + } + BuilderGenericMethod() { + super(); + } + public @Builder Map foo(int a, long b) { + return null; + } + public @java.lang.SuppressWarnings("all") MapBuilder builder() { + return new MapBuilder(); + } +} diff --git a/test/transform/resource/before/BuilderGenericMethod.java b/test/transform/resource/before/BuilderGenericMethod.java new file mode 100644 index 00000000..63dcb4db --- /dev/null +++ b/test/transform/resource/before/BuilderGenericMethod.java @@ -0,0 +1,11 @@ +import java.util.List; + +import lombok.Builder; +import java.util.*; + +class BuilderGenericMethod { + @Builder + public Map foo(int a, long b) { + return null; + } +} -- cgit