-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Migrate nullability annotations to JSpecify #4990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>
Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>
Thank you!
I cherry-picked the changes from #4864 as they haven't been merged yet.
Good. I already reviewed #4864 and currently working on the refactoring mentioned here: #4864 (comment). So since you cherry-picked the changes here, can we close #4864 in favour of this PR? I will continue on this one since it includes everything.
BTW, this will definitely require at least two passes. But I think it should be ok to ship in the last milestone (6.0 M4). I will keep you posted. Thank you again for this amazing effort!
So since you cherry-picked the changes here, can we close #4864 in favour of this PR?
Totally up to you! If you prefer a single PR for all modules, #4864 can definitely be closed.
On the other side, #4864 is "stable" and touches only spring-batch-infrastructure, while here there is plenty of work in progress across all the other modules...
spring-batch-core to JSpecify annotations (削除ここまで)That's fine. I will focus on this one and adapt where needed.
okay.. after digging into this, nullability specs and checks will be very difficult to implement correctly with the current state of things in Spring Batch (nothing to do with this PR).
The current model with default constructors (to make persistence and serialisation tools happy) is just broken by design. What makes things difficult is the fact that there is a single model used as a domain model and a persistence/serialization model at the same time. I remember I came across this problem when I introduced the first NoSQL store implementation.
I need to think more about the best way to proceed with the current state of things and the current context. I will get back with an update here asap.
The current model with default constructors (to make persistence and serialisation tools happy) is just broken by design.
Indeed, that is essentially why I used @SuppressWarnings("DataFlowIssue") extensively.
I'll stay tuned! 👀
antshaykolo
commented
Sep 25, 2025
main
This commit updates nullability checks based on the new changes introduced in #1870. The main refinements are: - Add constructors with mandatory properties - Avoid accepting nullable parameters where appropriate Some classes have been excluded from nullability checks util we fully remove the "no-arg constructor + setters + afterPropertiesSet" approach to creating objects. Resolves #4673
Rebased, squashed and merged as 551e26d. Thanks again for this amazing contribution!
As discussed, I refined things in 21af80d after doing a big preparatory work in 90d8959. I also excluded deprecated APIs from nullability checks to reduce the surface (see 0d49dba).
This refinement is the first stable pass, more refinements will come in #4988 and #4989
This is the next step for:
I cherry-picked the changes from #4864 as they haven't been merged yet.
I initially wanted to target
spring-batch-coreonly as the next module, but, as discussed at #4989 (comment), the clock is ticking 🙂I'm just pushing what I have, and please consider that there are also changes from other modules. Feel free to reuse them if it helps, or just discard them!
If I can support with anything, please don't hesitate to let me know.