[Spring] fix missing @Nullable on toIndentedString param when useJspecify=true - #24567
Open
SubhamAshok wants to merge 2 commits into
Open
[Spring] fix missing @Nullable on toIndentedString param when useJspecify=true#24567SubhamAshok wants to merge 2 commits into
SubhamAshok wants to merge 2 commits into
Conversation
…cify=true The nullableAnnotation partial strips the annotation in jspecify mode and nothing re-inserts it, since Object is not wrapped in the jSpecifyDatatype lambda. Wrap it like the rest of the template does. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
@SubhamAshok I think everyone makes it too complex by using It gives side effects with the complex conditions on the property (required, isNullable, defaultValue....) My PR is work in progress: #24563 |
…partial Review feedback: nullableAnnotation is meant for a CodegenProperty context and its conditions cause side effects here. The method always accepts null, so state it directly. useOptional samples now get the annotation too. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Author
Contributor
|
@SubhamAshok My PR is ready #24563 It includes the same fix for toIndentedString, and a lot more for fluent setters... Can you review? |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With useJspecify=true, toIndentedString(Object o) is generated without @nullable. The nullableAnnotation partial strips the annotation and nothing puts it back, since Object is not wrapped in the jSpecifyDatatype lambda. Without jspecify the output is @nullable Object o, so the two modes disagree.
This matters because the model package is @NullMarked and toString() passes nullable fields into this method. NullAway and IntelliJ report an error on every generated model.
Fix: wrap Object in the jSpecifyDatatype lambda, same pattern as the rest of the template. This complements #24563 which fixes the constructor and fluent setter spots. Relates to #23848.
Checked both modes. jspecify=true now gives @nullable Object o with the import. jspecify=false output is byte identical to before. Added an assertion to SpringCodegenTest#testJspecify, all 8 combos pass. Regenerated samples for bin/configs/spring-boot-4-jspecify.yaml, the diff is one line.
PR checklist
cc Java Spring technical committee: @cachescrubber @welshm @MelleD @atextor @manedev79 @javisst @borsch @banlevente @Zomzog @martin-mfg @KannaKim
Summary by cubic
Fixes missing @nullable on toIndentedString(Object o) when
useJspecify=trueby hardcoding the annotation in the Spring POJO template. This aligns jspecify and non-jspecify outputs and removes nullness errors in@NullMarkedmodels. Complements #24563 and relates to #23848.@NullableontoIndentedString(@Nullable Object o)inpojo.mustacheinstead of using thenullableAnnotationpartial, avoiding context side effects and applying the fix to use-optional variants too.org.jspecify.annotations.Nullable; regenerate affected samples.Written for commit 6bd8ee2. Summary will update on new commits.