Skip to content

Commit

Permalink
Merge pull request #395 from openrewrite/csharp-catch-only-rethrow
Browse files Browse the repository at this point in the history 
Adjust `CatchClauseOnlyRethrows` to work with C#
  • Loading branch information
@Laurens-W
Laurens-W authored Nov 28, 2024
2 parents 8a0343b + f1a83f4 commit 14f7766
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 23 deletions.
9 changes: 9 additions & 0 deletions src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@

import org.openrewrite.ExecutionContext;
import org.openrewrite.Recipe;
import org.openrewrite.SourceFile;
import org.openrewrite.TreeVisitor;
import org.openrewrite.csharp.tree.Cs;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.tree.Expression;
Expand Down Expand Up @@ -113,6 +115,13 @@ private boolean onlyRethrows(J.Try.Catch aCatch) {
}

Expression exception = ((J.Throw) aCatch.getBody().getStatements().get(0)).getException();

// In C# an implicit rethrow is possible
if (getCursor().firstEnclosing(SourceFile.class) instanceof Cs &&
exception instanceof J.Empty) {
return true;
}

JavaType catchParameterType = aCatch.getParameter().getType();
if (!(catchParameterType instanceof JavaType.MultiCatch)) {
JavaType.FullyQualified catchType = TypeUtils.asFullyQualified(catchParameterType);
Expand Down
100 changes: 77 additions & 23 deletions src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,19 @@

import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.ExecutionContext;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.Tree;
import org.openrewrite.csharp.tree.Cs;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.Space;
import org.openrewrite.marker.Markers;
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("ALL")
class CatchClauseOnlyRethrowsTest implements RewriteTest {
Expand All @@ -38,7 +47,7 @@ void rethrownButWithDifferentMessage() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -63,7 +72,7 @@ void catchShouldBePreservedBecauseLessSpecificCatchFollows() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -90,7 +99,7 @@ void catchShouldBePreservedBecauseLessSpecificCatchFollowsWithMultiCast() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -116,7 +125,7 @@ void tryCanBeRemoved() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -130,7 +139,7 @@ void foo() throws IOException {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
new FileReader("").read();
Expand All @@ -150,7 +159,7 @@ void tryCanBeRemovedWithMultiCatch() {
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;
class A {
void foo() throws IOException {
try {
Expand All @@ -166,16 +175,16 @@ void foo() throws IOException {
}
""",
"""
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;
class A {
void foo() throws IOException {
new FileReader("").read();
}
}
"""
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;
class A {
void foo() throws IOException {
new FileReader("").read();
}
}
"""
)
);
}
Expand All @@ -189,7 +198,7 @@ void multiCatchPreservedOnDifferentThrow() {
import java.io.FileReader;
import java.io.IOException;
import java.io.FileNotFoundException;
class A {
void foo() throws IOException {
try {
Expand All @@ -214,7 +223,7 @@ void tryShouldBePreservedBecauseFinally() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -230,7 +239,7 @@ void foo() throws IOException {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -253,7 +262,7 @@ void tryShouldBePreservedBecauseResources() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try(FileReader fr = new FileReader("")) {
Expand All @@ -267,7 +276,7 @@ void foo() throws IOException {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try(FileReader fr = new FileReader("")) {
Expand All @@ -288,7 +297,7 @@ void wrappingAndRethrowingIsUnchanged() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() {
try {
Expand All @@ -311,7 +320,7 @@ void loggingAndRethrowingIsUnchanged() {
"""
import java.io.FileReader;
import java.io.IOException;
class A {
void foo() throws IOException {
try {
Expand All @@ -326,4 +335,49 @@ void foo() throws IOException {
)
);
}

@Test
void verifyCsharpImplicitThrow() {
rewriteRun(
spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
@Override
public J visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
// C# can rethrow the caught exception implicitly and so the `e` Identifier is removed by the inline visitor below
Cs.CompilationUnit cscu = JavaToCsharp.compilationUnit(cu);
cscu = (Cs.CompilationUnit) new JavaVisitor<ExecutionContext>() {
@Override
public J visitThrow(J.Throw thrown, ExecutionContext ctx) {
if (thrown.getException() instanceof J.Identifier) {
return thrown.withException(new J.Empty(Tree.randomId(), Space.EMPTY, Markers.EMPTY));
}
return thrown;
}
}.visit(cscu, new InMemoryExecutionContext());
// Exercise the regular recipe with the now modified CSharp compilation unit
return (J) new CatchClauseOnlyRethrows().getVisitor().visit(cscu, ctx);
}
})),
//language=java
java(
"""
class A {
void foo() throws IllegalAccessException {
try {
throw new IllegalAccessException();
} catch (Exception e) {
throw e; // `e` is removed below
}
}
}
""",
"""
class A {
void foo() throws IllegalAccessException {
throw new IllegalAccessException();
}
}
"""
)
);
}
}
46 changes: 46 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2024 the original author or authors.
* <p>
* 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
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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.openrewrite.csharp.tree.Cs;
import org.openrewrite.java.tree.J;
import org.openrewrite.java.tree.JRightPadded;
import org.openrewrite.java.tree.Statement;

import java.util.List;

public class JavaToCsharp {

public static Cs.CompilationUnit compilationUnit(J.CompilationUnit cu) {
return new Cs.CompilationUnit(
cu.getId(),
cu.getPrefix(),
cu.getMarkers(),
cu.getSourcePath(),
cu.getFileAttributes(),
cu.getCharset().name(),
cu.isCharsetBomMarked(),
cu.getChecksum(),
List.of(),
List.of(),
List.of(),
cu.getClasses().stream()
.map(Statement.class::cast)
.map(JRightPadded::build)
.toList(),
cu.getEof());
}
}

0 comments on commit 14f7766

Please sign in to comment.