-
Notifications
You must be signed in to change notification settings - Fork 78
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
Recipes for methods and classes renamed or moved when switching to Java 17 #430
Changes from 21 commits
916bec7
d1d7271
60548d1
48b640c
d94c875
2dd45da
b113d01
3d73a92
da81b1a
d3d0705
63c632f
3d4aca8
d4bdae2
cf78d9b
910538a
0e30ecf
6f97692
ff7629a
abf75f1
c5ccd9d
7dcc706
9fc5103
34ae7c5
170eff2
f03cc5e
43e5d42
11ec8fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,10 @@ recipeList: | |
groupId: com.sonatype.clm | ||
artifactId: clm-maven-plugin | ||
newVersion: 2.47.6-01 | ||
|
||
- org.openrewrite.java.migrate.RemovedZipFinalizeMethods | ||
- org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl | ||
- org.openrewrite.java.migrate.SunNetSslPackageUnavailable | ||
- org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant | ||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
name: org.openrewrite.java.migrate.JavaVersion17 | ||
|
@@ -122,3 +125,66 @@ recipeList: | |
- org.openrewrite.java.ChangeMethodAccessLevel: | ||
methodPattern: "*..* premain(java.lang.String, java.lang.instrument.Instrumentation)" | ||
newAccessLevel: public | ||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
`java.util.zip.Inflater` and `java.util.zip.Deflater` as it is no longer available in Java SE 12 and later. | ||
tags: | ||
- java17 | ||
recipeList: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a test for this recipe. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was about running tests where we need an older version of Java and we cant get that to work -
|
||
- org.openrewrite.java.ChangeMethodName: | ||
methodPattern: "java.util.zip.Inflater finalize()" | ||
newMethodName: end | ||
ignoreDefinition: true | ||
matchOverrides: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we ignore definitions, we shouldn't need to do anything with overrides. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
- org.openrewrite.java.ChangeMethodName: | ||
methodPattern: "java.util.zip.Deflater finalize()" | ||
newMethodName: end | ||
ignoreDefinition: true | ||
- org.openrewrite.java.ChangeMethodName: | ||
methodPattern: "java.util.ZipFile finalize()" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be |
||
newMethodName: close | ||
ignoreDefinition: true | ||
timtebeek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
name: org.openrewrite.java.migrate.RemovedSSLSessionGetPeerCertificateChainMethodImpl | ||
displayName: Remove `SSLSession.getPeerCertificateChain()` method | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
description: > | ||
The `javax.net.ssl.SSLSession.getPeerCertificateChain()` method implementation was removed from the SunJSSE provider and HTTP client implementation in Java SE 15. | ||
The default implementation will now throw an `UnsupportedOperationException`. | ||
Applications using this method should be updated to use the `javax.net.ssl.SSLSession.getPeerCertificates()` method instead. | ||
tags: | ||
- java17 | ||
recipeList: | ||
- org.openrewrite.java.ChangeMethodName: | ||
methodPattern: "*.net.ssl.SSLSession getPeerCertificateChain()" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason we aren't using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. During one test I had it as *, I reverted it back to javax now |
||
newMethodName: getPeerCertificates | ||
ignoreDefinition: true | ||
matchOverrides: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we ignore definitions, we shouldn't need to do anything with overrides. |
||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
name: org.openrewrite.java.migrate.SunNetSslPackageUnavailable | ||
displayName: Remove `com.sun.net.ssl` package | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
description: > | ||
The internal API `com.sun.net.ssl` is removed. The package was intended for internal use only and replacement APIs can be found in the `javax.net.ssl` package. | ||
tags: | ||
- java17 | ||
recipeList: | ||
- org.openrewrite.java.ChangePackage: | ||
oldPackageName: com.sun.net.ssl | ||
newPackageName: javax.net.ssl | ||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
name: org.openrewrite.java.migrate.RemovedRMIConnectorServerCredentialTypesConstant | ||
displayName: Remove `RMIConnectorServer.CREDENTIAL_TYPES` constant | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
description: > | ||
This recipe replaces `RMIConnectorServer.CREDENTIAL_TYPES` constant with the `RMIConnectorServer.CREDENTIALS_FILTER_PATTERN` constant and a filter pattern. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove extra space between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last bit about the pattern There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in next commit |
||
tags: | ||
- java17 | ||
recipeList: | ||
- org.openrewrite.java.ReplaceConstantWithAnotherConstant: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a test for this recipe. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
existingFullyQualifiedConstantName: javax.management.remote.rmi.RMIConnectorServer.CREDENTIAL_TYPES | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. It seems like there may have been a typo in the link I looked at initially. |
||
fullyQualifiedConstantName: javax.management.remote.rmi.RMIConnectorServer.CREDENTIALS_FILTER_PATTERN |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/* | ||
* Copyright 2024 the original author or authors. | ||
* <p> | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* <p> | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* <p> | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.openrewrite.java.migrate; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.openrewrite.DocumentExample; | ||
import org.openrewrite.InMemoryExecutionContext; | ||
import org.openrewrite.config.Environment; | ||
import org.openrewrite.java.JavaParser; | ||
import org.openrewrite.java.migrate.jakarta.RemoveBeanIsNullable; | ||
import org.openrewrite.test.RecipeSpec; | ||
import org.openrewrite.test.RewriteTest; | ||
|
||
import static org.openrewrite.java.Assertions.*; | ||
|
||
class SunNetSslPackageUnavailableTest implements RewriteTest { | ||
|
||
@Override | ||
public void defaults(RecipeSpec spec) { | ||
spec.parser(JavaParser.fromJavaVersion() | ||
.classpathFromResources(new InMemoryExecutionContext(), | ||
"com.sun.net.ssl")) | ||
.recipeFromResource("/META-INF/rewrite/java-version-17.yml", "org.openrewrite.java.migrate.SunNetSslPackageUnavailable"); | ||
} | ||
|
||
@Test | ||
void sunNetSslPackageUnavailableTest(){ | ||
rewriteRun( | ||
//language=java | ||
java( | ||
""" | ||
import com.sun.net.ssl.HttpsURLConnection; | ||
|
||
class TestSunNetSsl { | ||
void useThePackages() { | ||
HttpsURLConnection con; | ||
} | ||
} | ||
""", | ||
""" | ||
import javax.net.ssl.HttpsURLConnection; | ||
|
||
class TestSunNetSsl { | ||
void useThePackages() { | ||
HttpsURLConnection con; | ||
} | ||
} | ||
""" | ||
) | ||
); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
/* | ||
* Copyright 2023 the original author or authors. | ||
* Copyright 2024 the original author or authors. | ||
* <p> | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
|
@@ -17,6 +17,7 @@ | |
|
||
import org.junit.jupiter.api.Test; | ||
import org.openrewrite.DocumentExample; | ||
import org.openrewrite.InMemoryExecutionContext; | ||
import org.openrewrite.config.Environment; | ||
import org.openrewrite.java.marker.JavaVersion; | ||
import org.openrewrite.test.RecipeSpec; | ||
|
@@ -527,4 +528,42 @@ void lombokBumpedGoingTo17() { | |
) | ||
); | ||
} | ||
|
||
@Test | ||
void removedSSLSessionGetPeerCertificateChainMethodImplTest(){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If possible, please split each recipe into its own test method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
rewriteRun( | ||
//language=java | ||
java( | ||
""" | ||
import java.security.cert.Certificate; | ||
import javax.net.ssl.SSLContext; | ||
import javax.net.ssl.SSLEngine; | ||
import javax.net.ssl.SSLSession; | ||
class RemovedSSLSessionGetPeerCertificateChainMethodImplApp { | ||
void test() throws Exception { | ||
SSLEngine sslEngine = SSLContext.getDefault().createSSLEngine(); | ||
SSLSession session = sslEngine.getHandshakeSession(); | ||
session.getPeerCertificateChain(); //This should trigger | ||
Certificate[] certs = session.getPeerCertificates(); //This should not trigger | ||
} | ||
} | ||
""", | ||
""" | ||
import java.security.cert.Certificate; | ||
import javax.net.ssl.SSLContext; | ||
import javax.net.ssl.SSLEngine; | ||
import javax.net.ssl.SSLSession; | ||
class RemovedSSLSessionGetPeerCertificateChainMethodImplApp { | ||
void test() throws Exception { | ||
SSLEngine sslEngine = SSLContext.getDefault().createSSLEngine(); | ||
SSLSession session = sslEngine.getHandshakeSession(); | ||
session.getPeerCertificates(); //This should trigger | ||
Certificate[] certs = session.getPeerCertificates(); //This should not trigger | ||
} | ||
} | ||
""" | ||
) | ||
); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
toReplace