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

AnnotateNullableMethods should have a parameter for the FQN of the nullable type #428

Open
Bananeweizen opened this issue Jan 7, 2025 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Bananeweizen
Copy link
Contributor

What problem are you trying to solve?

AnnotateNullableMethods currently adds a fixed Nullable annotation, the one that OpenRewrite uses itself. The fully qualified name for that nullable type should be parameterizable, as many projects use other nullable types. Even OpenRewrite changed its code base just recently to switch to JSpecify.

Describe the solution you'd like

one new argument for the FQN to use, with the default of the current value to not cause migration effort

Have you considered any alternatives or workarounds?

Right now people with other annotations also can just run this recipe and then do a search-replace of the import.

Are you interested in contributing this feature to OpenRewrite?

Yes. Is there a convention for naming parameters or would you suggest a name? "nullableType"?

@Bananeweizen Bananeweizen added the enhancement New feature or request label Jan 7, 2025
@timtebeek
Copy link
Contributor

hi @Bananeweizen ; thanks for the suggestion! One thing to check perhaps before we make the nullable annotation configurable, is if your desired argument supports this style of use:

public static Outer.@Nullable Inner test() { return null; }

If yes, then fine to make it optionally configurable with the existing value the default; Otherwise it might complicate matters a bit.

@timtebeek timtebeek moved this to Backlog in OpenRewrite Jan 7, 2025
@timtebeek timtebeek added the good first issue Good for newcomers label Jan 7, 2025
@Bananeweizen
Copy link
Contributor Author

@timtebeek Your question is basically: Is my null annotation a declaration annotation (Java 5) or a type annotation (Java 8) using @Target({...}). AFAIK all type annotations fit your needs (or the FQN case like java.io.@Nullable File) and similar. My null annotation (Eclipse JDT) definitely fits, and I would also expect other major null annotations to be type annotations nowadays.

@timtebeek
Copy link
Contributor

Perfect, then indeed ok to add a nullableType option to AnnotateNullableMethods which takes a fully qualified class name. 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

2 participants