From a454fd74cf040b3aa4f061377478d784bd2157ff Mon Sep 17 00:00:00 2001 From: Jan Rieke Date: Mon, 24 Sep 2018 17:48:03 +0200 Subject: SuperBuilder: allow customization of selected methods --- .../eclipse/handlers/HandleSuperBuilder.java | 35 +++++-- .../lombok/javac/handlers/HandleSuperBuilder.java | 34 ++++++- .../after-delombok/SuperBuilderCustomized.java | 102 +++++++++++++++++++++ .../resource/after-ecj/SuperBuilderCustomized.java | 86 +++++++++++++++++ .../resource/before/SuperBuilderCustomized.java | 33 +++++++ 5 files changed, 278 insertions(+), 12 deletions(-) create mode 100644 test/transform/resource/after-delombok/SuperBuilderCustomized.java create mode 100644 test/transform/resource/after-ecj/SuperBuilderCustomized.java create mode 100644 test/transform/resource/before/SuperBuilderCustomized.java diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java index 97e38904..9a3275c2 100644 --- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java @@ -259,14 +259,28 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { generateBuilderBasedConstructor(tdParent, typeParams, builderFields, annotationNode, builderClassName, superclassBuilderClass != null); - // Create the abstract builder class. + // Create the abstract builder class, or reuse an existing one. EclipseNode builderType = findInnerClass(tdParent, builderClassName); if (builderType == null) { builderType = generateBuilderAbstractClass(tdParent, builderClassName, superclassBuilderClass, typeParams, ast, classGenericName, builderGenericName); } else { - annotationNode.addError("@SuperBuilder does not support customized builders. Use @Builder instead."); - return; + TypeDeclaration builderTypeDeclaration = (TypeDeclaration) builderType.get(); + if ((builderTypeDeclaration.modifiers & (ClassFileConstants.AccStatic | ClassFileConstants.AccAbstract)) == 0) { + annotationNode.addError("Existing Builder must be an abstract static inner class."); + return; + } + sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderType, annotationNode); + // Generate errors for @Singular BFDs that have one already defined node. + for (BuilderFieldData bfd : builderFields) { + SingularData sd = bfd.singularData; + if (sd == null) continue; + EclipseSingularizer singularizer = sd.getSingularizer(); + if (singularizer == null) continue; + if (singularizer.checkForAlreadyExistingNodesAndGenerateError(builderType, sd)) { + bfd.singularData = null; + } + } } // Check validity of @ObtainVia fields, and add check if adding cleaning for @Singular is necessary. @@ -338,13 +352,18 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { return; } - // Create the builder implementation class. + // Create the builder implementation class, or reuse an existing one. EclipseNode builderImplType = findInnerClass(tdParent, builderImplClassName); if (builderImplType == null) { builderImplType = generateBuilderImplClass(tdParent, builderImplClassName, builderClassName, typeParams, ast); } else { - annotationNode.addError("@SuperBuilder does not support customized builders. Use @Builder instead."); - return; + TypeDeclaration builderImplTypeDeclaration = (TypeDeclaration) builderImplType.get(); + if ((builderImplTypeDeclaration.modifiers & ClassFileConstants.AccAbstract) != 0 || + (builderImplTypeDeclaration.modifiers & ClassFileConstants.AccStatic) == 0) { + annotationNode.addError("Existing BuilderImpl must be a non-abstract static inner class."); + return; + } + sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderImplType, annotationNode); } if (toBuilder) { @@ -362,7 +381,9 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { // Create the self() and build() methods in the BuilderImpl. injectMethod(builderImplType, generateSelfMethod(builderImplType, typeParams, p)); - injectMethod(builderImplType, generateBuildMethod(tdParent, buildMethodName, returnType, ast)); + if (methodExists(buildMethodName, builderImplType, -1) == MemberExistsResult.NOT_EXISTS) { + injectMethod(builderImplType, generateBuildMethod(tdParent, buildMethodName, returnType, ast)); + } // Add the builder() method to the annotated class. if (methodExists(builderMethodName, tdParent, -1) == MemberExistsResult.NOT_EXISTS) { diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java index c84988ea..8af0a7c0 100644 --- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java @@ -27,6 +27,8 @@ import static lombok.javac.handlers.JavacHandlerUtil.*; import java.util.ArrayList; +import javax.lang.model.element.Modifier; + import org.mangosdk.spi.ProviderFor; import com.sun.tools.javac.code.BoundKind; @@ -227,8 +229,23 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { builderType = generateBuilderAbstractClass(annotationNode, tdParent, builderClassName, superclassBuilderClassExpression, typeParams, superclassTypeParams, classGenericName, builderGenericName); } else { - annotationNode.addError("@SuperBuilder does not support customized builders. Use @Builder instead."); - return; + JCClassDecl builderTypeDeclaration = (JCClassDecl) builderType.get(); + if (!builderTypeDeclaration.getModifiers().getFlags().contains(Modifier.STATIC) + || !builderTypeDeclaration.getModifiers().getFlags().contains(Modifier.ABSTRACT)) { + annotationNode.addError("Existing Builder must be an abstract static inner class."); + return; + } + sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderType, annotationNode); + // Generate errors for @Singular BFDs that have one already defined node. + for (BuilderFieldData bfd : builderFields) { + SingularData sd = bfd.singularData; + if (sd == null) continue; + JavacSingularizer singularizer = sd.getSingularizer(); + if (singularizer == null) continue; + if (singularizer.checkForAlreadyExistingNodesAndGenerateError(builderType, sd)) { + bfd.singularData = null; + } + } } // Generate the fields in the abstract builder class that hold the values for the instance. @@ -281,8 +298,13 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { if (builderImplType == null) { builderImplType = generateBuilderImplClass(annotationNode, tdParent, builderImplClassName, builderClassName, typeParams); } else { - annotationNode.addError("@SuperBuilder does not support customized builders. Use @Builder instead."); - return; + JCClassDecl builderImplTypeDeclaration = (JCClassDecl) builderImplType.get(); + if (!builderImplTypeDeclaration.getModifiers().getFlags().contains(Modifier.STATIC) + || builderImplTypeDeclaration.getModifiers().getFlags().contains(Modifier.ABSTRACT)) { + annotationNode.addError("Existing BuilderImpl must be a non-abstract static inner class."); + return; + } + sanityCheckForMethodGeneratingAnnotationsOnBuilderClass(builderImplType, annotationNode); } // Create a simple constructor for the BuilderImpl class. @@ -291,7 +313,9 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { // Create the self() and build() methods in the BuilderImpl. injectMethod(builderImplType, generateSelfMethod(builderImplType, typeParams)); - injectMethod(builderImplType, generateBuildMethod(buildMethodName, tdParent, builderImplType, thrownExceptions)); + if (methodExists(buildMethodName, builderImplType, -1) == MemberExistsResult.NOT_EXISTS) { + injectMethod(builderImplType, generateBuildMethod(buildMethodName, tdParent, builderImplType, thrownExceptions)); + } recursiveSetGeneratedBy(builderImplType.get(), ast, annotationNode.getContext()); } diff --git a/test/transform/resource/after-delombok/SuperBuilderCustomized.java b/test/transform/resource/after-delombok/SuperBuilderCustomized.java new file mode 100644 index 00000000..dcc2613f --- /dev/null +++ b/test/transform/resource/after-delombok/SuperBuilderCustomized.java @@ -0,0 +1,102 @@ +import java.util.List; +public class SuperBuilderCustomized { + public static class Parent { + public static abstract class ParentBuilder> { + @java.lang.SuppressWarnings("all") + private int field1; + public B resetToDefault() { + field1 = 0; + return self(); + } + @java.lang.SuppressWarnings("all") + protected abstract B self(); + @java.lang.SuppressWarnings("all") + public abstract C build(); + @java.lang.SuppressWarnings("all") + public B field1(final int field1) { + this.field1 = field1; + return self(); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "SuperBuilderCustomized.Parent.ParentBuilder(field1=" + this.field1 + ")"; + } + } + int field1; + @java.lang.SuppressWarnings("all") + private static final class ParentBuilderImpl extends ParentBuilder { + @java.lang.SuppressWarnings("all") + private ParentBuilderImpl() { + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected ParentBuilderImpl self() { + return this; + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public Parent build() { + return new Parent(this); + } + } + @java.lang.SuppressWarnings("all") + protected Parent(final ParentBuilder b) { + this.field1 = b.field1; + } + @java.lang.SuppressWarnings("all") + public static ParentBuilder builder() { + return new ParentBuilderImpl(); + } + } + public static class Child extends Parent { + private static final class ChildBuilderImpl extends ChildBuilder { + @Override + public Child build() { + this.resetToDefault(); + return new Child(this); + } + @java.lang.SuppressWarnings("all") + private ChildBuilderImpl() { + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected ChildBuilderImpl self() { + return this; + } + } + double field2; + public static ChildBuilder builder() { + return new ChildBuilderImpl().field2(10.0); + } + @java.lang.SuppressWarnings("all") + public static abstract class ChildBuilder> extends Parent.ParentBuilder { + @java.lang.SuppressWarnings("all") + private double field2; + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected abstract B self(); + @java.lang.Override + @java.lang.SuppressWarnings("all") + public abstract C build(); + @java.lang.SuppressWarnings("all") + public B field2(final double field2) { + this.field2 = field2; + return self(); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "SuperBuilderCustomized.Child.ChildBuilder(super=" + super.toString() + ", field2=" + this.field2 + ")"; + } + } + @java.lang.SuppressWarnings("all") + protected Child(final ChildBuilder b) { + super(b); + this.field2 = b.field2; + } + } + public static void test() { + Child x = Child.builder().field2(1.0).field1(5).resetToDefault().build(); + } +} diff --git a/test/transform/resource/after-ecj/SuperBuilderCustomized.java b/test/transform/resource/after-ecj/SuperBuilderCustomized.java new file mode 100644 index 00000000..7336a662 --- /dev/null +++ b/test/transform/resource/after-ecj/SuperBuilderCustomized.java @@ -0,0 +1,86 @@ +import java.util.List; +public class SuperBuilderCustomized { + public static @lombok.experimental.SuperBuilder class Parent { + public static abstract class ParentBuilder> { + private @java.lang.SuppressWarnings("all") int field1; + public ParentBuilder() { + super(); + } + public B resetToDefault() { + field1 = 0; + return self(); + } + protected abstract @java.lang.SuppressWarnings("all") B self(); + public abstract @java.lang.SuppressWarnings("all") C build(); + public @java.lang.SuppressWarnings("all") B field1(final int field1) { + this.field1 = field1; + return self(); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (("SuperBuilderCustomized.Parent.ParentBuilder(field1=" + this.field1) + ")"); + } + } + private static final @java.lang.SuppressWarnings("all") class ParentBuilderImpl extends ParentBuilder { + private ParentBuilderImpl() { + super(); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") ParentBuilderImpl self() { + return this; + } + public @java.lang.Override @java.lang.SuppressWarnings("all") Parent build() { + return new Parent(this); + } + } + int field1; + protected @java.lang.SuppressWarnings("all") Parent(final ParentBuilder b) { + super(); + this.field1 = b.field1; + } + public static @java.lang.SuppressWarnings("all") ParentBuilder builder() { + return new ParentBuilderImpl(); + } + } + public static @lombok.experimental.SuperBuilder class Child extends Parent { + private static final class ChildBuilderImpl extends ChildBuilder { + private ChildBuilderImpl() { + super(); + } + public @Override Child build() { + this.resetToDefault(); + return new Child(this); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") ChildBuilderImpl self() { + return this; + } + } + public static abstract @java.lang.SuppressWarnings("all") class ChildBuilder> extends Parent.ParentBuilder { + private @java.lang.SuppressWarnings("all") double field2; + public ChildBuilder() { + super(); + } + protected abstract @java.lang.Override @java.lang.SuppressWarnings("all") B self(); + public abstract @java.lang.Override @java.lang.SuppressWarnings("all") C build(); + public @java.lang.SuppressWarnings("all") B field2(final double field2) { + this.field2 = field2; + return self(); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (((("SuperBuilderCustomized.Child.ChildBuilder(super=" + super.toString()) + ", field2=") + this.field2) + ")"); + } + } + double field2; + public static ChildBuilder builder() { + return new ChildBuilderImpl().field2(10.0); + } + protected @java.lang.SuppressWarnings("all") Child(final ChildBuilder b) { + super(b); + this.field2 = b.field2; + } + } + public SuperBuilderCustomized() { + super(); + } + public static void test() { + Child x = Child.builder().field2(1.0).field1(5).resetToDefault().build(); + } +} diff --git a/test/transform/resource/before/SuperBuilderCustomized.java b/test/transform/resource/before/SuperBuilderCustomized.java new file mode 100644 index 00000000..58f2797c --- /dev/null +++ b/test/transform/resource/before/SuperBuilderCustomized.java @@ -0,0 +1,33 @@ +import java.util.List; + +public class SuperBuilderCustomized { + @lombok.experimental.SuperBuilder + public static class Parent { + public static abstract class ParentBuilder> { + public B resetToDefault() { + field1 = 0; + return self(); + } + } + int field1; + } + + @lombok.experimental.SuperBuilder + public static class Child extends Parent { + private static final class ChildBuilderImpl extends ChildBuilder { + @Override + public Child build() { + this.resetToDefault(); + return new Child(this); + } + } + double field2; + public static ChildBuilder builder() { + return new ChildBuilderImpl().field2(10.0); + } + } + + public static void test() { + Child x = Child.builder().field2(1.0).field1(5).resetToDefault().build(); + } +} -- cgit