From 4d6abed3627d5359f2a3834ed3b06aa7e2faa5ff Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 25 Aug 2026 22:51:37 +0200 Subject: [PATCH 1/2] Drop the `rewrite-analysis` dependency from `URLConstructorToURICreate` A single recipe pulled in the whole module, only to resolve a `String` identifier back to the literal it was initialized with. Walking the enclosing blocks for the matching variable declaration covers the fields and locals that the recipe actually sees, and it drops the `RecipeRunException` catch that was there solely because `ConstantFold` throws on a lambda parameter. The lookup is by name rather than by dataflow, so a variable reassigned before use, or shadowed elsewhere, can now fold to a different value. That only shifts whether the path is judged valid; the argument is passed through to `URI.create` untouched either way. --- build.gradle.kts | 1 - .../net/URLConstructorToURICreate.java | 39 ++++++++++++------- .../net/URLConstructorToURICreateTest.java | 30 ++++++++++++++ 3 files changed, 55 insertions(+), 15 deletions(-) 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..ddbc276d2e 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,11 @@ 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.JavaParser; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -32,6 +30,7 @@ import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; import org.openrewrite.java.tree.TypeUtils; import java.net.URI; @@ -81,19 +80,31 @@ 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 (variable.getSimpleName().equals(identifier.getSimpleName())) { + Expression initializer = variable.getInitializer(); + if (initializer instanceof J.Literal && ((J.Literal) initializer).getValue() instanceof String) { + return (String) ((J.Literal) initializer).getValue(); + } + return null; + } + } + } + } } - } else { - // null indicates no path extractable - return null; } + return null; } 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..ea8ad46718 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,36 @@ 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(); + } + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/620") @Test void urlCheckConstantRelativePath() { From abd27b5275730f62f221420921c4646665e0807d Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 25 Aug 2026 23:35:02 +0200 Subject: [PATCH 2/2] Resolve the identifier by type, and refuse to fold a reassigned variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matching the declaration on its name alone crossed two boundaries the previous `ConstantFold` respected. A local shadowing a field is a distinct variable, so comparing `JavaType.Variable` — which carries the owner — keeps the walk going past the shadowing declaration to the one the identifier actually binds to. And a variable written to anywhere in the file no longer holds the value it was declared with, which is the condition `ConstantFold` expressed as a single assigned value. An assignment through `this` reaches the same variable as a bare name, so the target is unwrapped before comparing. --- .../net/URLConstructorToURICreate.java | 47 ++++++++++++- .../net/URLConstructorToURICreateTest.java | 66 +++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) 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 ddbc276d2e..dfeda36e23 100644 --- a/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java +++ b/src/main/java/org/openrewrite/java/migrate/net/URLConstructorToURICreate.java @@ -22,6 +22,7 @@ import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaParser; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; @@ -29,11 +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 { @@ -92,9 +95,11 @@ public J visitNewClass(J.NewClass nc, ExecutionContext ctx) { for (Statement statement : ((J.Block) c.getValue()).getStatements()) { if (statement instanceof J.VariableDeclarations) { for (J.VariableDeclarations.NamedVariable variable : ((J.VariableDeclarations) statement).getVariables()) { - if (variable.getSimpleName().equals(identifier.getSimpleName())) { + if (isSameVariable(identifier, variable)) { Expression initializer = variable.getInitializer(); - if (initializer instanceof J.Literal && ((J.Literal) initializer).getValue() instanceof String) { + if (initializer instanceof J.Literal && + ((J.Literal) initializer).getValue() instanceof String && + !isReassigned(variable)) { return (String) ((J.Literal) initializer).getValue(); } return null; @@ -107,6 +112,44 @@ public J visitNewClass(J.NewClass nc, ExecutionContext ctx) { 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) { if (path == null) { return true; 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 ea8ad46718..8b0614ccf6 100644 --- a/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java +++ b/src/test/java/org/openrewrite/java/migrate/net/URLConstructorToURICreateTest.java @@ -213,6 +213,72 @@ void urlConstructor() { ); } + @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() {