Skip to content

Decorate all Assert implementations with @CheckReturnValue#46766

Closed
scordio wants to merge 1 commit into
spring-projects:mainfrom
scordio:gh-32683-CheckReturnValue
Closed

Decorate all Assert implementations with @CheckReturnValue#46766
scordio wants to merge 1 commit into
spring-projects:mainfrom
scordio:gh-32683-CheckReturnValue

Conversation

@scordio

@scordio scordio commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

See #32683.

Deliverables

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

@snicoll

snicoll commented Aug 11, 2025

Copy link
Copy Markdown
Member

See #32683 (comment)

@snicoll snicoll added the status: on-hold We can't start working on this issue yet label Aug 11, 2025
@scordio
scordio marked this pull request as draft August 11, 2025 07:30
@scordio

scordio commented Aug 11, 2025

Copy link
Copy Markdown
Contributor Author

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

<allow class="org.springframework.lang.Contract" />

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

snicoll commented Aug 11, 2025

Copy link
Copy Markdown
Member

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

snicoll commented Sep 10, 2025

Copy link
Copy Markdown
Member

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

@scordio

scordio commented Sep 10, 2025

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Member

Thanks, @scordio. There's no rush.

@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch 2 times, most recently from 76f6b34 to 934b746 Compare September 14, 2025 12:21
@scordio scordio changed the title Annotate ApplicationContextAssert methods with @CheckReturnValue Decorate all Assert implementations with @CheckReturnValue Sep 14, 2025
@scordio

scordio commented Sep 14, 2025

Copy link
Copy Markdown
Contributor Author

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

snicoll commented Sep 14, 2025

Copy link
Copy Markdown
Member

@scordio you need Java 24 to build the project

@scordio

This comment was marked as outdated.

@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch from 934b746 to df47244 Compare September 14, 2025 21:20
@snicoll snicoll added type: enhancement A general enhancement and removed status: on-hold We can't start working on this issue yet status: waiting-for-triage An issue we've not yet triaged labels Sep 16, 2025
@snicoll snicoll added this to the 4.0.x milestone Sep 16, 2025
@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch from df47244 to d045ca2 Compare September 19, 2025 16:56
@snicoll snicoll modified the milestones: 4.0.x, 4.x Oct 5, 2025
@snicoll

snicoll commented Oct 5, 2025

Copy link
Copy Markdown
Member

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

@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch from d045ca2 to 802b3db Compare October 10, 2025 07:07
@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch 2 times, most recently from c744ebb to 17cb2f0 Compare November 3, 2025 10:04
@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch 2 times, most recently from 0b2f0e8 to bbb9c1c Compare November 15, 2025 14:23
@scordio
scordio marked this pull request as ready for review November 15, 2025 15:52

@wilkinsona wilkinsona left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much, @scordio.

I've left a couple of very minor comments. No need for you to do anything about them unless you really want to. We can take care of them when we merge this. That'll take a little while as it'll hopefully go in Spring Boot 4.1 and we need to first get 4.0 out of the door and then create the 4.0.x maintenance branch.

@scordio

scordio commented Nov 17, 2025

Copy link
Copy Markdown
Contributor Author

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

Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>
@scordio
scordio force-pushed the gh-32683-CheckReturnValue branch from bbb9c1c to 18513ed Compare November 17, 2025 11:55
@snicoll snicoll self-assigned this Nov 19, 2025
snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Nov 28, 2025
See spring-projectsgh-46766

Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>
@snicoll snicoll modified the milestones: 4.x, 4.1.x Nov 28, 2025
@snicoll

snicoll commented Nov 28, 2025

Copy link
Copy Markdown
Member

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

snicoll pushed a commit that referenced this pull request Dec 19, 2025
See gh-46766

Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>
@snicoll snicoll closed this in 90faddf Dec 19, 2025
@snicoll snicoll modified the milestones: 4.1.x, 4.1.0-M1 Dec 19, 2025
@snicoll

snicoll commented Dec 19, 2025

Copy link
Copy Markdown
Member

Good stuff @scordio - Thanks very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants