From 4a34ed75b49f259823739df5691f1d81f37c12ca Mon Sep 17 00:00:00 2001 From: Jan Rieke Date: Fri, 13 Dec 2019 23:32:16 +0100 Subject: avoid more name clashes for builder type param; fixes #2297 --- .../eclipse/handlers/HandleSuperBuilder.java | 37 ++++-- .../lombok/javac/handlers/HandleSuperBuilder.java | 35 +++++- .../after-delombok/SuperBuilderNameClashes.java | 126 +++++++++++++++++++++ .../after-ecj/SuperBuilderNameClashes.java | 104 +++++++++++++++++ .../resource/before/SuperBuilderNameClashes.java | 17 +++ .../SuperBuilderNameClashes.java.messages | 3 + 6 files changed, 309 insertions(+), 13 deletions(-) mode change 100755 => 100644 src/core/lombok/eclipse/handlers/HandleSuperBuilder.java create mode 100644 test/transform/resource/after-delombok/SuperBuilderNameClashes.java create mode 100644 test/transform/resource/after-ecj/SuperBuilderNameClashes.java create mode 100644 test/transform/resource/before/SuperBuilderNameClashes.java create mode 100644 test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java old mode 100755 new mode 100644 index 306c6439..12fa9b2c --- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java @@ -29,6 +29,7 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import org.eclipse.jdt.internal.compiler.ast.ASTNode; @@ -222,12 +223,12 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { // are the generics for our builder. String classGenericName = "C"; String builderGenericName = "B"; - // If these generics' names collide with any generics on the annotated class, modify them. + // We have to make sure that the generics' names do not collide with any generics on the annotated class, + // the classname itself, or any member type name of the annotated class. // For instance, if there are generics on the annotated class, use "C2" and "B3" for our builder. - java.util.List typeParamStrings = new ArrayList(); - for (TypeParameter typeParam : typeParams) typeParamStrings.add(typeParam.toString()); - classGenericName = generateNonclashingNameFor(classGenericName, typeParamStrings); - builderGenericName = generateNonclashingNameFor(builderGenericName, typeParamStrings); + java.util.Set usedNames = gatherUsedTypeNames(typeParams, td); + classGenericName = generateNonclashingNameFor(classGenericName, usedNames); + builderGenericName = generateNonclashingNameFor(builderGenericName, usedNames); TypeReference extendsClause = td.superclass; TypeReference superclassBuilderClass = null; @@ -415,7 +416,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { } } } - + private EclipseNode generateBuilderAbstractClass(EclipseNode tdParent, String builderClass, TypeReference superclassBuilderClass, TypeParameter[] typeParams, ASTNode source, String classGenericName, String builderGenericName) { @@ -1052,7 +1053,29 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler { return null; } - private String generateNonclashingNameFor(String classGenericName, java.util.List typeParamStrings) { + private java.util.Set gatherUsedTypeNames(TypeParameter[] typeParams, TypeDeclaration td) { + java.util.HashSet usedNames = new HashSet(); + + // 1. Add type parameter names. + for (TypeParameter typeParam : typeParams) + usedNames.add(typeParam.toString()); + + // 2. Add class name. + usedNames.add(String.valueOf(td.name)); + + // 3. Add used type names. + if (td.fields != null) { + for (FieldDeclaration field : td.fields) { + char[][] typeName = field.type.getTypeName(); + if (typeName.length >= 1) // Add the first token, because only that can collide. + usedNames.add(String.valueOf(typeName[0])); + } + } + + return usedNames; + } + + private String generateNonclashingNameFor(String classGenericName, java.util.Set typeParamStrings) { if (!typeParamStrings.contains(classGenericName)) return classGenericName; int counter = 2; while (typeParamStrings.contains(classGenericName + counter)) counter++; diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java index ba330325..650b0964 100644 --- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java +++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java @@ -27,6 +27,7 @@ import static lombok.javac.handlers.JavacHandlerUtil.*; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import javax.lang.model.element.Modifier; @@ -209,12 +210,12 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { // are the generics for our builder. String classGenericName = "C"; String builderGenericName = "B"; - // If these generics' names collide with any generics on the annotated class, modify them. + // We have to make sure that the generics' names do not collide with any generics on the annotated class, + // the classname itself, or any member type name of the annotated class. // For instance, if there are generics on the annotated class, use "C2" and "B3" for our builder. - java.util.List typeParamStrings = new ArrayList(); - for (JCTypeParameter typeParam : typeParams) typeParamStrings.add(typeParam.getName().toString()); - classGenericName = generateNonclashingNameFor(classGenericName, typeParamStrings); - builderGenericName = generateNonclashingNameFor(builderGenericName, typeParamStrings); + java.util.HashSet usedNames = gatherUsedTypeNames(typeParams, td); + classGenericName = generateNonclashingNameFor(classGenericName, usedNames); + builderGenericName = generateNonclashingNameFor(builderGenericName, usedNames); thrownExceptions = List.nil(); @@ -987,7 +988,29 @@ public class HandleSuperBuilder extends JavacAnnotationHandler { return null; } - private String generateNonclashingNameFor(String classGenericName, java.util.List typeParamStrings) { + private java.util.HashSet gatherUsedTypeNames(List typeParams, JCClassDecl td) { + java.util.HashSet usedNames = new HashSet(); + + // 1. Add type parameter names. + for (JCTypeParameter typeParam : typeParams) + usedNames.add(typeParam.getName().toString()); + + // 2. Add class name. + usedNames.add(td.name.toString()); + + // 3. Add used type names. + for (JCTree member : td.getMembers()) { + if (member.getKind() == com.sun.source.tree.Tree.Kind.VARIABLE && member instanceof JCVariableDecl) { + JCTree type = ((JCVariableDecl)member).getType(); + if (type instanceof JCIdent) + usedNames.add(((JCIdent)type).getName().toString()); + } + } + + return usedNames; + } + + private String generateNonclashingNameFor(String classGenericName, java.util.HashSet typeParamStrings) { if (!typeParamStrings.contains(classGenericName)) return classGenericName; int counter = 2; while (typeParamStrings.contains(classGenericName + counter)) counter++; diff --git a/test/transform/resource/after-delombok/SuperBuilderNameClashes.java b/test/transform/resource/after-delombok/SuperBuilderNameClashes.java new file mode 100644 index 00000000..a33fd6b4 --- /dev/null +++ b/test/transform/resource/after-delombok/SuperBuilderNameClashes.java @@ -0,0 +1,126 @@ +public class SuperBuilderNameClashes { + public static class GenericsClash { + @java.lang.SuppressWarnings("all") + public static abstract class GenericsClashBuilder, B2 extends GenericsClashBuilder> { + @java.lang.SuppressWarnings("all") + protected abstract B2 self(); + @java.lang.SuppressWarnings("all") + public abstract C3 build(); + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "SuperBuilderNameClashes.GenericsClash.GenericsClashBuilder()"; + } + } + @java.lang.SuppressWarnings("all") + private static final class GenericsClashBuilderImpl extends GenericsClashBuilder, GenericsClashBuilderImpl> { + @java.lang.SuppressWarnings("all") + private GenericsClashBuilderImpl() { + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected GenericsClashBuilderImpl self() { + return this; + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public GenericsClash build() { + return new GenericsClash(this); + } + } + @java.lang.SuppressWarnings("all") + protected GenericsClash(final GenericsClashBuilder b) { + } + @java.lang.SuppressWarnings("all") + public static GenericsClashBuilder builder() { + return new GenericsClashBuilderImpl(); + } + } + public static class B { + @java.lang.SuppressWarnings("all") + public static abstract class BBuilder> { + @java.lang.SuppressWarnings("all") + protected abstract B2 self(); + @java.lang.SuppressWarnings("all") + public abstract C build(); + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "SuperBuilderNameClashes.B.BBuilder()"; + } + } + @java.lang.SuppressWarnings("all") + private static final class BBuilderImpl extends BBuilder { + @java.lang.SuppressWarnings("all") + private BBuilderImpl() { + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected BBuilderImpl self() { + return this; + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public B build() { + return new B(this); + } + } + @java.lang.SuppressWarnings("all") + protected B(final BBuilder b) { + } + @java.lang.SuppressWarnings("all") + public static BBuilder builder() { + return new BBuilderImpl(); + } + } + public static class C2 { + } + public static class C { + C2 c2; + @java.lang.SuppressWarnings("all") + public static abstract class CBuilder> { + @java.lang.SuppressWarnings("all") + private C2 c2; + @java.lang.SuppressWarnings("all") + protected abstract B self(); + @java.lang.SuppressWarnings("all") + public abstract C3 build(); + @java.lang.SuppressWarnings("all") + public B c2(final C2 c2) { + this.c2 = c2; + return self(); + } + @java.lang.Override + @java.lang.SuppressWarnings("all") + public java.lang.String toString() { + return "SuperBuilderNameClashes.C.CBuilder(c2=" + this.c2 + ")"; + } + } + @java.lang.SuppressWarnings("all") + private static final class CBuilderImpl extends CBuilder { + @java.lang.SuppressWarnings("all") + private CBuilderImpl() { + } + + @java.lang.Override + @java.lang.SuppressWarnings("all") + protected CBuilderImpl self() { + return this; + } + + @java.lang.Override + @java.lang.SuppressWarnings("all") + public C build() { + return new C(this); + } + } + @java.lang.SuppressWarnings("all") + protected C(final CBuilder b) { + this.c2 = b.c2; + } + @java.lang.SuppressWarnings("all") + public static CBuilder builder() { + return new CBuilderImpl(); + } + } +} diff --git a/test/transform/resource/after-ecj/SuperBuilderNameClashes.java b/test/transform/resource/after-ecj/SuperBuilderNameClashes.java new file mode 100644 index 00000000..dd238b5d --- /dev/null +++ b/test/transform/resource/after-ecj/SuperBuilderNameClashes.java @@ -0,0 +1,104 @@ +public class SuperBuilderNameClashes { + public static @lombok.experimental.SuperBuilder class GenericsClash { + public static abstract @java.lang.SuppressWarnings("all") class GenericsClashBuilder, B2 extends GenericsClashBuilder> { + public GenericsClashBuilder() { + super(); + } + protected abstract @java.lang.SuppressWarnings("all") B2 self(); + public abstract @java.lang.SuppressWarnings("all") C3 build(); + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return "SuperBuilderNameClashes.GenericsClash.GenericsClashBuilder()"; + } + } + private static final @java.lang.SuppressWarnings("all") class GenericsClashBuilderImpl extends GenericsClashBuilder, GenericsClashBuilderImpl> { + private GenericsClashBuilderImpl() { + super(); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") GenericsClashBuilderImpl self() { + return this; + } + public @java.lang.Override @java.lang.SuppressWarnings("all") GenericsClash build() { + return new GenericsClash(this); + } + } + protected @java.lang.SuppressWarnings("all") GenericsClash(final GenericsClashBuilder b) { + super(); + } + public static @java.lang.SuppressWarnings("all") GenericsClashBuilder builder() { + return new GenericsClashBuilderImpl(); + } + } + public static @lombok.experimental.SuperBuilder class B { + public static abstract @java.lang.SuppressWarnings("all") class BBuilder> { + public BBuilder() { + super(); + } + protected abstract @java.lang.SuppressWarnings("all") B2 self(); + public abstract @java.lang.SuppressWarnings("all") C build(); + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return "SuperBuilderNameClashes.B.BBuilder()"; + } + } + private static final @java.lang.SuppressWarnings("all") class BBuilderImpl extends BBuilder { + private BBuilderImpl() { + super(); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") BBuilderImpl self() { + return this; + } + public @java.lang.Override @java.lang.SuppressWarnings("all") B build() { + return new B(this); + } + } + protected @java.lang.SuppressWarnings("all") B(final BBuilder b) { + super(); + } + public static @java.lang.SuppressWarnings("all") BBuilder builder() { + return new BBuilderImpl(); + } + } + public static class C2 { + public C2() { + super(); + } + } + public static @lombok.experimental.SuperBuilder class C { + public static abstract @java.lang.SuppressWarnings("all") class CBuilder> { + private @java.lang.SuppressWarnings("all") C2 c2; + public CBuilder() { + super(); + } + protected abstract @java.lang.SuppressWarnings("all") B self(); + public abstract @java.lang.SuppressWarnings("all") C3 build(); + public @java.lang.SuppressWarnings("all") B c2(final C2 c2) { + this.c2 = c2; + return self(); + } + public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() { + return (("SuperBuilderNameClashes.C.CBuilder(c2=" + this.c2) + ")"); + } + } + private static final @java.lang.SuppressWarnings("all") class CBuilderImpl extends CBuilder { + private CBuilderImpl() { + super(); + } + protected @java.lang.Override @java.lang.SuppressWarnings("all") CBuilderImpl self() { + return this; + } + public @java.lang.Override @java.lang.SuppressWarnings("all") C build() { + return new C(this); + } + } + C2 c2; + protected @java.lang.SuppressWarnings("all") C(final CBuilder b) { + super(); + this.c2 = b.c2; + } + public static @java.lang.SuppressWarnings("all") CBuilder builder() { + return new CBuilderImpl(); + } + } + public SuperBuilderNameClashes() { + super(); + } +} \ No newline at end of file diff --git a/test/transform/resource/before/SuperBuilderNameClashes.java b/test/transform/resource/before/SuperBuilderNameClashes.java new file mode 100644 index 00000000..a0b58452 --- /dev/null +++ b/test/transform/resource/before/SuperBuilderNameClashes.java @@ -0,0 +1,17 @@ +public class SuperBuilderNameClashes { + @lombok.experimental.SuperBuilder + public static class GenericsClash { + } + + @lombok.experimental.SuperBuilder + public static class B { + } + + public static class C2 { + } + + @lombok.experimental.SuperBuilder + public static class C { + C2 c2; + } +} diff --git a/test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages b/test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages new file mode 100644 index 00000000..59774b92 --- /dev/null +++ b/test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages @@ -0,0 +1,3 @@ +3 WARNING The type parameter B is hiding the type SuperBuilderNameClashes.B +3 WARNING The type parameter C is hiding the type SuperBuilderNameClashes.C +3 WARNING The type parameter C2 is hiding the type SuperBuilderNameClashes.C2 \ No newline at end of file -- cgit