Skip to content
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

Fixed process nullable annotations #923

Merged
merged 6 commits into from
Feb 3, 2023

Conversation

altro3
Copy link
Collaborator

@altro3 altro3 commented Jan 31, 2023

Fixed #921

Copy link

@nbrugger-tgm nbrugger-tgm left a comment

Choose a reason for hiding this comment

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

Loogs good to me, maybe pattern matching (like *.Nullable) could be considered but that could have unintended side effects in very rare cases. But that's up to you / just an idea

@altro3 altro3 mentioned this pull request Feb 1, 2023
|| element.getType().isOptional()
|| element.hasStereotype(Nullable.class)
|| element.hasStereotype("jakarta.annotation.Nullable")
|| element.hasStereotype("org.jetbrains.annotations.Nullable");
Copy link
Contributor

Choose a reason for hiding this comment

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

Please consider supporting other nullability annotations https://kotlinlang.org/docs/java-interop.html#nullability-annotations?

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be implemented using an annotation transformer to transform the annotations to AnnotationUtil.NULLABLE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

add all other annotations too

@altro3 altro3 force-pushed the fix_nullable branch 2 times, most recently from cc9877b to 2545aa3 Compare February 2, 2023 11:07
@altro3
Copy link
Collaborator Author

altro3 commented Feb 2, 2023

@sdelamo @dstepanov I've added support all nullable annotations from @sdelamo 's link. But I think i found bug in micronaut:

element.hasStereotype("org.eclipse.jdt.annotation.Nullable")
element.hasStereotype("org.jspecify.annotations.Nullable")

These two annotations not found.

I think problem is here: both annotions have Target TYPE_USE

@Target(TYPE_USE)

return element.isNullable()
|| element.getType().isOptional()
|| element.hasStereotype(Nullable.class)
|| element.hasStereotype("javax.annotation.Nullable")
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dstepanov No, can't. Because it since micronaut 4.0, but this fix for release 4.8.3 which based on micronaut 3.8.3

Copy link
Contributor

Choose a reason for hiding this comment

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

It should work in 3.8.3 too.

Copy link
Collaborator Author

@altro3 altro3 Feb 2, 2023

Choose a reason for hiding this comment

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

@dstepanov no! this class added in micronaut 4.0:
изображение

Can't find this class in 3.8.x branch

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the class is just an example how to do it. Just copy it, modify the annotation name and define it in the service entires.

@dstepanov
Copy link
Contributor

@altro3 Javac might be acting strange with those types. Maybe you can create an issue with a reproducible test in the core?

gradle/libs.versions.toml Outdated Show resolved Hide resolved
@altro3
Copy link
Collaborator Author

altro3 commented Feb 2, 2023

gradle/libs.versions.toml Outdated Show resolved Hide resolved
gradle/libs.versions.toml Outdated Show resolved Hide resolved
@sdelamo sdelamo merged commit d6451c9 into micronaut-projects:4.8.x Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants