Decorate all Assert implementations with @CheckReturnValue

[scordio]

See #32683.

Deliverables

• [x] Annotate all existing Assert implementations
• [x] Build ArchUnit rule to ensure consistency

[snicoll]

See https://github.com/spring-projects/spring-boot/issues/32683#issuecomment-3173564225

[scordio]

As I need to extend Checkstyle's import-control.xml, would it be fine to add a top-level allow statement close to:

https://github.com/spring-projects/spring-boot/blob/a17684921d9fb7fa70289aa821ac0a1c4eab3545/config/checkstyle/import-control.xml#L6

or should I define a more fine-grained rule, restricting it to the following packages?

• org.springframework.boot.test.context.assertj
• org.springframework.boot.test.json

[snicoll]

would it be fine to add a top-level allow statement close to

Yep. The intention is to restrict only to certain AssertJ entry point and this is one of them so I think that's ok.

[snicoll]

@scordio how is it going, anything I can do to help?

[scordio]

Apologies, life happened and this fell off my radar! 🤦

If not in a rush, I'm happy to continue on it over the weekend.

[wilkinsona]

Thanks, @scordio. There's no rush.

[scordio]

When building the project locally, I get two NullAway errors on :core:spring-boot:compileJava:

/Users/stefano/Projects/spring-boot/core/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java:424: error: [NullAway] Method returns @Nullable String @Nullable [], but overridden method returns String [], which has mismatched type parameter nullability
		public @Nullable String @Nullable [] getParameterNames(Constructor<?> constructor) {
		                                     ^
    (see http://t.uber.com/nullaway )

/Users/stefano/Projects/spring-boot/core/spring-boot/src/main/java/org/springframework/boot/logging/structured/StructuredLogFormatterFactory.java:240: error: [NullAway] referenced method returns @Nullable, but functional interface method java.util.function.Function.apply(T) returns @NonNull
					ArgumentResolver.from(StructuredLogFormatterFactory.this.instantiator::getArg));
					                      ^
    (see http://t.uber.com/nullaway )

but the CI seems fine 🤔

[snicoll]

@scordio you need Java 24 to build the project

[scordio]

Build ArchUnit rule to ensure consistency

Some more fine-tuning is needed, but I'm getting there 🙂

Architecture Violation [Priority: MEDIUM] - Rule 'methods that are declared in classes that implement org.assertj.core.api.Assert and not don't return same type should be annotated with @CheckReturnValue' was violated (14 times):
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.contextFailedToStartWhenExpecting(java.lang.Throwable, java.lang.String, [Ljava.lang.Object;)> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:491)
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.findBean(java.lang.String)> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:382)
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.getApplicationContext()> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:482)
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.getPrimary([Ljava.lang.String;, org.springframework.boot.test.context.assertj.ApplicationContextAssert$Scope)> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:294)
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.getStartupFailure()> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:486)
Method <org.springframework.boot.test.context.assertj.ApplicationContextAssert.isPrimary(java.lang.String, org.springframework.boot.test.context.assertj.ApplicationContextAssert$Scope)> is not annotated with @CheckReturnValue in (ApplicationContextAssert.java:310)
Method <org.springframework.boot.test.json.JsonContentAssert.compare(java.lang.CharSequence, org.skyscreamer.jsonassert.JSONCompareMode)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:1007)
Method <org.springframework.boot.test.json.JsonContentAssert.compare(java.lang.CharSequence, org.skyscreamer.jsonassert.comparator.JSONComparator)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:1023)
Method <org.springframework.boot.test.json.JsonContentAssert.compareForNull(java.lang.CharSequence)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:1039)
Method <org.springframework.boot.test.json.JsonContentAssert.extractingJsonPathValue(java.lang.CharSequence, [Ljava.lang.Object;, java.lang.Class, java.lang.String)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:999)
Method <org.springframework.boot.test.json.JsonContentAssert.isEqualTo(java.lang.Object)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:56)
Method <org.springframework.boot.test.json.JsonContentAssert.isEqualTo(java.lang.Object)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:56)
Method <org.springframework.boot.test.json.JsonContentAssert.isNotEqualTo(java.lang.Object)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:56)
Method <org.springframework.boot.test.json.JsonContentAssert.isNotEqualTo(java.lang.Object)> is not annotated with @CheckReturnValue in (JsonContentAssert.java:56)

[snicoll]

I've moved to this to 4.x as we're approaching RC1 but not worries either way. Thanks for your efforts!

[scordio]

Thanks, @wilkinsona! I'll address them later in the day.

[snicoll]

This PR is ready to be merged once main switches to 4.1.0-SNAPSHOT.