Harmonize generated class name conventions

This commit moves the responsibility of naming classes to the
GenerationContext. This was already largely the case before, except that
the concept of a "mainTarget" and "featureNamePrefix" was specific
to bean factory initialization contributors.

ClassNameGenerator should now be instantiated with a default target
and an optional feature name prefix. As a result, it does no longer
generate class names in the "__" package.

GeneratedClasses can now provide a new, unique, GeneratedClass or
offer a container for retrieving the same GeneratedClass based on an
identifier. This lets all contributors use this facility rather than
creating JavaFile manually. This also means that ClassNameGenerator
is no longer exposed.

Because the naming conventions are now part of the GenerationContext, it
is required to be able to retrieve a specialized version of it if a
code generation round needs to use different naming conventions. A new
withName method has been added to that effect.

Closes gh-28585

spring-core/src/test/java/org/springframework/aot/generate/ClassNameGeneratorTests.java

@@ -32,12 +32,26 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
  */
 class ClassNameGeneratorTests {
 
-	private final ClassNameGenerator generator = new ClassNameGenerator();
+	private final ClassNameGenerator generator = new ClassNameGenerator(Object.class);
 
 	@Test
-	void generateClassNameWhenTargetClassIsNullUsesAotPackage() {
-		ClassName generated = this.generator.generateClassName((Class<?>) null, "test");
-		assertThat(generated).hasToString("__.Test");
+	void generateClassNameWhenTargetClassIsNullUsesMainTarget() {
+		ClassName generated = this.generator.generateClassName(null, "test");
+		assertThat(generated).hasToString("java.lang.Object__Test");
+	}
+
+	@Test
+	void generateClassNameUseFeatureNamePrefix() {
+		ClassName generated = new ClassNameGenerator(Object.class, "One")
+				.generateClassName(InputStream.class, "test");
+		assertThat(generated).hasToString("java.io.InputStream__OneTest");
+	}
+
+	@Test
+	void generateClassNameWithNoTextFeatureNamePrefix() {
+		ClassName generated = new ClassNameGenerator(Object.class, "  ")
+				.generateClassName(InputStream.class, "test");
+		assertThat(generated).hasToString("java.io.InputStream__Test");
 	}
 
 	@Test
@@ -59,8 +73,7 @@ class ClassNameGeneratorTests {
 
 	@Test
 	void generateClassNameWithClassWhenLowercaseFeatureNameGeneratesName() {
-		ClassName generated = this.generator.generateClassName(InputStream.class,
-				"bytes");
+		ClassName generated = this.generator.generateClassName(InputStream.class, "bytes");
 		assertThat(generated).hasToString("java.io.InputStream__Bytes");
 	}
 
@@ -68,7 +81,7 @@ class ClassNameGeneratorTests {
 	void generateClassNameWithClassWhenInnerClassGeneratesName() {
 		ClassName generated = this.generator.generateClassName(TestBean.class, "EventListener");
 		assertThat(generated)
-			.hasToString("org.springframework.aot.generate.ClassNameGeneratorTests_TestBean__EventListener");
+				.hasToString("org.springframework.aot.generate.ClassNameGeneratorTests_TestBean__EventListener");
 	}
 
 	@Test
@@ -81,6 +94,15 @@ class ClassNameGeneratorTests {
 		assertThat(generated3).hasToString("java.io.InputStream__Bytes2");
 	}
 
+	@Test
+	void getClassNameWhenMultipleCallsReturnsSameName() {
+		String name1 = this.generator.getClassName(InputStream.class, "bytes");
+		String name2 = this.generator.getClassName(InputStream.class, "bytes");
+		String name3 = this.generator.getClassName(InputStream.class, "bytes");
+		assertThat(name1).hasToString("java.io.InputStream__Bytes")
+				.isEqualTo(name2).isEqualTo(name3);
+	}
+
 	static class TestBean {
 
 	}

spring-core/src/test/java/org/springframework/aot/generate/DefaultGenerationContextTests.java

@@ -16,9 +16,14 @@
 
 package org.springframework.aot.generate;
 
+import java.util.function.Consumer;
+
 import org.junit.jupiter.api.Test;
 
+import org.springframework.aot.generate.GeneratedFiles.Kind;
 import org.springframework.aot.hint.RuntimeHints;
+import org.springframework.core.testfixture.aot.generate.TestTarget;
+import org.springframework.javapoet.TypeSpec.Builder;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -31,9 +36,12 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
  */
 class DefaultGenerationContextTests {
 
-	private final ClassNameGenerator classNameGenerator = new ClassNameGenerator();
+	private static final Consumer<Builder> typeSpecCustomizer = type -> {};
 
-	private final GeneratedFiles generatedFiles = new InMemoryGeneratedFiles();
+	private final GeneratedClasses generatedClasses = new GeneratedClasses(
+			new ClassNameGenerator(TestTarget.class));
+
+	private final InMemoryGeneratedFiles generatedFiles = new InMemoryGeneratedFiles();
 
 	private final RuntimeHints runtimeHints = new RuntimeHints();
 
@@ -41,9 +49,7 @@ class DefaultGenerationContextTests {
 	@Test
 	void createWithOnlyGeneratedFilesCreatesContext() {
 		DefaultGenerationContext context = new DefaultGenerationContext(
-				this.generatedFiles);
-		assertThat(context.getClassNameGenerator())
-				.isInstanceOf(ClassNameGenerator.class);
+				new ClassNameGenerator(TestTarget.class), this.generatedFiles);
 		assertThat(context.getGeneratedFiles()).isSameAs(this.generatedFiles);
 		assertThat(context.getRuntimeHints()).isInstanceOf(RuntimeHints.class);
 	}
@@ -51,24 +57,23 @@ class DefaultGenerationContextTests {
 	@Test
 	void createCreatesContext() {
 		DefaultGenerationContext context = new DefaultGenerationContext(
-				this.classNameGenerator, this.generatedFiles, this.runtimeHints);
-		assertThat(context.getClassNameGenerator()).isNotNull();
+				this.generatedClasses, this.generatedFiles, this.runtimeHints);
 		assertThat(context.getGeneratedFiles()).isNotNull();
 		assertThat(context.getRuntimeHints()).isNotNull();
 	}
 
 	@Test
-	void createWhenClassNameGeneratorIsNullThrowsException() {
+	void createWhenGeneratedClassesIsNullThrowsException() {
 		assertThatIllegalArgumentException()
 				.isThrownBy(() -> new DefaultGenerationContext(null, this.generatedFiles,
 						this.runtimeHints))
-				.withMessage("'classNameGenerator' must not be null");
+				.withMessage("'generatedClasses' must not be null");
 	}
 
 	@Test
 	void createWhenGeneratedFilesIsNullThrowsException() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> new DefaultGenerationContext(this.classNameGenerator,
+				.isThrownBy(() -> new DefaultGenerationContext(this.generatedClasses,
 						null, this.runtimeHints))
 				.withMessage("'generatedFiles' must not be null");
 	}
@@ -76,30 +81,71 @@ class DefaultGenerationContextTests {
 	@Test
 	void createWhenRuntimeHintsIsNullThrowsException() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> new DefaultGenerationContext(this.classNameGenerator,
+				.isThrownBy(() -> new DefaultGenerationContext(this.generatedClasses,
 						this.generatedFiles, null))
 				.withMessage("'runtimeHints' must not be null");
 	}
 
 	@Test
-	void getClassNameGeneratorReturnsClassNameGenerator() {
+	void getGeneratedClassesReturnsClassNameGenerator() {
 		DefaultGenerationContext context = new DefaultGenerationContext(
-				this.classNameGenerator, this.generatedFiles, this.runtimeHints);
-		assertThat(context.getClassNameGenerator()).isSameAs(this.classNameGenerator);
+				this.generatedClasses, this.generatedFiles, this.runtimeHints);
+		assertThat(context.getGeneratedClasses()).isSameAs(this.generatedClasses);
 	}
 
 	@Test
 	void getGeneratedFilesReturnsGeneratedFiles() {
 		DefaultGenerationContext context = new DefaultGenerationContext(
-				this.classNameGenerator, this.generatedFiles, this.runtimeHints);
+				this.generatedClasses, this.generatedFiles, this.runtimeHints);
 		assertThat(context.getGeneratedFiles()).isSameAs(this.generatedFiles);
 	}
 
 	@Test
 	void getRuntimeHintsReturnsRuntimeHints() {
 		DefaultGenerationContext context = new DefaultGenerationContext(
-				this.classNameGenerator, this.generatedFiles, this.runtimeHints);
+				this.generatedClasses, this.generatedFiles, this.runtimeHints);
 		assertThat(context.getRuntimeHints()).isSameAs(this.runtimeHints);
 	}
 
+	@Test
+	void withNameUpdateNamingConvention() {
+		DefaultGenerationContext context = new DefaultGenerationContext(
+				new ClassNameGenerator(TestTarget.class), this.generatedFiles);
+		GenerationContext anotherContext = context.withName("Another");
+		GeneratedClass generatedClass = anotherContext.getGeneratedClasses()
+				.forFeature("Test").generate(typeSpecCustomizer);
+		assertThat(generatedClass.getName().simpleName()).endsWith("__AnotherTest");
+	}
+
+	@Test
+	void withNameKeepTrackOfAllGeneratedFiles() {
+		DefaultGenerationContext context = new DefaultGenerationContext(
+				new ClassNameGenerator(TestTarget.class), this.generatedFiles);
+		context.getGeneratedClasses().forFeature("Test").generate(typeSpecCustomizer);
+		GenerationContext anotherContext = context.withName("Another");
+		assertThat(anotherContext.getGeneratedClasses()).isNotSameAs(context.getGeneratedClasses());
+		assertThat(anotherContext.getGeneratedFiles()).isSameAs(context.getGeneratedFiles());
+		assertThat(anotherContext.getRuntimeHints()).isSameAs(context.getRuntimeHints());
+		anotherContext.getGeneratedClasses().forFeature("Test").generate(typeSpecCustomizer);
+		context.writeGeneratedContent();
+		assertThat(this.generatedFiles.getGeneratedFiles(Kind.SOURCE)).hasSize(2);
+	}
+
+	@Test
+	void withNameGenerateUniqueName() {
+		DefaultGenerationContext context = new DefaultGenerationContext(
+				new ClassNameGenerator(Object.class), this.generatedFiles);
+		context.withName("Test").getGeneratedClasses()
+				.forFeature("Feature").generate(typeSpecCustomizer);
+		context.withName("Test").getGeneratedClasses()
+				.forFeature("Feature").generate(typeSpecCustomizer);
+		context.withName("Test").getGeneratedClasses()
+				.forFeature("Feature").generate(typeSpecCustomizer);
+		context.writeGeneratedContent();
+		assertThat(this.generatedFiles.getGeneratedFiles(Kind.SOURCE)).containsOnlyKeys(
+				"java/lang/Object__TestFeature.java",
+				"java/lang/Object__Test1Feature.java",
+				"java/lang/Object__Test2Feature.java");
+	}
+
 }

spring-core/src/test/java/org/springframework/aot/generate/GeneratedClassTests.java

@@ -16,77 +16,43 @@
 
 package org.springframework.aot.generate;
 
+import java.util.function.Consumer;
+
 import org.junit.jupiter.api.Test;
 
 import org.springframework.javapoet.ClassName;
-import org.springframework.javapoet.JavaFile;
-import org.springframework.javapoet.TypeSpec;
+import org.springframework.javapoet.TypeSpec.Builder;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 
 /**
  * Tests for {@link GeneratedClass}.
  *
  * @author Phillip Webb
+ * @author Stephane Nicoll
  */
 class GeneratedClassTests {
 
 	@Test
 	void getNameReturnsName() {
 		ClassName name = ClassName.bestGuess("com.example.Test");
-		GeneratedClass generatedClass = new GeneratedClass(this::generateJavaFile, name);
+		GeneratedClass generatedClass = new GeneratedClass(emptyTypeSpec(), name);
 		assertThat(generatedClass.getName()).isSameAs(name);
 	}
 
 	@Test
-	void generateJavaFileSuppliesGeneratedMethods() {
+	void generateJavaFileIncludesGeneratedMethods() {
 		ClassName name = ClassName.bestGuess("com.example.Test");
-		GeneratedClass generatedClass = new GeneratedClass(this::generateJavaFile, name);
+		GeneratedClass generatedClass = new GeneratedClass(emptyTypeSpec(), name);
 		MethodGenerator methodGenerator = generatedClass.getMethodGenerator();
 		methodGenerator.generateMethod("test")
 				.using(builder -> builder.addJavadoc("Test Method"));
 		assertThat(generatedClass.generateJavaFile().toString()).contains("Test Method");
 	}
 
-	@Test
-	void generateJavaFileWhenHasBadPackageThrowsException() {
-		ClassName name = ClassName.bestGuess("com.example.Test");
-		GeneratedClass generatedClass = new GeneratedClass(
-				this::generateBadPackageJavaFile, name);
-		assertThatIllegalStateException()
-				.isThrownBy(
-						() -> assertThat(generatedClass.generateJavaFile().toString()))
-				.withMessageContaining("should be in package");
-	}
 
-	@Test
-	void generateJavaFileWhenHasBadNameThrowsException() {
-		ClassName name = ClassName.bestGuess("com.example.Test");
-		GeneratedClass generatedClass = new GeneratedClass(this::generateBadNameJavaFile,
-				name);
-		assertThatIllegalStateException()
-				.isThrownBy(
-						() -> assertThat(generatedClass.generateJavaFile().toString()))
-				.withMessageContaining("should be named");
-	}
-
-	private JavaFile generateJavaFile(ClassName className, GeneratedMethods methods) {
-		TypeSpec.Builder classBuilder = TypeSpec.classBuilder(className);
-		methods.doWithMethodSpecs(classBuilder::addMethod);
-		return JavaFile.builder(className.packageName(), classBuilder.build()).build();
-	}
-
-	private JavaFile generateBadPackageJavaFile(ClassName className,
-			GeneratedMethods methods) {
-		TypeSpec.Builder classBuilder = TypeSpec.classBuilder(className);
-		return JavaFile.builder("naughty", classBuilder.build()).build();
-	}
-
-	private JavaFile generateBadNameJavaFile(ClassName className,
-			GeneratedMethods methods) {
-		TypeSpec.Builder classBuilder = TypeSpec.classBuilder("Naughty");
-		return JavaFile.builder(className.packageName(), classBuilder.build()).build();
+	private Consumer<Builder> emptyTypeSpec() {
+		return type -> {};
 	}
 
 }

spring-core/src/test/java/org/springframework/aot/generate/GeneratedClassesTests.java

@@ -16,27 +16,34 @@
 
 package org.springframework.aot.generate;
 
+import java.io.IOException;
+import java.util.function.Consumer;
+
 import org.junit.jupiter.api.Test;
 
-import org.springframework.aot.generate.ClassGenerator.JavaFileGenerator;
-import org.springframework.javapoet.ClassName;
-import org.springframework.javapoet.JavaFile;
+import org.springframework.aot.generate.GeneratedFiles.Kind;
 import org.springframework.javapoet.TypeSpec;
+import org.springframework.javapoet.TypeSpec.Builder;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 
 /**
  * Tests for {@link GeneratedClasses}.
  *
  * @author Phillip Webb
+ * @author Stephane Nicoll
  */
 class GeneratedClassesTests {
 
-	private GeneratedClasses generatedClasses = new GeneratedClasses(
-			new ClassNameGenerator());
+	private static final Consumer<TypeSpec.Builder> emptyTypeCustomizer = type -> {};
 
-	private static final JavaFileGenerator JAVA_FILE_GENERATOR = GeneratedClassesTests::generateJavaFile;
+	private final GeneratedClasses generatedClasses = new GeneratedClasses(
+			new ClassNameGenerator(Object.class));
 
 	@Test
 	void createWhenClassNameGeneratorIsNullThrowsException() {
@@ -45,61 +52,118 @@ class GeneratedClassesTests {
 	}
 
 	@Test
-	void getOrGenerateWithClassTargetWhenJavaFileGeneratorIsNullThrowsException() {
+	void forFeatureComponentWhenTargetIsNullThrowsException() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> this.generatedClasses.getOrGenerateClass(null,
-						TestTarget.class, "test"))
-				.withMessage("'javaFileGenerator' must not be null");
+				.isThrownBy(() -> this.generatedClasses.forFeatureComponent("test", null))
+				.withMessage("'component' must not be null");
 	}
 
 	@Test
-	void getOrGenerateWithClassTargetWhenTargetIsNullThrowsException() {
+	void forFeatureComponentWhenFeatureNameIsEmptyThrowsException() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> this.generatedClasses
-						.getOrGenerateClass(JAVA_FILE_GENERATOR, (Class<?>) null, "test"))
-				.withMessage("'target' must not be null");
-	}
-
-	@Test
-	void getOrGenerateWithClassTargetWhenFeatureIsNullThrowsException() {
-		assertThatIllegalArgumentException()
-				.isThrownBy(() -> this.generatedClasses
-						.getOrGenerateClass(JAVA_FILE_GENERATOR, TestTarget.class, null))
+				.isThrownBy(() -> this.generatedClasses.forFeatureComponent("", TestComponent.class))
 				.withMessage("'featureName' must not be empty");
 	}
 
 	@Test
-	void getOrGenerateWhenNewReturnsGeneratedMethod() {
+	void forFeatureWhenFeatureNameIsEmptyThrowsException() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> this.generatedClasses.forFeature(""))
+				.withMessage("'featureName' must not be empty");
+	}
+
+	@Test
+	void generateWhenTypeSpecCustomizerIsNullThrowsException() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> this.generatedClasses
+						.forFeatureComponent("test", TestComponent.class).generate(null))
+				.withMessage("'typeSpecCustomizer' must not be null");
+	}
+
+	@Test
+	void forFeatureUsesDefaultTarget() {
+		GeneratedClass generatedClass = this.generatedClasses
+				.forFeature("Test").generate(emptyTypeCustomizer);
+		assertThat(generatedClass.getName()).hasToString("java.lang.Object__Test");
+	}
+
+	@Test
+	void forFeatureComponentUsesComponent() {
+		GeneratedClass generatedClass = this.generatedClasses
+				.forFeatureComponent("Test", TestComponent.class).generate(emptyTypeCustomizer);
+		assertThat(generatedClass.getName().toString()).endsWith("TestComponent__Test");
+	}
+
+	@Test
+	void generateReturnsDifferentInstances() {
+		Consumer<Builder> typeCustomizer = mockTypeCustomizer();
 		GeneratedClass generatedClass1 = this.generatedClasses
-				.getOrGenerateClass(JAVA_FILE_GENERATOR, TestTarget.class, "one");
+				.forFeatureComponent("one", TestComponent.class).generate(typeCustomizer);
 		GeneratedClass generatedClass2 = this.generatedClasses
-				.getOrGenerateClass(JAVA_FILE_GENERATOR, TestTarget.class, "two");
+				.forFeatureComponent("one", TestComponent.class).generate(typeCustomizer);
+		assertThat(generatedClass1).isNotSameAs(generatedClass2);
+		assertThat(generatedClass1.getName().simpleName()).endsWith("__One");
+		assertThat(generatedClass2.getName().simpleName()).endsWith("__One1");
+	}
+
+	@Test
+	void getOrGenerateWhenNewReturnsGeneratedMethod() {
+		Consumer<Builder> typeCustomizer = mockTypeCustomizer();
+		GeneratedClass generatedClass1 = this.generatedClasses
+				.forFeatureComponent("one", TestComponent.class).getOrGenerate("facet", typeCustomizer);
+		GeneratedClass generatedClass2 = this.generatedClasses
+				.forFeatureComponent("two", TestComponent.class).getOrGenerate("facet", typeCustomizer);
 		assertThat(generatedClass1).isNotNull().isNotEqualTo(generatedClass2);
 		assertThat(generatedClass2).isNotNull();
 	}
 
 	@Test
 	void getOrGenerateWhenRepeatReturnsSameGeneratedMethod() {
-		GeneratedClasses generated = this.generatedClasses;
-		GeneratedClass generatedClass1 = generated.getOrGenerateClass(JAVA_FILE_GENERATOR,
-				TestTarget.class, "one");
-		GeneratedClass generatedClass2 = generated.getOrGenerateClass(JAVA_FILE_GENERATOR,
-				TestTarget.class, "one");
-		GeneratedClass generatedClass3 = generated.getOrGenerateClass(JAVA_FILE_GENERATOR,
-				TestTarget.class, "one");
-		GeneratedClass generatedClass4 = generated.getOrGenerateClass(JAVA_FILE_GENERATOR,
-				TestTarget.class, "two");
+		Consumer<Builder> typeCustomizer = mockTypeCustomizer();
+		GeneratedClass generatedClass1 = this.generatedClasses
+				.forFeatureComponent("one", TestComponent.class).getOrGenerate("facet", typeCustomizer);
+		GeneratedClass generatedClass2 = this.generatedClasses
+				.forFeatureComponent("one", TestComponent.class).getOrGenerate("facet", typeCustomizer);
+		GeneratedClass generatedClass3 = this.generatedClasses
+				.forFeatureComponent("one", TestComponent.class).getOrGenerate("facet", typeCustomizer);
 		assertThat(generatedClass1).isNotNull().isSameAs(generatedClass2)
-				.isSameAs(generatedClass3).isNotSameAs(generatedClass4);
+				.isSameAs(generatedClass3);
+		verifyNoInteractions(typeCustomizer);
+		generatedClass1.generateJavaFile();
+		verify(typeCustomizer).accept(any());
 	}
 
-	static JavaFile generateJavaFile(ClassName className,
-			GeneratedMethods generatedMethods) {
-		TypeSpec typeSpec = TypeSpec.classBuilder(className).addJavadoc("Test").build();
-		return JavaFile.builder(className.packageName(), typeSpec).build();
+	@Test
+	@SuppressWarnings("unchecked")
+	void writeToInvokeTypeSpecCustomizer() throws IOException {
+		Consumer<TypeSpec.Builder> typeSpecCustomizer = mock(Consumer.class);
+		this.generatedClasses.forFeatureComponent("one", TestComponent.class)
+				.generate(typeSpecCustomizer);
+		verifyNoInteractions(typeSpecCustomizer);
+		InMemoryGeneratedFiles generatedFiles = new InMemoryGeneratedFiles();
+		this.generatedClasses.writeTo(generatedFiles);
+		verify(typeSpecCustomizer).accept(any());
+		assertThat(generatedFiles.getGeneratedFiles(Kind.SOURCE)).hasSize(1);
 	}
 
-	private static class TestTarget {
+	@Test
+	void withNameUpdatesNamingConventions() {
+		GeneratedClass generatedClass1 = this.generatedClasses
+				.forFeatureComponent("one", TestComponent.class).generate(emptyTypeCustomizer);
+		GeneratedClass generatedClass2 = this.generatedClasses.withName("Another")
+				.forFeatureComponent("one", TestComponent.class).generate(emptyTypeCustomizer);
+		assertThat(generatedClass1.getName().toString()).endsWith("TestComponent__One");
+		assertThat(generatedClass2.getName().toString()).endsWith("TestComponent__AnotherOne");
+	}
+
+
+	@SuppressWarnings("unchecked")
+	private Consumer<TypeSpec.Builder> mockTypeCustomizer() {
+		return mock(Consumer.class);
+	}
+
+
+	private static class TestComponent {
 
 	}