Merge pull request #46753 from nosan

* pr/46753:
  Add ArchRule that checks that the type of exposed bean is not private

Closes gh-46753

Author: snicoll

buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java

@@ -43,6 +43,7 @@
 import org.gradle.api.file.FileTree;
 import org.gradle.api.provider.ListProperty;
 import org.gradle.api.provider.Property;
+import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.Classpath;
 import org.gradle.api.tasks.IgnoreEmptyDirectories;
 import org.gradle.api.tasks.Input;
@@ -53,6 +54,7 @@
 import org.gradle.api.tasks.PathSensitive;
 import org.gradle.api.tasks.PathSensitivity;
 import org.gradle.api.tasks.SkipWhenEmpty;
+import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.api.tasks.VerificationException;
 
@@ -75,9 +77,17 @@ public ArchitectureCheck() {
 		getRules().addAll(getProhibitObjectsRequireNonNull().convention(true)
 			.map(whenTrue(ArchitectureRules::noClassesShouldCallObjectsRequireNonNull)));
 		getRules().addAll(ArchitectureRules.standard());
+		getRules().addAll(whenMainSources(
+				() -> Collections.singletonList(ArchitectureRules.allBeanMethodsShouldReturnNonPrivateType())));
 		getRuleDescriptions().set(getRules().map(this::asDescriptions));
 	}
 
+	private Provider<List<ArchRule>> whenMainSources(Supplier<List<ArchRule>> rules) {
+		return getSourceSet().convention(SourceSet.MAIN_SOURCE_SET_NAME)
+			.map(SourceSet.MAIN_SOURCE_SET_NAME::equals)
+			.map(whenTrue(rules));
+	}
+
 	private Transformer<List<ArchRule>, Boolean> whenTrue(Supplier<List<ArchRule>> rules) {
 		return (in) -> (!in) ? Collections.emptyList() : rules.get();
 	}
@@ -170,6 +180,9 @@ final FileTree getInputClasses() {
 	@Internal
 	public abstract Property<Boolean> getProhibitObjectsRequireNonNull();
 
+	@Internal
+	abstract Property<String> getSourceSet();
+
 	@Input // Use descriptions as input since rules aren't serializable
 	abstract ListProperty<String> getRuleDescriptions();
 

buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitecturePlugin.java

@@ -49,6 +49,7 @@ private void registerTasks(Project project) {
 			TaskProvider<ArchitectureCheck> checkPackageTangles = project.getTasks()
 				.register("checkArchitecture" + StringUtils.capitalize(sourceSet.getName()), ArchitectureCheck.class,
 						(task) -> {
+							task.getSourceSet().set(sourceSet.getName());
 							task.getCompileClasspath().from(sourceSet.getCompileClasspath());
 							task.setClasses(sourceSet.getOutput().getClassesDirs());
 							task.getResourcesDirectory().set(sourceSet.getOutput().getResourcesDir());

buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureRules.java

@@ -33,6 +33,7 @@
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.domain.JavaClass.Predicates;
 import com.tngtech.archunit.core.domain.JavaMethod;
+import com.tngtech.archunit.core.domain.JavaModifier;
 import com.tngtech.archunit.core.domain.JavaParameter;
 import com.tngtech.archunit.core.domain.JavaType;
 import com.tngtech.archunit.core.domain.properties.CanBeAnnotated;
@@ -93,6 +94,20 @@ static List<ArchRule> standard() {
 		return List.copyOf(rules);
 	}
 
+	static ArchRule allBeanMethodsShouldReturnNonPrivateType() {
+		return methodsThatAreAnnotatedWith("org.springframework.context.annotation.Bean").should(check(
+				"not return types declared with the %s modifier, as such types are incompatible with Spring AOT processing"
+					.formatted(JavaModifier.PRIVATE),
+				(method, events) -> {
+					JavaClass returnType = method.getRawReturnType();
+					if (returnType.getModifiers().contains(JavaModifier.PRIVATE)) {
+						addViolation(events, method, "%s returns %s which is declared as %s".formatted(
+								method.getDescription(), returnType.getDescription(), returnType.getModifiers()));
+					}
+				}))
+			.allowEmptyShould(true);
+	}
+
 	private static ArchRule allPackagesShouldBeFreeOfTangles() {
 		return SlicesRuleDefinition.slices().matching("(**)").should().beFreeOfCycles();
 	}

buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java

@@ -16,7 +16,9 @@
 
 package org.springframework.boot.build.architecture;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.function.Consumer;
@@ -160,6 +162,21 @@ void whenClassCallsStringToUpperCaseWithLocaleShouldNotFail() throws IOException
 		runGradleWithCompiledClasses("string/toUpperCaseWithLocale", shouldHaveEmptyFailureReport());
 	}
 
+	@Test
+	void whenBeanMethodExposePrivateTypeShouldFailAndWriteReport() throws IOException {
+		runGradleWithCompiledClasses("beans/privatebean", shouldHaveFailureReportWithMessage(
+				"methods that are annotated with @Bean should not return types declared with the PRIVATE modifier,"
+						+ " as such types are incompatible with Spring AOT processing",
+				"Method <org.springframework.boot.build.architecture.beans.privatebean.PrivateBean.myBean()> "
+						+ "returns Class <org.springframework.boot.build.architecture.beans.privatebean.PrivateBean$MyBean>"
+						+ " which is declared as [PRIVATE, STATIC, FINAL]"));
+	}
+
+	@Test
+	void whenBeanMethodExposeNonPrivateTypeeShouldNotFail() throws IOException {
+		runGradleWithCompiledClasses("beans/regular", shouldHaveEmptyFailureReport());
+	}
+
 	@Test
 	void whenBeanPostProcessorBeanMethodIsNotStaticWithExternalClass() throws IOException {
 		Files.writeString(this.buildFile, """
@@ -196,17 +213,22 @@ IntegrationMBeanExporter integrationMBeanExporter() {
 
 	private Consumer<GradleRunner> shouldHaveEmptyFailureReport() {
 		return (gradleRunner) -> {
-			assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
-				.contains("Task :checkArchitectureMain");
-			assertThat(failureReport()).isEmptyFile();
+			try {
+				assertThat(gradleRunner.build().getOutput()).contains("BUILD SUCCESSFUL")
+					.contains("Task :checkArchitectureMain");
+				assertThat(failureReport()).isEmpty();
+			}
+			catch (Exception ex) {
+				throw new AssertionError("Expected build to succeed but it failed\n" + failureReport(), ex);
+			}
 		};
 	}
 
-	private Consumer<GradleRunner> shouldHaveFailureReportWithMessage(String message) {
+	private Consumer<GradleRunner> shouldHaveFailureReportWithMessage(String... messages) {
 		return (gradleRunner) -> {
 			assertThat(gradleRunner.buildAndFail().getOutput()).contains("BUILD FAILED")
 				.contains("Task :checkArchitectureMain FAILED");
-			assertThat(failureReport()).content().contains(message);
+			assertThat(failureReport()).contains(messages);
 		};
 	}
 
@@ -235,8 +257,17 @@ private void runGradle(Consumer<GradleRunner> callback) {
 			.withPluginClasspath());
 	}
 
-	private Path failureReport() {
-		return this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
+	private String failureReport() {
+		try {
+			Path failureReport = this.projectDir.resolve("build/checkArchitectureMain/failure-report.txt");
+			return Files.readString(failureReport, StandardCharsets.UTF_8);
+		}
+		catch (FileNotFoundException ex) {
+			return "Failure report does not exist";
+		}
+		catch (IOException ex) {
+			return "Failure report could not be read: " + ex.getMessage();
+		}
 	}
 
 }

buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/privatebean/PrivateBean.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.build.architecture.beans.privatebean;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration(proxyBeanMethods = false)
+class PrivateBean {
+
+	@Bean
+	static MyBean myBean() {
+		return new MyBean();
+	}
+
+	private static final class MyBean {
+
+	}
+
+}

buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/regular/RegularBean.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.build.architecture.beans.regular;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration(proxyBeanMethods = false)
+class RegularBean {
+
+	@Bean
+	static PackagePrivate packagePrivateBean() {
+		return new PackagePrivate();
+	}
+
+	@Bean
+	static Protected protectedBean() {
+		return new Protected();
+	}
+
+	@Bean
+	static Public publicBean() {
+		return new Public();
+	}
+
+	static final class PackagePrivate {
+
+	}
+
+	protected static final class Protected {
+
+	}
+
+	public static final class Public {
+
+	}
+
+}