From 5b42a63e10569e1320875cb8a40fb00950f1aef9 Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Tue, 26 Nov 2024 14:53:38 +0100 Subject: [PATCH 01/11] Add Csharp specific check for empty throw statements --- .../staticanalysis/CatchClauseOnlyRethrows.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java index 53b978fce..d31a84a23 100755 --- a/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java +++ b/src/main/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrows.java @@ -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; @@ -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); From 4add0df9d27142a7a89f0216b3e35781f3baa86d Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Tue, 26 Nov 2024 17:05:01 +0100 Subject: [PATCH 02/11] Add questionable unit test --- .../CatchClauseOnlyRethrowsTest.java | 96 ++++++++++++++----- 1 file changed, 73 insertions(+), 23 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index c6c93ebf1..ee07611e5 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -17,9 +17,20 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.FileAttributes; +import org.openrewrite.InMemoryExecutionContext; +import org.openrewrite.Tree; +import org.openrewrite.csharp.tree.Cs; +import org.openrewrite.java.tree.*; +import org.openrewrite.marker.Markers; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.nio.file.Path; +import java.util.Collections; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; @SuppressWarnings("ALL") @@ -38,7 +49,7 @@ void rethrownButWithDifferentMessage() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -63,7 +74,7 @@ void catchShouldBePreservedBecauseLessSpecificCatchFollows() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -90,7 +101,7 @@ void catchShouldBePreservedBecauseLessSpecificCatchFollowsWithMultiCast() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -116,7 +127,7 @@ void tryCanBeRemoved() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -130,7 +141,7 @@ void foo() throws IOException { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { new FileReader("").read(); @@ -150,7 +161,7 @@ void tryCanBeRemovedWithMultiCatch() { import java.io.FileReader; import java.io.IOException; import java.io.FileNotFoundException; - + class A { void foo() throws IOException { try { @@ -166,16 +177,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(); + } + } + """ ) ); } @@ -189,7 +200,7 @@ void multiCatchPreservedOnDifferentThrow() { import java.io.FileReader; import java.io.IOException; import java.io.FileNotFoundException; - + class A { void foo() throws IOException { try { @@ -214,7 +225,7 @@ void tryShouldBePreservedBecauseFinally() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -230,7 +241,7 @@ void foo() throws IOException { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -253,7 +264,7 @@ void tryShouldBePreservedBecauseResources() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try(FileReader fr = new FileReader("")) { @@ -267,7 +278,7 @@ void foo() throws IOException { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try(FileReader fr = new FileReader("")) { @@ -288,7 +299,7 @@ void wrappingAndRethrowingIsUnchanged() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() { try { @@ -311,7 +322,7 @@ void loggingAndRethrowingIsUnchanged() { """ import java.io.FileReader; import java.io.IOException; - + class A { void foo() throws IOException { try { @@ -326,4 +337,43 @@ void foo() throws IOException { ) ); } + + @Test + void testCsharpImplicitThrow() { + Cs.CompilationUnit compilationUnit = new Cs.CompilationUnit(Tree.randomId(), Space.EMPTY, + Markers.EMPTY, Path.of("test.cs"), + new FileAttributes(null, null, null, true, true, true, 0l), + null, false, null, null, null, null, + List.of(JRightPadded.build( + new J.ClassDeclaration(Tree.randomId(), Space.EMPTY, Markers.EMPTY, + Collections.emptyList(), Collections.emptyList(), + new J.ClassDeclaration.Kind(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), J.ClassDeclaration.Kind.Type.Class), + new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), "doSome", null, null), + null, null, null, null, null, + new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), + List.of(JRightPadded.build(new J.Try(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, + new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), + List.of(JRightPadded.build(new J.Throw(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.NewClass( + Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, Space.EMPTY, TypeTree.build("java.lang.IllegalAccessException"), JContainer.empty(), null, null)))), + Space.EMPTY), + List.of(new J.Try.Catch(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.ControlParentheses(Tree.randomId(), Space.EMPTY, Markers.EMPTY, + JRightPadded.build(new J.VariableDeclarations(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), Collections.emptyList(), TypeTree.build("java.lang.IllegalAccessException"), null, List.of(), List.of(JRightPadded.build( + new J.VariableDeclarations.NamedVariable(Tree.randomId(), Space.EMPTY, Markers.EMPTY, + new J.Identifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, Collections.emptyList(), "e", null, null), + List.of(), null, null)))))), + new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), + List.of(JRightPadded.build(new J.Throw(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.Empty(Tree.randomId(), Space.EMPTY, Markers.EMPTY)))), + Space.EMPTY))), + null))), + Space.EMPTY), + null))) + , Space.EMPTY); + + assertThat(compilationUnit.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); + assertThat(((J.ClassDeclaration)compilationUnit.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.Try.class); + + Cs.CompilationUnit output = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor().visit(compilationUnit, new InMemoryExecutionContext()); + assertThat(output.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); + assertThat(((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.Throw.class); + } } From 5c675c636146d2f930665103cb0106374c57afed Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Tue, 26 Nov 2024 17:29:14 +0100 Subject: [PATCH 03/11] Fix review comment --- .../openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index ee07611e5..dec07e77c 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -339,7 +339,7 @@ void foo() throws IOException { } @Test - void testCsharpImplicitThrow() { + void verifyCsharpImplicitThrow() { Cs.CompilationUnit compilationUnit = new Cs.CompilationUnit(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Path.of("test.cs"), new FileAttributes(null, null, null, true, true, true, 0l), From 70cc111e43c8b5a5731c1241aea844f80f17d188 Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Wed, 27 Nov 2024 15:16:29 +0100 Subject: [PATCH 04/11] Improve test --- .../CatchClauseOnlyRethrowsTest.java | 92 ++++++++++--------- .../staticanalysis/JavaToCsharp.java | 32 +++++++ 2 files changed, 81 insertions(+), 43 deletions(-) create mode 100644 src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index dec07e77c..285effad2 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -16,20 +16,14 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; -import org.openrewrite.DocumentExample; -import org.openrewrite.FileAttributes; -import org.openrewrite.InMemoryExecutionContext; -import org.openrewrite.Tree; +import org.openrewrite.*; import org.openrewrite.csharp.tree.Cs; +import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; -import java.nio.file.Path; -import java.util.Collections; -import java.util.List; - import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; @@ -340,40 +334,52 @@ void foo() throws IOException { @Test void verifyCsharpImplicitThrow() { - Cs.CompilationUnit compilationUnit = new Cs.CompilationUnit(Tree.randomId(), Space.EMPTY, - Markers.EMPTY, Path.of("test.cs"), - new FileAttributes(null, null, null, true, true, true, 0l), - null, false, null, null, null, null, - List.of(JRightPadded.build( - new J.ClassDeclaration(Tree.randomId(), Space.EMPTY, Markers.EMPTY, - Collections.emptyList(), Collections.emptyList(), - new J.ClassDeclaration.Kind(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), J.ClassDeclaration.Kind.Type.Class), - new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), "doSome", null, null), - null, null, null, null, null, - new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), - List.of(JRightPadded.build(new J.Try(Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, - new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), - List.of(JRightPadded.build(new J.Throw(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.NewClass( - Tree.randomId(), Space.EMPTY, Markers.EMPTY, null, Space.EMPTY, TypeTree.build("java.lang.IllegalAccessException"), JContainer.empty(), null, null)))), - Space.EMPTY), - List.of(new J.Try.Catch(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.ControlParentheses(Tree.randomId(), Space.EMPTY, Markers.EMPTY, - JRightPadded.build(new J.VariableDeclarations(Tree.randomId(), Space.EMPTY, Markers.EMPTY, Collections.emptyList(), Collections.emptyList(), TypeTree.build("java.lang.IllegalAccessException"), null, List.of(), List.of(JRightPadded.build( - new J.VariableDeclarations.NamedVariable(Tree.randomId(), Space.EMPTY, Markers.EMPTY, - new J.Identifier(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, Collections.emptyList(), "e", null, null), - List.of(), null, null)))))), - new J.Block(Tree.randomId(), Space.EMPTY, Markers.EMPTY, JRightPadded.build(false), - List.of(JRightPadded.build(new J.Throw(Tree.randomId(), Space.EMPTY, Markers.EMPTY, new J.Empty(Tree.randomId(), Space.EMPTY, Markers.EMPTY)))), - Space.EMPTY))), - null))), - Space.EMPTY), - null))) - , Space.EMPTY); - - assertThat(compilationUnit.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); - assertThat(((J.ClassDeclaration)compilationUnit.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.Try.class); - - Cs.CompilationUnit output = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor().visit(compilationUnit, new InMemoryExecutionContext()); - assertThat(output.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); - assertThat(((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.Throw.class); + rewriteRun( + spec -> spec.recipe(Recipe.noop()), + //language=java + java( + """ + class A { + void foo() throws IllegalAccessException { + try { + throw new IllegalAccessException(); + } catch (Exception e) { + throw e; + } + } + }""" + , spec -> spec.beforeRecipe(compUnit -> { + Cs.CompilationUnit cSharpCompUnit = (Cs.CompilationUnit) new JavaVisitor() { + @Override + public J visitThrow(J.Throw thrown, ExecutionContext executionContext) { + if (thrown.getException() instanceof J.Identifier) { + return thrown.withException(new J.Empty(Tree.randomId(), Space.EMPTY, Markers.EMPTY)); + } + return thrown; + } + }.visit(JavaToCsharp.compilationUnit(compUnit), new InMemoryExecutionContext()); + + assertThat(cSharpCompUnit.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); + assertThat(((J.ClassDeclaration)cSharpCompUnit.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.MethodDeclaration.class); + J.MethodDeclaration md = (J.MethodDeclaration) ((J.ClassDeclaration)cSharpCompUnit.getMembers().get(0)).getBody().getStatements().get(0); + assertThat(md.getBody().getStatements().get(0)).isInstanceOf(J.Try.class); + + Cs.CompilationUnit output = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor().visit(cSharpCompUnit, new InMemoryExecutionContext()); + + assertThat(output.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); + assertThat(((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.MethodDeclaration.class); + md = (J.MethodDeclaration) ((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0); + assertThat(md.getBody().getStatements().get(0)).isInstanceOf(J.Throw.class); + } + ) + ) + ); + } + + public JavaVisitor getJavaToCsharpVisitor() { + return new JavaVisitor() { + + + }; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java new file mode 100644 index 000000000..ba3fd564a --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java @@ -0,0 +1,32 @@ +/* + * Copyright 2024 the original author or authors. + *

+ * 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 + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * 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; +import java.util.stream.Collectors; + +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(cd -> JRightPadded.build(cd)).collect(Collectors.toList()), cu.getEof()); + } +} From bb8d98ae7405d510875d427468ad857d6d595714 Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Wed, 27 Nov 2024 15:18:51 +0100 Subject: [PATCH 05/11] Add comment for clarity --- .../openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index 285effad2..c3cedf6b1 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -344,7 +344,7 @@ void foo() throws IllegalAccessException { try { throw new IllegalAccessException(); } catch (Exception e) { - throw e; + throw e; // C# can rethrow the caught exception implicitly and so the `e` Identifier is removed by the inline visitor below } } }""" From 6a9a2ad9828cc7dd72a2eb6bf0e0b1fd19e24dcd Mon Sep 17 00:00:00 2001 From: Laurens Westerlaken Date: Thu, 28 Nov 2024 09:58:27 +0100 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../staticanalysis/CatchClauseOnlyRethrowsTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index c3cedf6b1..ee40b3999 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -19,7 +19,8 @@ import org.openrewrite.*; import org.openrewrite.csharp.tree.Cs; import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.tree.*; +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; @@ -351,7 +352,7 @@ void foo() throws IllegalAccessException { , spec -> spec.beforeRecipe(compUnit -> { Cs.CompilationUnit cSharpCompUnit = (Cs.CompilationUnit) new JavaVisitor() { @Override - public J visitThrow(J.Throw thrown, ExecutionContext executionContext) { + 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)); } @@ -377,7 +378,7 @@ public J visitThrow(J.Throw thrown, ExecutionContext executionContext) { } public JavaVisitor getJavaToCsharpVisitor() { - return new JavaVisitor() { + return new JavaVisitor<>() { }; From afe2c77c6b97ce289043b0f8d456e71f5e304793 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 28 Nov 2024 12:08:48 +0100 Subject: [PATCH 07/11] Apply suggestions from code review --- .../CatchClauseOnlyRethrowsTest.java | 55 +++++++++---------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index ee40b3999..2d945878c 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -345,42 +345,37 @@ void foo() throws IllegalAccessException { try { throw new IllegalAccessException(); } catch (Exception e) { - throw e; // C# can rethrow the caught exception implicitly and so the `e` Identifier is removed by the inline visitor below + throw e; // `e` is removed below } } - }""" - , spec -> spec.beforeRecipe(compUnit -> { + } + """, + spec -> spec.beforeRecipe(compUnit -> { + // C# can rethrow the caught exception implicitly and so the `e` Identifier is removed by the inline visitor below Cs.CompilationUnit cSharpCompUnit = (Cs.CompilationUnit) new JavaVisitor() { - @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(JavaToCsharp.compilationUnit(compUnit), new InMemoryExecutionContext()); - - assertThat(cSharpCompUnit.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); - assertThat(((J.ClassDeclaration)cSharpCompUnit.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.MethodDeclaration.class); - J.MethodDeclaration md = (J.MethodDeclaration) ((J.ClassDeclaration)cSharpCompUnit.getMembers().get(0)).getBody().getStatements().get(0); - assertThat(md.getBody().getStatements().get(0)).isInstanceOf(J.Try.class); - - Cs.CompilationUnit output = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor().visit(cSharpCompUnit, new InMemoryExecutionContext()); - - assertThat(output.getMembers().get(0)).isInstanceOf(J.ClassDeclaration.class); - assertThat(((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0)).isInstanceOf(J.MethodDeclaration.class); - md = (J.MethodDeclaration) ((J.ClassDeclaration)output.getMembers().get(0)).getBody().getStatements().get(0); - assertThat(md.getBody().getStatements().get(0)).isInstanceOf(J.Throw.class); - } + @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(JavaToCsharp.compilationUnit(compUnit), new InMemoryExecutionContext()); + + Cs.CompilationUnit after = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor() + .visit(cSharpCompUnit, new InMemoryExecutionContext()); + + var classA = (J.ClassDeclaration) after.getMembers().get(0); + var methodFoo = (J.MethodDeclaration) classA.getBody().getStatements().get(0); + var firstStatement = methodFoo.getBody().getStatements().get(0); + assertThat(firstStatement) + .isInstanceOf(J.Throw.class) // The `try/catch` block removed + .extracting(s -> ((J.Throw) s).getException()) + .isInstanceOf(J.NewClass.class); + } ) ) ); } - public JavaVisitor getJavaToCsharpVisitor() { - return new JavaVisitor<>() { - - - }; - } } From 2feec02822f074a938b98ebd952c734c039963e7 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 28 Nov 2024 12:16:31 +0100 Subject: [PATCH 08/11] Use regular before/after text blocks --- .../CatchClauseOnlyRethrowsTest.java | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java index 2d945878c..f34d89657 100755 --- a/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/CatchClauseOnlyRethrowsTest.java @@ -16,7 +16,10 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; -import org.openrewrite.*; +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; @@ -25,8 +28,8 @@ import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; -import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.test.RewriteTest.toRecipe; @SuppressWarnings("ALL") class CatchClauseOnlyRethrowsTest implements RewriteTest { @@ -336,7 +339,24 @@ void foo() throws IOException { @Test void verifyCsharpImplicitThrow() { rewriteRun( - spec -> spec.recipe(Recipe.noop()), + 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() { + @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( """ @@ -350,32 +370,14 @@ void foo() throws IllegalAccessException { } } """, - spec -> spec.beforeRecipe(compUnit -> { - // C# can rethrow the caught exception implicitly and so the `e` Identifier is removed by the inline visitor below - Cs.CompilationUnit cSharpCompUnit = (Cs.CompilationUnit) new JavaVisitor() { - @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(JavaToCsharp.compilationUnit(compUnit), new InMemoryExecutionContext()); - - Cs.CompilationUnit after = (Cs.CompilationUnit) new CatchClauseOnlyRethrows().getVisitor() - .visit(cSharpCompUnit, new InMemoryExecutionContext()); - - var classA = (J.ClassDeclaration) after.getMembers().get(0); - var methodFoo = (J.MethodDeclaration) classA.getBody().getStatements().get(0); - var firstStatement = methodFoo.getBody().getStatements().get(0); - assertThat(firstStatement) - .isInstanceOf(J.Throw.class) // The `try/catch` block removed - .extracting(s -> ((J.Throw) s).getException()) - .isInstanceOf(J.NewClass.class); + """ + class A { + void foo() throws IllegalAccessException { + throw new IllegalAccessException(); + } } - ) + """ ) ); } - } From 8c3d41a4ddb1b7dcc3364996f71a107fa310b8b8 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 28 Nov 2024 12:18:36 +0100 Subject: [PATCH 09/11] Use newlines in JavaToCsharp.compilationUnit for cleaner future diffs --- .../staticanalysis/JavaToCsharp.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java index ba3fd564a..aa3d1949e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java +++ b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java @@ -23,10 +23,27 @@ import java.util.List; import java.util.stream.Collectors; +import static java.util.stream.Collectors.toList; + 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(cd -> JRightPadded.build(cd)).collect(Collectors.toList()), cu.getEof()); + 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) + .collect(toList()), + cu.getEof()); } } From 9f0bb94d913d99eb73129cf745814da59cfaa5be Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 28 Nov 2024 12:25:42 +0100 Subject: [PATCH 10/11] Remove unused import Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java index aa3d1949e..c0821e834 100644 --- a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java +++ b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java @@ -21,8 +21,6 @@ import org.openrewrite.java.tree.Statement; import java.util.List; -import java.util.stream.Collectors; - import static java.util.stream.Collectors.toList; public class JavaToCsharp { From f1a83f40fd1ff9359387f98bb7c4ea6edd9d2b1e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 28 Nov 2024 12:32:39 +0100 Subject: [PATCH 11/11] Use Stream.toList() --- src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java index c0821e834..06b5b6221 100644 --- a/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java +++ b/src/test/java/org/openrewrite/staticanalysis/JavaToCsharp.java @@ -21,7 +21,6 @@ import org.openrewrite.java.tree.Statement; import java.util.List; -import static java.util.stream.Collectors.toList; public class JavaToCsharp { @@ -41,7 +40,7 @@ public static Cs.CompilationUnit compilationUnit(J.CompilationUnit cu) { cu.getClasses().stream() .map(Statement.class::cast) .map(JRightPadded::build) - .collect(toList()), + .toList(), cu.getEof()); } }