From cf0ec93de591d8bdef4f342b90bebcbe9c1065ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Wed, 20 Nov 2024 09:37:45 +0100 Subject: [PATCH 1/4] add Test to reproduce issue 608 --- .../lang/var/UseVarForObjectsTest.java | 105 +++++++++++++----- 1 file changed, 75 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java index 717e13bef..ae4b68d2d 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java @@ -32,35 +32,6 @@ public void defaults(RecipeSpec spec) { .allSources(s -> s.markers(javaVersion(10))); } - @Test - @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/550") - void genericType() { - rewriteRun( - java( - """ - import java.io.Serializable; - - abstract class Outer { - abstract T doIt(); - void trigger() { - T x = doIt(); - } - } - """, - """ - import java.io.Serializable; - - abstract class Outer { - abstract T doIt(); - void trigger() { - var x = doIt(); - } - } - """ - ) - ); - } - @Nested class Applicable { @DocumentExample @@ -336,12 +307,86 @@ void m() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/550") + void genericType() { + rewriteRun( + java( + """ + import java.io.Serializable; + + abstract class Outer { + abstract T doIt(); + void trigger() { + T x = doIt(); + } + } + """, + """ + import java.io.Serializable; + + abstract class Outer { + abstract T doIt(); + void trigger() { + var x = doIt(); + } + } + """ + ) + ); + } } } @Nested class NotApplicable { + @Test + @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/608") + void genericTypeInStaticMethod() { + /* + * This can be migrated from `String string = Global.cast(o)` to `var string = Global.cast(o)`. + * But we decided to not migrate, because it is very unconventional Java. + */ + rewriteRun( + java( + """ + package example; + + class Global { + static T cast(Object o) { + return (T) o; + } + } + class User { + public String test() { + Object o = "Hello"; + String string = Global.cast(o); + return string; + } + } + """, + """ + package example; + + class Global { + static T cast(Object o) { + return (T) o; + } + } + class User { + public String test() { + var o = "Hello"; + String string = Global.cast(o); + return string; + } + } + """ + ) + ); + } + @Test @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/551") void arrayInitializer() { @@ -350,7 +395,7 @@ void arrayInitializer() { java( """ package com.example.app; - + class A { void m() { String[] dictionary = {"aa", "b", "aba", "ba"}; From 1ab2cbf740cc5fa53abc8e944cf5fe75346e5669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Wed, 20 Nov 2024 17:16:42 +0100 Subject: [PATCH 2/4] (Hot) Fix issue 608 by disable migration of variables that are initialized by static methods --- .../migrate/lang/var/DeclarationCheck.java | 28 +++++++++++++++++++ .../migrate/lang/var/UseVarForObject.java | 6 ++-- .../lang/var/UseVarForObjectsTest.java | 6 ++-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java index d90d62cb8..426ac8333 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java @@ -18,6 +18,8 @@ import org.openrewrite.Cursor; import org.openrewrite.java.tree.*; +import javax.annotation.Nullable; + import static java.util.Objects.requireNonNull; final class DeclarationCheck { @@ -198,4 +200,30 @@ private static boolean isInsideInitializer(Cursor cursor, int nestedBlockLevel) return isInsideInitializer(requireNonNull(cursor.getParent()), nestedBlockLevel); } + + /** + * Checks whether the initializer {@linkplain Expression} is a {@linkplain J.MethodInvocation} targeting a static method. + * + * @param initializer {@linkplain J.VariableDeclarations.NamedVariable#getInitializer()} value + * @return true iff is initialized by static method + */ + public static boolean initializedByStaticMethod(@Nullable Expression initializer) { + if (initializer == null) { + return false; + } + initializer = initializer.unwrap(); + + if (!(initializer instanceof J.MethodInvocation)) { + // no MethodInvocation -> false + return false; + } + + J.MethodInvocation invocation = (J.MethodInvocation) initializer; + if (invocation.getMethodType() == null) { + // not a static method -> false + return false; + } + + return invocation.getMethodType().getFlags().contains(Flag.Static); + } } diff --git a/src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForObject.java b/src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForObject.java index e79094495..6504fefdd 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForObject.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/var/UseVarForObject.java @@ -75,8 +75,10 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations v boolean isPrimitive = DeclarationCheck.isPrimitive(vd); boolean usesGenerics = DeclarationCheck.useGenerics(vd); boolean usesTernary = DeclarationCheck.initializedByTernary(vd); - boolean usesArrayInitializer = vd.getVariables().get(0).getInitializer() instanceof J.NewArray; - if (isPrimitive || usesGenerics || usesTernary || usesArrayInitializer) { + Expression initializer = vd.getVariables().get(0).getInitializer(); + boolean usesArrayInitializer = initializer instanceof J.NewArray; + boolean initializedByStaticMethod = DeclarationCheck.initializedByStaticMethod(initializer); + if (isPrimitive || usesGenerics || usesTernary || usesArrayInitializer || initializedByStaticMethod) { return vd; } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java index ae4b68d2d..b269c0439 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java @@ -270,6 +270,7 @@ void m() { } @Test + @Disabled("in favor to https://github.com/openrewrite/rewrite-migrate-java/issues/608 we skip all static methods ATM") void staticMethods() { //language=java rewriteRun( @@ -345,10 +346,7 @@ class NotApplicable { @Test @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/608") void genericTypeInStaticMethod() { - /* - * This can be migrated from `String string = Global.cast(o)` to `var string = Global.cast(o)`. - * But we decided to not migrate, because it is very unconventional Java. - */ + // ATM the recipe skips all static method initialized variables rewriteRun( java( """ From 7a97a2d28f6366c3ea83be3d512038131288f3b1 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 Nov 2024 17:47:55 +0100 Subject: [PATCH 3/4] Minor polish --- .../openrewrite/java/migrate/lang/var/DeclarationCheck.java | 2 +- .../java/migrate/lang/var/UseVarForObjectsTest.java | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java index 426ac8333..d869207d1 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java @@ -224,6 +224,6 @@ public static boolean initializedByStaticMethod(@Nullable Expression initializer return false; } - return invocation.getMethodType().getFlags().contains(Flag.Static); + return invocation.getMethodType().hasFlags(Flag.Static); } } diff --git a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java index b269c0439..504861d18 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/var/UseVarForObjectsTest.java @@ -313,6 +313,7 @@ void m() { @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/550") void genericType() { rewriteRun( + //language=java java( """ import java.io.Serializable; @@ -348,6 +349,7 @@ class NotApplicable { void genericTypeInStaticMethod() { // ATM the recipe skips all static method initialized variables rewriteRun( + //language=java java( """ package example; @@ -360,7 +362,7 @@ static T cast(Object o) { class User { public String test() { Object o = "Hello"; - String string = Global.cast(o); + String string = Global.cast(o); // static method unchanged return string; } } @@ -376,7 +378,7 @@ static T cast(Object o) { class User { public String test() { var o = "Hello"; - String string = Global.cast(o); + String string = Global.cast(o); // static method unchanged return string; } } From 3a34f3d13877b6b1790a14207f0de6623306a098 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 Nov 2024 17:48:42 +0100 Subject: [PATCH 4/4] Use org.jspecify.annotations.Nullable --- .../openrewrite/java/migrate/lang/var/DeclarationCheck.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java index d869207d1..dc4b48b60 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/var/DeclarationCheck.java @@ -15,11 +15,10 @@ */ package org.openrewrite.java.migrate.lang.var; +import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.java.tree.*; -import javax.annotation.Nullable; - import static java.util.Objects.requireNonNull; final class DeclarationCheck {