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

@DependentRequired, @DependentSchema and @SchemaProperty are missing @Target #676

Open
Azquelt opened this issue Dec 11, 2024 · 1 comment
Labels

Comments

@Azquelt
Copy link
Member

Azquelt commented Dec 11, 2024

I believe all of these annotations are only intended to be used as nested annotations within @Schema, so they should have @Target({}).

Without @Target they can be placed anywhere, but they'll likely be ignored.

Most are also missing @Retention so if placed on a Java element, they won't be visible at runtime. When used as a nested annotation (as intended for these annotations), @Retention has no effect.

I'm unsure whether fixing this would constitute a breaking change:

  • I don't think it would prevent anything loading at runtime (since annotation errors are generally ignored at runtime).
  • It would break source-compatibility, users who used these annotations directly on a Java element would get compile errors.
  • I don't think any implementation should be processing these annotations if they're placed on a Java element.
@Azquelt Azquelt added the bug label Dec 11, 2024
@MikeEdgar
Copy link
Member

I suspect strict semver would say it's a breaking change requiring a major version, but perhaps we can choose to be flexible on that since the missing @Target was indeed a bug. I'd say the pragmatic thing would be to fix this in the next minor release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants