From 4ee504d0046b17490d2cfcf217f6baf53dd5cacb Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 18 May 2018 16:39:22 +0700 Subject: [PATCH] Boolean simplification inspections improved IDEA-192236 Support opposite expression pairs like `x == null` and `x != null` in boolean expression simplifiers IDEA-192161 Support "a && b || a" case in "Simplifiable boolean expression" inspection --- ...nExpressionMayBeConditionalInspection.java | 17 +-- ...mplifiableBooleanExpressionInspection.java | 124 ++++++++++-------- ...fiableConditionalExpressionInspection.java | 27 ++-- .../src/com/siyeh/ig/psiutils/BoolUtils.java | 42 ++++++ .../com/siyeh/ig/psiutils/CommentTracker.java | 33 +++++ .../simple_condition/Binary.after.java | 5 + .../igfixes/simple_condition/Binary.java | 5 + ...plifiableConditionalExpressionFixTest.java | 4 + ...leanExpressionMayBeConditionalFixTest.java | 14 ++ .../SimplifiableBooleanExpressionFixTest.java | 52 ++++++++ 10 files changed, 240 insertions(+), 83 deletions(-) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.after.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/BooleanExpressionMayBeConditionalInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/BooleanExpressionMayBeConditionalInspection.java index 90b748007eea..bf80719090ce 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/BooleanExpressionMayBeConditionalInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/BooleanExpressionMayBeConditionalInspection.java @@ -28,7 +28,10 @@ import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; -import com.siyeh.ig.psiutils.*; +import com.siyeh.ig.psiutils.BoolUtils; +import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.SideEffectChecker; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -133,17 +136,7 @@ public class BooleanExpressionMayBeConditionalInspection extends BaseInspection } final PsiExpression expression1 = ParenthesesUtils.stripParentheses(lBinaryExpression.getLOperand()); final PsiExpression expression2 = ParenthesesUtils.stripParentheses(rBinaryExpression.getLOperand()); - if (expression1 == null || expression2 == null || - ParenthesesUtils.stripParentheses(lBinaryExpression.getROperand()) == null || - ParenthesesUtils.stripParentheses(rBinaryExpression.getROperand()) == null) { - return; - } - if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(BoolUtils.getNegated(expression1), expression2) && - !SideEffectChecker.mayHaveSideEffects(expression2)) { - registerError(expression); - } - else if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(expression1, BoolUtils.getNegated(expression2)) && - !SideEffectChecker.mayHaveSideEffects(expression1)) { + if (BoolUtils.areExpressionsOpposite(expression1, expression2) && !SideEffectChecker.mayHaveSideEffects(expression1)) { registerError(expression); } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableBooleanExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableBooleanExpressionInspection.java index 290c3445a386..c0fafd957da7 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableBooleanExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableBooleanExpressionInspection.java @@ -19,21 +19,22 @@ import com.intellij.codeInspection.CleanupLocalInspectionTool; import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.project.Project; import com.intellij.psi.*; -import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; -import com.siyeh.ig.psiutils.BoolUtils; -import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.EquivalenceChecker; -import com.siyeh.ig.psiutils.ParenthesesUtils; +import com.siyeh.ig.psiutils.*; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Arrays; + +import static com.intellij.util.ObjectUtils.tryCast; + /** * @author Bas Leijdekkers */ @@ -118,22 +119,42 @@ public class SimplifiableBooleanExpressionInspection extends BaseInspection impl @NonNls static String calculateReplacementExpression(PsiBinaryExpression expression, CommentTracker commentTracker) { - final PsiExpression rhs1 = ParenthesesUtils.stripParentheses(expression.getROperand()); - if (rhs1 == null) { - return null; - } - final PsiExpression lhs = ParenthesesUtils.stripParentheses(expression.getLOperand()); - if (!(lhs instanceof PsiBinaryExpression)) { - return null; - } - final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)lhs; - final PsiExpression rhs2 = binaryExpression.getROperand(); - if (rhs2 == null) { - return null; - } - return ParenthesesUtils.getText(commentTracker.markUnchanged(rhs1), ParenthesesUtils.OR_PRECEDENCE) + "||" + - ParenthesesUtils.getText(commentTracker.markUnchanged(rhs2), ParenthesesUtils.OR_PRECEDENCE); + PsiPolyadicExpression conjunction = + tryCast(PsiUtil.skipParenthesizedExprDown(expression.getLOperand()), PsiPolyadicExpression.class); + if (conjunction == null) return null; + final PsiExpression rightDisjunct = ParenthesesUtils.stripParentheses(expression.getROperand()); + if (rightDisjunct == null) return null; + if (hasOperand(conjunction, rightDisjunct)) return commentTracker.text(rightDisjunct); + PsiExpression[] operands = conjunction.getOperands(); + boolean isFirst; + if (BoolUtils.areExpressionsOpposite(operands[0], rightDisjunct)) { + isFirst = true; + } + else if (BoolUtils.areExpressionsOpposite(operands[operands.length - 1], rightDisjunct)) { + isFirst = false; + } + else { + return null; + } + String conjunctionRemnant; + if (operands.length == 2) { + conjunctionRemnant = commentTracker.text(operands[isFirst ? 1 : 0], ParenthesesUtils.OR_PRECEDENCE); + } + else { + if (isFirst) { + conjunctionRemnant = commentTracker.rangeText(operands[1], operands[operands.length - 1]); + } + else { + conjunctionRemnant = commentTracker.rangeText(operands[0], operands[operands.length - 2]); + } + if (expression.getLOperand() instanceof PsiParenthesizedExpression) { + conjunctionRemnant = "(" + conjunctionRemnant + ")"; + } + } + return isFirst ? + commentTracker.text(rightDisjunct, ParenthesesUtils.OR_PRECEDENCE) + "||" + conjunctionRemnant : + conjunctionRemnant + "||" + commentTracker.text(rightDisjunct, ParenthesesUtils.OR_PRECEDENCE); } @Override @@ -146,21 +167,13 @@ public class SimplifiableBooleanExpressionInspection extends BaseInspection impl @Override public void visitPrefixExpression(PsiPrefixExpression expression) { super.visitPrefixExpression(expression); - final IElementType tokenType = expression.getOperationTokenType(); - if (!JavaTokenType.EXCL.equals(tokenType)) { - return; - } - final PsiExpression operand = ParenthesesUtils.stripParentheses(expression.getOperand()); - if (!(operand instanceof PsiBinaryExpression)) { - return; - } - final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)operand; - final IElementType binaryTokenType = binaryExpression.getOperationTokenType(); - if (!JavaTokenType.XOR.equals(binaryTokenType)) { - return; - } - final PsiExpression lhs = ParenthesesUtils.stripParentheses(binaryExpression.getLOperand()); - final PsiExpression rhs = ParenthesesUtils.stripParentheses(binaryExpression.getROperand()); + if (!JavaTokenType.EXCL.equals(expression.getOperationTokenType())) return; + + PsiBinaryExpression maybeXor = tryCast(PsiUtil.skipParenthesizedExprDown(expression.getOperand()), PsiBinaryExpression.class); + if (maybeXor == null || !JavaTokenType.XOR.equals(maybeXor.getOperationTokenType())) return; + + final PsiExpression lhs = ParenthesesUtils.stripParentheses(maybeXor.getLOperand()); + final PsiExpression rhs = ParenthesesUtils.stripParentheses(maybeXor.getROperand()); if (lhs == null || rhs == null) { return; } @@ -168,28 +181,29 @@ public class SimplifiableBooleanExpressionInspection extends BaseInspection impl } @Override - public void visitBinaryExpression(PsiBinaryExpression expression) { - super.visitBinaryExpression(expression); - final IElementType tokenType1 = expression.getOperationTokenType(); - if (!JavaTokenType.OROR.equals(tokenType1)) { - return; + public void visitBinaryExpression(PsiBinaryExpression disjunction) { + super.visitBinaryExpression(disjunction); + if (!JavaTokenType.OROR.equals(disjunction.getOperationTokenType())) return; + PsiPolyadicExpression conjunction = + tryCast(PsiUtil.skipParenthesizedExprDown(disjunction.getLOperand()), PsiPolyadicExpression.class); + if (conjunction == null || !JavaTokenType.ANDAND.equals(conjunction.getOperationTokenType())) return; + + final PsiExpression rightDisjunct = ParenthesesUtils.stripParentheses(disjunction.getROperand()); + if (hasOperand(conjunction, rightDisjunct) && !SideEffectChecker.mayHaveSideEffects(conjunction)) { + registerError(disjunction, disjunction); } - final PsiExpression lhs1 = ParenthesesUtils.stripParentheses(expression.getLOperand()); - if (!(lhs1 instanceof PsiBinaryExpression)) { - return; + PsiExpression[] operands = conjunction.getOperands(); + if ((BoolUtils.areExpressionsOpposite(operands[0], rightDisjunct) || + BoolUtils.areExpressionsOpposite(operands[operands.length - 1], rightDisjunct)) && + !SideEffectChecker.mayHaveSideEffects(rightDisjunct)) { + registerError(disjunction, disjunction); } - final PsiBinaryExpression binaryExpression = (PsiBinaryExpression)lhs1; - final IElementType tokenType2 = binaryExpression.getOperationTokenType(); - if (!JavaTokenType.ANDAND.equals(tokenType2)) { - return; - } - final PsiExpression lhs2 = ParenthesesUtils.stripParentheses(binaryExpression.getLOperand()); - final PsiExpression rhs1 = ParenthesesUtils.stripParentheses(expression.getROperand()); - final PsiExpression negated = BoolUtils.getNegated(rhs1); - if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(lhs2, negated)) { - return; - } - registerError(expression, expression); } } + + private static boolean hasOperand(PsiPolyadicExpression polyadic, PsiExpression operand) { + if (operand == null) return false; + EquivalenceChecker equivalence = EquivalenceChecker.getCanonicalPsiEquivalence(); + return Arrays.stream(polyadic.getOperands()).anyMatch(op -> equivalence.expressionsAreEquivalent(op, operand)); + } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableConditionalExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableConditionalExpressionInspection.java index 16e4e2843cc5..858a888c1ac3 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableConditionalExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/SimplifiableConditionalExpressionInspection.java @@ -21,6 +21,7 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.PsiConditionalExpression; import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiType; +import com.intellij.psi.util.PsiPrecedenceUtil; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; @@ -28,7 +29,6 @@ import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.BoolUtils; import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.EquivalenceChecker; import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -88,13 +88,14 @@ public class SimplifiableConditionalExpressionInspection extends BaseInspection final PsiExpression condition = expression.getCondition(); assert thenExpression != null; assert elseExpression != null; - if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(BoolUtils.getNegated(thenExpression), elseExpression)) { - return ParenthesesUtils.getText(tracker.markUnchanged(condition), ParenthesesUtils.EQUALITY_PRECEDENCE) + " != " + - BoolUtils.getNegatedExpressionText(thenExpression, ParenthesesUtils.EQUALITY_PRECEDENCE, tracker); - } - else if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, BoolUtils.getNegated(elseExpression))) { - return ParenthesesUtils.getText(tracker.markUnchanged(condition), ParenthesesUtils.EQUALITY_PRECEDENCE) + " == " + - ParenthesesUtils.getText(tracker.markUnchanged(thenExpression), ParenthesesUtils.EQUALITY_PRECEDENCE); + if (BoolUtils.areExpressionsOpposite(thenExpression, elseExpression)) { + if (BoolUtils.isNegation(thenExpression)) { + return ParenthesesUtils.getText(tracker.markUnchanged(condition), PsiPrecedenceUtil.RELATIONAL_PRECEDENCE) + " != " + + ParenthesesUtils.getText(tracker.markUnchanged(elseExpression), PsiPrecedenceUtil.RELATIONAL_PRECEDENCE); + } else { + return ParenthesesUtils.getText(tracker.markUnchanged(condition), PsiPrecedenceUtil.RELATIONAL_PRECEDENCE) + " == " + + ParenthesesUtils.getText(tracker.markUnchanged(thenExpression), PsiPrecedenceUtil.RELATIONAL_PRECEDENCE); + } } if (BoolUtils.isTrue(thenExpression)) { final String elseExpressionText = ParenthesesUtils.getText(tracker.markUnchanged(elseExpression), ParenthesesUtils.OR_PRECEDENCE); @@ -137,15 +138,9 @@ public class SimplifiableConditionalExpressionInspection extends BaseInspection } final boolean thenConstant = BoolUtils.isFalse(thenExpression) || BoolUtils.isTrue(thenExpression); final boolean elseConstant = BoolUtils.isFalse(elseExpression) || BoolUtils.isTrue(elseExpression); - if (thenConstant == elseConstant) { - if (EquivalenceChecker.getCanonicalPsiEquivalence() - .expressionsAreEquivalent(BoolUtils.getNegated(thenExpression), elseExpression) || - EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(thenExpression, BoolUtils.getNegated(elseExpression))) { - registerError(expression, expression); - } - return; + if (thenConstant != elseConstant || BoolUtils.areExpressionsOpposite(thenExpression, elseExpression)) { + registerError(expression, expression); } - registerError(expression, expression); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/BoolUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/BoolUtils.java index a544a8937500..d7d5288bbef1 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/BoolUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/BoolUtils.java @@ -15,9 +15,11 @@ */ package com.siyeh.ig.psiutils; +import com.intellij.codeInspection.dataFlow.value.DfaRelationValue; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiUtil; import com.intellij.util.Function; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NonNls; @@ -203,4 +205,44 @@ public class BoolUtils { } return PsiKeyword.FALSE.equals(expression.getText()); } + + /** + * Checks whether two supplied boolean expressions are opposite to each other (e.g. "a == null" and "a != null") + * + * @param expression1 first expression + * @param expression2 second expression + * @return true if it's determined that the expressions are opposite to each other. + */ + @Contract(value = "null, _ -> false; _, null -> false", pure = true) + public static boolean areExpressionsOpposite(@Nullable PsiExpression expression1, @Nullable PsiExpression expression2) { + expression1 = PsiUtil.skipParenthesizedExprDown(expression1); + expression2 = PsiUtil.skipParenthesizedExprDown(expression2); + if (expression1 == null || expression2 == null) return false; + EquivalenceChecker equivalence = EquivalenceChecker.getCanonicalPsiEquivalence(); + if (isNegation(expression1)) { + return equivalence.expressionsAreEquivalent(getNegated(expression1), expression2); + } + if (isNegation(expression2)) { + return equivalence.expressionsAreEquivalent(getNegated(expression2), expression1); + } + if (expression1 instanceof PsiBinaryExpression && expression2 instanceof PsiBinaryExpression) { + PsiBinaryExpression binOp1 = (PsiBinaryExpression)expression1; + PsiBinaryExpression binOp2 = (PsiBinaryExpression)expression2; + DfaRelationValue.RelationType rel1 = DfaRelationValue.RelationType.fromElementType(binOp1.getOperationTokenType()); + DfaRelationValue.RelationType rel2 = DfaRelationValue.RelationType.fromElementType(binOp2.getOperationTokenType()); + if (rel1 == null || rel2 == null) return false; + PsiType type = binOp1.getLOperand().getType(); + // a > b and a <= b are not strictly opposite due to NaN semantics + if (type == null || type.equals(PsiType.FLOAT) || type.equals(PsiType.DOUBLE)) return false; + if (rel1 == rel2.getNegated()) { + return equivalence.expressionsAreEquivalent(binOp1.getLOperand(), binOp2.getLOperand()) && + equivalence.expressionsAreEquivalent(binOp1.getROperand(), binOp2.getROperand()); + } + if (rel1.getFlipped() == rel2.getNegated()) { + return equivalence.expressionsAreEquivalent(binOp1.getLOperand(), binOp2.getROperand()) && + equivalence.expressionsAreEquivalent(binOp1.getROperand(), binOp2.getLOperand()); + } + } + return false; + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java index b06b5c585a66..a0c64620255d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/CommentTracker.java @@ -79,6 +79,39 @@ public class CommentTracker { return element; } + /** + * Marks the range of elements as unchanged and returns their text. The unchanged elements are assumed to be preserved + * in the resulting code as is, so the comments from them will not be extracted. + * + * @param firstElement first element to mark + * @param lastElement last element to mark (must be equal to firstElement or its sibling) + * @return a text to be inserted into refactored code + * @throws IllegalArgumentException if firstElement and lastElements are not siblings or firstElement goes after last element + */ + public String rangeText(@NotNull PsiElement firstElement, @NotNull PsiElement lastElement) { + checkState(); + PsiElement e; + StringBuilder result = new StringBuilder(); + for (e = firstElement; e != null && e != lastElement; e = e.getNextSibling()) { + addIgnored(e); + result.append(e.getText()); + } + if (e == null) { + throw new IllegalArgumentException("Elements must be siblings: " + firstElement + " and " + lastElement); + } + addIgnored(lastElement); + result.append(lastElement.getText()); + return result.toString(); + } + + /** + * Marks the range of elements as unchanged. The unchanged elements are assumed to be preserved + * in the resulting code as is, so the comments from them will not be extracted. + * + * @param firstElement first element to mark + * @param lastElement last element to mark (must be equal to firstElement or its sibling) + * @throws IllegalArgumentException if firstElement and lastElements are not siblings or firstElement goes after last element + */ public void markRangeUnchanged(@NotNull PsiElement firstElement, @NotNull PsiElement lastElement) { checkState(); PsiElement e; diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.after.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.after.java new file mode 100644 index 000000000000..eeda4bd678f2 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.after.java @@ -0,0 +1,5 @@ +class D { + void f(int x, int y, boolean b) { + final boolean sss = b == (x > y); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.java b/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.java new file mode 100644 index 000000000000..0fd8c4596499 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igfixes/simple_condition/Binary.java @@ -0,0 +1,5 @@ +class D { + void f(int x, int y, boolean b) { + final boolean sss = b ? x > y : x <= y; + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/SimplifiableConditionalExpressionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/SimplifiableConditionalExpressionFixTest.java index c8978f09fffe..209222c39572 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/SimplifiableConditionalExpressionFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/SimplifiableConditionalExpressionFixTest.java @@ -36,4 +36,8 @@ public class SimplifiableConditionalExpressionFixTest extends IGQuickFixesTestCa public void testInverted() { doTest(); } + + public void testBinary() { + doTest(); + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/BooleanExpressionMayBeConditionalFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/BooleanExpressionMayBeConditionalFixTest.java index b71437ba07a7..ca7fb5e2e213 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/BooleanExpressionMayBeConditionalFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/BooleanExpressionMayBeConditionalFixTest.java @@ -75,6 +75,20 @@ public class BooleanExpressionMayBeConditionalFixTest extends IGQuickFixesTestCa "}"); } + public void testComparison() { + doTest(InspectionGadgetsBundle.message("if.may.be.conditional.quickfix"), + "class X {\n" + + " boolean test(int x, int y, int z) {\n" + + " return (x > y && z == 1) || /**/(x <= y && z == 2);\n" + + " }\n" + + "}", + "class X {\n" + + " boolean test(int x, int y, int z) {\n" + + " return x > y ? z == 1 : z == 2;\n" + + " }\n" + + "}"); + } + @Override protected BaseInspection getInspection() { return new BooleanExpressionMayBeConditionalInspection(); diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/SimplifiableBooleanExpressionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/SimplifiableBooleanExpressionFixTest.java index 63c55b8a8d93..678c7a433c67 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/SimplifiableBooleanExpressionFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/controlflow/SimplifiableBooleanExpressionFixTest.java @@ -45,6 +45,58 @@ public class SimplifiableBooleanExpressionFixTest extends IGQuickFixesTestCase { "}"); } + public void testAndOrExpression3() { + doMemberTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix"), + "boolean fff(boolean a, boolean b, boolean c) {" + + " return a && b && c/**/|| !a;" + + "}", + "boolean fff(boolean a, boolean b, boolean c) {" + + " return !a || b && c;" + + "}"); + } + + public void testAndOrExpression3Middle() { + // While this particular case could be safely transformed to "a && c || !b", the order of execution is changed which may + // affect dereferencing (e.g. "a != null && b != null && a.foo(b.bar()) || b == null") is safe, but replacement is not. + // Proper replacement would be "(a || !b) && (!b || c)", but it's not shorter than the original code + assertQuickfixNotAvailable(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix"), + "class X {\n" + + " boolean fff(boolean a, boolean b, boolean c) { \n" + + " return a && b && c/**/ || !b;\n" + + " }\n" + + "}"); + } + + public void testAndOrExpression3Parentheses() { + doMemberTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix"), + "boolean fff(boolean a, boolean b, boolean c) {" + + " return (a && b && !c)/**/|| c;" + + "}", + "boolean fff(boolean a, boolean b, boolean c) {" + + " return (a && b) || c;" + + "}"); + } + + public void testAndOrExpressionComparisons() { + doMemberTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix"), + "boolean fff(int a, int b, int c) {" + + " return a > b && b > c /**/|| a <= b;" + + "}", + "boolean fff(int a, int b, int c) {" + + " return a <= b || b > c;" + + "}"); + } + + public void testAndOrNonNegated() { + doMemberTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix"), + "boolean fff(int a, int b, int c) {" + + " return a > b && b > c && b > 0 /**/|| (b > c);" + + "}", + "boolean fff(int a, int b, int c) {" + + " return b > c;" + + "}"); + } + @Override protected BaseInspection getInspection() { return new SimplifiableBooleanExpressionInspection();