Convert 5-arg URL constructor with URLStreamHandler to URL.of - #1241
Open
harrisleesh wants to merge 1 commit into
Open
harrisleesh wants to merge 1 commit into
harrisleesh wants to merge 1 commit into
Conversation
The URLStreamHandler variant of the deprecated URL constructor was left out of URLConstructorsToNewURI, so occurrences like new URL(protocol, host, port, file, handler) were not migrated. Convert them to URL.of(new URI(protocol, null, host, port, file, null, null), handler), which preserves the custom handler. Fixes: openrewrite#849
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's changed?
URLConstructorsToNewURInow also matches the deprecated 5-arg constructornew URL(String protocol, String host, int port, String file, URLStreamHandler handler)and converts it toURL.of(new URI(protocol, null, host, port, file, null, null), handler), preserving the custom stream handler. The URI construction follows the same shape already used for the 3- and 4-arg variants.What's your motivation?
java.net.URL#<init>(java.lang.String,java.lang.String,int,java.lang.String,java.net.URLStreamHandler)[Deprecated in Java 21] #849 — this variant was left out and @timtebeek welcomed a contribution to fill the gap.Anything in particular you'd like reviewers to focus on?
URL.of(URI, URLStreamHandler)requires Java 20+, while the 3/4-arg replacements only needURI.toURL(). I kept everything in the same recipe since it is listed under the Java 21 migration, but happy to gate the 5-arg case behind a version precondition if you prefer.URLThreeArgumentConstructordoes not introduce newly thrown exception #467 applies: the URI constructor throwsURISyntaxExceptionwhere the URL constructor threwMalformedURLException.Any additional context
Heads-up on validation: I could not resolve
org.openrewriteartifacts from the Code Genome repository locally yet, so I verified the change in a standalone project against rewrite 8.90.4 — the new test fails without the recipe change and both tests pass with it. Relying on CI for the exact HEAD build; if anything fails I'll follow up promptly.Checklist
./gradlew buildlocally, and committed any resulting changes torecipes.csv(blocked on Code Genome credentials — no new recipe class was added, sorecipes.csvshould be unaffected)