Recipes for methods and classes renamed or moved when switching to Java 17 #430

ranuradh · 2024-03-11T14:34:06Z

This PR contains 4 recipes for Java SE 17 migrations.
They are:

org.openrewrite.java.migrate.RemovedZipFinalizeMethods
org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl
org.openrewrite.java.migrate.SunNetSslPackageUnavailable
org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant

Checklist

I've added unit tests to cover both positive and negative cases - Added them where possible
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java

timtebeek · 2024-03-11T14:41:35Z

Hi @ranuradh ! Since you last contributed we added some automated code reviews which post comments on PRs. You'll want to apply their suggestions, or make adjustments to ensure there's no new suggestions after your last commit.

…vailableTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

….java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

timtebeek · 2024-03-12T16:57:33Z

Thanks a lot @ranuradh ! @cjobinabo will you do the first round of review here? Just to be sure it's clear who's in the lead.

timtebeek

Great start; some small touch ups to be applied in bulk from this page:
https://github.com/openrewrite/rewrite-migrate-java/pull/430/files

Other than that I think we can see this through quite quickly if @cjobinabo agrees.

src/main/resources/META-INF/rewrite/java-version-17.yml

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java

Co-authored-by: Tim te Beek <tim@moderne.io>

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

….java Co-authored-by: Tim te Beek <tim@moderne.io>

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

ranuradh · 2024-03-12T18:55:23Z

@timtebeek I saw all the touch ups and committed them. I took note of not using public in test classes and some formatting. This week @cjobinabo has a tech event that he is attending I think if you review and if we can merge it that will be one less review for him when he gets back. I don't think he will mind it.

src/main/resources/META-INF/rewrite/java-version-17.yml

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java

ranuradh · 2024-03-18T15:45:17Z

@timtebeek and @cjobinabo can we close and merge this. I thought this was done.Thanks!

timtebeek · 2024-03-18T19:12:23Z

Ah I had thought @cjobinabo would do the final check, as I'm out for the next couple weeks.

ranuradh · 2024-03-18T19:15:20Z

No worries he is back and should be able to take care of it. See you after a couple of weeks!

cjobinabo

Thanks for looking into these recipes. I left a few comments about missing tests, wording updates, and issues I noticed with some recipes.

cjobinabo · 2024-03-18T21:45:36Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+tags:
+  - java17
+recipeList:
+  - org.openrewrite.java.ReplaceConstantWithAnotherConstant:


I don't see a test for this recipe.

Hi @cjobinabo since it wasnt possible to find a jar/jdk upgrade we decided to not have a test for this.. Refer to our slack conversation

The only recipes we discussed on Slack before I left were about the removed finalize method and the removed ConstantBootstraps constructor. Perhaps this is a conversation you had with someone else. If you can't find a jar with the dependencies, I would suggest mocking one up like we have done in the past. I would rarely suggest omitting a test.

cjobinabo · 2024-03-18T21:45:57Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+   `java.util.zip.Inflater` and `java.util.zip.Deflater` as it is no longer available in Java SE 12 and later.
+tags:
+  - java17
+recipeList:


I don't see a test for this recipe.

Hi @cjobinabo since it wasnt possible to find a jar with the issue , we decided to not have a test for this.. Refer to our slack conversation

Is this a conversation you and I had, or you and Tim? If you couldn't find a jar with the dependency, I would have suggested mocking one up as we have before.

This was about running tests where we need an older version of Java and we cant get that to work -

so while there's no good way to unit test such recipes, you'll have to trust us that the particular case works when run on Java 8 or 11

cjobinabo · 2024-03-18T21:49:44Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+      newMethodName: end
+      ignoreDefinition: true
+  - org.openrewrite.java.ChangeMethodName:
+      methodPattern: "java.util.ZipFile finalize()"


This should be java.util.zip.ZipFile

cjobinabo · 2024-03-18T21:50:48Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+      methodPattern: "java.util.zip.Inflater finalize()"
+      newMethodName: end
+      ignoreDefinition: true
+      matchOverrides: true


If we ignore definitions, we shouldn't need to do anything with overrides.

fixed in next commit

cjobinabo · 2024-03-18T21:52:33Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+name: org.openrewrite.java.migrate.RemovedZipFinalizeMethods
+displayName: Remove `finalize` method in `java.util.zip.Zipfile`, `java.util.zip.Inflater` and `java.util.zip.Deflater`
+description: >
+  The `finalize` methods in `java.util.zip.ZipFile` is replaced with `close` method and is replaced by `end` method in 


methods should be method

fixed in next commit

cjobinabo · 2024-03-18T22:05:23Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+---
+type: specs.openrewrite.org/v1beta/recipe
+name: org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl
+displayName: Remove `SSLSession.getPeerCertificateChain()` method


Remove to Replace.

fixed in next commit

cjobinabo · 2024-03-18T22:06:48Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+---
+type: specs.openrewrite.org/v1beta/recipe
+name: org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant
+displayName: Remove `RMIConnectorServer.CREDENTIAL_TYPES` constant


Remove to Replace.

fixed in next commit

cjobinabo · 2024-03-18T22:07:10Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+name: org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant
+displayName: Remove `RMIConnectorServer.CREDENTIAL_TYPES` constant
+description: >
+  This recipe  replaces `RMIConnectorServer.CREDENTIAL_TYPES` constant with the `RMIConnectorServer.CREDENTIALS_FILTER_PATTERN` constant and a filter pattern.


Remove extra space between recipe replaces. Also add the before RMIConnectorServer.CREDENTIAL_TYPES

The last bit about the pattern and a filter pattern can be removed

fixed in next commit

cjobinabo · 2024-03-18T22:14:00Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+  - java17
+recipeList:
+  - org.openrewrite.java.ReplaceConstantWithAnotherConstant:
+      existingFullyQualifiedConstantName: javax.management.remote.rmi.RMIConnectorServer.CREDENTIAL_TYPES


CREDENTIAL_TYPES should be CREDENTIAL_TYPE

In this rule I see the this rule flags references to the RMIConnectorServer.CREDENTIAL_TYPES constant and jmx.remote.rmi.server.credential.types string literal. The RMIConnectorServer.CREDENTIAL_TYPES constant was removed in Java SE 15. The constant can be replaced with the RMIConnectorServer.CREDENTIALS_FILTER_PATTERN constant and a filter pattern.

You're right. It seems like there may have been a typo in the link I looked at initially.

cjobinabo · 2024-03-18T22:17:08Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+---
+type: specs.openrewrite.org/v1beta/recipe
+name: org.openrewrite.java.migrate.SunNetSslPackageUnavailable
+displayName: Remove `com.sun.net.ssl` package


Remove to Replace.

fixed in next commit

cjobinabo · 2024-03-18T22:27:08Z

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java

@@ -527,4 +528,42 @@ void lombokBumpedGoingTo17() {
          )
        );
    }
+
+    @Test
+    void removedSSLSessionGetPeerCertificateChainMethodImplTest(){


If possible, please split each recipe into its own test method.

Actually I thought unless we had multiple cases it was better to put them in the common Test.java, I could add it to another text if u feel strongly but I went with what I saw in the file.

I left the earlier comment here because I thought this test method was attempting to test more than one recipe. But that actually does not appear to be the case, so nothing needs to be changed here.

cjobinabo · 2024-03-19T04:41:26Z

src/main/resources/META-INF/rewrite/java-version-17.yml

+name: org.openrewrite.java.migrate.RemovedZipFinalizeMethods
+displayName: Replace `finalize` method in `java.util.zip.Zipfile`, `java.util.zip.Inflater` and `java.util.zip.Deflater`
+description: >
+  The `finalize` method in `java.util.zip.ZipFile` is replaced with `close` method and is replaced by `end` method in 


In this sentence, let's add the before close

fixed in next commit

…lizeMethods

github-actions

Some suggestions could not be made:

src/main/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersion.java
- lines 1-0

github-actions

Some suggestions could not be made:

src/main/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersion.java
- lines 1-0

cjobinabo

Looks good. Thanks again for working on this.

…lizeMethods

adding 4 recipes + tests for JavaSE 17

916bec7

ranuradh requested review from timtebeek and cjobinabo March 11, 2024 14:34

github-actions bot reviewed Mar 11, 2024

View reviewed changes

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java Outdated Show resolved Hide resolved

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java Outdated Show resolved Hide resolved

changed license to 2024 in test and a small text update to .yml

d1d7271

github-actions bot reviewed Mar 11, 2024

View reviewed changes

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java Outdated Show resolved Hide resolved

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java Outdated Show resolved Hide resolved

ranuradh and others added 3 commits March 11, 2024 10:41

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

60548d1

…vailableTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…

48b640c

….java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

remove extra spaces in test

d94c875

ranuradh self-assigned this Mar 11, 2024

ranuradh added the recipe Recipe requested label Mar 11, 2024

timtebeek reviewed Mar 12, 2024

View reviewed changes

timtebeek changed the title ~~adding 4 recipes + tests for JavaSE 17~~ Recipes for methods and classes renamed or moved when switching to Java 17 Mar 12, 2024

ranuradh and others added 12 commits March 12, 2024 14:42

Update src/main/resources/META-INF/rewrite/java-version-17.yml

2dd45da

Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/main/resources/META-INF/rewrite/java-version-17.yml

b113d01

Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/main/resources/META-INF/rewrite/java-version-17.yml

3d73a92

Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

da81b1a

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

d3d0705

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

63c632f

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…

3d4aca8

….java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…

d4bdae2

….java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…

cf78d9b

….java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test…

910538a

….java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

0e30ecf

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

Update src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUna…

6f97692

…vailableTest.java Co-authored-by: Tim te Beek <tim@moderne.io>

timtebeek reviewed Mar 13, 2024

View reviewed changes

src/main/resources/META-INF/rewrite/java-version-17.yml Outdated Show resolved Hide resolved

Merge branch 'main' into recipe_RemovedZipFinalizeMethods

ff7629a

timtebeek reviewed Mar 15, 2024

View reviewed changes

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java Outdated Show resolved Hide resolved

Apply suggestions from code review

abf75f1

timtebeek reviewed Mar 15, 2024

View reviewed changes

src/test/java/org/openrewrite/java/migrate/UpgradeToJava17Test.java Outdated Show resolved Hide resolved

Apply suggestions from code review

c5ccd9d

timtebeek reviewed Mar 15, 2024

View reviewed changes

src/test/java/org/openrewrite/java/migrate/SunNetSslPackageUnavailableTest.java Outdated Show resolved Hide resolved

Apply suggestions from code review

7dcc706

cjobinabo reviewed Mar 18, 2024

View reviewed changes

AnuRam123 added 2 commits March 18, 2024 18:59

updated text in .yml

9fc5103

small fixes to java-version-17.yml

34ae7c5

cjobinabo reviewed Mar 19, 2024

View reviewed changes

AnuRam123 added 2 commits March 19, 2024 09:34

added small correction to text

170eff2

Merge remote-tracking branch 'origin/main' into recipe_RemovedZipFina…

f03cc5e

…lizeMethods

github-actions bot reviewed Mar 19, 2024

View reviewed changes

update license in java-version-17.yml

43e5d42

github-actions bot reviewed Mar 19, 2024

View reviewed changes

cjobinabo approved these changes Mar 19, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into recipe_RemovedZipFina…

11ec8fa

…lizeMethods

ranuradh merged commit b6cea71 into openrewrite:main Mar 20, 2024
2 checks passed

ranuradh deleted the recipe_RemovedZipFinalizeMethods branch March 20, 2024 12:50

Recipes for methods and classes renamed or moved when switching to Java 17 #430

Recipes for methods and classes renamed or moved when switching to Java 17 #430

Conversation

ranuradh commented Mar 11, 2024 • edited Loading

Checklist

timtebeek commented Mar 11, 2024

timtebeek commented Mar 12, 2024

timtebeek left a comment

Choose a reason for hiding this comment

ranuradh commented Mar 12, 2024 • edited Loading

ranuradh commented Mar 18, 2024

timtebeek commented Mar 18, 2024

ranuradh commented Mar 18, 2024

cjobinabo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ranuradh Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjobinabo Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

ranuradh Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

cjobinabo left a comment

Choose a reason for hiding this comment

ranuradh commented Mar 11, 2024 •

edited

Loading

ranuradh commented Mar 12, 2024 •

edited

Loading

ranuradh Mar 18, 2024 •

edited

Loading

cjobinabo Mar 18, 2024 •

edited

Loading

ranuradh Mar 18, 2024 •

edited

Loading