Revise nullability in HttpEntity and ResponseEntity#36238
Revise nullability in HttpEntity and ResponseEntity#36238ngocnhan-tran1996 wants to merge 1 commit intospring-projects:mainfrom
Conversation
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
spring-projects-issues
added
the
status: waiting-for-triage
|
|
||
| private final HttpHeaders headers; | ||
|
|
||
| private final @Nullable T body; |
There was a problem hiding this comment.
Here, you should just leverage the nullability defined at type level, so please remove the explicit @Nullable override here.
There was a problem hiding this comment.
I have removed but the compileJava task build fail
Within @NullMarked, remember, unannotated types are the same as if they were annotated with @nonnull. Since the bound of E is the same as @nonnull Number, and not @nullable Number, that means the type argument for E can't be a type that includes null. @nullable Integer can't be the type argument, then, since that can include null. (In other words: @nullable Integer is not a subtype of Number.)
There was a problem hiding this comment.
You need to update the rest of the codebase to fix NullAway errors, with for example here changing the return value to HttpEntity<? extends @Nullable Object> build() for example.
| * @param headers the entity headers | ||
| * @since 7.0 | ||
| */ | ||
| public HttpEntity(@Nullable T body, @Nullable HttpHeaders headers) { |
There was a problem hiding this comment.
Here, you should just leverage the nullability defined at type level, so please remove the explicit @Nullable override here and in other similar use cases.
sdeleuze
added
the
in: web
sbrannen
changed the title
@Nullable to ResponseEntity generic type
sdeleuze
left a comment
sdeleuze
left a comment
There was a problem hiding this comment.
After discussing with the team, we would like to clarify how entity of empty value translates on both Java and Kotlin signature level before deciding if we move forward.
| * @see RequestEntity | ||
| */ | ||
| public class ResponseEntity<T> extends HttpEntity<T> { | ||
| public class ResponseEntity<T extends @Nullable Object> extends HttpEntity<T> { |
There was a problem hiding this comment.
Can you check how ResponseEntity of "empty value" translates in a sample application checked with NullAway, using https://github.com/sdeleuze/jspecify-nullaway-demo as a basis maybe?
I guess that would be ResponseEntity<@Nullable Void> based on jspecify/jspecify#51 but worth to check.
|
|
||
|
|
||
| @Test | ||
| fun ofNullNullableType() { |
There was a problem hiding this comment.
Please add a test for empty value, ie. how ResponseEntity<@Nullable Void> translates to Kotlin. Is it ResponseEntity<Unit>?
4 participants
Based on the IDE warning shown in the screenshot
This change adds
@Nullableto the generic bound of ResponseEntity to make the nullability contract.Related to gh-36236