diff --git a/src/main/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNested.java b/src/main/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNested.java new file mode 100644 index 0000000000..8ffefb526d --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNested.java @@ -0,0 +1,388 @@ +/* + * Copyright 2022 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 static org.openrewrite.Tree.randomId; +import static org.openrewrite.java.tree.J.Binary.Type.Equal; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.openrewrite.ExecutionContext; +import org.openrewrite.Recipe; +import org.openrewrite.Tree; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.marker.JavaVersion; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.Flag; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JContainer; +import org.openrewrite.java.tree.JRightPadded; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.marker.Markers; + + +public class TernaryOperatorsShouldNotBeNested extends Recipe { + + @Override + public String getDisplayName() { + return "Ternary operators should not be nested"; + } + + @Override + public String getDescription() { + return "Nested ternary operators can be hard to read quickly. Prefer simpler constructs for improved readability. " + + "If supported, this recipe will try to replace nested ternaries with switch expressions."; + } + @Override + public Set getTags() { + return Collections.singleton("RSPEC-3358"); + } + + @Override + public TreeVisitor getVisitor() { + return new JavaIsoVisitor() { + @Override + public J.CompilationUnit visitCompilationUnit( + final J.CompilationUnit cu, + final ExecutionContext executionContext + ) { + if (cu.getMarkers() + .findFirst(JavaVersion.class) + .filter(javaVersion -> javaVersion.getMajorVersion() >= 14) + .isPresent()) { + doAfterVisit(new UseSwitchExpressionVisitor()); + } + doAfterVisit(new UseIfVisitor()); + return cu; + } + }; + } + + private static class UseIfVisitor extends JavaVisitor { + + @Override + public J visitLambda(final J.Lambda lambda, final ExecutionContext executionContext) { + J result = rewriteNestedTernary(lambda); + if (result == lambda) { + return super.visitLambda(lambda, executionContext); + } + doAfterVisit(new RemoveUnneededBlock().getVisitor()); + return autoFormat(lambda.withBody(result.withPrefix(Space.SINGLE_SPACE)), executionContext); + } + + @Override + public J visitReturn(final J.Return retrn, final ExecutionContext executionContext) { + J result = rewriteNestedTernary(retrn); + if (result == retrn) { + return super.visitReturn(retrn, executionContext); + } + doAfterVisit(new RemoveUnneededBlock().getVisitor()); + return autoFormat(result, executionContext); + } + + private Statement rewriteNestedTernary(final Statement parent) { + return findTernary(parent).map(ternary -> { + if (!isNestedTernary(ternary)) { + return parent; + } + J.If iff = ifOf(ternary); + J.Return otherwise = returnOf(ternary.getFalsePart()); + return blockOf(iff, rewriteNestedTernary(otherwise)).withPrefix(parent.getPrefix()); + }).orElse(parent); + } + + + private J.If ifOf(final J.Ternary ternary) { + return new J.If( + Tree.randomId(), + ternary.getPrefix(), + Markers.EMPTY, + new J.ControlParentheses<>(Tree.randomId(), Space.EMPTY, Markers.EMPTY, + JRightPadded.build(ternary.getCondition()) + ).withComments(ternary.getCondition().getComments()), + JRightPadded.build(blockOf(rewriteNestedTernary(returnOf(ternary.getTruePart() + .withComments(ternary.getTruePart().getComments()))))), + null + ); + } + + private static boolean isNestedTernary(final J possibleTernary) { + int result = determineNestingLevels(possibleTernary, 0); + return result > 1; + } + + private static int determineNestingLevels(final J possibleTernary, final int level) { + if (!(possibleTernary instanceof J.Ternary)) { + return level; + } + J.Ternary ternary = (J.Ternary) possibleTernary; + int truePath = determineNestingLevels(ternary.getTruePart(), level + 1); + int falsePath = determineNestingLevels(ternary.getFalsePart(), level + 1); + return Math.max(falsePath, truePath); + } + + private static Optional findTernary(Statement parent) { + J possibleTernary = parent; + if (parent instanceof J.Return) { + possibleTernary = ((J.Return) parent).getExpression(); + } else if (parent instanceof J.Lambda) { + possibleTernary = ((J.Lambda) parent).getBody(); + } + if (possibleTernary instanceof J.Ternary) { + return Optional.of(possibleTernary).map(J.Ternary.class::cast); + } + return Optional.empty(); + } + + } + + + static class UseSwitchExpressionVisitor extends JavaVisitor { + + @Override + public J visitTernary(final J.Ternary ternary, final ExecutionContext executionContext) { + return findConditionIdentifier(ternary).map(switchVar -> { + List nestList = findNestedTernaries(ternary, switchVar); + if (nestList.size() < 2) { + return null; + } + return autoFormat(toSwitch(switchVar, nestList), executionContext); + }).map(J.class::cast) + .orElseGet(() -> super.visitTernary(ternary, executionContext)); + } + + private List findNestedTernaries(final J.Ternary ternary, final J.Identifier switchVar) { + List nestList = new ArrayList<>(); + J.Ternary next = ternary; + while (next.getFalsePart() instanceof J.Ternary) { + if (next.getTruePart() instanceof J.Ternary) { + //as long as we do not use pattern matching, an "and" nested ternary will never work for a switch: + // Example: a equals a and a equals b will never be true + return Collections.emptyList(); + } + J.Ternary nested = (J.Ternary) next.getFalsePart(); + if (!findConditionIdentifier(nested) + .filter(found -> isEqualVariable(switchVar, found)) + .isPresent()) { + return Collections.emptyList(); + } + nestList.add(next); + next = nested; + } + nestList.add(next); + return nestList; + } + + private static boolean isEqualVariable(final J.Identifier switchVar, @Nullable final J found) { + if (!(found instanceof J.Identifier)) { + return false; + } + J.Identifier foundVar = (J.Identifier) found; + return Objects.equals(foundVar.getFieldType(), switchVar.getFieldType()); + } + + private J.SwitchExpression toSwitch(final J.Identifier switchVar, final List nestList) { + J.Ternary last = nestList.get(nestList.size() - 1); + return new J.SwitchExpression( + Tree.randomId(), + Space.SINGLE_SPACE, + Markers.EMPTY, + new J.ControlParentheses<>( + Tree.randomId(), + switchVar.getPrefix().withWhitespace(" "), + switchVar.getMarkers(), + JRightPadded.build(switchVar.withPrefix(Space.EMPTY)) + ), + blockOf(Stream.concat( + nestList.stream().map(ternary -> toCase(switchVar, ternary)), + Stream.of(toDefault(last)) + ).collect(Collectors.toList())) + .withPrefix(Space.SINGLE_SPACE) + ); + } + + private J.Case toCase(final J.Identifier switchVar, final J.Ternary ternary) { + Expression compare; + if (ternary.getCondition() instanceof J.MethodInvocation) { + J.MethodInvocation inv = ((J.MethodInvocation) ternary.getCondition()); + if (isObjectsEquals(inv)) { + maybeRemoveImport("java.util.Objects"); + compare = isVariable(inv.getArguments().get(0)) + ? inv.getArguments().get(1) + : inv.getArguments().get(0); + } else { + compare = isEqualVariable(switchVar, inv.getSelect()) + ? inv.getArguments().get(0) + : inv.getSelect(); + } + } else if (isEqualsBinary(ternary.getCondition())) { + J.Binary bin = ((J.Binary) ternary.getCondition()); + compare = isEqualVariable(switchVar, bin.getLeft()) + ? bin.getRight() + : bin.getLeft(); + } else { + throw new IllegalArgumentException( + "Only J.Binary or J.MethodInvocation are expected as ternary conditions when creating a switch case"); + } + return new J.Case( + Tree.randomId(), + ternary.getPrefix().withWhitespace(" "), + ternary.getMarkers(), + J.Case.Type.Rule, + JContainer.build( + Collections.singletonList(JRightPadded.build(compare.withPrefix(Space.SINGLE_SPACE)) + .withAfter(Space.SINGLE_SPACE)) + ), + JContainer.build(Collections.emptyList()), + JRightPadded.build(ternary.getTruePart()) + ); + } + + private J.Case toDefault(final J.Ternary ternary) { + return new J.Case( + Tree.randomId(), + Space.EMPTY, + ternary.getMarkers(), + J.Case.Type.Rule, + JContainer.build(Collections.singletonList(JRightPadded.build(new J.Identifier( + randomId(), + Space.EMPTY, + Markers.EMPTY, + "default", + null, + null + )).withAfter(Space.SINGLE_SPACE))), + JContainer.build(Collections.emptyList()), + JRightPadded.build(ternary.getFalsePart()) + ); + } + + private Optional findConditionIdentifier(final J.Ternary ternary) { + J.Identifier result = null; + if (ternary.getCondition() instanceof J.MethodInvocation) { + J.MethodInvocation inv = (J.MethodInvocation) ternary.getCondition(); + if (!inv.getSimpleName().equals("equals")) { + return Optional.empty(); + } + if (inv.getArguments().size() == 1) { + J other = null; + if (isVariable(inv.getSelect())) { + result = (J.Identifier) inv.getSelect(); + other = inv.getArguments().get(0); + } + if (inv.getArguments().get(0) instanceof J.Identifier) { + result = (J.Identifier) inv.getArguments().get(0); + other = inv.getSelect(); + } + if (!isConstant(other)) { + return Optional.empty(); + } + } + } else if (isEqualsBinary(ternary.getCondition())) { + J.Binary bin = (J.Binary) ternary.getCondition(); + result = xorVariable(bin.getLeft(), bin.getRight()); + } + return Optional.ofNullable(result); + + } + + @Nullable + private static J.Identifier xorVariable(J first, J second) { + J.Identifier result = null; + if (isVariable(first) && isVariable(second)) { + return null; + } + if (isVariable(first)) { + result = (J.Identifier) first; + } + if (isVariable(second)) { + result = (J.Identifier) second; + } + return result; + } + + private static boolean isVariable(@Nullable J maybeVariable) { + if (maybeVariable == null) { + return false; + } + if (!(maybeVariable instanceof J.Identifier)) { + return false; + } + J.Identifier identifier = (J.Identifier) maybeVariable; + if (identifier.getFieldType() == null) { + return false; + } + return !identifier.getFieldType().hasFlags(Flag.Final) || !identifier.getFieldType().hasFlags(Flag.Static); + } + + private static boolean isConstant(@Nullable J maybeConstant) { + if (maybeConstant == null) { + return false; + } + if (maybeConstant instanceof J.Literal) { + return true; + } + if (!(maybeConstant instanceof J.Identifier)) { + return false; + } + J.Identifier identifier = (J.Identifier) maybeConstant; + if (identifier.getFieldType() == null) { + return false; + } + return !identifier.getFieldType().hasFlags(Flag.Final) || !identifier.getFieldType().hasFlags(Flag.Static); + } + + private static boolean isObjectsEquals(J.MethodInvocation inv) { + if (inv.getSelect() instanceof J.Identifier) { + J.Identifier maybeObjects = (J.Identifier) inv.getSelect(); + boolean isObjects = TypeUtils.isOfClassType(maybeObjects.getType(), "java.util.Objects"); + return isObjects && "equals".equals(inv.getSimpleName()); + } + return false; + } + + private static boolean isEqualsBinary(J maybeEqualsBinary) { + return maybeEqualsBinary instanceof J.Binary && ((J.Binary) maybeEqualsBinary).getOperator().equals(Equal); + } + } + + private static J.Return returnOf(Expression expression) { + return new J.Return(Tree.randomId(), Space.EMPTY, Markers.EMPTY, expression.withPrefix(Space.EMPTY)) + .withComments(expression.getComments()); + } + + private static J.Block blockOf(Statement... statements) { + return blockOf(Arrays.asList(statements)); + } + + private static J.Block blockOf(List statements) { + return J.Block.createEmptyBlock().withStatements(statements); + } + +} diff --git a/src/test/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNestedTest.java b/src/test/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNestedTest.java new file mode 100644 index 0000000000..4192c97325 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/TernaryOperatorsShouldNotBeNestedTest.java @@ -0,0 +1,1364 @@ +/* + * Copyright 2022 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 static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.javaVersion; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ExpectedToFail; +import org.openrewrite.DocumentExample; +import org.openrewrite.Issue; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +class TernaryOperatorsShouldNotBeNestedTest { + + @Nested + class SwitchExpressionNotSupported implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new TernaryOperatorsShouldNotBeNested()).allSources(s -> s.markers(javaVersion(11))); + } + + @Test + @DocumentExample + void doReplaceNestedOrTernaryWithIfFollowedByTernary() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "a" : "b".equals(b) ? "b" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return "a"; + } + return "b".equals(b) ? "b" : "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryPrimitiveCondition() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(int a, int b) { + return a == 1 ? "one" : b == 2 ? "two" : "other"; + } + } + """, + """ + class Test { + public String determineSomething(int a, int b) { + if (a == 1) { + return "one"; + } + return b == 2 ? "two" : "other"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryPrimitiveConditionInverted() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(int a, int b) { + return 1 == a ? "one" : 2 == b ? "two" : "other"; + } + } + """, + """ + class Test { + public String determineSomething(int a, int b) { + if (1 == a) { + return "one"; + } + return 2 == b ? "two" : "other"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryRecursive() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b, String c) { + return "a".equals(a) ? "a" : "b".equals(b) ? "b" : "c".equals(b) ? "c" :"nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b, String c) { + if ("a".equals(a)) { + return "a"; + } + if ("b".equals(b)) { + return "b"; + } + return "c".equals(b) ? "c" : "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedAndTernaryWithIfThenTernary() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "b".equals(b) ? "b" : "a" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return "b".equals(b) ? "b" : "a"; + } + return "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedAndOrTernaryWithIfThenTernaryElseTernary() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b, String c) { + return "a".equals(a) ? "b".equals(b) ? "b" : "a" : "c".equals(c) ? "c" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b, String c) { + if ("a".equals(a)) { + return "b".equals(b) ? "b" : "a"; + } + return "c".equals(c) ? "c" : "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceMultiLevelTernaries() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String letter) { + return "a".equals(letter) ? "a" : + "b".equals(letter) ? "b" : + "c".equals(letter) ? "c" : + letter.contains("d") ? letter.startsWith("d") ? letter.equals("d") ? "equals" : "startsWith" : "contains" : + "e".equals(letter) ? "e" : + "f".equals(letter) ? "f" : + "g".equals(letter) ? "g" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String letter) { + if ("a".equals(letter)) { + return "a"; + } + if ("b".equals(letter)) { + return "b"; + } + if ("c".equals(letter)) { + return "c"; + } + if (letter.contains("d")) { + if (letter.startsWith("d")) { + return letter.equals("d") ? "equals" : "startsWith"; + } + return "contains"; + } + if ("e".equals(letter)) { + return "e"; + } + if ("f".equals(letter)) { + return "f"; + } + return "g".equals(letter) ? "g" : "nope"; + } + } + """ + ) + ); + } + + @ExpectedToFail("Comment `dont forget about c` falls off. It is part of a `before` that is dropped when falsePart is extracted") + @Test + void doReplaceMultiLevelTernariesWithComments() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String letter) { + return "a".equals(letter) ? "a" : //look its a + "b".equals(letter) ? "b" : //b is also here + "c".equals(letter) ? "c" /* dont forget about c */ : + // d is important too + letter.contains("d") ? letter.startsWith("d") ? letter.equals("d") ? "equals" : "startsWith" : "contains" : + "e".equals(letter) ? "e" : //e + "f".equals(letter) ? "f" : //f + "g".equals(letter) ? "g" : "nope"; //and nope if nope + } + } + """, + """ + class Test { + public String determineSomething(String letter) { + if ("a".equals(letter)) { + return "a"; + } + //look its a + if ("b".equals(letter)) { + return "b"; + } + //b is also here + if ("c".equals(letter)) { + return "c"; /* dont forget about c */ + } + // d is important too + if (letter.contains("d")) { + if (letter.startsWith("d")) { + return letter.equals("d") ? "equals" : "startsWith"; + } + return "contains"; + } + if ("e".equals(letter)) { + return "e"; + } + //e + if ("f".equals(letter)) { + return "f"; + }//f + return "g".equals(letter) ? "g" : "nope"; //and nope if nope + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/112") + @ExpectedToFail("only directly returned ternaries are taken into account") + @Test + void doReplaceNestedOrAssignmentTernaryWithIfElse() { + rewriteRun( + //language=java + java( + """ + class Test { + public void doThing(String a, String b) { + String result = "a".equals(a) ? "a" : "b".equals(b) ? "b" : "nope"; + System.out.println(result); + } + } + """, + """ + class Test { + public void doThing(String a, String b) { + String result; + if ("a".equals(a)) { + result = "a"; + } + else { + result = "b".equals(b) ? "b" : "nope"; + } + System.out.println(result); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceNonNestedOrTernaryInStream() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> item.startsWith("a") ? "a" : "nope").collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryInStreamWithIfInBlock() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> item.startsWith("a") ? "a" : item.startsWith("b") ? "b" : "nope").collect(Collectors.toSet()); + } + } + """, + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> { + if (item.startsWith("a")) { + return "a"; + } + return item.startsWith("b") ? "b" : "nope"; + }).collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryInStreamContainingComments() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map( /* look a lambda */ item -> + //look a ternary + item.startsWith("a") ? "a" : item.startsWith("b") ? "b" : "nope" + ).collect(Collectors.toSet()); + } + } + """, + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map( /* look a lambda */ item -> { + //look a ternary + if (item.startsWith("a")) { + return "a"; + } + return item.startsWith("b") ? "b" : "nope"; + } + ).collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + + @Test + void doReplaceNestedOrTernaryContainingNull() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? null : "b".equals(b) ? "b" : null; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return null; + } + return "b".equals(b) ? "b" : null; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "foo" + "bar" : "b".equals(b) ? a + b : b + a; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return "foo" + "bar"; + } + return "b".equals(b) ? a + b : b + a; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingComments() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + //this should be before the if and followed by a new line + + return "a".equals(a) ? "a" : "b".equals(b) ? "b" : "nope"; //this should be behind the ternary + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + //this should be before the if and followed by a new line + + if ("a".equals(a)) { + return "a"; + } + return "b".equals(b) ? "b" : "nope"; //this should be behind the ternary + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingMethodCall() { + //language=java + rewriteRun( + java(""" + class M{ + static String a(){return "a";} + static String b(){return "b";} + static String c(){return "c";} + static String nope(){return "nope";} + } + """), + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? M.a() : "b".equals(b) ? M.b() : M.nope(); + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return M.a(); + } + return "b".equals(b) ? M.b() : M.nope(); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingNewlines() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) + ? null + : "b".equals(b) + ? "b" + : null; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return null; + } + return "b".equals(b) + ? "b" + : null; + } + } + """ + ) + ); + } + } + + @Nested + class SwitchExpressionSupported implements RewriteTest { + + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new TernaryOperatorsShouldNotBeNested()).allSources(s -> s.markers(javaVersion(14))); + } + + @Nested + class ReplaceWithSwitchExpression { + @Test + @DocumentExample + void doReplaceNestedOrTernaryWithSwitchExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "a" : "b".equals(a) ? "b" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> "a"; + case "b" -> "b"; + default -> "nope"; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryPrimitiveCondition() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(int a) { + return a == 1 ? "one" : a == 2 ? "two" : "other"; + } + } + """, + """ + class Test { + public String determineSomething(int a) { + return switch (a) { + case 1 -> "one"; + case 2 -> "two"; + default -> "other"; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryPrimitiveConditionWithConstants() { + rewriteRun( + //language=java + java( + """ + class Test { + private static final int ONE = 1; + private static final int TWO = 2; + public String determineSomething(int a) { + return a == ONE ? "one" : a == TWO ? "two" : "other"; + } + } + """, + """ + class Test { + private static final int ONE = 1; + private static final int TWO = 2; + public String determineSomething(int a) { + return switch (a) { + case ONE -> "one"; + case TWO -> "two"; + default -> "other"; + }; + } + } + """ + ) + ); + } + + + @Test + void doReplaceNestedOrTernaryPrimitiveNotEqualsCondition() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(int a) { + return a >= 3 ? "a lot" : a == 2 ? "two" : "other"; + } + } + """, + """ + class Test { + public String determineSomething(int a) { + if (a >= 3) { + return "a lot"; + } + return a == 2 ? "two" : "other"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryPrimitiveConditionInverted() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(int a) { + return 1 == a ? "one" : 2 == a ? "two" : "other"; + } + } + """, + """ + class Test { + public String determineSomething(int a) { + return switch (a) { + case 1 -> "one"; + case 2 -> "two"; + default -> "other"; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryWithSwitchExpressionInvertedEquals() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return a.equals("a") ? "a" : a.equals("b") ? b : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> "a"; + case "b" -> b; + default -> "nope"; + }; + } + } + """ + ) + ); + } + + @ExpectedToFail("switch(null) is not supported before Java 18. This would break null safety.") + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/157") + @Test + void doReplaceNestedOrTernaryWithSwitchExpressionNullSafeEquals() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + class Test { + public String determineSomething(String a, String b) { + return Objects.equals(a, "a") ? "a" : Objects.equals(a, "b") ? b : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> "a"; + case "b" -> b; + default -> "nope"; + }; + } + } + """ + ) + ); + } + + @ExpectedToFail("switch(null) is not supported before Java 18. This would break null safety.") + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/157") + @Test + void doReplaceNestedOrTernaryWithSwitchExpressionNullSafeEqualsInverted() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + class Test { + public String determineSomething(String a, String b) { + return Objects.equals("a", a) ? "a" : Objects.equals("b", a) ? b : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> "a"; + case "b" -> b; + default -> "nope"; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryRecursive() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a) { + return "a".equals(a) ? "a" : "b".equals(a) ? "b" : "c".equals(a) ? "c" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a) { + return switch (a) { + case "a" -> "a"; + case "b" -> "b"; + case "c" -> "c"; + default -> "nope"; + }; + } + } + """ + ) + ); + } + + + @Test + void doReplaceMultiLevelTernariesWithSwitchExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String letter) { + return "a".equals(letter) ? "a" : //look its a + "b".equals(letter) ? "b" : //b is also here + "c".equals(letter) ? "c" : /* dont forget about c */ + // d is important too + "d".equals(letter) ? "d" : + "e".equals(letter) ? "e" : //e + "f".equals(letter) ? "f" : //f + "g".equals(letter) ? "g" : "nope"; //and nope if nope + } + } + """, + """ + class Test { + public String determineSomething(String letter) { + return switch (letter) { + case "a" -> "a"; //look its a + case "b" -> "b"; //b is also here + case "c" -> "c"; /* dont forget about c */ + // d is important too + case "d" -> "d"; + case "e" -> "e"; //e + case "f" -> "f"; //f + case "g" -> "g"; + default -> "nope"; + }; //and nope if nope + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrAssignmentTernaryWithSwitch() { + rewriteRun( + //language=java + java( + """ + class Test { + public void doThing(String a, String b) { + String result = "a".equals(a) ? "a" : "b".equals(a) ? b : "nope"; + System.out.println(result); + } + } + """, + """ + class Test { + public void doThing(String a, String b) { + String result = switch (a) { + case "a" -> "a"; + case "b" -> b; + default -> "nope"; + }; + System.out.println(result); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryInStreamWithSwitch() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> item.equals("a") ? "a" : item.equals("b") ? "b" : "nope").collect(Collectors.toSet()); + } + } + """, + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> switch (item) { + case "a" -> "a"; + case "b" -> "b"; + default -> "nope"; + }).collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryInStreamContainingComments() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map( /* look a lambda */ item -> + //look a ternary + item.equals("a") ? "a" : item.equals("b") ? "b" : "nope" + ).collect(Collectors.toSet()); + } + } + """, + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map( /* look a lambda */ item -> switch (item) { //look a ternary + case "a" -> "a"; + case "b" -> "b"; + default -> "nope"; + } + ).collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingNull() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a) { + return "a".equals(a) ? null : "b".equals(a) ? "b" : null; + } + } + """, + """ + class Test { + public String determineSomething(String a) { + return switch (a) { + case "a" -> null; + case "b" -> "b"; + default -> null; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingExpression() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "foo" + "bar" : "b".equals(a) ? a + b : b + a; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> "foo" + "bar"; + case "b" -> a + b; + default -> b + a; + }; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingComments() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + //this should be before the if and followed by a new line + + return "a".equals(a) ? "a" : "b".equals(a) ? "b" : "nope"; //this should be behind the ternary + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + //this should be before the if and followed by a new line + + return switch (a) { + case "a" -> "a"; + case "b" -> "b"; + default -> "nope"; + }; //this should be behind the ternary + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryContainingMethodCall() { + //language=java + rewriteRun( + java(""" + class M{ + static String a(){return "a";} + static String b(){return "b";} + static String c(){return "c";} + static String nope(){return "nope";} + } + """), + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? M.a() : "b".equals(a) ? M.b() : M.nope(); + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "a" -> M.a(); + case "b" -> M.b(); + default -> M.nope(); + }; + } + } + """ + ) + ); + } + + @ExpectedToFail("Pattern matching not yet implemented") + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/158") + @Test + void doReplaceNestedOrTernaryInStreamWithPatternMatchingSwitch() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> item.startsWith("a") ? "a" : item.startsWith("b") ? "b" : "nope").collect(Collectors.toSet()); + } + } + """, + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> + switch (item) { + case String st && st.startsWith("a") -> "a"; + case String st && st.startsWith("b") -> "b"; + default -> "nope"; + } + ).collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + @ExpectedToFail("not yet implemented collapsing cases") + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/159") + @Test + void doReplaceNestedOrTernaryContainingNullCollapsingSameCases() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a) { + return "a".equals(a) ? null : "b".equals(a) ? "b" : "c".equals(a) ? "c" : null; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + return switch (a) { + case "b" -> "b"; + case "c" -> "c"; + default -> null; + }; + } + } + """ + ) + ); + } + } + + @Nested + class ReplaceWithIf { + + @Test + void doReplaceNestedOrTernaryRecursiveWithIfsWhenMultipleVariablesAreUsed() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b, String c) { + return "a".equals(a) ? "a" : "b".equals(b) ? "b" : "c".equals(c) ? "c" :"nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b, String c) { + if ("a".equals(a)) { + return "a"; + } + if ("b".equals(b)) { + return "b"; + } + return "c".equals(c) ? "c" : "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedAndTernaryWithIfThenTernary() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return "a".equals(a) ? "b".equals(b) ? "b" : "a" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if ("a".equals(a)) { + return "b".equals(b) ? "b" : "a"; + } + return "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedAndOrTernaryWithIfThenTernaryElseTernary() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b, String c) { + return "a".equals(a) ? "b".equals(b) ? "b" : "a" : "c".equals(c) ? "c" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b, String c) { + if ("a".equals(a)) { + return "b".equals(b) ? "b" : "a"; + } + return "c".equals(c) ? "c" : "nope"; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceNestedOrTernaryWithIfWhenUsingNullSafeEquals() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + class Test { + public String determineSomething(String a, String b) { + return Objects.equals(a, "a") ? "a" : Objects.equals(a, "b") ? b : "nope"; + } + } + """, + """ + import java.util.Objects; + class Test { + public String determineSomething(String a, String b) { + if (Objects.equals(a, "a")) { + return "a"; + } + return Objects.equals(a, "b") ? b : "nope"; + } + } + """ + ) + ); + } + + @Test + void doReplaceNestedOrTernaryWithNonConstantEqualsWithIf() { + rewriteRun( + //language=java + java( + """ + class Test { + public String determineSomething(String a, String b) { + return a.equals("a".toString()) ? "a" : "b".equals(a) ? "b" : "nope"; + } + } + """, + """ + class Test { + public String determineSomething(String a, String b) { + if (a.equals("a".toString())) { + return "a"; + } + return "b".equals(a) ? "b" : "nope"; + } + } + """ + ) + ); + } + + } + + @Nested + class DoNotReplace { + + @Test + void doNotReplaceNonNestedOrTernaryInStreamWithSwitch() { + rewriteRun( + //language=java + java( + """ + import java.util.Set; + import java.util.Arrays; + import java.util.List; + import java.util.stream.Collectors; + class Test { + public Set makeASet() { + List s = Arrays.asList("a","b","c","nope"); + return s.stream().map(item -> item.startsWith("a") ? "a" : "nope").collect(Collectors.toSet()); + } + } + """ + ) + ); + } + + } + + } + + +} \ No newline at end of file