Skip to content

Commit

Permalink
RemoveMethodInvocationsVisitor can remove static method (#4754)
Browse files Browse the repository at this point in the history
* Add test on RemoveMethodInvocationsVisitor to remove static method

* First implementation

* Improved implementation

* Improved tests

* Improved tests

* Improved tests

* Improved tests [skip ci]

* Complete it

* Remove redundant parentheses

* Better mame of tests

* Add failing test for scenario not yet covered

* Improvement

---------

Co-authored-by: lingenj <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>
  • Loading branch information
3 people authored Dec 10, 2024
1 parent 3403f7e commit 54a2057
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,26 +69,32 @@ public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx)
return j;
}

private @Nullable J removeMethods(Expression expression, int depth, boolean isLambdaBody, Stack<Space> selectAfter) {
if (!(expression instanceof J.MethodInvocation)) {
private @Nullable J removeMethods(@Nullable Expression expression, int depth, boolean isLambdaBody, Stack<Space> selectAfter) {
if (!(expression instanceof J.MethodInvocation) || ((J.MethodInvocation) expression).getMethodType() == null) {
return expression;
}

boolean isStatement = isStatement();
J.MethodInvocation m = (J.MethodInvocation) expression;
boolean isStatic = m.getMethodType().hasFlags(Flag.Static);

if (m.getMethodType() == null || m.getSelect() == null) {
if (isStatic && !isStatementInParentBlock(m)) {
return expression;
}

boolean isStatement = isStatement();

if (matchers.entrySet().stream().anyMatch(entry -> matches(m, entry.getKey(), entry.getValue()))) {
boolean hasSameReturnType = TypeUtils.isAssignableTo(m.getMethodType().getReturnType(), m.getSelect().getType());
boolean removable = (isStatement && depth == 0) || hasSameReturnType;
boolean hasSameReturnType = m.getSelect() != null && TypeUtils.isAssignableTo(m.getMethodType().getReturnType(), m.getSelect().getType());
boolean removable = ((isStatement || isStatic) && depth == 0) || hasSameReturnType;
if (!removable) {
return expression;
}

if (m.getSelect() instanceof J.Identifier || m.getSelect() instanceof J.NewClass) {
maybeRemoveImport(m.getMethodType().getDeclaringType());

if (m.getSelect() == null) {
return null;
} else if (m.getSelect() instanceof J.Identifier || m.getSelect() instanceof J.NewClass) {
boolean keepSelect = depth != 0;
if (keepSelect) {
selectAfter.add(getSelectAfter(m));
Expand Down Expand Up @@ -131,6 +137,11 @@ private boolean isStatement() {
).getValue() instanceof J.Block;
}

private boolean isStatementInParentBlock(Statement method) {
J.Block parentBlock = getCursor().firstEnclosing(J.Block.class);
return parentBlock == null || parentBlock.getStatements().contains(method);
}

private boolean isLambdaBody() {
if (getCursor().getParent() == null) {
return false;
Expand Down Expand Up @@ -241,7 +252,7 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {

@Value
@With
static class ToBeRemoved implements Marker {
private static class ToBeRemoved implements Marker {
UUID id;
static <J2 extends J> J2 withMarker(J2 j) {
return j.withMarkers(j.getMarkers().addIfAbsent(new ToBeRemoved(randomId())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
*/
package org.openrewrite.java;

import java.util.List;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.ExpectedToFail;
import org.openrewrite.DocumentExample;
import org.openrewrite.Recipe;
import org.openrewrite.test.RewriteTest;

import java.util.List;

import static org.openrewrite.java.Assertions.java;
import static org.openrewrite.test.RewriteTest.toRecipe;
Expand Down Expand Up @@ -172,7 +173,6 @@ void method() {
}

@Test
@ExpectedToFail
void removeWithoutSelect() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("Test foo()")),
Expand Down Expand Up @@ -492,4 +492,140 @@ public void method() {
)
);
}

@Test
void removeStaticMethodFromImport() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("org.junit.jupiter.api.Assertions assertTrue(..)")),
// language=java
java(
"""
import static org.junit.jupiter.api.Assertions.assertTrue;
class Test {
void method() {
assertTrue(true);
}
}
""",
"""
class Test {
void method() {
}
}
"""
)
);
}

@Test
void keepStaticMethodFromImportWithAssignment() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")),
// language=java
java(
"""
import java.util.List;
import static java.util.Collections.emptyList;
class Test {
void method() {
List<Object> emptyList = emptyList();
emptyList.isEmpty();
}
}
"""
)
);
}

@Test
void keepStaticMethodFromImportClassField() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")),
// language=java
java(
"""
import java.util.List;
import static java.util.Collections.emptyList;
class Test {
List<Object> emptyList = emptyList();
void method() {
emptyList.isEmpty();
}
}
"""
)
);
}

@Test
void removeStaticMethod() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("org.junit.jupiter.api.Assertions assertTrue(..)")),
// language=java
java(
"""
import org.junit.jupiter.api.Assertions;
class Test {
void method() {
Assertions.assertTrue(true);
}
}
""",
"""
class Test {
void method() {
}
}
"""
)
);
}

@Test
void keepStaticMethodWithAssignment() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")),
// language=java
java(
"""
import java.util.List;
import java.util.Collections;
class Test {
void method() {
List<Object> emptyList = Collections.emptyList();
emptyList.size();
}
}
"""
)
);
}

@Test
void keepStaticMethodClassField() {
rewriteRun(
spec -> spec.recipe(createRemoveMethodsRecipe("java.util.Collections emptyList()")),
// language=java
java(
"""
import java.util.List;
import java.util.Collections;
class Test {
List<Object> emptyList = Collections.emptyList();
void method() {
emptyList.size();
}
}
"""
)
);
}
}

0 comments on commit 54a2057

Please sign in to comment.