Skip to content

Commit

Permalink
Fix use static import adding imports whose names conflict.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sambsnyd
sambsnyd committed Jul 19, 2024
1 parent 105bdab commit 3de7723
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 27 deletions.
72 changes: 45 additions & 27 deletions rewrite-java-test/src/test/java/org/openrewrite/java/UseStaticImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import static org.openrewrite.java.Assertions.java;

@SuppressWarnings({"UnnecessaryCallToStringValueOf", "UnnecessaryBoxing", "RedundantTypeArguments", "rawtypes"})
class UseStaticImportTest implements RewriteTest {
@Test
void replaceWithStaticImports() {
Expand All @@ -30,7 +31,7 @@ void replaceWithStaticImports() {
java(
"""
package asserts;
public class Assert {
public static void assertTrue(boolean b) {}
public static void assertFalse(boolean b) {}
Expand All @@ -41,9 +42,9 @@ public static void assertEquals(int m, int n) {}
java(
"""
package test;
import asserts.Assert;
class Test {
void test() {
Assert.assertTrue(true);
Expand All @@ -54,9 +55,9 @@ void test() {
""",
"""
package test;
import static asserts.Assert.*;
class Test {
void test() {
assertTrue(true);
Expand All @@ -78,7 +79,7 @@ void ignoreMethodsWithTypeParameter() {
"""
import java.util.Collections;
import java.util.List;
public class Reproducer {
public void methodWithTypeParameter() {
List<Object> list = Collections.<Object>emptyList();
Expand All @@ -96,19 +97,19 @@ void sameMethodLocallyNoStaticImport() {
java(
"""
package com.helloworld;
import java.util.Collections;
import java.util.List;
public class SameMethodNameLocally {
public void avoidCollision() {
List<Object> list = Collections.emptyList();
}
private int emptyList(String canHaveDifferentArguments) {
}
}
"""
"""
)
);
}
Expand All @@ -121,9 +122,9 @@ void sameMethodImportedNoStaticImport() {
java(
"""
package com.helloworld;
import static java.lang.Integer.valueOf;
public class SameMethodNameImported {
public void avoidCollision() {
String a = String.valueOf("1");
Expand All @@ -145,7 +146,7 @@ void sameMethodsNoStaticImport() {
java(
"""
package com.helloworld;
public class SameMethodNames {
public void avoidCollision() {
String a = String.valueOf("1");
Expand All @@ -155,9 +156,9 @@ public void avoidCollision() {
""",
"""
package com.helloworld;
import static java.lang.String.valueOf;
public class SameMethodNames {
public void avoidCollision() {
String a = valueOf("1");
Expand All @@ -177,7 +178,7 @@ void doReplaceWhenWildcard() {
"""
import java.util.Collections;
import java.util.List;
class SameMethodNameLocally {
void avoidCollision() {
List<Object> list = Collections.emptyList();
Expand All @@ -186,9 +187,9 @@ void avoidCollision() {
""",
"""
import java.util.List;
import static java.util.Collections.emptyList;
class SameMethodNameLocally {
void avoidCollision() {
List<Object> list = emptyList();
Expand All @@ -208,10 +209,10 @@ void junit5Assertions() {
java(
"""
package org.openrewrite;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;
class SampleTest {
@Test
void sample() {
Expand All @@ -221,11 +222,11 @@ void sample() {
""",
"""
package org.openrewrite;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
class SampleTest {
@Test
void sample() {
Expand All @@ -246,7 +247,7 @@ void javadocLinkUnchanged() {
"""
import java.util.Collections;
import java.util.List;
public class WithJavadoc {
/**
* This method uses {@link Collections#emptyList()}.
Expand All @@ -259,9 +260,9 @@ public void mustNotChangeTheJavadocAbove() {
"""
import java.util.Collections;
import java.util.List;
import static java.util.Collections.emptyList;
public class WithJavadoc {
/**
* This method uses {@link Collections#emptyList()}.
Expand Down Expand Up @@ -293,9 +294,9 @@ void reproduce() {
""",
"""
import java.util.function.Predicate;
import static java.util.function.Predicate.not;
public class Reproducer {
void reproduce() {
Predicate<Object> predicate = x -> false;
Expand Down Expand Up @@ -326,4 +327,21 @@ void reproduce() {
)
);
}

@Test
void noStaticImportOfMethodMatchingInstanceMethod() {
// Cannot do a static import of Arrays.toString() because it is ambiguous with Object.toString()
rewriteRun(
spec -> spec.recipe(new UseStaticImport("java..* *(..)")),
java("""
import java.util.Arrays;
class A {
String s(String[] strings) {
return Arrays.toString(strings);
}
}
"""
));
}
}
45 changes: 45 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/UseStaticImport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import lombok.*;
import lombok.experimental.FieldDefaults;
import org.openrewrite.*;
import org.openrewrite.internal.lang.Nullable;
import org.openrewrite.java.search.DeclaresMethod;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Flag;
Expand Down Expand Up @@ -85,6 +86,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu
if (m.getTypeParameters() != null && !m.getTypeParameters().isEmpty()) {
return m;
}

if (methodNameConflicts(m.getSimpleName(), getCursor())) {
return m;
}

if (m.getMethodType() != null) {
if (!m.getMethodType().hasFlags(Flag.Static)) {
return m;
Expand Down Expand Up @@ -120,4 +126,43 @@ public Javadoc visitReference(Javadoc.Reference reference, ExecutionContext ctx)
};
}
}

private static boolean methodNameConflicts(String methodName, Cursor cursor) {
Cursor cdCursor = cursor.dropParentUntil(it -> it instanceof J.ClassDeclaration || it == Cursor.ROOT_VALUE);
Object maybeCd = cdCursor.getValue();
if (!(maybeCd instanceof J.ClassDeclaration)) {
return false;
}
J.ClassDeclaration cd = (J.ClassDeclaration) maybeCd;
JavaType.FullyQualified ct = cd.getType();
if (ct == null) {
return false;
}

if(methodNameConflicts(methodName, ct)) {
return true;
}

return methodNameConflicts(methodName, cdCursor);
}

private static boolean methodNameConflicts(String methodName, @Nullable JavaType.FullyQualified ct) {
if(ct == null) {
return false;
}
for (JavaType.Method method : ct.getMethods()) {
if (method.getName().equals(methodName)) {
return true;
}
}
if (methodNameConflicts(methodName, ct.getSupertype())) {
return true;
}
for (JavaType.FullyQualified intf : ct.getInterfaces()) {
if (methodNameConflicts(methodName, intf)) {
return true;
}
}
return false;
}
}

0 comments on commit 3de7723

Please sign in to comment.