diff --git a/AUTHORS b/AUTHORS index 9ee773f5..a0130bc5 100755 --- a/AUTHORS +++ b/AUTHORS @@ -59,6 +59,7 @@ Rostislav Krasny <45571812+rosti-il@users.noreply.github.com> Samuel Pereira Sasha Koning Szymon Pacanowski +Taeeun Kim Taiki Sugawara Takuya Murakami Thomas Darimont diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java index 51aa2f21..64a3fa95 100755 --- a/src/core/lombok/eclipse/handlers/HandleBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java @@ -114,6 +114,7 @@ public class HandleBuilder extends EclipseAnnotationHandler { static final char[] BUILDER_TEMP_VAR = {'b', 'u', 'i', 'l', 'd', 'e', 'r'}; static final AbstractMethodDeclaration[] EMPTY_METHODS = {}; static final String TO_BUILDER_NOT_SUPPORTED = "@Builder(toBuilder=true) is only supported if you return your own type."; + static final String BUILDER_CLASS_NAME_ANNOTATION_CONFLICT = "builderClassName cannot be \"%s\" when using @%s; use @%s or choose another builder class name."; private static final boolean toBoolean(Object expr, boolean defaultValue) { if (expr == null) return defaultValue; @@ -122,6 +123,15 @@ public class HandleBuilder extends EclipseAnnotationHandler { return ((Boolean) expr).booleanValue(); } + static boolean checkBuilderClassNameAnnotationConflict(String annotationName, String qualifiedAnnotationName, String builderClassName, Annotation ast, EclipseNode annotationNode) { + char[][] typeName = ast.type == null ? null : ast.type.getTypeName(); + if (annotationName.equals(builderClassName) && (typeName == null || typeName.length == 1)) { + annotationNode.addError(String.format(BUILDER_CLASS_NAME_ANNOTATION_CONFLICT, builderClassName, annotationName, qualifiedAnnotationName)); + return false; + } + return true; + } + static class BuilderJob { CheckerFrameworkVersion checkerFramework; EclipseNode parentType; @@ -308,6 +318,9 @@ public class HandleBuilder extends EclipseAnnotationHandler { job.parentType = parent; TypeDeclaration td = (TypeDeclaration) parent.get(); + job.setBuilderClassName(job.replaceBuilderClassName(td.name)); + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("Builder", "lombok.Builder", job.builderClassName, ast, annotationNode)) return; List allFields = new ArrayList(); boolean valuePresent = (hasAnnotation(lombok.Value.class, parent) || hasAnnotation("lombok.experimental.Value", parent)); @@ -365,8 +378,6 @@ public class HandleBuilder extends EclipseAnnotationHandler { buildMethodReturnType = job.createBuilderParentTypeReference(); buildMethodThrownExceptions = null; nameOfBuilderMethod = null; - job.setBuilderClassName(job.replaceBuilderClassName(td.name)); - if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; } else if (parent.get() instanceof ConstructorDeclaration) { job.checkReturnValue = true; ConstructorDeclaration cd = (ConstructorDeclaration) parent.get(); @@ -478,6 +489,9 @@ public class HandleBuilder extends EclipseAnnotationHandler { return; } + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("Builder", "lombok.Builder", job.builderClassName, ast, annotationNode)) return; + if (fillParametersFrom != null) { for (EclipseNode param : fillParametersFrom.down()) { if (param.getKind() != Kind.ARGUMENT) continue; diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java index 7aeb03a6..36a7cf7a 100644 --- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java @@ -187,6 +187,12 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { job.parentType = parent; TypeDeclaration td = (TypeDeclaration) parent.get(); + job.builderAbstractClassName = job.builderClassName = job.replaceBuilderClassName(td.name); + job.builderAbstractClassNameArr = job.builderClassNameArr = job.builderAbstractClassName.toCharArray(); + job.builderImplClassName = job.builderAbstractClassName + "Impl"; + job.builderImplClassNameArr = job.builderImplClassName.toCharArray(); + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("SuperBuilder", "lombok.SuperBuilder", job.builderClassName, ast, annotationNode)) return; // Gather all fields of the class that should be set by the builder. List allFields = new ArrayList(); @@ -309,11 +315,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { superclassBuilderClass = new ParameterizedQualifiedTypeReference(tokens, typeArgsForTokens, 0, poss); } - job.builderAbstractClassName = job.builderClassName = job.replaceBuilderClassName(td.name); - job.builderAbstractClassNameArr = job.builderClassNameArr = job.builderAbstractClassName.toCharArray(); - job.builderImplClassName = job.builderAbstractClassName + "Impl"; - job.builderImplClassNameArr = job.builderImplClassName.toCharArray(); - // If there is no superclass, superclassBuilderClassExpression is still == null at this point. // You can use it to check whether to inherit or not. diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java index 595695cd..0b680624 100644 --- a/src/core/lombok/javac/handlers/HandleBuilder.java +++ b/src/core/lombok/javac/handlers/HandleBuilder.java @@ -96,6 +96,7 @@ public class HandleBuilder extends JavacAnnotationHandler { static final String VALUE_PREFIX = "$value"; static final String BUILDER_TEMP_VAR = "builder"; static final String TO_BUILDER_NOT_SUPPORTED = "@Builder(toBuilder=true) is only supported if you return your own type."; + static final String BUILDER_CLASS_NAME_ANNOTATION_CONFLICT = "builderClassName cannot be \"%s\" when using @%s; use @%s or choose another builder class name."; private static final boolean toBoolean(Object expr, boolean defaultValue) { if (expr == null) return defaultValue; @@ -103,6 +104,14 @@ public class HandleBuilder extends JavacAnnotationHandler { return ((Boolean) expr).booleanValue(); } + static boolean checkBuilderClassNameAnnotationConflict(String annotationName, String qualifiedAnnotationName, String builderClassName, JCAnnotation ast, JavacNode annotationNode) { + if (annotationName.equals(builderClassName) && !(ast.annotationType instanceof JCFieldAccess)) { + annotationNode.addError(String.format(BUILDER_CLASS_NAME_ANNOTATION_CONFLICT, builderClassName, annotationName, qualifiedAnnotationName)); + return false; + } + return true; + } + static class BuilderJob { CheckerFrameworkVersion checkerFramework; JavacNode parentType; @@ -251,6 +260,9 @@ public class HandleBuilder extends JavacAnnotationHandler { job.parentType = parent; job.checkReturnValue = true; JCClassDecl td = (JCClassDecl) parent.get(); + job.builderClassName = job.replaceBuilderClassName(td.name); + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("Builder", "lombok.Builder", job.builderClassName, ast, annotationNode)) return; ListBuffer allFields = new ListBuffer(); boolean valuePresent = (hasAnnotation(lombok.Value.class, parent) || hasAnnotation("lombok.experimental.Value", parent)); @@ -308,8 +320,6 @@ public class HandleBuilder extends JavacAnnotationHandler { job.typeParams = job.builderTypeParams = td.typarams; buildMethodThrownExceptions = List.nil(); nameOfBuilderMethod = null; - job.builderClassName = job.replaceBuilderClassName(td.name); - if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; } else if (fillParametersFrom != null && fillParametersFrom.getName().toString().equals("")) { JCMethodDecl jmd = (JCMethodDecl) fillParametersFrom.get(); if (!jmd.typarams.isEmpty()) { @@ -420,6 +430,9 @@ public class HandleBuilder extends JavacAnnotationHandler { return; } + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("Builder", "lombok.Builder", job.builderClassName, ast, annotationNode)) return; + if (fillParametersFrom != null) { for (JavacNode param : fillParametersFrom.down()) { if (param.getKind() != Kind.ARGUMENT) continue; diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java index fa0ba9dd..0b1fe686 100644 --- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java @@ -169,6 +169,9 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { job.parentType = parent; JCClassDecl td = (JCClassDecl) parent.get(); + job.builderClassName = job.replaceBuilderClassName(td.name); + if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; + if (!checkBuilderClassNameAnnotationConflict("SuperBuilder", "lombok.SuperBuilder", job.builderClassName, ast, annotationNode)) return; // Gather all fields of the class that should be set by the builder. ArrayList nonFinalNonDefaultedFields = null; @@ -217,8 +220,6 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { } job.typeParams = job.builderTypeParams = td.typarams; - job.builderClassName = job.replaceBuilderClassName(td.name); - if (!checkName("builderClassName", job.builderClassName, annotationNode)) return; // are the generics for our builder. String classGenericName = "C"; diff --git a/test/transform/resource/after-delombok/BuilderClassNameCollisionQualified.java b/test/transform/resource/after-delombok/BuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..f2be2f5c --- /dev/null +++ b/test/transform/resource/after-delombok/BuilderClassNameCollisionQualified.java @@ -0,0 +1,44 @@ +class BuilderClassNameCollisionQualified { + private java.util.function.Function mapper; + @java.lang.SuppressWarnings("all") + @lombok.Generated + BuilderClassNameCollisionQualified(final java.util.function.Function mapper) { + this.mapper = mapper; + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + public static class Builder { + @java.lang.SuppressWarnings("all") + @lombok.Generated + private java.util.function.Function mapper; + @java.lang.SuppressWarnings("all") + @lombok.Generated + Builder() { + } + /** + * @return {@code this}. + */ + @java.lang.SuppressWarnings("all") + @lombok.Generated + public BuilderClassNameCollisionQualified.Builder mapper(final java.util.function.Function mapper) { + this.mapper = mapper; + return this; + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + public BuilderClassNameCollisionQualified build() { + return new BuilderClassNameCollisionQualified(this.mapper); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + @lombok.Generated + public java.lang.String toString() { + return "BuilderClassNameCollisionQualified.Builder(mapper=" + this.mapper + ")"; + } + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + public static BuilderClassNameCollisionQualified.Builder builder() { + return new BuilderClassNameCollisionQualified.Builder(); + } +} diff --git a/test/transform/resource/after-delombok/SuperBuilderClassNameCollisionQualified.java b/test/transform/resource/after-delombok/SuperBuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..59cfe743 --- /dev/null +++ b/test/transform/resource/after-delombok/SuperBuilderClassNameCollisionQualified.java @@ -0,0 +1,61 @@ +class SuperBuilderClassNameCollisionQualified { + T value; + @java.lang.SuppressWarnings("all") + @lombok.Generated + public static abstract class SuperBuilder, B extends SuperBuilderClassNameCollisionQualified.SuperBuilder> { + @java.lang.SuppressWarnings("all") + @lombok.Generated + private T value; + /** + * @return {@code this}. + */ + @java.lang.SuppressWarnings("all") + @lombok.Generated + public B value(final T value) { + this.value = value; + return self(); + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + protected abstract B self(); + @java.lang.SuppressWarnings("all") + @lombok.Generated + public abstract C build(); + @java.lang.Override + @java.lang.SuppressWarnings("all") + @lombok.Generated + public java.lang.String toString() { + return "SuperBuilderClassNameCollisionQualified.SuperBuilder(value=" + this.value + ")"; + } + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + private static final class SuperBuilderImpl extends SuperBuilderClassNameCollisionQualified.SuperBuilder, SuperBuilderClassNameCollisionQualified.SuperBuilderImpl> { + @java.lang.SuppressWarnings("all") + @lombok.Generated + private SuperBuilderImpl() { + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + @lombok.Generated + protected SuperBuilderClassNameCollisionQualified.SuperBuilderImpl self() { + return this; + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + @lombok.Generated + public SuperBuilderClassNameCollisionQualified build() { + return new SuperBuilderClassNameCollisionQualified(this); + } + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + protected SuperBuilderClassNameCollisionQualified(final SuperBuilderClassNameCollisionQualified.SuperBuilder b) { + this.value = b.value; + } + @java.lang.SuppressWarnings("all") + @lombok.Generated + public static SuperBuilderClassNameCollisionQualified.SuperBuilder builder() { + return new SuperBuilderClassNameCollisionQualified.SuperBuilderImpl(); + } +} diff --git a/test/transform/resource/after-ecj/BuilderClassNameCollisionQualified.java b/test/transform/resource/after-ecj/BuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..f6cab45e --- /dev/null +++ b/test/transform/resource/after-ecj/BuilderClassNameCollisionQualified.java @@ -0,0 +1,29 @@ +@lombok.Builder(builderClassName = "Builder") class BuilderClassNameCollisionQualified { + public static @java.lang.SuppressWarnings("all") @lombok.Generated class Builder { + private @java.lang.SuppressWarnings("all") @lombok.Generated java.util.function.Function mapper; + @java.lang.SuppressWarnings("all") @lombok.Generated Builder() { + super(); + } + /** + * @return {@code this}. + */ + public @java.lang.SuppressWarnings("all") @lombok.Generated BuilderClassNameCollisionQualified.Builder mapper(final java.util.function.Function mapper) { + this.mapper = mapper; + return this; + } + public @java.lang.SuppressWarnings("all") @lombok.Generated BuilderClassNameCollisionQualified build() { + return new BuilderClassNameCollisionQualified(this.mapper); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") @lombok.Generated java.lang.String toString() { + return (("BuilderClassNameCollisionQualified.Builder(mapper=" + this.mapper) + ")"); + } + } + private java.util.function.Function mapper; + @java.lang.SuppressWarnings("all") @lombok.Generated BuilderClassNameCollisionQualified(final java.util.function.Function mapper) { + super(); + this.mapper = mapper; + } + public static @java.lang.SuppressWarnings("all") @lombok.Generated BuilderClassNameCollisionQualified.Builder builder() { + return new BuilderClassNameCollisionQualified.Builder(); + } +} diff --git a/test/transform/resource/after-ecj/SuperBuilderClassNameCollisionQualified.java b/test/transform/resource/after-ecj/SuperBuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..4d5c9adb --- /dev/null +++ b/test/transform/resource/after-ecj/SuperBuilderClassNameCollisionQualified.java @@ -0,0 +1,39 @@ +@lombok.SuperBuilder class SuperBuilderClassNameCollisionQualified { + public static abstract @java.lang.SuppressWarnings("all") @lombok.Generated class SuperBuilder, B extends SuperBuilderClassNameCollisionQualified.SuperBuilder> { + private @java.lang.SuppressWarnings("all") @lombok.Generated T value; + public SuperBuilder() { + super(); + } + /** + * @return {@code this}. + */ + public @java.lang.SuppressWarnings("all") @lombok.Generated B value(final T value) { + this.value = value; + return self(); + } + protected abstract @java.lang.SuppressWarnings("all") @lombok.Generated B self(); + public abstract @java.lang.SuppressWarnings("all") @lombok.Generated C build(); + public @java.lang.Override @java.lang.SuppressWarnings("all") @lombok.Generated java.lang.String toString() { + return (("SuperBuilderClassNameCollisionQualified.SuperBuilder(value=" + this.value) + ")"); + } + } + private static final @java.lang.SuppressWarnings("all") @lombok.Generated class SuperBuilderImpl extends SuperBuilderClassNameCollisionQualified.SuperBuilder, SuperBuilderClassNameCollisionQualified.SuperBuilderImpl> { + private SuperBuilderImpl() { + super(); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") @lombok.Generated SuperBuilderClassNameCollisionQualified.SuperBuilderImpl self() { + return this; + } + public @java.lang.Override @java.lang.SuppressWarnings("all") @lombok.Generated SuperBuilderClassNameCollisionQualified build() { + return new SuperBuilderClassNameCollisionQualified(this); + } + } + T value; + protected @java.lang.SuppressWarnings("all") @lombok.Generated SuperBuilderClassNameCollisionQualified(final SuperBuilderClassNameCollisionQualified.SuperBuilder b) { + super(); + this.value = b.value; + } + public static @java.lang.SuppressWarnings("all") @lombok.Generated SuperBuilderClassNameCollisionQualified.SuperBuilder builder() { + return new SuperBuilderClassNameCollisionQualified.SuperBuilderImpl(); + } +} diff --git a/test/transform/resource/before/BuilderClassNameCollision.java b/test/transform/resource/before/BuilderClassNameCollision.java new file mode 100644 index 00000000..89ca8c79 --- /dev/null +++ b/test/transform/resource/before/BuilderClassNameCollision.java @@ -0,0 +1,7 @@ +//skip compare content +import lombok.Builder; + +@Builder(builderClassName = "Builder") +class BuilderClassNameCollision { + T value; +} diff --git a/test/transform/resource/before/BuilderClassNameCollisionQualified.java b/test/transform/resource/before/BuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..493ac6a5 --- /dev/null +++ b/test/transform/resource/before/BuilderClassNameCollisionQualified.java @@ -0,0 +1,4 @@ +@lombok.Builder(builderClassName = "Builder") +class BuilderClassNameCollisionQualified { + private java.util.function.Function mapper; +} diff --git a/test/transform/resource/before/SuperBuilderClassNameCollision.java b/test/transform/resource/before/SuperBuilderClassNameCollision.java new file mode 100644 index 00000000..b708acc2 --- /dev/null +++ b/test/transform/resource/before/SuperBuilderClassNameCollision.java @@ -0,0 +1,8 @@ +//skip compare content +//CONF: lombok.builder.className = SuperBuilder +import lombok.SuperBuilder; + +@SuperBuilder +class SuperBuilderClassNameCollision { + int value; +} diff --git a/test/transform/resource/before/SuperBuilderClassNameCollisionQualified.java b/test/transform/resource/before/SuperBuilderClassNameCollisionQualified.java new file mode 100644 index 00000000..12394c0b --- /dev/null +++ b/test/transform/resource/before/SuperBuilderClassNameCollisionQualified.java @@ -0,0 +1,5 @@ +//CONF: lombok.builder.className = SuperBuilder +@lombok.SuperBuilder +class SuperBuilderClassNameCollisionQualified { + T value; +} diff --git a/test/transform/resource/messages-delombok/BuilderClassNameCollision.java.messages b/test/transform/resource/messages-delombok/BuilderClassNameCollision.java.messages new file mode 100644 index 00000000..0aa68c17 --- /dev/null +++ b/test/transform/resource/messages-delombok/BuilderClassNameCollision.java.messages @@ -0,0 +1 @@ +4 builderClassName cannot be "Builder" when using @Builder; use @lombok.Builder or choose another builder class name. diff --git a/test/transform/resource/messages-delombok/SuperBuilderClassNameCollision.java.messages b/test/transform/resource/messages-delombok/SuperBuilderClassNameCollision.java.messages new file mode 100644 index 00000000..1222e313 --- /dev/null +++ b/test/transform/resource/messages-delombok/SuperBuilderClassNameCollision.java.messages @@ -0,0 +1 @@ +5 builderClassName cannot be "SuperBuilder" when using @SuperBuilder; use @lombok.SuperBuilder or choose another builder class name. diff --git a/test/transform/resource/messages-ecj/BuilderClassNameCollision.java.messages b/test/transform/resource/messages-ecj/BuilderClassNameCollision.java.messages new file mode 100644 index 00000000..0aa68c17 --- /dev/null +++ b/test/transform/resource/messages-ecj/BuilderClassNameCollision.java.messages @@ -0,0 +1 @@ +4 builderClassName cannot be "Builder" when using @Builder; use @lombok.Builder or choose another builder class name. diff --git a/test/transform/resource/messages-ecj/SuperBuilderClassNameCollision.java.messages b/test/transform/resource/messages-ecj/SuperBuilderClassNameCollision.java.messages new file mode 100644 index 00000000..1222e313 --- /dev/null +++ b/test/transform/resource/messages-ecj/SuperBuilderClassNameCollision.java.messages @@ -0,0 +1 @@ +5 builderClassName cannot be "SuperBuilder" when using @SuperBuilder; use @lombok.SuperBuilder or choose another builder class name.