From 56a84408ae2dc445e86d133fb6dcedf9107fb6a6 Mon Sep 17 00:00:00 2001 From: Jonathan Leitschuh Date: Fri, 16 Feb 2024 22:22:30 -0500 Subject: [PATCH 1/8] Add Two Recipes - BufferedWriterCreation - SimplifyConstantTernaryExecution Signed-off-by: Jonathan Leitschuh --- .../BufferedWriterCreation.java | 158 +++++++++++++++ .../SimplifyConstantIfBranchExecution.java | 51 +++-- .../SimplifyConstantTernaryExecution.java | 67 +++++++ .../BufferedWriterCreationTest.java | 181 ++++++++++++++++++ .../SimplifyConstantTernaryExecutionTest.java | 79 ++++++++ 5 files changed, 509 insertions(+), 27 deletions(-) create mode 100644 src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java create mode 100644 src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java new file mode 100644 index 000000000..0103906a9 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -0,0 +1,158 @@ +/* + * 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.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.search.UsesMethod; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.template.Semantics; +import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor; +import org.openrewrite.java.tree.J; + +import java.util.Collections; +import java.util.Set; + +import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.SHORTEN_NAMES; + +public class BufferedWriterCreation extends Recipe { + + @Override + public String getDisplayName() { + return "Modernize BufferedWriter creation & prevent file descriptor leak"; + } + + @Override + public String getDescription() { + return "The code `new BufferedWriter(new FileWriter(f))` creates a `BufferedWriter` that does not close the underlying `FileWriter` when it is closed. " + + "This can lead to file descriptor leaks. " + + "Use `Files.newBufferedWriter` to create a `BufferedWriter` that closes the underlying file descriptor when it is closed."; + } + + @Override + public Set getTags() { + return Collections.singleton("CWE-755"); + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { + final JavaTemplate beforeFile = Semantics.expression( + this, + "beforeFile", + (java.io.File f) -> new java.io.BufferedWriter(new java.io.FileWriter(f)) + ).build(); + final JavaTemplate afterFile = Semantics.expression( + this, + "afterFile", + (java.io.File f) -> java.nio.file.Files.newBufferedWriter(f.toPath()) + ).build(); + + final JavaTemplate beforeString = Semantics.expression( + this, + "beforeString", + (String f) -> new java.io.BufferedWriter(new java.io.FileWriter(f)) + ).build(); + final JavaTemplate afterString = Semantics.expression( + this, + "afterString", + (String f) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath()) + ).build(); + + final JavaTemplate beforeFileBoolean = Semantics.expression( + this, + "beforeFileBoolean", + (java.io.File f, Boolean b) -> new java.io.BufferedWriter(new java.io.FileWriter(f, b)) + ).build(); + final JavaTemplate afterFileBoolean = Semantics.expression( + this, + "afterFileBoolean", + (java.io.File f, Boolean b) -> java.nio.file.Files.newBufferedWriter(f.toPath(), b ? java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) + ).build(); + + final JavaTemplate beforeStringBoolean = Semantics.expression( + this, + "beforeStringBoolean", + (String f, Boolean b) -> new java.io.BufferedWriter(new java.io.FileWriter(f, b)) + ).build(); + final JavaTemplate afterStringBoolean = Semantics.expression( + this, + "afterStringBoolean", + (String f, Boolean b) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath(), b ? java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) + ).build(); + + @Override + public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { + JavaTemplate.Matcher matcher; + J j1 = replaceOneArg(elem, ctx, beforeFile, afterFile); + if (j1 != null) return j1; + J j2 = replaceOneArg(elem, ctx, beforeString, afterString); + if (j2 != null) return j2; + J j3 = replaceTwoArg(elem, ctx, beforeFileBoolean, afterFileBoolean); + if (j3 != null) return j3; + J j4 = replaceTwoArg(elem, ctx, beforeStringBoolean, afterStringBoolean); + if (j4 != null) return j4; + return super.visitNewClass(elem, ctx); + } + + @Nullable + private J replaceOneArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { + JavaTemplate.Matcher matcher; + if ((matcher = before.matcher(getCursor())).find()) { + maybeRemoveImport("java.io.FileWriter"); + return embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + } + return null; + } + + @Nullable + private J replaceTwoArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { + JavaTemplate.Matcher matcher; + if ((matcher = before.matcher(getCursor())).find()) { + maybeRemoveImport("java.io.FileWriter"); + J j = embed( + after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0), matcher.parameter(1)), + getCursor(), + ctx, + SHORTEN_NAMES + ); + return (J) new SimplifyConstantTernaryExecution().getVisitor().visitNonNull(j, ctx, getCursor().getParentOrThrow()); + } + return null; + } + + }; + return Preconditions.check( + Preconditions.and( + new UsesType<>("java.io.BufferedWriter", true), + new UsesType<>("java.io.FileWriter", true), + new UsesMethod<>("java.io.BufferedWriter (..)"), + new UsesMethod<>("java.io.FileWriter (..)") + ), + javaVisitor + ); + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index 704a6201f..9a4ca70e1 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -15,10 +15,7 @@ */ package org.openrewrite.staticanalysis; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Recipe; -import org.openrewrite.SourceFile; -import org.openrewrite.TreeVisitor; +import org.openrewrite.*; import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor; @@ -57,7 +54,7 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { if (bl != block) { bl = (J.Block) new RemoveUnneededBlock.RemoveUnneededBlockStatementVisitor() .visitNonNull(bl, ctx, getCursor().getParentOrThrow()); - EmptyBlockStyle style = ((SourceFile) getCursor().firstEnclosingOrThrow(JavaSourceFile.class)) + EmptyBlockStyle style = getCursor().firstEnclosingOrThrow(JavaSourceFile.class) .getStyle(EmptyBlockStyle.class); if (style == null) { style = Checkstyle.emptyBlock(); @@ -68,27 +65,11 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { return bl; } - @SuppressWarnings("unchecked") - private E cleanupBooleanExpression( - E expression, ExecutionContext ctx - ) { - E ex1 = - (E) new UnnecessaryParenthesesVisitor() - .visitNonNull(expression, ctx, getCursor().getParentOrThrow()); - ex1 = (E) new SimplifyBooleanExpressionVisitor() - .visitNonNull(ex1, ctx, getCursor().getParentTreeCursor()); - if (expression == ex1 || isLiteralFalse(ex1) || isLiteralTrue(ex1)) { - return ex1; - } - // Run recursively until no further changes are needed - return cleanupBooleanExpression(ex1, ctx); - } - @Override public J visitIf(J.If if_, ExecutionContext ctx) { J.If if__ = (J.If) super.visitIf(if_, ctx); - J.ControlParentheses cp = cleanupBooleanExpression(if__.getIfCondition(), ctx); + J.ControlParentheses cp = cleanupBooleanExpression(if__.getIfCondition(), getCursor(), ctx); if__ = if__.withIfCondition(cp); // The compile-time constant value of the if condition control parentheses. @@ -146,14 +127,30 @@ public J visitIf(J.If if_, ExecutionContext ctx) { return J.Block.createEmptyBlock(); } } + } - private static boolean isLiteralTrue(@Nullable Expression expression) { - return J.Literal.isLiteralValue(expression, Boolean.TRUE); + @SuppressWarnings("unchecked") + static E cleanupBooleanExpression( + E expression, Cursor c, ExecutionContext ctx + ) { + E ex1 = + (E) new UnnecessaryParenthesesVisitor<>() + .visitNonNull(expression, ctx, c.getParentOrThrow()); + ex1 = (E) new SimplifyBooleanExpressionVisitor() + .visitNonNull(ex1, ctx, c.getParentTreeCursor()); + if (expression == ex1 || isLiteralFalse(ex1) || isLiteralTrue(ex1)) { + return ex1; } + // Run recursively until no further changes are needed + return cleanupBooleanExpression(ex1, c, ctx); + } - private static boolean isLiteralFalse(@Nullable Expression expression) { - return J.Literal.isLiteralValue(expression, Boolean.FALSE); - } + private static boolean isLiteralTrue(@Nullable Expression expression) { + return J.Literal.isLiteralValue(expression, Boolean.TRUE); + } + + private static boolean isLiteralFalse(@Nullable Expression expression) { + return J.Literal.isLiteralValue(expression, Boolean.FALSE); } } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java new file mode 100644 index 000000000..d98e472e6 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java @@ -0,0 +1,67 @@ +/* + * 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.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.Repeat; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; + +import java.time.Duration; + +public class SimplifyConstantTernaryExecution extends Recipe { + @Override + public String getDisplayName() { + return "Simplify constant ternary branch execution"; + } + + @Override + public String getDescription() { + return "Checks for ternary expressions that are always `true` or `false` and simplifies them."; + } + + @Override + public @Nullable Duration getEstimatedEffortPerOccurrence() { + return Duration.ofSeconds(15); + } + + @Override + public TreeVisitor getVisitor() { + JavaVisitor v = new JavaVisitor() { + @Override + public J visitTernary(J.Ternary ternary, ExecutionContext executionContext) { + J.Ternary t = (J.Ternary) super.visitTernary(ternary, executionContext); + Expression condition = + SimplifyConstantIfBranchExecution.cleanupBooleanExpression( + t.getCondition(), + getCursor(), + executionContext + ); + if (J.Literal.isLiteralValue(condition, true)) { + return autoFormat(t.getTruePart(), executionContext); + } else if (J.Literal.isLiteralValue(condition, false)) { + return autoFormat(t.getFalsePart(), executionContext); + } + return t; + } + }; + return Repeat.repeatUntilStable(v); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java new file mode 100644 index 000000000..79fd26fc6 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java @@ -0,0 +1,181 @@ +/* + * 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.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("EmptyTryBlock") +public class BufferedWriterCreationTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new BufferedWriterCreation()); + } + + @Test + @DocumentExample + void bufferedReaderCreation() { + rewriteRun( + java( + """ + import java.io.BufferedWriter; + import java.io.FileWriter; + import java.io.File; + import java.io.IOException; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(File f) throws IOException { + try (BufferedWriter writer = new BufferedWriter(new FileWriter(f))) { + + } + } + } + """, + """ + import java.io.BufferedWriter; + import java.io.File; + import java.io.IOException; + import java.nio.file.Files; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(File f) throws IOException { + try (BufferedWriter writer = Files.newBufferedWriter(f.toPath())) { + + } + } + } + """ + ) + ); + } + + @Test + void bufferedReaderCreationAppend() { + rewriteRun( + java( + """ + import java.io.BufferedWriter; + import java.io.FileWriter; + import java.io.File; + import java.io.IOException; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(File f) throws IOException { + try (BufferedWriter writer = new BufferedWriter(new FileWriter(f, true))) { + + } + } + } + """, + """ + import java.io.BufferedWriter; + import java.io.File; + import java.io.IOException; + import java.nio.file.Files; + import java.nio.file.StandardOpenOption; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(File f) throws IOException { + try (BufferedWriter writer = Files.newBufferedWriter(f.toPath(), StandardOpenOption.APPEND)) { + + } + } + } + """ + ) + ); + } + + @Test + void bufferedReaderStringCreation() { + rewriteRun( + java( + """ + import java.io.BufferedWriter; + import java.io.FileWriter; + import java.io.File; + import java.io.IOException; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(String f) throws IOException { + try (BufferedWriter writer = new BufferedWriter(new FileWriter(f))) { + + } + } + } + """, + """ + import java.io.BufferedWriter; + import java.io.File; + import java.io.IOException; + import java.nio.file.Files; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(String f) throws IOException { + try (BufferedWriter writer = Files.newBufferedWriter(new File(f).toPath())) { + + } + } + } + """ + ) + ); + } + + @Test + void bufferedReaderStringCreationAppend() { + rewriteRun( + java( + """ + import java.io.BufferedWriter; + import java.io.FileWriter; + import java.io.File; + import java.io.IOException; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(String f) throws IOException { + try (BufferedWriter writer = new BufferedWriter(new FileWriter(f, false))) { + + } + } + } + """, + """ + import java.io.BufferedWriter; + import java.io.File; + import java.io.IOException; + import java.nio.file.Files; + import java.nio.file.StandardOpenOption; + + public class BufferedWriterCreationTest { + public void createBufferedWriter(String f) throws IOException { + try (BufferedWriter writer = Files.newBufferedWriter(new File(f).toPath(), StandardOpenOption.CREATE)) { + + } + } + } + """ + ) + ); + } + + +} diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java new file mode 100644 index 000000000..70aa60bd4 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java @@ -0,0 +1,79 @@ +/* + * 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.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("ConstantConditionalExpression") +public class SimplifyConstantTernaryExecutionTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new SimplifyConstantTernaryExecution()); + } + + @Test + @DocumentExample + void simplifyConstantTernaryExecution() { + // Given + rewriteRun( + java( + """ + class Test { + void test() { + System.out.println(true ? "foo" : "bar"); + } + } + """, + """ + class Test { + void test() { + System.out.println("foo"); + } + } + """ + ) + ); + } + + @Test + void simplifyUnnecessaryParenthesesTernary() { + // Given + rewriteRun( + java( + """ + class Test { + void test() { + System.out.println((true) ? "foo" : "bar"); + } + } + """, + """ + class Test { + void test() { + System.out.println("foo"); + } + } + """ + ) + ); + } +} From 8820542d932a035ad1c98d750dcacb595b4a7216 Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Mon, 19 Feb 2024 09:53:19 +0100 Subject: [PATCH 2/8] Apply auto formatting to owning LST element --- .../SimplifyConstantTernaryExecution.java | 20 ++++++++++++++----- .../BufferedWriterCreationTest.java | 2 +- .../SimplifyConstantTernaryExecutionTest.java | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java index d98e472e6..7b5236f60 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java @@ -46,21 +46,31 @@ public String getDescription() { public TreeVisitor getVisitor() { JavaVisitor v = new JavaVisitor() { @Override - public J visitTernary(J.Ternary ternary, ExecutionContext executionContext) { - J.Ternary t = (J.Ternary) super.visitTernary(ternary, executionContext); + public J visitTernary(J.Ternary ternary, ExecutionContext ctx) { + J.Ternary t = (J.Ternary) super.visitTernary(ternary, ctx); Expression condition = SimplifyConstantIfBranchExecution.cleanupBooleanExpression( t.getCondition(), getCursor(), - executionContext + ctx ); if (J.Literal.isLiteralValue(condition, true)) { - return autoFormat(t.getTruePart(), executionContext); + getCursor().getParentTreeCursor().putMessage("AUTO_FORMAT", true); + return t.getTruePart(); } else if (J.Literal.isLiteralValue(condition, false)) { - return autoFormat(t.getFalsePart(), executionContext); + getCursor().getParentTreeCursor().putMessage("AUTO_FORMAT", true); + return t.getFalsePart(); } return t; } + + @Override + public @Nullable J postVisit(J tree, ExecutionContext ctx) { + if (getCursor().pollMessage("AUTO_FORMAT") != null) { + return autoFormat(tree, ctx); + } + return super.postVisit(tree, ctx); + } }; return Repeat.repeatUntilStable(v); } diff --git a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java index 79fd26fc6..5dd7ecb7f 100644 --- a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java @@ -23,7 +23,7 @@ import static org.openrewrite.java.Assertions.java; @SuppressWarnings("EmptyTryBlock") -public class BufferedWriterCreationTest implements RewriteTest { +class BufferedWriterCreationTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java index 70aa60bd4..2922b6550 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java @@ -23,7 +23,7 @@ import static org.openrewrite.java.Assertions.java; @SuppressWarnings("ConstantConditionalExpression") -public class SimplifyConstantTernaryExecutionTest implements RewriteTest { +class SimplifyConstantTernaryExecutionTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { From fc838e830fc3622fb5ca738a723a887490d0d7b4 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 14:25:13 +0100 Subject: [PATCH 3/8] Apply formatter and best practices --- .../BufferedWriterCreation.java | 25 +++++++++++-------- .../SimplifyConstantIfBranchExecution.java | 6 +++-- .../SimplifyConstantTernaryExecution.java | 2 +- .../BufferedWriterCreationTest.java | 6 +++-- .../SimplifyConstantTernaryExecutionTest.java | 4 +-- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java index 0103906a9..109488b57 100644 --- a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -15,11 +15,11 @@ */ package org.openrewrite.staticanalysis; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Preconditions; import org.openrewrite.Recipe; import org.openrewrite.TreeVisitor; -import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesMethod; @@ -101,20 +101,26 @@ public TreeVisitor getVisitor() { @Override public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { - JavaTemplate.Matcher matcher; J j1 = replaceOneArg(elem, ctx, beforeFile, afterFile); - if (j1 != null) return j1; + if (j1 != null) { + return j1; + } J j2 = replaceOneArg(elem, ctx, beforeString, afterString); - if (j2 != null) return j2; + if (j2 != null) { + return j2; + } J j3 = replaceTwoArg(elem, ctx, beforeFileBoolean, afterFileBoolean); - if (j3 != null) return j3; + if (j3 != null) { + return j3; + } J j4 = replaceTwoArg(elem, ctx, beforeStringBoolean, afterStringBoolean); - if (j4 != null) return j4; + if (j4 != null) { + return j4; + } return super.visitNewClass(elem, ctx); } - @Nullable - private J replaceOneArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { + private @Nullable J replaceOneArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { JavaTemplate.Matcher matcher; if ((matcher = before.matcher(getCursor())).find()) { maybeRemoveImport("java.io.FileWriter"); @@ -128,8 +134,7 @@ private J replaceOneArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate befo return null; } - @Nullable - private J replaceTwoArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { + private @Nullable J replaceTwoArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { JavaTemplate.Matcher matcher; if ((matcher = before.matcher(getCursor())).find()) { maybeRemoveImport("java.io.FileWriter"); diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index 7b5dfe0c0..0ab366a84 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -16,7 +16,10 @@ package org.openrewrite.staticanalysis; import org.jspecify.annotations.Nullable; -import org.openrewrite.*; +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.cleanup.SimplifyBooleanExpressionVisitor; import org.openrewrite.java.cleanup.UnnecessaryParenthesesVisitor; @@ -153,5 +156,4 @@ private static boolean isLiteralTrue(@Nullable Expression expression) { private static boolean isLiteralFalse(@Nullable Expression expression) { return J.Literal.isLiteralValue(expression, Boolean.FALSE); } - } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java index 7b5236f60..ba8a58c4d 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java @@ -15,11 +15,11 @@ */ package org.openrewrite.staticanalysis; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; import org.openrewrite.Repeat; import org.openrewrite.TreeVisitor; -import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; diff --git a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java index 5dd7ecb7f..bdace9a97 100644 --- a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java @@ -34,6 +34,7 @@ public void defaults(RecipeSpec spec) { @DocumentExample void bufferedReaderCreation() { rewriteRun( + //language=java java( """ import java.io.BufferedWriter; @@ -70,6 +71,7 @@ public void createBufferedWriter(File f) throws IOException { @Test void bufferedReaderCreationAppend() { rewriteRun( + //language=java java( """ import java.io.BufferedWriter; @@ -107,6 +109,7 @@ public void createBufferedWriter(File f) throws IOException { @Test void bufferedReaderStringCreation() { rewriteRun( + //language=java java( """ import java.io.BufferedWriter; @@ -143,6 +146,7 @@ public void createBufferedWriter(String f) throws IOException { @Test void bufferedReaderStringCreationAppend() { rewriteRun( + //language=java java( """ import java.io.BufferedWriter; @@ -176,6 +180,4 @@ public void createBufferedWriter(String f) throws IOException { ) ); } - - } diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java index 2922b6550..c03e2e03f 100644 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java @@ -33,8 +33,8 @@ public void defaults(RecipeSpec spec) { @Test @DocumentExample void simplifyConstantTernaryExecution() { - // Given rewriteRun( + //language=java java( """ class Test { @@ -56,8 +56,8 @@ void test() { @Test void simplifyUnnecessaryParenthesesTernary() { - // Given rewriteRun( + //language=java java( """ class Test { From 610c1c3db413839dcdff233db289b7c3e76730d9 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 14:37:15 +0100 Subject: [PATCH 4/8] Drop unnecessary UsesType --- .../openrewrite/staticanalysis/BufferedWriterCreation.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java index 109488b57..60a3c4b58 100644 --- a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -23,7 +23,6 @@ import org.openrewrite.java.JavaTemplate; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.search.UsesMethod; -import org.openrewrite.java.search.UsesType; import org.openrewrite.java.template.Semantics; import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor; import org.openrewrite.java.tree.J; @@ -37,7 +36,7 @@ public class BufferedWriterCreation extends Recipe { @Override public String getDisplayName() { - return "Modernize BufferedWriter creation & prevent file descriptor leak"; + return "Modernize `BufferedWriter` creation & prevent file descriptor leak"; } @Override @@ -148,12 +147,10 @@ public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { } return null; } - }; + return Preconditions.check( Preconditions.and( - new UsesType<>("java.io.BufferedWriter", true), - new UsesType<>("java.io.FileWriter", true), new UsesMethod<>("java.io.BufferedWriter (..)"), new UsesMethod<>("java.io.FileWriter (..)") ), From c81972da9539c4b7175166ef67a882d1ee98c185 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 14:42:47 +0100 Subject: [PATCH 5/8] Minor polish --- .../BufferedWriterCreation.java | 6 ++-- .../SimplifyConstantIfBranchExecution.java | 28 ++++++------------- .../SimplifyConstantTernaryExecution.java | 7 +---- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java index 60a3c4b58..f1af593ad 100644 --- a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -84,7 +84,8 @@ public TreeVisitor getVisitor() { final JavaTemplate afterFileBoolean = Semantics.expression( this, "afterFileBoolean", - (java.io.File f, Boolean b) -> java.nio.file.Files.newBufferedWriter(f.toPath(), b ? java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) + (java.io.File f, Boolean b) -> java.nio.file.Files.newBufferedWriter(f.toPath(), b ? + java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) ).build(); final JavaTemplate beforeStringBoolean = Semantics.expression( @@ -95,7 +96,8 @@ public TreeVisitor getVisitor() { final JavaTemplate afterStringBoolean = Semantics.expression( this, "afterStringBoolean", - (String f, Boolean b) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath(), b ? java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) + (String f, Boolean b) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath(), b ? + java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) ).build(); @Override diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index 0ab366a84..70d59066e 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -15,7 +15,6 @@ */ package org.openrewrite.staticanalysis; -import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.ExecutionContext; import org.openrewrite.Recipe; @@ -78,9 +77,9 @@ public J visitIf(J.If if_, ExecutionContext ctx) { // The compile-time constant value of the if condition control parentheses. final Optional compileTimeConstantBoolean; - if (isLiteralTrue(cp.getTree())) { + if (J.Literal.isLiteralValue(cp.getTree(), Boolean.TRUE)) { compileTimeConstantBoolean = Optional.of(true); - } else if (isLiteralFalse(cp.getTree())) { + } else if (J.Literal.isLiteralValue(cp.getTree(), Boolean.FALSE)) { compileTimeConstantBoolean = Optional.of(false); } else { // The condition is not a literal, so we can't simplify it. @@ -134,26 +133,15 @@ public J visitIf(J.If if_, ExecutionContext ctx) { } @SuppressWarnings("unchecked") - static E cleanupBooleanExpression( - E expression, Cursor c, ExecutionContext ctx - ) { - E ex1 = - (E) new UnnecessaryParenthesesVisitor<>() - .visitNonNull(expression, ctx, c.getParentOrThrow()); - ex1 = (E) new SimplifyBooleanExpressionVisitor() - .visitNonNull(ex1, ctx, c.getParentTreeCursor()); - if (expression == ex1 || isLiteralFalse(ex1) || isLiteralTrue(ex1)) { + static E cleanupBooleanExpression(E expression, Cursor c, ExecutionContext ctx) { + E ex1 = (E) new UnnecessaryParenthesesVisitor<>().visitNonNull(expression, ctx, c.getParentOrThrow()); + ex1 = (E) new SimplifyBooleanExpressionVisitor().visitNonNull(ex1, ctx, c.getParentTreeCursor()); + if (expression == ex1 || + J.Literal.isLiteralValue(ex1, Boolean.FALSE) || + J.Literal.isLiteralValue(ex1, Boolean.TRUE)) { return ex1; } // Run recursively until no further changes are needed return cleanupBooleanExpression(ex1, c, ctx); } - - private static boolean isLiteralTrue(@Nullable Expression expression) { - return J.Literal.isLiteralValue(expression, Boolean.TRUE); - } - - private static boolean isLiteralFalse(@Nullable Expression expression) { - return J.Literal.isLiteralValue(expression, Boolean.FALSE); - } } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java index ba8a58c4d..7f1dfe327 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java @@ -48,12 +48,7 @@ public TreeVisitor getVisitor() { @Override public J visitTernary(J.Ternary ternary, ExecutionContext ctx) { J.Ternary t = (J.Ternary) super.visitTernary(ternary, ctx); - Expression condition = - SimplifyConstantIfBranchExecution.cleanupBooleanExpression( - t.getCondition(), - getCursor(), - ctx - ); + Expression condition = SimplifyConstantIfBranchExecution.cleanupBooleanExpression(t.getCondition(), getCursor(), ctx); if (J.Literal.isLiteralValue(condition, true)) { getCursor().getParentTreeCursor().putMessage("AUTO_FORMAT", true); return t.getTruePart(); From ee9fc7b1c16c0d604e67352601e4ead2b433c40c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 15:36:30 +0100 Subject: [PATCH 6/8] Drop now unnecessary SimplifyConstantTernaryExecution Following https://github.com/openrewrite/rewrite/pull/4617 --- .../BufferedWriterCreation.java | 6 +- .../SimplifyConstantTernaryExecution.java | 72 ----------------- .../SimplifyConstantTernaryExecutionTest.java | 79 ------------------- 3 files changed, 3 insertions(+), 154 deletions(-) delete mode 100644 src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java delete mode 100644 src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java index f1af593ad..4085a5929 100644 --- a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -31,6 +31,7 @@ import java.util.Set; import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.SHORTEN_NAMES; +import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.SIMPLIFY_BOOLEANS; public class BufferedWriterCreation extends Recipe { @@ -139,13 +140,12 @@ public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { JavaTemplate.Matcher matcher; if ((matcher = before.matcher(getCursor())).find()) { maybeRemoveImport("java.io.FileWriter"); - J j = embed( + return embed( after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0), matcher.parameter(1)), getCursor(), ctx, - SHORTEN_NAMES + SHORTEN_NAMES, SIMPLIFY_BOOLEANS ); - return (J) new SimplifyConstantTernaryExecution().getVisitor().visitNonNull(j, ctx, getCursor().getParentOrThrow()); } return null; } diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java deleted file mode 100644 index 7f1dfe327..000000000 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecution.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * 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.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Recipe; -import org.openrewrite.Repeat; -import org.openrewrite.TreeVisitor; -import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; - -import java.time.Duration; - -public class SimplifyConstantTernaryExecution extends Recipe { - @Override - public String getDisplayName() { - return "Simplify constant ternary branch execution"; - } - - @Override - public String getDescription() { - return "Checks for ternary expressions that are always `true` or `false` and simplifies them."; - } - - @Override - public @Nullable Duration getEstimatedEffortPerOccurrence() { - return Duration.ofSeconds(15); - } - - @Override - public TreeVisitor getVisitor() { - JavaVisitor v = new JavaVisitor() { - @Override - public J visitTernary(J.Ternary ternary, ExecutionContext ctx) { - J.Ternary t = (J.Ternary) super.visitTernary(ternary, ctx); - Expression condition = SimplifyConstantIfBranchExecution.cleanupBooleanExpression(t.getCondition(), getCursor(), ctx); - if (J.Literal.isLiteralValue(condition, true)) { - getCursor().getParentTreeCursor().putMessage("AUTO_FORMAT", true); - return t.getTruePart(); - } else if (J.Literal.isLiteralValue(condition, false)) { - getCursor().getParentTreeCursor().putMessage("AUTO_FORMAT", true); - return t.getFalsePart(); - } - return t; - } - - @Override - public @Nullable J postVisit(J tree, ExecutionContext ctx) { - if (getCursor().pollMessage("AUTO_FORMAT") != null) { - return autoFormat(tree, ctx); - } - return super.postVisit(tree, ctx); - } - }; - return Repeat.repeatUntilStable(v); - } -} diff --git a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java b/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java deleted file mode 100644 index c03e2e03f..000000000 --- a/src/test/java/org/openrewrite/staticanalysis/SimplifyConstantTernaryExecutionTest.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * 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.junit.jupiter.api.Test; -import org.openrewrite.DocumentExample; -import org.openrewrite.test.RecipeSpec; -import org.openrewrite.test.RewriteTest; - -import static org.openrewrite.java.Assertions.java; - -@SuppressWarnings("ConstantConditionalExpression") -class SimplifyConstantTernaryExecutionTest implements RewriteTest { - - @Override - public void defaults(RecipeSpec spec) { - spec.recipe(new SimplifyConstantTernaryExecution()); - } - - @Test - @DocumentExample - void simplifyConstantTernaryExecution() { - rewriteRun( - //language=java - java( - """ - class Test { - void test() { - System.out.println(true ? "foo" : "bar"); - } - } - """, - """ - class Test { - void test() { - System.out.println("foo"); - } - } - """ - ) - ); - } - - @Test - void simplifyUnnecessaryParenthesesTernary() { - rewriteRun( - //language=java - java( - """ - class Test { - void test() { - System.out.println((true) ? "foo" : "bar"); - } - } - """, - """ - class Test { - void test() { - System.out.println("foo"); - } - } - """ - ) - ); - } -} From 923b38eec22b5eb5815b84c0317b235f0838448c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 15:38:22 +0100 Subject: [PATCH 7/8] Restore private `cleanupBooleanExpression` --- .../SimplifyConstantIfBranchExecution.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java index 70d59066e..d5295f4fb 100644 --- a/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java +++ b/src/main/java/org/openrewrite/staticanalysis/SimplifyConstantIfBranchExecution.java @@ -67,6 +67,19 @@ public J visitBlock(J.Block block, ExecutionContext ctx) { return bl; } + @SuppressWarnings("unchecked") + private static E cleanupBooleanExpression(E expression, Cursor c, ExecutionContext ctx) { + E ex1 = (E) new UnnecessaryParenthesesVisitor<>().visitNonNull(expression, ctx, c.getParentOrThrow()); + ex1 = (E) new SimplifyBooleanExpressionVisitor().visitNonNull(ex1, ctx, c.getParentTreeCursor()); + if (expression == ex1 || + J.Literal.isLiteralValue(ex1, Boolean.FALSE) || + J.Literal.isLiteralValue(ex1, Boolean.TRUE)) { + return ex1; + } + // Run recursively until no further changes are needed + return cleanupBooleanExpression(ex1, c, ctx); + } + @Override public J visitIf(J.If if_, ExecutionContext ctx) { J.If if__ = (J.If) super.visitIf(if_, ctx); @@ -131,17 +144,4 @@ public J visitIf(J.If if_, ExecutionContext ctx) { } } } - - @SuppressWarnings("unchecked") - static E cleanupBooleanExpression(E expression, Cursor c, ExecutionContext ctx) { - E ex1 = (E) new UnnecessaryParenthesesVisitor<>().visitNonNull(expression, ctx, c.getParentOrThrow()); - ex1 = (E) new SimplifyBooleanExpressionVisitor().visitNonNull(ex1, ctx, c.getParentTreeCursor()); - if (expression == ex1 || - J.Literal.isLiteralValue(ex1, Boolean.FALSE) || - J.Literal.isLiteralValue(ex1, Boolean.TRUE)) { - return ex1; - } - // Run recursively until no further changes are needed - return cleanupBooleanExpression(ex1, c, ctx); - } } From a40918b5044da8159fe08df7041f4a22aa4ce970 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Sun, 27 Oct 2024 16:40:20 +0100 Subject: [PATCH 8/8] Convert to regular Refaster recipe Following https://github.com/openrewrite/rewrite-templating/pull/114 --- .../BufferedWriterCreation.java | 200 ++++++------------ .../BufferedWriterCreationTest.java | 2 +- 2 files changed, 70 insertions(+), 132 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java index 4085a5929..6d7552bf9 100644 --- a/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java +++ b/src/main/java/org/openrewrite/staticanalysis/BufferedWriterCreation.java @@ -15,148 +15,86 @@ */ package org.openrewrite.staticanalysis; -import org.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Preconditions; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; -import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.search.UsesMethod; -import org.openrewrite.java.template.Semantics; -import org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor; -import org.openrewrite.java.tree.J; +import com.google.errorprone.refaster.annotation.AfterTemplate; +import com.google.errorprone.refaster.annotation.BeforeTemplate; +import org.openrewrite.java.template.Primitive; +import org.openrewrite.java.template.RecipeDescriptor; -import java.util.Collections; -import java.util.Set; +import java.io.BufferedWriter; +import java.io.IOException; -import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.SHORTEN_NAMES; -import static org.openrewrite.java.template.internal.AbstractRefasterJavaVisitor.EmbeddingOption.SIMPLIFY_BOOLEANS; +@RecipeDescriptor( + name = "Modernize `BufferedWriter` creation & prevent file descriptor leaks", + description = "The code `new BufferedWriter(new FileWriter(f))` creates a `BufferedWriter` that does not close the underlying `FileWriter` when it is closed. " + + "This can lead to file descriptor leaks as per [CWE-755](https://cwe.mitre.org/data/definitions/755.html). " + + "Use `Files.newBufferedWriter` to create a `BufferedWriter` that closes the underlying file descriptor when it is closed." +) +public class BufferedWriterCreation { -public class BufferedWriterCreation extends Recipe { + @RecipeDescriptor( + name = "Convert `new BufferedWriter(new FileWriter(File))` to `Files.newBufferedWriter(Path)`", + description = "Convert `new BufferedWriter(new FileWriter(f))` to `Files.newBufferedWriter(f.toPath())`." + ) + static class BufferedWriterFromNewFileWriterWithFileArgument { + @BeforeTemplate + BufferedWriter before(java.io.File f) throws IOException { + return new BufferedWriter(new java.io.FileWriter(f)); + } - @Override - public String getDisplayName() { - return "Modernize `BufferedWriter` creation & prevent file descriptor leak"; + @AfterTemplate + BufferedWriter after(java.io.File f) throws IOException { + return java.nio.file.Files.newBufferedWriter(f.toPath()); + } } - @Override - public String getDescription() { - return "The code `new BufferedWriter(new FileWriter(f))` creates a `BufferedWriter` that does not close the underlying `FileWriter` when it is closed. " + - "This can lead to file descriptor leaks. " + - "Use `Files.newBufferedWriter` to create a `BufferedWriter` that closes the underlying file descriptor when it is closed."; - } + @RecipeDescriptor( + name = "Convert `new BufferedWriter(new FileWriter(String))` to `Files.newBufferedWriter(Path)`", + description = "Convert `new BufferedWriter(new FileWriter(s))` to `Files.newBufferedWriter(new java.io.File(s).toPath())`." + ) + static class BufferedWriterFromNewFileWriterWithStringArgument { + @BeforeTemplate + BufferedWriter before(String s) throws IOException { + return new BufferedWriter(new java.io.FileWriter(s)); + } - @Override - public Set getTags() { - return Collections.singleton("CWE-755"); + @AfterTemplate + BufferedWriter after(String s) throws IOException { + return java.nio.file.Files.newBufferedWriter(new java.io.File(s).toPath()); + } } - @Override - public TreeVisitor getVisitor() { - JavaVisitor javaVisitor = new AbstractRefasterJavaVisitor() { - final JavaTemplate beforeFile = Semantics.expression( - this, - "beforeFile", - (java.io.File f) -> new java.io.BufferedWriter(new java.io.FileWriter(f)) - ).build(); - final JavaTemplate afterFile = Semantics.expression( - this, - "afterFile", - (java.io.File f) -> java.nio.file.Files.newBufferedWriter(f.toPath()) - ).build(); - - final JavaTemplate beforeString = Semantics.expression( - this, - "beforeString", - (String f) -> new java.io.BufferedWriter(new java.io.FileWriter(f)) - ).build(); - final JavaTemplate afterString = Semantics.expression( - this, - "afterString", - (String f) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath()) - ).build(); - - final JavaTemplate beforeFileBoolean = Semantics.expression( - this, - "beforeFileBoolean", - (java.io.File f, Boolean b) -> new java.io.BufferedWriter(new java.io.FileWriter(f, b)) - ).build(); - final JavaTemplate afterFileBoolean = Semantics.expression( - this, - "afterFileBoolean", - (java.io.File f, Boolean b) -> java.nio.file.Files.newBufferedWriter(f.toPath(), b ? - java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) - ).build(); - - final JavaTemplate beforeStringBoolean = Semantics.expression( - this, - "beforeStringBoolean", - (String f, Boolean b) -> new java.io.BufferedWriter(new java.io.FileWriter(f, b)) - ).build(); - final JavaTemplate afterStringBoolean = Semantics.expression( - this, - "afterStringBoolean", - (String f, Boolean b) -> java.nio.file.Files.newBufferedWriter(new java.io.File(f).toPath(), b ? - java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE) - ).build(); + @RecipeDescriptor( + name = "Convert `new BufferedWriter(new FileWriter(File, boolean))` to `Files.newBufferedWriter(Path, StandardOpenOption)`", + description = "Convert `new BufferedWriter(new FileWriter(f, b))` to `Files.newBufferedWriter(f.toPath(), b ? StandardOpenOption.APPEND : StandardOpenOption.CREATE)`." + ) + static class BufferedWriterFromNewFileWriterWithFileAndBooleanArguments { + @BeforeTemplate + BufferedWriter before(java.io.File f, @Primitive Boolean b) throws IOException { + return new BufferedWriter(new java.io.FileWriter(f, b)); + } - @Override - public J visitNewClass(J.NewClass elem, ExecutionContext ctx) { - J j1 = replaceOneArg(elem, ctx, beforeFile, afterFile); - if (j1 != null) { - return j1; - } - J j2 = replaceOneArg(elem, ctx, beforeString, afterString); - if (j2 != null) { - return j2; - } - J j3 = replaceTwoArg(elem, ctx, beforeFileBoolean, afterFileBoolean); - if (j3 != null) { - return j3; - } - J j4 = replaceTwoArg(elem, ctx, beforeStringBoolean, afterStringBoolean); - if (j4 != null) { - return j4; - } - return super.visitNewClass(elem, ctx); - } - - private @Nullable J replaceOneArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { - JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { - maybeRemoveImport("java.io.FileWriter"); - return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0)), - getCursor(), - ctx, - SHORTEN_NAMES - ); - } - return null; - } + @AfterTemplate + BufferedWriter after(java.io.File f, @Primitive Boolean b) throws IOException { + return java.nio.file.Files.newBufferedWriter(f.toPath(), b ? + java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE); + } + } - private @Nullable J replaceTwoArg(J.NewClass elem, ExecutionContext ctx, JavaTemplate before, JavaTemplate after) { - JavaTemplate.Matcher matcher; - if ((matcher = before.matcher(getCursor())).find()) { - maybeRemoveImport("java.io.FileWriter"); - return embed( - after.apply(getCursor(), elem.getCoordinates().replace(), matcher.parameter(0), matcher.parameter(1)), - getCursor(), - ctx, - SHORTEN_NAMES, SIMPLIFY_BOOLEANS - ); - } - return null; - } - }; + @RecipeDescriptor( + name = "Convert `new BufferedWriter(new FileWriter(String, boolean))` to `Files.newBufferedWriter(Path, StandardOpenOption)`", + description = "Convert `new BufferedWriter(new FileWriter(s, b))` to `Files.newBufferedWriter(new java.io.File(s).toPath(), b ? StandardOpenOption.APPEND : StandardOpenOption.CREATE)`." + ) + static class BufferedWriterFromNewFileWriterWithStringAndBooleanArguments { + @BeforeTemplate + BufferedWriter before(String s, @Primitive Boolean b) throws IOException { + return new BufferedWriter(new java.io.FileWriter(s, b)); + } - return Preconditions.check( - Preconditions.and( - new UsesMethod<>("java.io.BufferedWriter (..)"), - new UsesMethod<>("java.io.FileWriter (..)") - ), - javaVisitor - ); + @AfterTemplate + BufferedWriter after(String s, @Primitive Boolean b) throws IOException { + return java.nio.file.Files.newBufferedWriter(new java.io.File(s).toPath(), b ? + java.nio.file.StandardOpenOption.APPEND : java.nio.file.StandardOpenOption.CREATE); + } } + } diff --git a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java index bdace9a97..b1f560f71 100644 --- a/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/BufferedWriterCreationTest.java @@ -27,7 +27,7 @@ class BufferedWriterCreationTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new BufferedWriterCreation()); + spec.recipe(new BufferedWriterCreationRecipes()); } @Test