avoid name clash between classname and builder type param; fixes proj…

…ectlombok#2297
janrieke · Dec 13, 2019 · efd9d31 · efd9d31
1 parent 08faffd
commit efd9d31
Show file tree

Hide file tree

Showing 6 changed files with 240 additions and 10 deletions.
diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
@@ -222,12 +222,14 @@ public void handle(AnnotationValues<SuperBuilder> annotation, Annotation ast, Ec
 		// <C, B> 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.
+		// If these generics' names collide with any generics on the annotated class or the classname itself, modify them.
 		// For instance, if there are generics <B, B2, C> on the annotated class, use "C2" and "B3" for our builder.
-		java.util.List<String> typeParamStrings = new ArrayList<String>();
-		for (TypeParameter typeParam : typeParams) typeParamStrings.add(typeParam.toString());
-		classGenericName = generateNonclashingNameFor(classGenericName, typeParamStrings);
-		builderGenericName = generateNonclashingNameFor(builderGenericName, typeParamStrings);
+		// FIXME: Shouldn't we also add all classes from import statments? Otherwise importing a class "mypackage.B" would also break the code.
+		java.util.List<String> usedNames = new ArrayList<String>();
+		for (TypeParameter typeParam : typeParams) usedNames.add(typeParam.toString());
+		usedNames.add(String.valueOf(td.name));
+		classGenericName = generateNonclashingNameFor(classGenericName, usedNames);
+		builderGenericName = generateNonclashingNameFor(builderGenericName, usedNames);
 
 		TypeReference extendsClause = td.superclass;
 		TypeReference superclassBuilderClass = null;

diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
@@ -208,12 +208,14 @@ public void handle(AnnotationValues<SuperBuilder> annotation, JCAnnotation ast,
 		// <C, B> 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.
+		// If these generics' names collide with any generics on the annotated class or the classname itself, modify them.
 		// For instance, if there are generics <B, B2, C> on the annotated class, use "C2" and "B3" for our builder.
-		java.util.List<String> typeParamStrings = new ArrayList<String>();
-		for (JCTypeParameter typeParam : typeParams) typeParamStrings.add(typeParam.getName().toString());
-		classGenericName = generateNonclashingNameFor(classGenericName, typeParamStrings);
-		builderGenericName = generateNonclashingNameFor(builderGenericName, typeParamStrings);
+		// FIXME: Shouldn't we also add all classes from import statments? Otherwise importing a class "mypackage.B" would also break the code.
+		java.util.List<String> usedNames = new ArrayList<String>();
+		for (JCTypeParameter typeParam : typeParams) usedNames.add(typeParam.getName().toString());
+		usedNames.add(td.name.toString());
+		classGenericName = generateNonclashingNameFor(classGenericName, usedNames);
+		builderGenericName = generateNonclashingNameFor(builderGenericName, usedNames);
 
 		thrownExceptions = List.nil();
 

diff --git a/test/transform/resource/after-delombok/SuperBuilderNameClashes.java b/test/transform/resource/after-delombok/SuperBuilderNameClashes.java
@@ -0,0 +1,116 @@
+import java.util.List;
+public class SuperBuilderNameClashes {
+	public static class GenericsClash<B, C, C2> {
+		@java.lang.SuppressWarnings("all")
+		public static abstract class GenericsClashBuilder<B, C, C2, C3 extends GenericsClash<B, C, C2>, B2 extends GenericsClashBuilder<B, C, C2, C3, B2>> {
+			@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<B, C, C2> extends GenericsClashBuilder<B, C, C2, GenericsClash<B, C, C2>, GenericsClashBuilderImpl<B, C, C2>> {
+			@java.lang.SuppressWarnings("all")
+			private GenericsClashBuilderImpl() {
+			}
+			@java.lang.Override
+			@java.lang.SuppressWarnings("all")
+			protected GenericsClashBuilderImpl<B, C, C2> self() {
+				return this;
+			}
+			@java.lang.Override
+			@java.lang.SuppressWarnings("all")
+			public GenericsClash<B, C, C2> build() {
+				return new GenericsClash<B, C, C2>(this);
+			}
+		}
+		@java.lang.SuppressWarnings("all")
+		protected GenericsClash(final GenericsClashBuilder<B, C, C2, ?, ?> b) {
+		}
+		@java.lang.SuppressWarnings("all")
+		public static <B, C, C2> GenericsClashBuilder<B, C, C2, ?, ?> builder() {
+			return new GenericsClashBuilderImpl<B, C, C2>();
+		}
+	}
+	public static class B {
+		@java.lang.SuppressWarnings("all")
+		public static abstract class BBuilder<C extends B, B2 extends BBuilder<C, B2>> {
+			@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<B, BBuilderImpl> {
+			@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 C {
+		@java.lang.SuppressWarnings("all")
+		public static abstract class CBuilder<C2 extends C, B extends CBuilder<C2, B>> {
+			@java.lang.SuppressWarnings("all")
+			protected abstract B self();
+			@java.lang.SuppressWarnings("all")
+			public abstract C2 build();
+			@java.lang.Override
+			@java.lang.SuppressWarnings("all")
+			public java.lang.String toString() {
+				return "SuperBuilderNameClashes.C.CBuilder()";
+			}
+		}
+		@java.lang.SuppressWarnings("all")
+		private static final class CBuilderImpl extends CBuilder<C, CBuilderImpl> {
+			@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) {
+		}
+		@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
@@ -0,0 +1,93 @@
+import java.util.List;
+public class SuperBuilderNameClashes {
+  public static @lombok.experimental.SuperBuilder class GenericsClash<B, C, C2> {
+    public static abstract @java.lang.SuppressWarnings("all") class GenericsClashBuilder<B, C, C2, C3 extends GenericsClash<B, C, C2>, B2 extends GenericsClashBuilder<B, C, C2, C3, B2>> {
+      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<B, C, C2> extends GenericsClashBuilder<B, C, C2, GenericsClash<B, C, C2>, GenericsClashBuilderImpl<B, C, C2>> {
+      private GenericsClashBuilderImpl() {
+        super();
+      }
+      protected @java.lang.Override @java.lang.SuppressWarnings("all") GenericsClashBuilderImpl<B, C, C2> self() {
+        return this;
+      }
+      public @java.lang.Override @java.lang.SuppressWarnings("all") GenericsClash<B, C, C2> build() {
+        return new GenericsClash<B, C, C2>(this);
+      }
+    }
+    protected @java.lang.SuppressWarnings("all") GenericsClash(final GenericsClashBuilder<B, C, C2, ?, ?> b) {
+      super();
+    }
+    public static @java.lang.SuppressWarnings("all") <B, C, C2>GenericsClashBuilder<B, C, C2, ?, ?> builder() {
+      return new GenericsClashBuilderImpl<B, C, C2>();
+    }
+  }
+  public static @lombok.experimental.SuperBuilder class B {
+    public static abstract @java.lang.SuppressWarnings("all") class BBuilder<C extends B, B2 extends BBuilder<C, B2>> {
+      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<B, BBuilderImpl> {
+      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 @lombok.experimental.SuperBuilder class C {
+    public static abstract @java.lang.SuppressWarnings("all") class CBuilder<C2 extends C, B extends CBuilder<C2, B>> {
+      public CBuilder() {
+        super();
+      }
+      protected abstract @java.lang.SuppressWarnings("all") B self();
+      public abstract @java.lang.SuppressWarnings("all") C2 build();
+      public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+        return "SuperBuilderNameClashes.C.CBuilder()";
+      }
+    }
+    private static final @java.lang.SuppressWarnings("all") class CBuilderImpl extends CBuilder<C, CBuilderImpl> {
+      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);
+      }
+    }
+    protected @java.lang.SuppressWarnings("all") C(final CBuilder<?, ?> b) {
+      super();
+    }
+    public static @java.lang.SuppressWarnings("all") CBuilder<?, ?> builder() {
+      return new CBuilderImpl();
+    }
+  }
+  public SuperBuilderNameClashes() {
+    super();
+  }
+}
diff --git a/test/transform/resource/before/SuperBuilderNameClashes.java b/test/transform/resource/before/SuperBuilderNameClashes.java
@@ -0,0 +1,15 @@
+import java.util.List;
+
+public class SuperBuilderNameClashes {
+	@lombok.experimental.SuperBuilder
+	public static class GenericsClash<B, C, C2> {
+	}
+
+	@lombok.experimental.SuperBuilder
+	public static class B {
+	}
+
+	@lombok.experimental.SuperBuilder
+	public static class C {
+	}
+}
diff --git a/test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages b/test/transform/resource/messages-ecj/SuperBuilderNameClashes.java.messages
@@ -0,0 +1,2 @@
+5 WARNING The type parameter B is hiding the type SuperBuilderNameClashes.B
+5 WARNING The type parameter C is hiding the type SuperBuilderNameClashes.C