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

Java 14+ : add @Serial annotation to serialVersionUID fields #96

Closed
yeikel opened this issue Apr 22, 2022 · 8 comments
Closed

Java 14+ : add @Serial annotation to serialVersionUID fields #96

yeikel opened this issue Apr 22, 2022 · 8 comments
Labels
recipe Recipe requested

Comments

@yeikel
Copy link
Contributor

yeikel commented Apr 22, 2022

Relevant :
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-September/081364.html
https://stackoverflow.com/questions/63783474/what-is-the-use-of-serial-annotation-as-of-java-14

@pway99 pway99 added the recipe Recipe requested label Apr 22, 2022
@melloware
Copy link
Contributor

+1 for this. Dont know how easy this would be though.

@melloware
Copy link
Contributor

@timtebeek i am interested in doing this one but need a hint on how to get started. The general idea is

  1. detect private static final long serialVersionUID = 1L;
  2. Add maybeAddImport("java.io.Serial", false);
  3. Add @Serial above the private static final long serialVersionUID

@timtebeek
Copy link
Contributor

hi @melloware, welcome back! It seems this open issue was missed when the recipe was implemented in another castle rewrite-static-analysis

You can already use that existing recipe; I've just pushed up a change to the latest snapshot to correct the casing:

Thanks for reaching out on this issue! Good to see this can be closed.

@melloware
Copy link
Contributor

OK wasn't sure where to ask but I wasn't seeing this run as part of a Java 17 migration i just did so I assumed this would have been part of an automatic upgrade recipe?

@timtebeek
Copy link
Contributor

I had doubted at the time to make this part of the upgrade recipe; the way I see it there might be some push back from folks that prefer a "minimal" migration, as opposed to having to review additional lines any place there's a serialVersionUID. What would be your argument for inclusion by default? Just to weigh the options.

@melloware
Copy link
Contributor

Yeah I guess my only argument is in INtelliJ when I open a Java 17 or 21 project that warning comes up on every class to add @Serial so its one less thing I have to do to N classes on a Migration. That was my only thought.

In theory I can run the 17 Recipe and then run this custom recipe. But there is a reason Java added this and to me it make sense to migrate it by default. Just my two cents.

@timtebeek
Copy link
Contributor

Ah I didn't know about the warnings in IntelliJ IDEA; that'd be a stronger reason to include this recipe by default. Would you want to push that up?

@melloware
Copy link
Contributor

Yes let me submit a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe requested
Projects
Archived in project
Development

No branches or pull requests

4 participants