Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont convert try catch fail when return #566

Merged
merged 5 commits into from
Aug 8, 2024
Merged

Dont convert try catch fail when return #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a13c52c
Solves https://github.com/openrewrite/rewrite-testing-frameworks/issu…
Laurens-W Aug 8, 2024
5404977
Apply org.openrewrite.recipes.OpenRewriteBestPractices
Laurens-W Aug 8, 2024
41dd21c
Polish: strip unnecessary elements, add highlighting, consistent new …
timtebeek Aug 8, 2024
e5a5e8c
Illustrate missed scenario with added test
timtebeek Aug 8, 2024
1baf659
Fix failing scenario with a nested visitor
timtebeek Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/main/java/org/openrewrite/java/testing/junit5/RemoveTryCatchFailBlocks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.*;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.*;

import java.util.Collections;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;

public class RemoveTryCatchFailBlocks extends Recipe {
Expand Down Expand Up @@ -67,6 +65,19 @@ public J visitTry(J.Try jtry, ExecutionContext ctx) {
return try_;
}

// Skip if any return is found, since we can't return from `assertDoesNotThrow`
AtomicBoolean returnFound = new AtomicBoolean(false);
new JavaIsoVisitor<AtomicBoolean>() {
@Override
public J.Return visitReturn(J.Return _return, AtomicBoolean atomicBoolean) {
atomicBoolean.set(true);
return _return;
}
}.visit(try_, returnFound, getCursor().getParentOrThrow());
if (returnFound.get()) {
return try_;
}

/*
Only one statement in the catch block, which is a fail(), with no or a simple String argument.
We would not want to convert for instance fail(cleanUpAndReturnMessage()) might still have side
Expand All @@ -81,9 +92,9 @@ We would not want to convert for instance fail(cleanUpAndReturnMessage()) might
return try_;
}
J.MethodInvocation failCall = (J.MethodInvocation) statement;
if (!ASSERT_FAIL_NO_ARG.matches(failCall)
&& !ASSERT_FAIL_STRING_ARG.matches(failCall)
&& !ASSERT_FAIL_THROWABLE_ARG.matches(failCall)) {
if (!ASSERT_FAIL_NO_ARG.matches(failCall) &&
!ASSERT_FAIL_STRING_ARG.matches(failCall) &&
!ASSERT_FAIL_THROWABLE_ARG.matches(failCall)) {
return try_;
}

Expand Down
106 changes: 77 additions & 29 deletions src/test/java/org/openrewrite/java/testing/junit5/RemoveTryCatchFailBlocksTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void removeTryCatchBlock() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -60,7 +60,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -83,7 +83,7 @@ void removeStaticImportFail() {
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.fail;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -98,7 +98,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -120,7 +120,7 @@ void removeTryCatchBlockWithoutMessage() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -135,7 +135,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -157,7 +157,7 @@ void removeTryCatchBlockWithFailString() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -172,7 +172,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -194,7 +194,7 @@ void failMethodArgIsNotGetMessage() {
"""
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;

class MyTest {
@Test
void aTest() {
Expand All @@ -204,7 +204,7 @@ void aTest() {
Assertions.fail(cleanUpAndReturnMessage());
}
}

String cleanUpAndReturnMessage() {
System.out.println("clean up code");
return "Oh no!";
Expand All @@ -223,7 +223,7 @@ void doesNotRunWithMultipleCatchBlocks() {
"""
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;

class MyTest {
@Test
void aTest() {
Expand All @@ -249,7 +249,7 @@ void catchHasMultipleStatements() {
"""
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;

class MyTest {
@Test
void aTest() {
Expand All @@ -275,7 +275,7 @@ void doesNotRunOnTryWithResources() {
import org.junit.jupiter.api.Test;
import java.io.PrintWriter;
import org.junit.jupiter.api.Assertions;

class MyTest {
@Test
void aTest() {
Expand All @@ -299,7 +299,7 @@ void statementsBeforeAndAfterTryBlock() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -320,7 +320,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand Down Expand Up @@ -348,7 +348,7 @@ void failWithStringThrowableArgs() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
void testMethod() {
Expand All @@ -363,7 +363,7 @@ void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
void testMethod() {
Expand All @@ -386,7 +386,7 @@ void failWithSupplierStringAsIdentifier() {
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.util.function.Supplier;

class MyTest {
@Test
void testMethod() {
Expand All @@ -412,7 +412,7 @@ void failWithSupplierStringAsLambda() {
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.util.function.Supplier;

class MyTest {
@Test
void testMethod() {
Expand All @@ -436,7 +436,7 @@ void failWithThrowable() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
void testMethod() {
Expand All @@ -451,7 +451,7 @@ void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
void testMethod() {
Expand All @@ -473,7 +473,7 @@ void multipleTryCatchBlocks() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -493,7 +493,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -518,7 +518,7 @@ void failHasBinaryWithException() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -533,7 +533,7 @@ public void testMethod() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -555,7 +555,7 @@ void failHasBinaryWithMethodCall() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -565,7 +565,7 @@ public void testMethod() {
Assertions.fail("The error is: " + anotherMethod());
}
}

public String anotherMethod() {
return "anotherMethod";
}
Expand Down Expand Up @@ -602,7 +602,7 @@ public void testMethod() {
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import java.io.FileOutputStream;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -625,7 +625,7 @@ void doesNotRunonTryFinally() {
"""
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class MyTest {
@Test
public void testMethod() {
Expand All @@ -642,4 +642,52 @@ public void testMethod() {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/547")
void noChangeOnDirectReturn() {
//language=java
rewriteRun(
java(
"""
import static org.junit.jupiter.api.Assertions.fail;

class Foo {
String getFoo() {
try {
return "foo";
} catch (RuntimeException e) {
fail();
}
}
}
"""
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-testing-frameworks/issues/547")
void noChangeOnNestedReturn() {
//language=java
rewriteRun(
java(
"""
import static org.junit.jupiter.api.Assertions.fail;

class Foo {
String getBar(boolean b) {
try {
if (b) {
return "bar";
}
} catch (RuntimeException e) {
fail();
}
}
}
"""
)
);
}
}
Loading