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

Fix CatchCauseOnlyRethrows onlyRethrows check for multi catch statements #355

Merged
merged 3 commits into from
Oct 22, 2024
Merged

Fix CatchCauseOnlyRethrows onlyRethrows check for multi catch statements #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e8d04a
Fix CatchCauseOnlyRethrows rethrows check for multi catch statements
Oct 7, 2024
7364f62
Merge branch 'main' into multi-catch-only-throws-support
timtebeek Oct 7, 2024
6b39457
Specifically check for `JavaType.MultiCatch`
timtebeek Oct 22, 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
19 changes: 12 additions & 7 deletions src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import org.openrewrite.TreeVisitor;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.tree.*;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JavaType;
import org.openrewrite.java.tree.TypeUtils;

import java.time.Duration;
import java.util.Set;
Expand All @@ -37,7 +40,7 @@ public String getDisplayName() {
@Override
public String getDescription() {
return "A `catch` clause that only rethrows the caught exception is unnecessary. " +
"Letting the exception bubble up as normal achieves the same result with less code.";
"Letting the exception bubble up as normal achieves the same result with less code.";
}

@Override
Expand Down Expand Up @@ -105,16 +108,18 @@ private boolean hasWiderExceptionType(J.Try.Catch aCatch, J.Try.Catch next) {

private boolean onlyRethrows(J.Try.Catch aCatch) {
if (aCatch.getBody().getStatements().size() != 1 ||
!(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) {
!(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) {
return false;
}

Expression exception = ((J.Throw) aCatch.getBody().getStatements().get(0)).getException();
JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(aCatch.getParameter().getType());
if (catchType == null || !catchType.equals(exception.getType())) {
return false;
JavaType catchParameterType = aCatch.getParameter().getType();
if (!(catchParameterType instanceof JavaType.MultiCatch)) {
JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(catchParameterType);
if (catchType == null || !catchType.equals(exception.getType())) {
return false;
}
}

if (exception instanceof J.Identifier) {
return ((J.Identifier) exception).getSimpleName().equals(aCatch.getParameter().getTree().getVariables().get(0).getSimpleName());
}
Expand Down
65 changes: 65 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,71 @@ void foo() throws IOException {
);
}

@Test
void tryCanBeRemovedWithMultiCatch() {
rewriteRun(
//language=java
java(
"""
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;

class A {
void foo() throws IOException {
try {
new FileReader("").read();
} catch (FileNotFoundException e) {
throw e;
} catch(IOException | ArrayIndexOutOfBoundsException e) {
throw e;
} catch(Exception e) {
throw e;
}
}
}
""",
"""
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;

class A {
void foo() throws IOException {
new FileReader("").read();
}
}
"""
)
);
}

@Test
void multiCatchPreservedOnDifferentThrow() {
rewriteRun(
//language=java
java(
"""
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;

class A {
void foo() throws IOException {
try {
new FileReader("").read();
} catch (FileNotFoundException e) {
throw e;
} catch(IOException | ArrayIndexOutOfBoundsException e) {
throw new IOException("another message", e);
}
}
}
"""
)
);
}

@Test
void tryShouldBePreservedBecauseFinally() {
rewriteRun(
Expand Down