Skip to content

Commit d24228b

Browse files
nielsdebruinNiels de Bruintimtebeek
authored
Fix CatchCauseOnlyRethrows onlyRethrows check for multi catch statements (#355)
* Fix CatchCauseOnlyRethrows rethrows check for multi catch statements * Specifically check for `JavaType.MultiCatch` --------- Co-authored-by: Niels de Bruin <niels.de.bruin@jdriven.com> Co-authored-by: Tim te Beek <tim@moderne.io>
1 parent 07686d2 commit d24228b

2 files changed

Lines changed: 77 additions & 7 deletions

File tree

src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
import org.openrewrite.TreeVisitor;
2121
import org.openrewrite.internal.ListUtils;
2222
import org.openrewrite.java.JavaIsoVisitor;
23-
import org.openrewrite.java.tree.*;
23+
import org.openrewrite.java.tree.Expression;
24+
import org.openrewrite.java.tree.J;
25+
import org.openrewrite.java.tree.JavaType;
26+
import org.openrewrite.java.tree.TypeUtils;
2427

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

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

106109
private boolean onlyRethrows(J.Try.Catch aCatch) {
107110
if (aCatch.getBody().getStatements().size() != 1 ||
108-
!(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) {
111+
!(aCatch.getBody().getStatements().get(0) instanceof J.Throw)) {
109112
return false;
110113
}
111114

112115
Expression exception = ((J.Throw) aCatch.getBody().getStatements().get(0)).getException();
113-
JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(aCatch.getParameter().getType());
114-
if (catchType == null || !catchType.equals(exception.getType())) {
115-
return false;
116+
JavaType catchParameterType = aCatch.getParameter().getType();
117+
if (!(catchParameterType instanceof JavaType.MultiCatch)) {
118+
JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(catchParameterType);
119+
if (catchType == null || !catchType.equals(exception.getType())) {
120+
return false;
121+
}
116122
}
117-
118123
if (exception instanceof J.Identifier) {
119124
return ((J.Identifier) exception).getSimpleName().equals(aCatch.getParameter().getTree().getVariables().get(0).getSimpleName());
120125
}

src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,71 @@ void foo() throws IOException {
141141
);
142142
}
143143

144+
@Test
145+
void tryCanBeRemovedWithMultiCatch() {
146+
rewriteRun(
147+
//language=java
148+
java(
149+
"""
150+
import java.io.FileReader;
151+
import java.io.IOException;
152+
import java.io.FileNotFoundException;
153+
154+
class A {
155+
void foo() throws IOException {
156+
try {
157+
new FileReader("").read();
158+
} catch (FileNotFoundException e) {
159+
throw e;
160+
} catch(IOException | ArrayIndexOutOfBoundsException e) {
161+
throw e;
162+
} catch(Exception e) {
163+
throw e;
164+
}
165+
}
166+
}
167+
""",
168+
"""
169+
import java.io.FileReader;
170+
import java.io.IOException;
171+
import java.io.FileNotFoundException;
172+
173+
class A {
174+
void foo() throws IOException {
175+
new FileReader("").read();
176+
}
177+
}
178+
"""
179+
)
180+
);
181+
}
182+
183+
@Test
184+
void multiCatchPreservedOnDifferentThrow() {
185+
rewriteRun(
186+
//language=java
187+
java(
188+
"""
189+
import java.io.FileReader;
190+
import java.io.IOException;
191+
import java.io.FileNotFoundException;
192+
193+
class A {
194+
void foo() throws IOException {
195+
try {
196+
new FileReader("").read();
197+
} catch (FileNotFoundException e) {
198+
throw e;
199+
} catch(IOException | ArrayIndexOutOfBoundsException e) {
200+
throw new IOException("another message", e);
201+
}
202+
}
203+
}
204+
"""
205+
)
206+
);
207+
}
208+
144209
@Test
145210
void tryShouldBePreservedBecauseFinally() {
146211
rewriteRun(

0 commit comments

Comments
 (0)