diff --git a/build.gradle.kts b/build.gradle.kts index c05b606231..e154453706 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -72,7 +72,6 @@ dependencies { implementation("org.openrewrite.recipe:rewrite-joda:$rewriteVersion") implementation("org.openrewrite.recipe:rewrite-jenkins:$rewriteVersion") implementation("org.openrewrite:rewrite-templating:$rewriteVersion") - implementation("org.openrewrite.meta:rewrite-analysis:$rewriteVersion") runtimeOnly("org.openrewrite:rewrite-java-8") runtimeOnly("org.openrewrite:rewrite-java-11") diff --git a/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java b/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java index 6145303ef7..dfeda36e23 100644 --- a/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java +++ b/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java @@ -17,13 +17,12 @@ import lombok.Getter; import org.jspecify.annotations.Nullable; +import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; -import org.openrewrite.analysis.constantfold.ConstantFold; -import org.openrewrite.analysis.util.CursorUtil; -import org.openrewrite.internal.RecipeRunException; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaParser; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -31,10 +30,13 @@ import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaSourceFile; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeUtils; import java.net.URI; +import java.util.concurrent.atomic.AtomicBoolean; public class URLConstructorToURICreate extends Recipe { @@ -81,19 +83,71 @@ public J visitNewClass(J.NewClass nc, ExecutionContext ctx) { } if (arg instanceof J.Identifier && TypeUtils.isOfType(arg.getType(), JavaType.Primitive.String)) { - // find constant value of the identifier - try { - return CursorUtil.findCursorForTree(getCursor(), arg) - .bind(c -> ConstantFold.findConstantLiteralValue(c, String.class)) - .toNull(); - } catch (RecipeRunException e) { - // `ConstantFold` does not support lambdas - return null; + return findLiteralInitializer((J.Identifier) arg); + } + // null indicates no path extractable + return null; + } + + private @Nullable String findLiteralInitializer(J.Identifier identifier) { + for (Cursor c = getCursor(); c != null; c = c.getParent()) { + if (c.getValue() instanceof J.Block) { + for (Statement statement : ((J.Block) c.getValue()).getStatements()) { + if (statement instanceof J.VariableDeclarations) { + for (J.VariableDeclarations.NamedVariable variable : ((J.VariableDeclarations) statement).getVariables()) { + if (isSameVariable(identifier, variable)) { + Expression initializer = variable.getInitializer(); + if (initializer instanceof J.Literal && + ((J.Literal) initializer).getValue() instanceof String && + !isReassigned(variable)) { + return (String) ((J.Literal) initializer).getValue(); + } + return null; + } + } + } + } } - } else { - // null indicates no path extractable - return null; } + return null; + } + + private boolean isSameVariable(Expression expression, J.VariableDeclarations.NamedVariable variable) { + if (expression instanceof J.FieldAccess) { + return isSameVariable(((J.FieldAccess) expression).getName(), variable); + } + if (!(expression instanceof J.Identifier)) { + return false; + } + JavaType.Variable fieldType = ((J.Identifier) expression).getFieldType(); + if (fieldType != null && variable.getVariableType() != null) { + return TypeUtils.isOfType(fieldType, variable.getVariableType()); + } + return variable.getSimpleName().equals(((J.Identifier) expression).getSimpleName()); + } + + private boolean isReassigned(J.VariableDeclarations.NamedVariable variable) { + JavaSourceFile sourceFile = getCursor().firstEnclosing(JavaSourceFile.class); + if (sourceFile == null) { + return true; + } + return new JavaIsoVisitor() { + @Override + public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean reassigned) { + if (isSameVariable(assignment.getVariable(), variable)) { + reassigned.set(true); + } + return super.visitAssignment(assignment, reassigned); + } + + @Override + public J.AssignmentOperation visitAssignmentOperation(J.AssignmentOperation assignOp, AtomicBoolean reassigned) { + if (isSameVariable(assignOp.getVariable(), variable)) { + reassigned.set(true); + } + return super.visitAssignmentOperation(assignOp, reassigned); + } + }.reduce(sourceFile, new AtomicBoolean()).get(); } private boolean isNotValidPath(@Nullable String path) { diff --git a/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java b/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java index 43b7d03995..8b0614ccf6 100644 --- a/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java +++ b/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java @@ -183,6 +183,102 @@ void urlConstructor() { ); } + @Test + void urlCheckLocalVariableAbsolutePath() { + rewriteRun( + //language=java + java( + """ + import java.net.URL; + + class Test { + void urlConstructor() { + String goodURL = "https://test.com"; + URL url1 = new URL(goodURL); + } + } + """, + """ + import java.net.URI; + import java.net.URL; + + class Test { + void urlConstructor() { + String goodURL = "https://test.com"; + URL url1 = URI.create(goodURL).toURL(); + } + } + """ + ) + ); + } + + @Test + void urlCheckReassignedVariablePath() { + rewriteRun( + //language=java + java( + """ + import java.net.URL; + + class Test { + void urlConstructor() throws Exception { + String url = "https://test.com"; + url = "not/valid/url"; + URL url1 = new URL(url); + } + } + """ + ) + ); + } + + @Test + void urlCheckFieldShadowedByLaterLocalPath() { + rewriteRun( + //language=java + java( + """ + import java.net.URL; + + class Test { + private String url = "not/valid/url"; + + void urlConstructor() throws Exception { + System.out.println(new URL(url)); + String url = "https://test.com"; + System.out.println(url); + } + } + """ + ) + ); + } + + @Test + void urlCheckFieldReassignedInAnotherMethodPath() { + rewriteRun( + //language=java + java( + """ + import java.net.URL; + + class Test { + private String url = "https://test.com"; + + void setUrl() { + this.url = "not/valid/url"; + } + + void urlConstructor() throws Exception { + System.out.println(new URL(url)); + } + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620") @Test void urlCheckConstantRelativePath() {