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

Move type annotations from methods and fields closer to the types #3880

Closed
vlsi opened this issue Jan 3, 2024 · 3 comments
Closed

Move type annotations from methods and fields closer to the types #3880

vlsi opened this issue Jan 3, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@vlsi
Copy link

vlsi commented Jan 3, 2024

What problem are you trying to solve?

I don't like how OpenRewrite moved @Nullable from type to a method.

-  @Nullable String getSlotName();
+  @Nullable
+  @Override
+  String getSlotName();

Describe the solution you'd like

@Nullable is a type annotation, so it applies to String type rather than getSlotName() method.

Since OpenRewrite knows the exact definition of the annotation, it should be able to infer that @Nullable is a type annotation while @Override is not.

So I would like the formatting to be

  @Override
  @Nullable String getSlotName();

The emerging jspecify uses only TYPE_USE, see https://github.com/jspecify/jspecify/blob/ee91db649ce2de81d39b55b52345730b3f0f2531/src/main/java/org/jspecify/annotations/Nullable.java#L142, so users should prefer

    @Nullable String newValue;

rather than

    @Nullable
    String newValue;
@vlsi vlsi added the enhancement New feature or request label Jan 3, 2024
@knutwannheden
Copy link
Contributor

At the moment I am not even sure that OpenRewrite knows the full definition of annotation types. While we have type attribution for annotations (like @Nullable) and the type attribution also lists the annotations present on that annotation type (e.g. @Target), we currently unfortunately don't include the annotation parameter values in the type attribution. So we don't know if @Target includes TYPE_USE or not. At parse time of the source this is available, but in the LST (specifically the type attribution) this information is lost.

We have an open issue for this here: #3504. Having a concrete use case for this enhancement is really useful. Should we add that as a comment to the mentioned issue and close this one?

@vlsi
Copy link
Author

vlsi commented Jan 3, 2024

Deduplicating issues sounds reasonable

@timtebeek
Copy link
Contributor

Thanks! Copied the above there just now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

3 participants