diff --git a/src/main/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoid.java b/src/main/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoid.java new file mode 100644 index 000000000..318fff817 --- /dev/null +++ b/src/main/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoid.java @@ -0,0 +1,115 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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.java.testing.cleanup; + +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.TypeUtils; + +import static java.util.Objects.requireNonNull; + +public class TestMethodsShouldBeVoid extends Recipe { + + @Override + public String getDisplayName() { + return "Test methods should have void return type"; + } + + @Override + public String getDescription() { + return "Test methods annotated with `@Test`, `@ParameterizedTest`, `@RepeatedTest`, `@TestTemplate` " + + "should have `void` return type. Non-void return types can cause test discovery issues, " + + "and warnings as of JUnit 5.13+. This recipe changes the return type to `void` and removes `return` statements."; + } + + @Override + public TreeVisitor getVisitor() { + return new JavaIsoVisitor() { + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { + J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); + + // Check if method has a test annotation & body + if (!hasTestAnnotation(m) || m.getBody() == null) { + return m; + } + + // Check if return type is already void + JavaType.Primitive voidType = JavaType.Primitive.Void; + if (m.getReturnTypeExpression() == null || TypeUtils.isOfType(m.getReturnTypeExpression().getType(), voidType)) { + return m; + } + + // Change return type to void + m = m.withReturnTypeExpression(new J.Primitive( + m.getReturnTypeExpression().getId(), + m.getReturnTypeExpression().getPrefix(), + m.getReturnTypeExpression().getMarkers(), + JavaType.Primitive.Void + )); + + // Update method type + if (m.getMethodType() != null) { + m = m.withMethodType(m.getMethodType().withReturnType(voidType)); + } + + // Remove return statements that are not in nested classes or lambdas + return m.withBody((J.Block) new RemoveDirectReturns().visitBlock(requireNonNull(m.getBody()), ctx)); + } + + private boolean hasTestAnnotation(J.MethodDeclaration method) { + for (J.Annotation annotation : method.getLeadingAnnotations()) { + if (TypeUtils.isOfClassType(annotation.getType(), "org.junit.Test") || + TypeUtils.isOfClassType(annotation.getType(), "org.junit.jupiter.api.RepeatedTest") || + TypeUtils.isOfClassType(annotation.getType(), "org.junit.jupiter.api.Test") || + TypeUtils.isOfClassType(annotation.getType(), "org.junit.jupiter.api.TestTemplate") || + TypeUtils.isOfClassType(annotation.getType(), "org.junit.jupiter.params.ParameterizedTest")) { + return true; + } + } + return false; + } + }; + } + + private static class RemoveDirectReturns extends JavaVisitor { + @Override + public J visitLambda(J.Lambda lambda, ExecutionContext ctx) { + return lambda; // Retain nested returns + } + + @Override + public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { + return newClass; // Retain nested returns + } + + @Override + public @Nullable J visitReturn(J.Return retrn, ExecutionContext ctx) { + return retrn.getExpression() instanceof Statement ? + // Retain any side effects from expressions in return statements + retrn.getExpression().withPrefix(retrn.getPrefix()) : + // Remove any other return statements + null; + } + } +} diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 736d2ac7e..62e99c74c 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -1344,6 +1344,32 @@ examples: language: java --- type: specs.openrewrite.org/v1beta/example +recipeName: org.openrewrite.java.testing.cleanup.TestMethodsShouldBeVoid +examples: +- description: '' + sources: + - before: | + import org.junit.jupiter.api.Test; + + class ATest { + @Test + int testMethod() { + int i = 42; + return i; + } + } + after: | + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + int i = 42; + } + } + language: java +--- +type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.java.testing.datafaker.JavaFakerToDataFaker examples: - description: '' diff --git a/src/main/resources/META-INF/rewrite/junit5.yml b/src/main/resources/META-INF/rewrite/junit5.yml index b2d3c5e14..44ee921e3 100755 --- a/src/main/resources/META-INF/rewrite/junit5.yml +++ b/src/main/resources/META-INF/rewrite/junit5.yml @@ -32,6 +32,7 @@ recipeList: - org.openrewrite.java.testing.cleanup.RemoveTestPrefix - org.openrewrite.java.testing.cleanup.SimplifyTestThrows - org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic + - org.openrewrite.java.testing.cleanup.TestMethodsShouldBeVoid - org.openrewrite.java.testing.junit5.AddParameterizedTestAnnotation - org.openrewrite.java.testing.junit5.RemoveDuplicateTestTemplates - org.openrewrite.java.testing.junit5.RemoveTryCatchFailBlocks diff --git a/src/test/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoidTest.java b/src/test/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoidTest.java new file mode 100644 index 000000000..c3935eeaa --- /dev/null +++ b/src/test/java/org/openrewrite/java/testing/cleanup/TestMethodsShouldBeVoidTest.java @@ -0,0 +1,358 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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.java.testing.cleanup; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.java.JavaParser; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("JUnitMalformedDeclaration") +class TestMethodsShouldBeVoidTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec + .parser(JavaParser.fromJavaVersion() + .classpathFromResources(new InMemoryExecutionContext(), "junit-jupiter-api-5", "junit-jupiter-params-5")) + .recipe(new TestMethodsShouldBeVoid()); + } + + @DocumentExample + @Test + void changeReturnTypeToVoid() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + int testMethod() { + int i = 42; + return i; + } + } + """, + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + int i = 42; + } + } + """ + ) + ); + } + + @Test + void removeReturnStatementOnly() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + String testMethod() { + System.out.println("test"); + return "done"; + } + } + """, + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + System.out.println("test"); + } + } + """ + ) + ); + } + + @Test + void preserveReturnInLambda() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + import java.util.function.Supplier; + + class ATest { + @Test + int testMethod() { + Supplier supplier = () -> { + return 42; + }; + return supplier.get(); + } + } + """, + """ + import org.junit.jupiter.api.Test; + import java.util.function.Supplier; + + class ATest { + @Test + void testMethod() { + Supplier supplier = () -> { + return 42; + }; + supplier.get(); + } + } + """ + ) + ); + } + + @Test + void preserveReturnInAnonymousClass() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + String testMethod() { + Runnable r = new Runnable() { + @Override + public void run() { + System.out.println("running"); + } + + public String getValue() { + return "value"; + } + }; + r.run(); + return "done"; + } + } + """, + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + Runnable r = new Runnable() { + @Override + public void run() { + System.out.println("running"); + } + + public String getValue() { + return "value"; + } + }; + r.run(); + } + } + """ + ) + ); + } + + @Test + void handleParameterizedTest() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.params.ParameterizedTest; + import org.junit.jupiter.params.provider.ValueSource; + + class ATest { + @ParameterizedTest + @ValueSource(strings = {"test1", "test2"}) + boolean testMethod(String value) { + System.out.println(value); + return value.length() > 0; + } + } + """, + """ + import org.junit.jupiter.params.ParameterizedTest; + import org.junit.jupiter.params.provider.ValueSource; + + class ATest { + @ParameterizedTest + @ValueSource(strings = {"test1", "test2"}) + void testMethod(String value) { + System.out.println(value); + } + } + """ + ) + ); + } + + @Test + void handleRepeatedTest() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.RepeatedTest; + + class ATest { + @RepeatedTest(3) + String testMethod() { + return "repeated"; + } + } + """, + """ + import org.junit.jupiter.api.RepeatedTest; + + class ATest { + @RepeatedTest(3) + void testMethod() { + } + } + """ + ) + ); + } + + @Test + void doNotChangeVoidMethods() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + System.out.println("already void"); + } + } + """ + ) + ); + } + + @Test + void doNotChangeNonTestMethods() { + //language=java + rewriteRun( + java( + """ + class ATest { + int notATestMethod() { + return 42; + } + + String helperMethod() { + return "helper"; + } + } + """ + ) + ); + } + + @Test + void handleMultipleReturns() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + int testMethod() { + if (true) { + return 1; + } else { + return 2; + } + } + } + """, + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + if (true) { + } else { + } + } + } + """ + ) + ); + } + + @Test + void preserveExpressionFromReturn() { + //language=java + rewriteRun( + java( + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + int testMethod() { + return calculateValue(); + } + + private int calculateValue() { + return 42; + } + } + """, + """ + import org.junit.jupiter.api.Test; + + class ATest { + @Test + void testMethod() { + calculateValue(); + } + + private int calculateValue() { + return 42; + } + } + """ + ) + ); + } +}