Merge pull request #46766 from scordio

* pr/46766:
  Polish "Decorate all Assert implementations with @CheckReturnValue"
  Decorate all Assert implementations with @CheckReturnValue

Closes gh-46766

Author: snicoll

build-plugin/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/AbstractArchiveIntegrationTests.java

@@ -38,6 +38,8 @@
 import org.assertj.core.api.ListAssert;
 import org.jspecify.annotations.Nullable;
 
+import org.springframework.lang.CheckReturnValue;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.contentOf;
 
@@ -172,6 +174,7 @@ JarAssert doesNotHaveEntryWithNameStartingWith(String prefix) {
 			return this;
 		}
 
+		@CheckReturnValue
 		ListAssert<String> entryNamesInPath(String path) {
 			List<String> matches = new ArrayList<>();
 			withJarFile((jarFile) -> withEntries(jarFile,

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

@@ -70,6 +70,7 @@
  * @author Phillip Webb
  * @author Dmytro Nosan
  * @author Moritz Halbritter
+ * @author Stefano Cordio
  */
 public abstract class ArchitectureCheck extends DefaultTask {
 
@@ -93,6 +94,8 @@ public ArchitectureCheck() {
 		getRules().addAll(whenMainSources(
 				() -> ArchitectureRules.configurationPropertiesDeprecation(ArchitectureCheckAnnotation.classFor(
 						getAnnotationClasses().get(), ArchitectureCheckAnnotation.DEPRECATED_CONFIGURATION_PROPERTY))));
+		getRules().addAll(whenMainSources(() -> Collections.singletonList(
+				ArchitectureRules.allCustomAssertionMethodsNotReturningSelfShouldBeAnnotatedWithCheckReturnValue())));
 		getRules().addAll(and(getNullMarkedEnabled(), isMainSourceSet()).map(whenTrue(() -> Collections.singletonList(
 				ArchitectureRules.packagesShouldBeAnnotatedWithNullMarked(getNullMarkedIgnoredPackages().get())))));
 		getRuleDescriptions().set(getRules().map(this::asDescriptions));

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

@@ -63,6 +63,7 @@
 
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.context.annotation.Role;
+import org.springframework.lang.CheckReturnValue;
 import org.springframework.util.ResourceUtils;
 
 /**
@@ -75,6 +76,7 @@
  * @author Phillip Webb
  * @author Ngoc Nhan
  * @author Moritz Halbritter
+ * @author Stefano Cordio
  */
 final class ArchitectureRules {
 
@@ -158,6 +160,26 @@ private static ArchRule allBeanMethodsShouldNotHaveConditionalOnClassAnnotation(
 			.allowEmptyShould(true);
 	}
 
+	static ArchRule allCustomAssertionMethodsNotReturningSelfShouldBeAnnotatedWithCheckReturnValue() {
+		return ArchRuleDefinition.methods()
+			.that()
+			.areDeclaredInClassesThat()
+			.implement("org.assertj.core.api.Assert")
+			.and()
+			.arePublic()
+			.and()
+			.doNotHaveModifier(JavaModifier.BRIDGE)
+			.and(doNotReturnSelfType())
+			.should()
+			.beAnnotatedWith(CheckReturnValue.class)
+			.allowEmptyShould(true);
+	}
+
+	private static DescribedPredicate<JavaMethod> doNotReturnSelfType() {
+		return DescribedPredicate.describe("do not return self type",
+				(method) -> !method.getRawReturnType().equals(method.getOwner()));
+	}
+
 	private static ArchRule allPackagesShouldBeFreeOfTangles() {
 		return SlicesRuleDefinition.slices()
 			.matching("(**)")

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

@@ -62,13 +62,18 @@
  * @author Scott Frederick
  * @author Ivan Malutin
  * @author Dmytro Nosan
+ * @author Stefano Cordio
  */
 class ArchitectureCheckTests {
 
-	private static final String SPRING_CONTEXT = "org.springframework:spring-context:6.2.9";
+	private static final String ASSERTJ_CORE = "org.assertj:assertj-core:3.27.4";
 
 	private static final String JUNIT_JUPITER = "org.junit.jupiter:junit-jupiter:5.12.0";
 
+	private static final String SPRING_CONTEXT = "org.springframework:spring-context:6.2.15";
+
+	private static final String SPRING_CORE = "org.springframework:spring-core:6.2.15";
+
 	private static final String SPRING_INTEGRATION_JMX = "org.springframework.integration:spring-integration-jmx:6.5.1";
 
 	private GradleBuild gradleBuild;
@@ -452,6 +457,23 @@ void whenDeprecatedConfigurationPropertyIsMissingSinceShouldFailAndWriteReport()
 				"DeprecatedConfigurationPropertySince.getProperty");
 	}
 
+	@Test
+	void whenCustomAssertionMethodNotReturningSelfIsAnnotatedWithCheckReturnValueShouldSucceedAndWriteEmptyReport()
+			throws IOException {
+		prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "assertj/checkReturnValue");
+		build(this.gradleBuild.withDependencies(ASSERTJ_CORE, SPRING_CORE), Task.CHECK_ARCHITECTURE_MAIN);
+	}
+
+	@Test
+	void whenCustomAssertionMethodNotReturningSelfIsNotAnnotatedWithCheckReturnValueShouldFailAndWriteReport()
+			throws IOException {
+		prepareTask(Task.CHECK_ARCHITECTURE_MAIN, "assertj/noCheckReturnValue");
+		buildAndFail(this.gradleBuild.withDependencies(ASSERTJ_CORE), Task.CHECK_ARCHITECTURE_MAIN,
+				"methods that are declared in classes that implement org.assertj.core.api.Assert and "
+						+ "are public and do not have modifier BRIDGE and do not return self type should be annotated "
+						+ "with @CheckReturnValue");
+	}
+
 	private void prepareTask(Task task, String... sourceDirectories) throws IOException {
 		for (String sourceDirectory : sourceDirectories) {
 			FileSystemUtils.copyRecursively(

buildSrc/src/test/java/org/springframework/boot/build/architecture/assertj/checkReturnValue/WithCheckReturnValue.java

@@ -0,0 +1,39 @@
+/*
+ * 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.assertj.checkReturnValue;
+
+import org.assertj.core.api.AbstractAssert;
+
+import org.springframework.lang.CheckReturnValue;
+
+public class WithCheckReturnValue extends AbstractAssert<WithCheckReturnValue, Object> {
+
+	WithCheckReturnValue() {
+		super(null, WithCheckReturnValue.class);
+	}
+
+	@CheckReturnValue
+	public Object notReturningSelf() {
+		return new Object();
+	}
+
+	@Override
+	public WithCheckReturnValue isEqualTo(Object expected) {
+		return super.isEqualTo(expected);
+	}
+
+}

buildSrc/src/test/java/org/springframework/boot/build/architecture/assertj/noCheckReturnValue/NoCheckReturnValue.java

@@ -0,0 +1,31 @@
+/*
+ * 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.assertj.noCheckReturnValue;
+
+import org.assertj.core.api.AbstractAssert;
+
+public class NoCheckReturnValue extends AbstractAssert<NoCheckReturnValue, Object> {
+
+	NoCheckReturnValue() {
+		super(null, NoCheckReturnValue.class);
+	}
+
+	public Object notReturningSelf() {
+		return new Object();
+	}
+
+}

config/checkstyle/import-control.xml

@@ -6,6 +6,9 @@
 	<allow pkg="io.micrometer.observation" />
 	<disallow pkg="io.micrometer" />
 
+	<!-- Improve DevEx with fluent APIs -->
+	<allow class="org.springframework.lang.CheckReturnValue" />
+
 	<!-- Use JSpecify for nullability (not Spring)  -->
 	<allow class="org.springframework.lang.Contract" />
 	<disallow pkg="org.springframework.lang" />

core/spring-boot-test/src/main/java/org/springframework/boot/test/context/assertj/ApplicationContextAssert.java

@@ -26,7 +26,6 @@
 import org.assertj.core.api.AbstractObjectArrayAssert;
 import org.assertj.core.api.AbstractObjectAssert;
 import org.assertj.core.api.AbstractThrowableAssert;
-import org.assertj.core.api.Assertions;
 import org.assertj.core.api.MapAssert;
 import org.assertj.core.error.BasicErrorMessageFactory;
 import org.jspecify.annotations.Nullable;
@@ -37,6 +36,7 @@
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.lang.CheckReturnValue;
 import org.springframework.util.Assert;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -224,13 +224,14 @@ public ApplicationContextAssert<C> doesNotHaveBean(String name) {
 	 * @return array assertions for the bean names
 	 * @throws AssertionError if the application context did not start
 	 */
+	@CheckReturnValue
 	public <T> AbstractObjectArrayAssert<?, String> getBeanNames(Class<T> type) {
 		if (this.startupFailure != null) {
 			throwAssertionError(contextFailedToStartWhenExpecting(this.startupFailure,
 					"to get beans names with type:%n <%s>", type));
 		}
-		return Assertions.assertThat(getApplicationContext().getBeanNamesForType(type))
-			.as("Bean names of type <%s> from <%s>", type, getApplicationContext());
+		return assertThat(getApplicationContext().getBeanNamesForType(type)).as("Bean names of type <%s> from <%s>",
+				type, getApplicationContext());
 	}
 
 	/**
@@ -249,6 +250,7 @@ public <T> AbstractObjectArrayAssert<?, String> getBeanNames(Class<T> type) {
 	 * @throws AssertionError if the application context contains multiple beans of the
 	 * given type
 	 */
+	@CheckReturnValue
 	public <T> AbstractObjectAssert<?, T> getBean(Class<T> type) {
 		return getBean(type, Scope.INCLUDE_ANCESTORS);
 	}
@@ -270,6 +272,7 @@ public <T> AbstractObjectAssert<?, T> getBean(Class<T> type) {
 	 * @throws AssertionError if the application context contains multiple beans of the
 	 * given type
 	 */
+	@CheckReturnValue
 	public <T> AbstractObjectAssert<?, T> getBean(Class<T> type, Scope scope) {
 		Assert.notNull(scope, "'scope' must not be null");
 		if (this.startupFailure != null) {
@@ -284,7 +287,7 @@ public <T> AbstractObjectAssert<?, T> getBean(Class<T> type, Scope scope) {
 							getApplicationContext(), type, names));
 		}
 		T bean = (name != null) ? getApplicationContext().getBean(name, type) : null;
-		return Assertions.assertThat(bean).as("Bean of type <%s> from <%s>", type, getApplicationContext());
+		return assertThat(bean).as("Bean of type <%s> from <%s>", type, getApplicationContext());
 	}
 
 	private @Nullable String getPrimary(String[] names, Scope scope) {
@@ -330,13 +333,14 @@ private boolean isPrimary(String name, Scope scope) {
 	 * is found
 	 * @throws AssertionError if the application context did not start
 	 */
+	@CheckReturnValue
 	public AbstractObjectAssert<?, Object> getBean(String name) {
 		if (this.startupFailure != null) {
 			throwAssertionError(
 					contextFailedToStartWhenExpecting(this.startupFailure, "to contain a bean of name:%n <%s>", name));
 		}
 		Object bean = findBean(name);
-		return Assertions.assertThat(bean).as("Bean of name <%s> from <%s>", name, getApplicationContext());
+		return assertThat(bean).as("Bean of name <%s> from <%s>", name, getApplicationContext());
 	}
 
 	/**
@@ -357,6 +361,7 @@ public AbstractObjectAssert<?, Object> getBean(String name) {
 	 * name but a different type
 	 */
 	@SuppressWarnings("unchecked")
+	@CheckReturnValue
 	public <T> AbstractObjectAssert<?, T> getBean(String name, Class<T> type) {
 		if (this.startupFailure != null) {
 			throwAssertionError(contextFailedToStartWhenExpecting(this.startupFailure,
@@ -368,8 +373,8 @@ public <T> AbstractObjectAssert<?, T> getBean(String name, Class<T> type) {
 					"%nExpecting:%n <%s>%nto contain a bean of name:%n <%s> (%s)%nbut found:%n <%s> of type <%s>",
 					getApplicationContext(), name, type, bean, bean.getClass()));
 		}
-		return Assertions.assertThat((T) bean)
-			.as("Bean of name <%s> and type <%s> from <%s>", name, type, getApplicationContext());
+		return assertThat((T) bean).as("Bean of name <%s> and type <%s> from <%s>", name, type,
+				getApplicationContext());
 	}
 
 	private @Nullable Object findBean(String name) {
@@ -395,6 +400,7 @@ public <T> AbstractObjectAssert<?, T> getBean(String name, Class<T> type) {
 	 * no beans are found
 	 * @throws AssertionError if the application context did not start
 	 */
+	@CheckReturnValue
 	public <T> MapAssert<String, T> getBeans(Class<T> type) {
 		return getBeans(type, Scope.INCLUDE_ANCESTORS);
 	}
@@ -414,14 +420,15 @@ public <T> MapAssert<String, T> getBeans(Class<T> type) {
 	 * no beans are found
 	 * @throws AssertionError if the application context did not start
 	 */
+	@CheckReturnValue
 	public <T> MapAssert<String, T> getBeans(Class<T> type, Scope scope) {
 		Assert.notNull(scope, "'scope' must not be null");
 		if (this.startupFailure != null) {
 			throwAssertionError(
 					contextFailedToStartWhenExpecting(this.startupFailure, "to get beans of type:%n <%s>", type));
 		}
-		return Assertions.assertThat(scope.getBeansOfType(getApplicationContext(), type))
-			.as("Beans of type <%s> from <%s>", type, getApplicationContext());
+		return assertThat(scope.getBeansOfType(getApplicationContext(), type)).as("Beans of type <%s> from <%s>", type,
+				getApplicationContext());
 	}
 
 	/**
@@ -434,6 +441,7 @@ public <T> MapAssert<String, T> getBeans(Class<T> type, Scope scope) {
 	 * @return assertions on the cause of the failure
 	 * @throws AssertionError if the application context started without a failure
 	 */
+	@CheckReturnValue
 	public AbstractThrowableAssert<?, ? extends Throwable> getFailure() {
 		hasFailed();
 		return assertThat(this.startupFailure);

core/spring-boot-test/src/main/java/org/springframework/boot/test/json/JsonContentAssert.java

@@ -40,6 +40,7 @@
 import org.skyscreamer.jsonassert.comparator.JSONComparator;
 
 import org.springframework.core.io.Resource;
+import org.springframework.lang.CheckReturnValue;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.function.ThrowingFunction;
@@ -917,6 +918,7 @@ public JsonContentAssert doesNotHaveEmptyJsonPathValue(CharSequence expression,
 	 * @return a new assertion object whose object under test is the extracted item
 	 * @throws AssertionError if the path is not valid
 	 */
+	@CheckReturnValue
 	public AbstractObjectAssert<?, Object> extractingJsonPathValue(CharSequence expression, Object... args) {
 		return Assertions.assertThat(new JsonPathValue(expression, args).getValue(false));
 	}
@@ -929,6 +931,7 @@ public AbstractObjectAssert<?, Object> extractingJsonPathValue(CharSequence expr
 	 * @return a new assertion object whose object under test is the extracted item
 	 * @throws AssertionError if the path is not valid or does not result in a string
 	 */
+	@CheckReturnValue
 	public AbstractCharSequenceAssert<?, String> extractingJsonPathStringValue(CharSequence expression,
 			Object... args) {
 		return Assertions.assertThat(extractingJsonPathValue(expression, args, String.class, "a string"));
@@ -942,6 +945,7 @@ public AbstractCharSequenceAssert<?, String> extractingJsonPathStringValue(CharS
 	 * @return a new assertion object whose object under test is the extracted item
 	 * @throws AssertionError if the path is not valid or does not result in a number
 	 */
+	@CheckReturnValue
 	public AbstractObjectAssert<?, Number> extractingJsonPathNumberValue(CharSequence expression, Object... args) {
 		return Assertions.assertThat(extractingJsonPathValue(expression, args, Number.class, "a number"));
 	}
@@ -954,6 +958,7 @@ public AbstractObjectAssert<?, Number> extractingJsonPathNumberValue(CharSequenc
 	 * @return a new assertion object whose object under test is the extracted item
 	 * @throws AssertionError if the path is not valid or does not result in a boolean
 	 */
+	@CheckReturnValue
 	public AbstractBooleanAssert<?> extractingJsonPathBooleanValue(CharSequence expression, Object... args) {
 		return Assertions.assertThat(extractingJsonPathValue(expression, args, Boolean.class, "a boolean"));
 	}
@@ -968,6 +973,7 @@ public AbstractBooleanAssert<?> extractingJsonPathBooleanValue(CharSequence expr
 	 * @throws AssertionError if the path is not valid or does not result in an array
 	 */
 	@SuppressWarnings("unchecked")
+	@CheckReturnValue
 	public <E> ListAssert<E> extractingJsonPathArrayValue(CharSequence expression, Object... args) {
 		return Assertions.assertThat(extractingJsonPathValue(expression, args, List.class, "an array"));
 	}
@@ -983,6 +989,7 @@ public <E> ListAssert<E> extractingJsonPathArrayValue(CharSequence expression, O
 	 * @throws AssertionError if the path is not valid or does not result in a map
 	 */
 	@SuppressWarnings("unchecked")
+	@CheckReturnValue
 	public <K, V> MapAssert<K, V> extractingJsonPathMapValue(CharSequence expression, Object... args) {
 		return Assertions.assertThat(extractingJsonPathValue(expression, args, Map.class, "a map"));
 	}