From e0f8104e43456629fb2bb467d59e90400e2f7b75 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 5 Dec 2022 17:14:40 +0100 Subject: [PATCH] [java-dfa] IDEA-307660 Avoid double reporting between "Constant values" and "Pointless boolean expression" GitOrigin-RevId: 97cf073bd5bcd69b8903e5cf2e915fb2a4261b6b --- .../dataFlow/ConstantValueInspection.java | 6 ++ ...uplicatedByPointlessBooleanInspection.java | 33 +++++++ .../DataFlowInspectionTest.java | 1 + .../PointlessBooleanExpressionInspection.java | 87 ++++++++++--------- 4 files changed, 87 insertions(+), 40 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/DuplicatedByPointlessBooleanInspection.java diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/dataFlow/ConstantValueInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/dataFlow/ConstantValueInspection.java index 4b7cbc2f62bf..6ab5e6355245 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/dataFlow/ConstantValueInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/dataFlow/ConstantValueInspection.java @@ -32,6 +32,7 @@ import com.intellij.util.IncorrectOperationException; import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.bugs.EqualsWithItselfInspection; +import com.siyeh.ig.controlflow.PointlessBooleanExpressionInspection; import com.siyeh.ig.fixes.EqualsToEqualityFix; import com.siyeh.ig.numeric.ComparisonToNaNInspection; import com.siyeh.ig.psiutils.*; @@ -325,6 +326,11 @@ public class ConstantValueInspection extends AbstractBaseJavaLocalInspectionTool if (method == null || !JavaMethodContractUtil.isPure(method)) return true; } } + if (new PointlessBooleanExpressionInspection().getExpressionKind(expression) != + PointlessBooleanExpressionInspection.BooleanExpressionKind.UNKNOWN) { + // avoid double reporting + return true; + } while (expression != null && BoolUtils.isNegation(expression)) { expression = BoolUtils.getNegated(expression); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/DuplicatedByPointlessBooleanInspection.java b/java/java-tests/testData/inspection/dataFlow/fixture/DuplicatedByPointlessBooleanInspection.java new file mode 100644 index 000000000000..8eb0e1e6ae65 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/DuplicatedByPointlessBooleanInspection.java @@ -0,0 +1,33 @@ +// IDEA-304296 +class BooleanOrEquals { + void foo(boolean a, boolean b) { + boolean c = !(b && false); + boolean d = a ^ b ^ true; + boolean x = a ^ !true ^ b; + + boolean y = false || c; + boolean z = b != true; + } + + static int i = 1; + public static void main(String[] args) { + boolean b = false; + if (i == 1 && (b |= true)) + System.out.println("i == 1"); + if (i == 1 && (b |= false)) + System.out.println("i == 1"); + if (b |= false) + System.out.println("i == 1"); + if (b |= true) + System.out.println("i == 1"); + if (b = true) + System.out.println("i == 1"); + System.out.println(b); + + if (i == 1 && (b &= true)) { } + if (i == 1 && (b &= false)) { } + if (b &= true) {} + if (b &= false) {} + } + +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index b377c0809034..60edd3bb12b0 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -723,4 +723,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testRewiringSubclassMethod() { doTest(); } public void testTryWithResourcesCloseThrows() { doTest(); } public void testBooleanOrEquals() { doTest(); } + public void testDuplicatedByPointlessBooleanInspection() { doTest(); } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java index f619c2a4bf7f..a1ea33df346e 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java @@ -41,7 +41,7 @@ import java.util.function.Predicate; import java.util.function.Supplier; public class PointlessBooleanExpressionInspection extends BaseInspection implements CleanupLocalInspectionTool { - private enum BooleanExpressionKind { + public enum BooleanExpressionKind { USELESS, USELESS_WITH_SIDE_EFFECTS, UNKNOWN } @@ -441,9 +441,6 @@ public class PointlessBooleanExpressionInspection extends BaseInspection impleme if (parent instanceof PsiExpression && getExpressionKind((PsiExpression)parent) != BooleanExpressionKind.UNKNOWN) { return; } - if (containsEscapingPatternVariable(expression)) { - return; - } final String replacement = buildSimplifiedExpression(expression, new StringBuilder(), new CommentTracker()).toString(); final Supplier newBodySupplier = @@ -457,52 +454,62 @@ public class PointlessBooleanExpressionInspection extends BaseInspection impleme } @NotNull - private BooleanExpressionKind getExpressionKind(PsiExpression expression) { + public BooleanExpressionKind getExpressionKind(PsiExpression expression) { + if ((expression instanceof PsiPrefixExpression || expression instanceof PsiPolyadicExpression) + && containsEscapingPatternVariable(expression)) { + return BooleanExpressionKind.UNKNOWN; + } if (expression instanceof PsiPrefixExpression || expression instanceof PsiAssignmentExpression) { return evaluate(expression) != null ? BooleanExpressionKind.USELESS : BooleanExpressionKind.UNKNOWN; } if (expression instanceof PsiPolyadicExpression polyadicExpression) { - final IElementType sign = polyadicExpression.getOperationTokenType(); - if (!booleanTokens.contains(sign)) { + return getPolyadicKind(polyadicExpression); + } + return BooleanExpressionKind.UNKNOWN; + } + + @NotNull + private BooleanExpressionKind getPolyadicKind(PsiPolyadicExpression expression) { + final IElementType sign = expression.getOperationTokenType(); + if (!booleanTokens.contains(sign)) { + return BooleanExpressionKind.UNKNOWN; + } + final PsiExpression[] operands = expression.getOperands(); + boolean containsConstant = false; + boolean stopCheckingSideEffects = false; + boolean sideEffectMayBeRemoved = false; + boolean reducedToConstant = false; + for (PsiExpression operand : operands) { + if (operand == null) { return BooleanExpressionKind.UNKNOWN; } - final PsiExpression[] operands = polyadicExpression.getOperands(); - boolean containsConstant = false; - boolean stopCheckingSideEffects = false; - boolean sideEffectMayBeRemoved = false; - boolean reducedToConstant = false; - for (PsiExpression operand : operands) { - if (operand == null) { - return BooleanExpressionKind.UNKNOWN; + final PsiType type = operand.getType(); + if (type == null || !type.equals(PsiType.BOOLEAN) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) { + return BooleanExpressionKind.UNKNOWN; + } + if (!stopCheckingSideEffects && SideEffectChecker.mayHaveSideEffects(operand)) { + sideEffectMayBeRemoved = true; + continue; + } + Boolean value = evaluate(operand); + if (value != null) { + containsConstant = true; + if ((JavaTokenType.ANDAND.equals(sign) && !value) || (JavaTokenType.OROR.equals(sign) && value)) { + stopCheckingSideEffects = true; + reducedToConstant = true; } - final PsiType type = operand.getType(); - if (type == null || !type.equals(PsiType.BOOLEAN) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) { - return BooleanExpressionKind.UNKNOWN; - } - if (!stopCheckingSideEffects && SideEffectChecker.mayHaveSideEffects(operand)) { - sideEffectMayBeRemoved = true; - continue; - } - Boolean value = evaluate(operand); - if (value != null) { - containsConstant = true; - if ((JavaTokenType.ANDAND.equals(sign) && !value) || (JavaTokenType.OROR.equals(sign) && value)) { - stopCheckingSideEffects = true; - reducedToConstant = true; - } - if ((JavaTokenType.AND.equals(sign) && !value) || (JavaTokenType.OR.equals(sign) && value)) { - reducedToConstant = true; - } + if ((JavaTokenType.AND.equals(sign) && !value) || (JavaTokenType.OR.equals(sign) && value)) { + reducedToConstant = true; } } - if (containsConstant) { - if (sideEffectMayBeRemoved && reducedToConstant) { - return CodeBlockSurrounder.canSurround(expression) - ? BooleanExpressionKind.USELESS_WITH_SIDE_EFFECTS - : BooleanExpressionKind.UNKNOWN; - } - return BooleanExpressionKind.USELESS; + } + if (containsConstant) { + if (sideEffectMayBeRemoved && reducedToConstant) { + return CodeBlockSurrounder.canSurround(expression) + ? BooleanExpressionKind.USELESS_WITH_SIDE_EFFECTS + : BooleanExpressionKind.UNKNOWN; } + return BooleanExpressionKind.USELESS; } return BooleanExpressionKind.UNKNOWN; }