Skip to content

Commit

Permalink
Fix var migration of variables initialized by static methods (#609)
Browse files Browse the repository at this point in the history
* add Test to reproduce issue 608

* (Hot) Fix issue 608 by disable migration of variables that are initialized by static methods

* Minor polish

* Use org.jspecify.annotations.Nullable

---------

Co-authored-by: Tim te Beek <[email protected]>
  • Loading branch information
MBoegers and timtebeek authored Nov 20, 2024
1 parent 6dce1fb commit d58fd75
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.openrewrite.java.migrate.lang.var;

import org.jspecify.annotations.Nullable;
import org.openrewrite.Cursor;
import org.openrewrite.java.tree.*;

Expand Down Expand Up @@ -198,4 +199,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().hasFlags(Flag.Static);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends Serializable> {
abstract T doIt();
void trigger() {
T x = doIt();
}
}
""",
"""
import java.io.Serializable;
abstract class Outer<T extends Serializable> {
abstract T doIt();
void trigger() {
var x = doIt();
}
}
"""
)
);
}

@Nested
class Applicable {
@DocumentExample
Expand Down Expand Up @@ -299,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(
Expand Down Expand Up @@ -336,12 +308,85 @@ void m() {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/550")
void genericType() {
rewriteRun(
//language=java
java(
"""
import java.io.Serializable;
abstract class Outer<T extends Serializable> {
abstract T doIt();
void trigger() {
T x = doIt();
}
}
""",
"""
import java.io.Serializable;
abstract class Outer<T extends Serializable> {
abstract T doIt();
void trigger() {
var x = doIt();
}
}
"""
)
);
}
}
}

@Nested
class NotApplicable {

@Test
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/608")
void genericTypeInStaticMethod() {
// ATM the recipe skips all static method initialized variables
rewriteRun(
//language=java
java(
"""
package example;
class Global {
static <T> T cast(Object o) {
return (T) o;
}
}
class User {
public String test() {
Object o = "Hello";
String string = Global.cast(o); // static method unchanged
return string;
}
}
""",
"""
package example;
class Global {
static <T> T cast(Object o) {
return (T) o;
}
}
class User {
public String test() {
var o = "Hello";
String string = Global.cast(o); // static method unchanged
return string;
}
}
"""
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/551")
void arrayInitializer() {
Expand All @@ -350,7 +395,7 @@ void arrayInitializer() {
java(
"""
package com.example.app;
class A {
void m() {
String[] dictionary = {"aa", "b", "aba", "ba"};
Expand Down

0 comments on commit d58fd75

Please sign in to comment.