diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitor.java b/src/main/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitor.java deleted file mode 100644 index da23007b2..000000000 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitor.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.staticanalysis; - -import lombok.AllArgsConstructor; -import org.jspecify.annotations.Nullable; -import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.tree.*; - -import java.util.function.BiPredicate; - -/** - * Removes all {@link MethodCall} matching both the - * {@link RemoveMethodCallVisitor#methodMatcher} - * and the - * {@link RemoveMethodCallVisitor#argumentPredicate} for all arguments. - *

- * Only removes {@link MethodCall} where the call's return value is unused. - */ -@AllArgsConstructor -// TODO Consider moving this to `org.openrewrite.java.RemoveMethodInvocationsVisitor` in rewrite-java -public class RemoveMethodCallVisitor

extends JavaIsoVisitor

{ - /** - * The {@link MethodCall} to match to be removed. - */ - private final MethodMatcher methodMatcher; - /** - * All arguments must match the predicate for the {@link MethodCall} to be removed. - */ - private final BiPredicate argumentPredicate; - - @SuppressWarnings("NullableProblems") - @Override - public J.@Nullable NewClass visitNewClass(J.NewClass newClass, P p) { - if (methodMatcher.matches(newClass) && predicateMatchesAllArguments(newClass) && isStatementInParentBlock(newClass)) { - if (newClass.getMethodType() != null) { - maybeRemoveImport(newClass.getMethodType().getDeclaringType()); - } - return null; - } - return super.visitNewClass(newClass, p); - } - - @SuppressWarnings("NullableProblems") - @Override - public J.@Nullable MethodInvocation visitMethodInvocation(J.MethodInvocation method, P p) { - // Find method invocations that match the specified method and arguments - if (methodMatcher.matches(method) && predicateMatchesAllArguments(method)) { - // If the method invocation is a standalone statement, remove it altogether - if (isStatementInParentBlock(method)) { - if (method.getMethodType() != null) { - maybeRemoveImport(method.getMethodType().getDeclaringType()); - } - return null; - } - - // If the method invocation is in a fluent chain, remove just the current invocation - if (method.getSelect() instanceof J.MethodInvocation && - TypeUtils.isOfType(method.getType(), method.getSelect().getType())) { - return super.visitMethodInvocation((J.MethodInvocation) method.getSelect(), p); - } - } - return super.visitMethodInvocation(method, p); - } - - private boolean predicateMatchesAllArguments(MethodCall method) { - for (int i = 0; i < method.getArguments().size(); i++) { - if (!argumentPredicate.test(i, method.getArguments().get(i))) { - return false; - } - } - return true; - } - - private boolean isStatementInParentBlock(Statement method) { - J.Block parentBlock = getCursor().firstEnclosing(J.Block.class); - return parentBlock == null || parentBlock.getStatements().contains(method); - } -} diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnneededAssertion.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnneededAssertion.java index f5b7dd9ae..2103b03a9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnneededAssertion.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnneededAssertion.java @@ -22,32 +22,31 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.internal.TypesInUse; +import org.openrewrite.java.RemoveMethodInvocationsVisitor; import org.openrewrite.java.search.UsesMethod; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.marker.SearchResult; -import java.util.function.BiPredicate; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; public class RemoveUnneededAssertion extends Recipe { - private static final MethodMatcher JUNIT_JUPITER_ASSERT_TRUE_MATCHER = - new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(..)"); - private static final MethodMatcher JUNIT_JUPITER_ASSERT_FALSE_MATCHER = - new MethodMatcher("org.junit.jupiter.api.Assertions assertFalse(..)"); - private static final MethodMatcher JUNIT_ASSERT_TRUE_MATCHER = - new MethodMatcher("org.junit.Assert assertTrue(boolean)"); - private static final MethodMatcher JUNIT_ASSERT_FALSE_MATCHER = - new MethodMatcher("org.junit.Assert assertFalse(boolean)"); + // Junit Jupiter + private static final MethodMatcher JUNIT_JUPITER_ASSERT_TRUE_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertTrue(..)"); + private static final MethodMatcher JUNIT_JUPITER_ASSERT_FALSE_MATCHER = new MethodMatcher("org.junit.jupiter.api.Assertions assertFalse(..)"); - private static final MethodMatcher JUNIT_ASSERT_MESSAGE_TRUE_MATCHER = - new MethodMatcher("org.junit.Assert assertTrue(String, boolean)"); - private static final MethodMatcher JUNIT_ASSERT_MESSAGE_FALSE_MATCHER = - new MethodMatcher("org.junit.Assert assertFalse(String, boolean)"); + // Junit 4 + private static final MethodMatcher JUNIT_ASSERT_TRUE_MATCHER = new MethodMatcher("org.junit.Assert assertTrue(boolean)"); + private static final MethodMatcher JUNIT_ASSERT_FALSE_MATCHER = new MethodMatcher("org.junit.Assert assertFalse(boolean)"); + private static final MethodMatcher JUNIT_ASSERT_MESSAGE_TRUE_MATCHER = new MethodMatcher("org.junit.Assert assertTrue(String, boolean)"); + private static final MethodMatcher JUNIT_ASSERT_MESSAGE_FALSE_MATCHER = new MethodMatcher("org.junit.Assert assertFalse(String, boolean)"); @Override public String getDisplayName() { - return "Remove Unneeded Assertions"; + return "Remove unneeded assertions"; } @Override @@ -57,95 +56,51 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { - return Preconditions.check( - Preconditions.or( - new UsesMethod<>(JUNIT_JUPITER_ASSERT_TRUE_MATCHER), - new UsesMethod<>(JUNIT_JUPITER_ASSERT_FALSE_MATCHER), - new UsesMethod<>(JUNIT_ASSERT_TRUE_MATCHER), - new UsesMethod<>(JUNIT_ASSERT_FALSE_MATCHER), - new UsesMethod<>(JUNIT_ASSERT_MESSAGE_TRUE_MATCHER), - new UsesMethod<>(JUNIT_ASSERT_MESSAGE_FALSE_MATCHER), - new JavaIsoVisitor() { - @Override - public J.Assert visitAssert(J.Assert _assert, ExecutionContext ctx) { - if (J.Literal.isLiteralValue(_assert.getCondition(), true)) { - return SearchResult.found(_assert); - } - return _assert; - } + TreeVisitor constraints = Preconditions.or( + new UsesMethod<>(JUNIT_JUPITER_ASSERT_TRUE_MATCHER), + new UsesMethod<>(JUNIT_JUPITER_ASSERT_FALSE_MATCHER), + new UsesMethod<>(JUNIT_ASSERT_TRUE_MATCHER), + new UsesMethod<>(JUNIT_ASSERT_FALSE_MATCHER), + new UsesMethod<>(JUNIT_ASSERT_MESSAGE_TRUE_MATCHER), + new UsesMethod<>(JUNIT_ASSERT_MESSAGE_FALSE_MATCHER), + new JavaIsoVisitor() { + @Override + public J.Assert visitAssert(J.Assert _assert, ExecutionContext ctx) { + if (J.Literal.isLiteralValue(_assert.getCondition(), true)) { + return SearchResult.found(_assert); } - ), new RemoveUnneededAssertionVisitor()); - } - - private static class RemoveUnneededAssertionVisitor extends JavaIsoVisitor { - @FunctionalInterface - private interface InvokeRemoveMethodCallVisitor { - J.CompilationUnit invoke( - J.CompilationUnit cu, - MethodMatcher methodMatcher, - BiPredicate argumentPredicate - ); - } - - @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - J.CompilationUnit compilationUnit = super.visitCompilationUnit(cu, ctx); - // We can compute the types in use once, because this logic only removes method calls, never adds them. - TypesInUse typesInUse = compilationUnit.getTypesInUse(); - InvokeRemoveMethodCallVisitor invokeRemoveMethodCallVisitor = (inputCu, methodMatcher, argumentPredicate) -> { - if (typesInUse.getUsedMethods().stream().anyMatch(methodMatcher::matches)) { - // Only visit the subtree when we know the method is present. - return (J.CompilationUnit) new RemoveMethodCallVisitor<>(methodMatcher, argumentPredicate) - .visitNonNull(cu, ctx, getCursor().getParentOrThrow()); + return _assert; + } } - return inputCu; - }; + ); - // Junit Jupiter - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_JUPITER_ASSERT_TRUE_MATCHER, - (arg, expr) -> !arg.equals(0) || J.Literal.isLiteralValue(expr, true) - ); - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_JUPITER_ASSERT_FALSE_MATCHER, - (arg, expr) -> !arg.equals(0) || J.Literal.isLiteralValue(expr, false) - ); + Predicate> isTrue = args -> J.Literal.isLiteralValue(args.get(0), true); + Predicate> isFalse = args -> J.Literal.isLiteralValue(args.get(0), false); - // Junit 4 - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_ASSERT_TRUE_MATCHER, - (arg, expr) -> arg.equals(0) && J.Literal.isLiteralValue(expr, true) - ); - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_ASSERT_FALSE_MATCHER, - (arg, expr) -> arg.equals(0) && J.Literal.isLiteralValue(expr, false) - ); - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_ASSERT_MESSAGE_TRUE_MATCHER, - (arg, expr) -> !arg.equals(1) || J.Literal.isLiteralValue(expr, true) - ); - compilationUnit = invokeRemoveMethodCallVisitor.invoke( - compilationUnit, - JUNIT_ASSERT_MESSAGE_FALSE_MATCHER, - (arg, expr) -> !arg.equals(1) || J.Literal.isLiteralValue(expr, false) - ); - return compilationUnit; - } + Map>> matchers = new HashMap<>(); + matchers.put(JUNIT_JUPITER_ASSERT_TRUE_MATCHER, isTrue); + matchers.put(JUNIT_JUPITER_ASSERT_FALSE_MATCHER, isFalse); + matchers.put(JUNIT_ASSERT_TRUE_MATCHER, isTrue); + matchers.put(JUNIT_ASSERT_FALSE_MATCHER, isFalse); + matchers.put(JUNIT_ASSERT_MESSAGE_TRUE_MATCHER, args -> J.Literal.isLiteralValue(args.get(1), true)); + matchers.put(JUNIT_ASSERT_MESSAGE_FALSE_MATCHER, args -> J.Literal.isLiteralValue(args.get(1), false)); + + return Preconditions.check(constraints, new JavaIsoVisitor() { + @Override + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit compilationUnit, ExecutionContext ctx) { + J.CompilationUnit cu = super.visitCompilationUnit(compilationUnit, ctx); + return (J.CompilationUnit) new RemoveMethodInvocationsVisitor(matchers) + .visitNonNull(cu, ctx, getCursor().getParentOrThrow()); + } - @Override - public J.@Nullable Assert visitAssert(J.Assert anAssert, ExecutionContext ctx) { - if (anAssert.getCondition() instanceof J.Literal) { - if (J.Literal.isLiteralValue(anAssert.getCondition(), true)) { - //noinspection ConstantConditions + @Override + @SuppressWarnings("NullableProblems") + public J.@Nullable Assert visitAssert(J.Assert _assert, ExecutionContext ctx) { + if (J.Literal.isLiteralValue(_assert.getCondition(), true)) { return null; } + return super.visitAssert(_assert, ctx); } - return super.visitAssert(anAssert, ctx); - } + }); } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitorTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitorTest.java deleted file mode 100644 index 21ac36efe..000000000 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveMethodCallVisitorTest.java +++ /dev/null @@ -1,169 +0,0 @@ -/* - * Copyright 2022 the original author or authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *

- * https://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.openrewrite.staticanalysis; - -import org.junit.jupiter.api.Test; -import org.openrewrite.DocumentExample; -import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.tree.J; -import org.openrewrite.test.RecipeSpec; -import org.openrewrite.test.RewriteTest; - -import static org.openrewrite.java.Assertions.java; -import static org.openrewrite.test.RewriteTest.toRecipe; - -@SuppressWarnings("FunctionName") -class RemoveMethodCallVisitorTest implements RewriteTest { - - @Override - public void defaults(RecipeSpec spec) { - spec.recipe(toRecipe(() -> new RemoveMethodCallVisitor<>(new MethodMatcher("* assertTrue(..)"), - (arg, expr) -> arg == 0 && J.Literal.isLiteralValue(expr, true)))); - } - - @DocumentExample - @Test - void assertTrueIsRemoved() { - rewriteRun( - //language=java - java( - """ - abstract class Test { - abstract void assertTrue(boolean condition); - - void test() { - System.out.println("Hello"); - assertTrue(true); - System.out.println("World"); - } - } - """, """ - abstract class Test { - abstract void assertTrue(boolean condition); - - void test() { - System.out.println("Hello"); - System.out.println("World"); - } - } - """ - ) - ); - } - - @Test - void assertTrueFalseIsNotRemoved() { - rewriteRun( - //language=java - java( - """ - abstract class Test { - abstract void assertTrue(boolean condition); - - void test() { - System.out.println("Hello"); - assertTrue(false); - System.out.println("World"); - } - } - """ - ) - ); - } - - @Test - void assertTrueTwoArgIsRemoved() { - rewriteRun( - spec -> spec.recipe(toRecipe(() -> new RemoveMethodCallVisitor<>( - new MethodMatcher("* assertTrue(..)"), (arg, expr) -> arg != 1 || J.Literal.isLiteralValue(expr, true) - ))), - //language=java - java( - """ - abstract class Test { - abstract void assertTrue(String message, boolean condition); - - void test() { - System.out.println("Hello"); - assertTrue("message", true); - System.out.println("World"); - } - } - """, - """ - abstract class Test { - abstract void assertTrue(String message, boolean condition); - - void test() { - System.out.println("Hello"); - System.out.println("World"); - } - } - """ - ) - ); - } - - @Test - void doesNotRemoveAssertTrueIfReturnValueIsUsed() { - rewriteRun( - //language=java - java( - """ - abstract class Test { - abstract int assertTrue(boolean condition); - - void test() { - System.out.println("Hello"); - int value = assertTrue(true); - System.out.println("World"); - } - } - """ - ) - ); - } - - @Test - void removeMethodCallFromFluentChain() { - rewriteRun( - spec -> spec.recipe(RewriteTest.toRecipe(() -> new RemoveMethodCallVisitor<>( - new MethodMatcher("java.lang.StringBuilder append(..)"), (i, e) -> true))), - // language=java - java( - """ - class Main { - void hello() { - final String s = new StringBuilder("hello") - .delete(1, 2) - .append("world") - .toString(); - } - } - """, - """ - class Main { - void hello() { - final String s = new StringBuilder("hello") - .delete(1, 2) - .toString(); - } - } - """ - ) - ); - } -}