diff --git a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java b/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java index 1fe1abef353a..6ad9c3d2ee8c 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureCheck.java +++ b/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> whenMainSources(Supplier> rules) { + return getSourceSet().convention(SourceSet.MAIN_SOURCE_SET_NAME) + .map(SourceSet.MAIN_SOURCE_SET_NAME::equals) + .map(whenTrue(rules)); + } + private Transformer, Boolean> whenTrue(Supplier> rules) { return (in) -> (!in) ? Collections.emptyList() : rules.get(); } @@ -170,6 +180,9 @@ final FileTree getInputClasses() { @Internal public abstract Property getProhibitObjectsRequireNonNull(); + @Internal + abstract Property getSourceSet(); + @Input // Use descriptions as input since rules aren't serializable abstract ListProperty getRuleDescriptions(); diff --git a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitecturePlugin.java b/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitecturePlugin.java index 5a1d7c8aa6e7..cb2cfdb0d4bc 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitecturePlugin.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitecturePlugin.java @@ -49,6 +49,7 @@ private void registerTasks(Project project) { TaskProvider 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()); diff --git a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureRules.java b/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureRules.java index 72202bea5435..54c154261feb 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/architecture/ArchitectureRules.java +++ b/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 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(); } diff --git a/buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java b/buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java index 87bb274d3680..b266641e9bc1 100644 --- a/buildSrc/src/test/java/org/springframework/boot/build/architecture/ArchitectureCheckTests.java +++ b/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 " + + "returns Class " + + " 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 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 shouldHaveFailureReportWithMessage(String message) { + private Consumer 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 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(); + } } } diff --git a/buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/privatebean/PrivateBean.java b/buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/privatebean/PrivateBean.java new file mode 100644 index 000000000000..1622b08eb049 --- /dev/null +++ b/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 { + + } + +} diff --git a/buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/regular/RegularBean.java b/buildSrc/src/test/java/org/springframework/boot/build/architecture/beans/regular/RegularBean.java new file mode 100644 index 000000000000..9d39e213539a --- /dev/null +++ b/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 { + + } + +}