Skip to content

Commit 7a21d21

Browse files
karthikNousherKARTHIK NOUSHERtimtebeekgithub-actions[bot]rlsanders4
authored
Improve Handling of Absolute, Relative, and Non-Literal URLs in URL Migration (#678)
* added new recipe to add a new convert uri method. * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Ran formatter * Fix tests * Update logic * Update string literal check * Update list * add test * Fix test * Get constant value * Polish pre condition handling --------- Co-authored-by: KARTHIK NOUSHER <[email protected]> Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Ralph-Sanders <[email protected]> Co-authored-by: Merlin Bögershausen <[email protected]>
1 parent aae42da commit 7a21d21

File tree

3 files changed

+263
-48
lines changed

3 files changed

+263
-48
lines changed

src/main/java/org/openrewrite/java/migrate/net/URLConstructorsToURI.java

Lines changed: 115 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -15,59 +15,129 @@
1515
*/
1616
package org.openrewrite.java.migrate.net;
1717

18-
import com.google.errorprone.refaster.annotation.AfterTemplate;
19-
import com.google.errorprone.refaster.annotation.BeforeTemplate;
20-
import org.openrewrite.java.template.RecipeDescriptor;
18+
import org.jspecify.annotations.Nullable;
19+
import org.openrewrite.ExecutionContext;
20+
import org.openrewrite.Preconditions;
21+
import org.openrewrite.Recipe;
22+
import org.openrewrite.TreeVisitor;
23+
import org.openrewrite.analysis.constantfold.ConstantFold;
24+
import org.openrewrite.analysis.util.CursorUtil;
25+
import org.openrewrite.java.JavaParser;
26+
import org.openrewrite.java.JavaTemplate;
27+
import org.openrewrite.java.JavaVisitor;
28+
import org.openrewrite.java.MethodMatcher;
29+
import org.openrewrite.java.search.UsesType;
30+
import org.openrewrite.java.tree.Expression;
31+
import org.openrewrite.java.tree.J;
32+
import org.openrewrite.java.tree.JavaType;
33+
import org.openrewrite.java.tree.TypeUtils;
2134

2235
import java.net.URI;
23-
import java.net.URL;
2436

25-
public class URLConstructorsToURI {
26-
@RecipeDescriptor(
27-
name = "Convert `new URL(String)` to `URI.create(String).toURL()`",
28-
description = "Converts `new URL(String)` constructors to `URI.create(String).toURL()`."
29-
)
30-
public static class URLSingleArgumentConstructor {
31-
@BeforeTemplate
32-
URL urlConstructor(String spec) throws Exception {
33-
return new URL(spec);
34-
}
37+
public class URLConstructorsToURI extends Recipe {
3538

36-
@AfterTemplate
37-
URL uriCreateToURL(String spec) throws Exception {
38-
return URI.create(spec).toURL();
39-
}
40-
}
39+
private static final String URI_FQN = "java.net.URI";
40+
private static final String URL_FQN = "java.net.URL";
41+
private static final MethodMatcher methodMatcherSingleArg = new MethodMatcher(URL_FQN + " <constructor>(java.lang.String)");
42+
private static final MethodMatcher methodMatcherThreeArg = new MethodMatcher(URL_FQN + " <constructor>(java.lang.String, java.lang.String, java.lang.String)");
43+
private static final MethodMatcher methodMatcherFourArg = new MethodMatcher(URL_FQN + " <constructor>(java.lang.String, java.lang.String, int, java.lang.String)");
4144

42-
@RecipeDescriptor(
43-
name = "Convert `new URL(String, String, String)` to `new URI(...).toURL()`",
44-
description = "Converts `new URL(String, String, String)` constructors to `new URI(...).toURL()`."
45-
)
46-
public static class URLThreeArgumentConstructor {
47-
@BeforeTemplate
48-
URL urlConstructor(String protocol, String host, String file) throws Exception {
49-
return new URL(protocol, host, file);
50-
}
45+
@Override
46+
public String getDisplayName() {
47+
return "Convert `new URL(String)` to `URI.create(String).toURL()`";
48+
}
5149

52-
@AfterTemplate
53-
URL newUriToUrl(String protocol, String host, String file) throws Exception {
54-
return new URI(protocol, null, host, -1, file, null, null).toURL();
55-
}
50+
@Override
51+
public String getDescription() {
52+
return "Converts `new URL(String)` constructors to `URI.create(String).toURL()`.";
5653
}
5754

58-
@RecipeDescriptor(
59-
name = "Convert `new URL(String, String, int, String)` to `new URI(...).toURL()`",
60-
description = "Converts `new URL(String, String, int, String)` constructors to `new URI(...).toURL()`."
61-
)
62-
public static class URLFourArgumentConstructor {
63-
@BeforeTemplate
64-
URL urlConstructor(String protocol, String host, int port, String file) throws Exception {
65-
return new URL(protocol, host, port, file);
66-
}
55+
@Override
56+
public TreeVisitor<?, ExecutionContext> getVisitor() {
57+
return Preconditions.check(new UsesType<>(URL_FQN, false),
58+
new JavaVisitor<ExecutionContext>() {
59+
@Override
60+
public J visitNewClass(J.NewClass nc, ExecutionContext ctx) {
61+
if (methodMatcherSingleArg.matches(nc)) {
62+
String path = extractPath(nc.getArguments().get(0));
63+
if (isNotValidPath(path)) {
64+
return nc;
65+
}
66+
67+
JavaTemplate template = JavaTemplate.builder("URI.create(#{any(String)}).toURL()")
68+
.imports(URI_FQN)
69+
.contextSensitive()
70+
.javaParser(JavaParser.fromJavaVersion())
71+
.build();
72+
maybeAddImport(URI_FQN);
73+
74+
return template.apply(getCursor(),
75+
nc.getCoordinates().replace(),
76+
nc.getArguments().get(0));
77+
} else {
78+
if (methodMatcherThreeArg.matches(nc)) {
79+
JavaTemplate template = JavaTemplate.builder("new URI(#{any(String)}, null, #{any(String)}, -1, #{any(String)}, null, null).toURL()")
80+
.imports(URI_FQN, URL_FQN)
81+
.contextSensitive()
82+
.javaParser(JavaParser.fromJavaVersion())
83+
.build();
84+
85+
maybeAddImport(URI_FQN);
86+
return template.apply(getCursor(), nc.getCoordinates().replace(),
87+
nc.getArguments().get(0),
88+
nc.getArguments().get(1),
89+
nc.getArguments().get(2));
90+
} else if (methodMatcherFourArg.matches(nc)) {
91+
JavaTemplate template = JavaTemplate.builder("new URI(#{any(String)}, null, #{any(String)}, #{any(int)}, #{any(String)}, null, null).toURL()")
92+
.imports(URI_FQN, URL_FQN)
93+
.contextSensitive()
94+
.javaParser(JavaParser.fromJavaVersion())
95+
.build();
96+
97+
maybeAddImport(URI_FQN);
98+
return template.apply(getCursor(), nc.getCoordinates().replace(),
99+
nc.getArguments().get(0),
100+
nc.getArguments().get(1),
101+
nc.getArguments().get(2),
102+
nc.getArguments().get(3));
103+
}
104+
}
105+
return super.visitNewClass(nc, ctx);
106+
}
107+
108+
@Nullable
109+
private String extractPath(Expression arg) {
110+
if (arg instanceof J.Literal &&
111+
TypeUtils.isOfType(arg.getType(), JavaType.Primitive.String)) {
112+
// Check if value is not null
113+
String literalValueSource = ((J.Literal) arg).getValueSource();
114+
// Remove quotations from string
115+
return literalValueSource != null ? literalValueSource.substring(1, literalValueSource.length() - 1).trim() : null;
116+
} else if (arg instanceof J.Identifier &&
117+
TypeUtils.isOfType(arg.getType(), JavaType.Primitive.String)) {
118+
// find constant value of the identifier
119+
return CursorUtil.findCursorForTree(getCursor(), arg)
120+
.bind(c -> ConstantFold.findConstantLiteralValue(c, String.class))
121+
.toNull();
122+
} else {
123+
// null indicates no path extractable
124+
return null;
125+
}
126+
}
127+
128+
private boolean isNotValidPath(@Nullable String path) {
129+
if (path == null) {
130+
return true;
131+
}
67132

68-
@AfterTemplate
69-
URL newUriToUrl(String protocol, String host, int port, String file) throws Exception {
70-
return new URI(protocol, null, host, port, file, null, null).toURL();
71-
}
133+
try {
134+
//noinspection ResultOfMethodCallIgnored
135+
URI.create(path).toURL();
136+
return false;
137+
} catch (Exception e) {
138+
return true;
139+
}
140+
}
141+
});
72142
}
73143
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ recipeList:
2929
- org.openrewrite.java.migrate.UpgradeBuildToJava21
3030
- org.openrewrite.java.migrate.RemoveIllegalSemicolons
3131
- org.openrewrite.java.migrate.lang.ThreadStopUnsupported
32-
- org.openrewrite.java.migrate.net.URLConstructorsToURIRecipes
32+
- org.openrewrite.java.migrate.net.URLConstructorsToURI
3333
- org.openrewrite.java.migrate.util.SequencedCollection
3434
- org.openrewrite.java.migrate.util.UseLocaleOf
3535
- org.openrewrite.staticanalysis.ReplaceDeprecatedRuntimeExecMethods

src/test/java/org/openrewrite/java/migrate/net/URLConstructorsToURITest.java

Lines changed: 147 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
class URLConstructorsToURITest implements RewriteTest {
2727
@Override
2828
public void defaults(RecipeSpec spec) {
29-
spec.recipe(new URLConstructorsToURIRecipes());
29+
spec.recipe(new URLConstructorsToURI());
3030
}
3131

3232
@Test
@@ -53,7 +53,7 @@ void urlConstructor(String spec) throws Exception {
5353
5454
class Test {
5555
void urlConstructor(String spec) throws Exception {
56-
URL url1 = URI.create(spec).toURL();
56+
URL url1 = new URL(spec);
5757
URL url2 = new URI(spec, null, "localhost", -1, "file", null, null).toURL();
5858
URL url3 = new URI(spec, null, "localhost", 8080, "file", null, null).toURL();
5959
}
@@ -62,4 +62,149 @@ void urlConstructor(String spec) throws Exception {
6262
)
6363
);
6464
}
65+
66+
@Test
67+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
68+
void urlCheckAbsolutePath() {
69+
rewriteRun(
70+
//language=java
71+
java(
72+
"""
73+
import java.net.URL;
74+
75+
class Test {
76+
void urlConstructor() {
77+
URL url1 = new URL("https://test.com");
78+
}
79+
}
80+
""",
81+
"""
82+
import java.net.URI;
83+
import java.net.URL;
84+
85+
class Test {
86+
void urlConstructor() {
87+
URL url1 = URI.create("https://test.com").toURL();
88+
}
89+
}
90+
"""
91+
)
92+
);
93+
}
94+
95+
@Test
96+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
97+
void urlCheckConstantAbsolutePath() {
98+
rewriteRun(
99+
//language=java
100+
java(
101+
"""
102+
import java.net.URL;
103+
104+
class Test {
105+
private String goodURL = "https://test.com";
106+
private String badURL = "not/valid/url";
107+
void urlConstructor() {
108+
URL url1 = new URL(goodURL);
109+
}
110+
}
111+
""",
112+
"""
113+
import java.net.URI;
114+
import java.net.URL;
115+
116+
class Test {
117+
private String goodURL = "https://test.com";
118+
private String badURL = "not/valid/url";
119+
void urlConstructor() {
120+
URL url1 = URI.create(goodURL).toURL();
121+
}
122+
}
123+
"""
124+
)
125+
);
126+
}
127+
128+
@Test
129+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
130+
void urlCheckConstantRelativePath() {
131+
rewriteRun(
132+
//language=java
133+
java(
134+
"""
135+
import java.net.URL;
136+
137+
class Test {
138+
private String goodURL = "https://test.com";
139+
private String badURL = "not/valid/url";
140+
void urlConstructor() {
141+
URL url1 = new URL(badURL);
142+
}
143+
}
144+
"""
145+
)
146+
);
147+
}
148+
149+
@Test
150+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
151+
void urlCheckRelativePath() {
152+
rewriteRun(
153+
spec -> spec.expectedCyclesThatMakeChanges(0),
154+
//language=java
155+
java(
156+
"""
157+
import java.net.URL;
158+
159+
class Test {
160+
void urlConstructor() {
161+
URL url1 = new URL("TEST-INF/test/testCase.wsdl");
162+
}
163+
}
164+
"""
165+
)
166+
);
167+
}
168+
169+
@Test
170+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
171+
void urlCheckNullPath() {
172+
rewriteRun(
173+
//language=java
174+
java(
175+
"""
176+
import java.net.URL;
177+
178+
class Test {
179+
void urlConstructor() {
180+
URL url1 = new URL(null);
181+
}
182+
}
183+
"""
184+
)
185+
);
186+
}
187+
188+
@Test
189+
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620")
190+
void urlCheckMethodInvocationParameter() {
191+
rewriteRun(
192+
//language=java
193+
java(
194+
"""
195+
import java.net.URL;
196+
197+
class Test {
198+
void urlConstructor(String spec) throws Exception {
199+
URL url1 = new URL(getString());
200+
}
201+
202+
String getString() {
203+
return "myURL";
204+
}
205+
}
206+
"""
207+
)
208+
);
209+
}
65210
}

0 commit comments

Comments
 (0)