diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernary.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernary.java new file mode 100644 index 0000000000..42136a9753 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernary.java @@ -0,0 +1,149 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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.InMemoryExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.Statement; + +import java.util.Collections; +import java.util.Set; + +public class ReplaceIfElseWithTernary extends Recipe { + @Override + public String getDisplayName() { + return "Replace simple if-else statements with ternary operator"; + } + + @Override + public String getDescription() { + return "Replaces simple `if-else` statements with a ternary operator for improved readability. " + + "Only applies to if-else blocks that contain a single assignment statement in each branch " + + "where both branches assign to the same variable."; + } + + @Override + public Set getTags() { + return Collections.singleton("RSPEC-S3358"); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaVisitor() { + @Override + public J visitIf(J.If if_, ExecutionContext ctx) { + J.If i = (J.If) super.visitIf(if_, ctx); + + // Check if we have an else block + if (i.getElsePart() == null || i.getElsePart().getBody() == null) { + return i; + } + + // Check if both then and else parts are single statements or blocks with single statements + Statement thenStatement = getSingleStatement(i.getThenPart()); + Statement elseStatement = getSingleStatement(i.getElsePart().getBody()); + + if (thenStatement == null || elseStatement == null) { + return i; + } + + // Don't convert if the then part contains another if statement (would create nested ternary) + if (thenStatement instanceof J.If) { + return i; + } + + // Check if both statements are assignments to the same variable + if (!(thenStatement instanceof J.Assignment) || !(elseStatement instanceof J.Assignment)) { + return i; + } + + J.Assignment thenAssign = (J.Assignment) thenStatement; + J.Assignment elseAssign = (J.Assignment) elseStatement; + + // Check if they assign to the same variable + if (!areAssignmentsToSameVariable(thenAssign, elseAssign)) { + return i; + } + + // Don't convert if either assignment already contains a ternary operator + if (containsTernary(thenAssign.getAssignment()) || containsTernary(elseAssign.getAssignment())) { + return i; + } + + // Create ternary expression + JavaTemplate template = JavaTemplate.builder("#{any()} = #{any()} ? #{any()} : #{any()}") + .build(); + + J.Assignment result = template.apply(getCursor(), i.getCoordinates().replace(), + thenAssign.getVariable(), + i.getIfCondition().getTree(), + thenAssign.getAssignment(), + elseAssign.getAssignment()); + + return result; + } + + private Statement getSingleStatement(Statement statement) { + if (statement instanceof J.Block) { + J.Block block = (J.Block) statement; + if (block.getStatements().size() == 1) { + return block.getStatements().get(0); + } + } else { + return statement; + } + return null; + } + + private boolean areAssignmentsToSameVariable(J.Assignment a1, J.Assignment a2) { + // Check if both assignments target the same variable + if (a1.getVariable() instanceof J.Identifier && a2.getVariable() instanceof J.Identifier) { + J.Identifier id1 = (J.Identifier) a1.getVariable(); + J.Identifier id2 = (J.Identifier) a2.getVariable(); + return id1.getSimpleName().equals(id2.getSimpleName()); + } else if (a1.getVariable() instanceof J.FieldAccess && a2.getVariable() instanceof J.FieldAccess) { + J.FieldAccess fa1 = (J.FieldAccess) a1.getVariable(); + J.FieldAccess fa2 = (J.FieldAccess) a2.getVariable(); + return fa1.toString().equals(fa2.toString()); + } + return false; + } + + private boolean containsTernary(J expr) { + if (expr instanceof J.Ternary) { + return true; + } + + // Check if expression contains a ternary operator + boolean[] hasTernary = {false}; + new JavaVisitor() { + @Override + public J visitTernary(J.Ternary ternary, ExecutionContext ctx) { + hasTernary[0] = true; + return ternary; + } + }.visit(expr, new InMemoryExecutionContext()); + + return hasTernary[0]; + } + }; + } +} diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index b173de112d..2ff9cabbef 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -360,44 +360,44 @@ examples: - description: '' sources: - before: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { + void foo() throws IOException { try { - throw new IllegalAccessException(); - } catch (Exception e) { - throw e; // `e` is removed below + new FileReader("").read(); + } catch (IOException e) { + throw e; } } } after: | + import java.io.FileReader; + import java.io.IOException; + class A { - void foo() throws IllegalAccessException { - throw new IllegalAccessException(); + void foo() throws IOException { + new FileReader("").read(); } } language: java - description: '' sources: - before: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { + void foo() throws IllegalAccessException { try { - new FileReader("").read(); - } catch (IOException e) { - throw e; + throw new IllegalAccessException(); + } catch (Exception e) { + throw e; // `e` is removed below } } } after: | - import java.io.FileReader; - import java.io.IOException; - class A { - void foo() throws IOException { - new FileReader("").read(); + void foo() throws IllegalAccessException { + throw new IllegalAccessException(); } } language: java @@ -2666,6 +2666,31 @@ examples: language: java --- type: specs.openrewrite.org/v1beta/example +recipeName: org.openrewrite.staticanalysis.ReplaceIfElseWithTernary +examples: +- description: '' + sources: + - before: | + class Test { + void method(int age) { + String category; + if (age >= 18) { + category = "Adult"; + } else { + category = "Minor"; + } + } + } + after: | + class Test { + void method(int age) { + String category; + category = age >= 18 ? "Adult" : "Minor"; + } + } + language: java +--- +type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference examples: - description: '' @@ -2938,18 +2963,6 @@ examples: type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanExpression examples: -- description: '' - sources: - - before: | - fun getSymbol() : String? { - return null - } - language: kotlin - - before: | - val isPositive = getSymbol().equals("+") == true - after: | - val isPositive = getSymbol().equals("+") - language: kotlin - description: '' sources: - before: | @@ -2969,6 +2982,18 @@ examples: } } language: java +- description: '' + sources: + - before: | + fun getSymbol() : String? { + return null + } + language: kotlin + - before: | + val isPositive = getSymbol().equals("+") == true + after: | + val isPositive = getSymbol().equals("+") + language: kotlin --- type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.staticanalysis.SimplifyBooleanExpressionWithDeMorgan diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernaryTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernaryTest.java new file mode 100644 index 0000000000..3248b5da3e --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceIfElseWithTernaryTest.java @@ -0,0 +1,291 @@ +/* + * Copyright 2025 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * 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.Issue; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +@SuppressWarnings("ConstantConditions") +class ReplaceIfElseWithTernaryTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceIfElseWithTernary()); + } + + @DocumentExample + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/683") + @Test + void simpleIfElseWithAssignment() { + rewriteRun( + java( + """ + class Test { + void method(int age) { + String category; + if (age >= 18) { + category = "Adult"; + } else { + category = "Minor"; + } + } + } + """, + """ + class Test { + void method(int age) { + String category; + category = age >= 18 ? "Adult" : "Minor"; + } + } + """ + ) + ); + } + + @Test + void ifElseWithoutBraces() { + rewriteRun( + java( + """ + class Test { + void method(boolean condition, String s1, String s2) { + String result; + if (condition) + result = s1; + else + result = s2; + } + } + """, + """ + class Test { + void method(boolean condition, String s1, String s2) { + String result; + result = condition ? s1 : s2; + } + } + """ + ) + ); + } + + @Test + void ifElseWithFieldAssignment() { + rewriteRun( + java( + """ + class Test { + String field; + + void method(boolean flag) { + if (flag) { + this.field = "true"; + } else { + this.field = "false"; + } + } + } + """, + """ + class Test { + String field; + + void method(boolean flag) { + this.field = flag ? "true" : "false"; + } + } + """ + ) + ); + } + + @Test + void ifElseWithNumericAssignment() { + rewriteRun( + java( + """ + class Test { + void method(boolean premium) { + double discount; + if (premium) { + discount = 0.2; + } else { + discount = 0.05; + } + } + } + """, + """ + class Test { + void method(boolean premium) { + double discount; + discount = premium ? 0.2 : 0.05; + } + } + """ + ) + ); + } + + @Test + void ifElseWithComplexCondition() { + rewriteRun( + java( + """ + class Test { + void method(int x, int y) { + String result; + if (x > 10 && y < 5) { + result = "A"; + } else { + result = "B"; + } + } + } + """, + """ + class Test { + void method(int x, int y) { + String result; + result = x > 10 && y < 5 ? "A" : "B"; + } + } + """ + ) + ); + } + + @Test + void doNotChangeIfNoElse() { + rewriteRun( + java( + """ + class Test { + void method(boolean condition) { + String result = "default"; + if (condition) { + result = "changed"; + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeIfMultipleStatements() { + rewriteRun( + java( + """ + class Test { + void method(boolean condition) { + String result; + if (condition) { + System.out.println("True branch"); + result = "true"; + } else { + result = "false"; + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeIfDifferentVariables() { + rewriteRun( + java( + """ + class Test { + void method(boolean condition) { + String result1; + String result2; + if (condition) { + result1 = "true"; + } else { + result2 = "false"; + } + } + } + """ + ) + ); + } + + @Test + void doNotChangeIfNotAssignment() { + rewriteRun( + java( + """ + class Test { + void method(boolean condition) { + if (condition) { + System.out.println("true"); + } else { + System.out.println("false"); + } + } + } + """ + ) + ); + } + + @Test + void convertNestedIfElseButAvoidNestedTernary() { + rewriteRun( + java( + """ + class Test { + void method(boolean c1, boolean c2) { + String result; + if (c1) { + if (c2) { + result = "A"; + } else { + result = "B"; + } + } else { + result = "C"; + } + } + } + """, + """ + class Test { + void method(boolean c1, boolean c2) { + String result; + if (c1) { + result = c2 ? "A" : "B"; + } else { + result = "C"; + } + } + } + """ + ) + ); + } +}