-
Notifications
You must be signed in to change notification settings - Fork 41.4k
Detect @Nullable on TYPE_USE parameters in configuration metadata #46854
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
base: main
Are you sure you want to change the base?
Detect @Nullable on TYPE_USE parameters in configuration metadata #46854
Conversation
Update MetadataGenerationEnvironment#getAnnotation to also inspect type-use annotations (element.asType().getAnnotationMirrors()) in addition to declaration annotations. This ensures that @nullable on method parameter types is correctly detected when generating configuration metadata for Actuator endpoints. Previously, parameters annotated with @nullable in TYPE_USE position could be missed, causing them to be marked as required in metadata. With this change, both declaration-level and type-use @nullable annotations are recognized. No new dependencies are introduced, and existing public APIs remain unchanged. Signed-off-by: wonyongg <111210881+wonyongg@users.noreply.github.com>
Thanks for the PR.
How do you know? Without additional tests that validates this is taken into account in the same way as |
- Add NullableParameterEndpoint test sample with @nullable parameter - Add OptionalParameterEndpoint test sample for comparison - Add test to verify @nullable and @OptionalParameter are treated equivalently - Ensures TYPE_USE annotation detection works correctly for optional parameters Signed-off-by: wonyongg <111210881+wonyongg@users.noreply.github.com>
Thanks for the feedback! I've added comprehensive tests that validate the @nullable annotation detection works correctly for Actuator endpoint parameters. The tests include:
These tests demonstrate that the updated getAnnotation method correctly detects @nullable annotations on method parameters, ensuring they are treated the same as @OptionalParameter annotations when determining if a parameter is optional. This validates the core fix in the PR where TYPE_USE annotations are now properly discovered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we're there yet. This is the wrong annotation, please review.
|
||
import org.springframework.boot.configurationsample.Endpoint; | ||
import org.springframework.boot.configurationsample.ReadOperation; | ||
import org.springframework.lang.Nullable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the wrong class. It should be org.jspecify.annotations.Nullable
@Target({ ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD, ElementType.TYPE_USE }) | ||
@Retention(RetentionPolicy.SOURCE) | ||
@Documented | ||
public @interface Nullable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this. The annotations that are copied are from Spring Boot itself as we can't rely on them without creating a cycle.
Update
MetadataGenerationEnvironment#getAnnotation
to also inspect type-use annotations (element.asType().getAnnotationMirrors()
) in addition to declaration annotations. This ensures that@Nullable
on method parameter types is correctly detected when generating configuration metadata for Actuator endpoints.Previously, parameters annotated with
@Nullable
in TYPE_USE position could be missed, causing them to be marked as required in metadata. With this change, both declaration-level and type-use@Nullable
annotations are recognized.No new dependencies are introduced, and existing public APIs remain unchanged.
This pull request addresses the issue described in #46837.
Future improvement:
Currently, the processor only looks for
org.springframework.lang.Nullable
. If there is interest, we could follow up with a small, separate PR to support additional well-known@Nullable
annotations (e.g.org.jspecify.annotations.Nullable
,javax.annotation.Nullable
).