Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BufferedWriterCreation recipe #258

Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* 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.ExecutionContext;
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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<String> getTags() {
return Collections.singleton("CWE-755");
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
knutwannheden marked this conversation as resolved.
Show resolved Hide resolved
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;
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
J j4 = replaceTwoArg(elem, ctx, beforeStringBoolean, afterStringBoolean);
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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) {
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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 <constructor>(..)"),
new UsesMethod<>("java.io.FileWriter <constructor>(..)")
),
javaVisitor
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -68,27 +65,11 @@ public J visitBlock(J.Block block, ExecutionContext ctx) {
return bl;
}

@SuppressWarnings("unchecked")
private <E extends Expression> 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<Expression> cp = cleanupBooleanExpression(if__.getIfCondition(), ctx);
J.ControlParentheses<Expression> cp = cleanupBooleanExpression(if__.getIfCondition(), getCursor(), ctx);
if__ = if__.withIfCondition(cp);

// The compile-time constant value of the if condition control parentheses.
Expand Down Expand Up @@ -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 extends Expression> 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);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* 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.ExecutionContext;
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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.";
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public @Nullable Duration getEstimatedEffortPerOccurrence() {
return Duration.ofSeconds(15);
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
JavaVisitor<ExecutionContext> v = new JavaVisitor<ExecutionContext>() {
@Override
public J visitTernary(J.Ternary ternary, ExecutionContext executionContext) {
J.Ternary t = (J.Ternary) super.visitTernary(ternary, executionContext);
Expression condition =
SimplifyConstantIfBranchExecution.cleanupBooleanExpression(
t.getCondition(),
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
timtebeek marked this conversation as resolved.
Show resolved Hide resolved
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);
}
}
Loading
Loading