diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 035ab8a1eaa8..7374c40c297b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -382,7 +382,8 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec if (parent instanceof PsiStatement) return !(parent instanceof PsiReturnStatement); if (parent instanceof PsiPolyadicExpression) { IElementType tokenType = ((PsiPolyadicExpression)parent).getOperationTokenType(); - return tokenType.equals(JavaTokenType.ANDAND) || tokenType.equals(JavaTokenType.OROR); + return tokenType.equals(JavaTokenType.ANDAND) || tokenType.equals(JavaTokenType.OROR) || + tokenType.equals(JavaTokenType.AND) || tokenType.equals(JavaTokenType.OR); } if (parent instanceof PsiConditionalExpression) { return PsiTreeUtil.isAncestor(((PsiConditionalExpression)parent).getCondition(), expression, false); @@ -395,7 +396,11 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec if (shouldBeSuppressed(ref) || constant == ConstantResult.UNKNOWN) return; List fixes = new SmartList<>(); String presentableName = constant.toString(); - fixes.add(new ReplaceWithConstantValueFix(presentableName, presentableName)); + if (constant.value() instanceof Boolean) { + fixes.add(createSimplifyBooleanExpressionFix(ref, (Boolean)constant.value())); + } else { + fixes.add(new ReplaceWithConstantValueFix(presentableName, presentableName)); + } Object value = constant.value(); boolean isAssertion = isAssertionEffectively(ref, constant); if (isAssertion && DONT_REPORT_TRUE_ASSERT_STATEMENTS) return; diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java index 0819da718749..98a56c960d3d 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java @@ -56,25 +56,25 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { if (subExpression == null) { return getFamilyName(); } - return getIntentionText(subExpression, mySubExpressionValue) + (shouldExtractSideEffect() ? " extracting side effects" : ""); + String suffix = ""; + if (SideEffectChecker.mayHaveSideEffects(subExpression)) { + suffix = canExtractSideEffect(subExpression) ? " extracting side effects" : " (may change semantics)"; + } + return getIntentionText(subExpression, mySubExpressionValue) + suffix; } - private boolean shouldExtractSideEffect() { - PsiExpression subExpression = getSubExpression(); - if (subExpression != null && - SideEffectChecker.mayHaveSideEffects(subExpression)) { - if (ControlFlowUtils.canExtractStatement(subExpression)) return true; - if (!mySubExpressionValue) { - PsiElement parent = PsiUtil.skipParenthesizedExprUp(subExpression.getParent()); - if (parent instanceof PsiWhileStatement || parent instanceof PsiForStatement) return true; - // code like "if (foo || alwaysFalseWithSideEffects) {}" - if (parent instanceof PsiPolyadicExpression) { - PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent; - if (polyadic.getOperationTokenType().equals(JavaTokenType.OROR) - && PsiTreeUtil.isAncestor(ArrayUtil.getLastElement(polyadic.getOperands()), subExpression, false) - && PsiUtil.skipParenthesizedExprUp(parent.getParent()) instanceof PsiIfStatement) { - return true; - } + private boolean canExtractSideEffect(PsiExpression subExpression) { + if (ControlFlowUtils.canExtractStatement(subExpression)) return true; + if (!mySubExpressionValue) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(subExpression.getParent()); + if (parent instanceof PsiWhileStatement || parent instanceof PsiForStatement) return true; + // code like "if (foo || alwaysFalseWithSideEffects) {}" + if (parent instanceof PsiPolyadicExpression) { + PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent; + if (polyadic.getOperationTokenType().equals(JavaTokenType.OROR) + && PsiTreeUtil.isAncestor(ArrayUtil.getLastElement(polyadic.getOperands()), subExpression, false) + && PsiUtil.skipParenthesizedExprUp(parent.getParent()) instanceof PsiIfStatement) { + return true; } } } @@ -136,7 +136,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { PsiExpression subExpression = getSubExpression(); if (subExpression == null) return; CommentTracker ct = new CommentTracker(); - if (shouldExtractSideEffect()) { + if (SideEffectChecker.mayHaveSideEffects(subExpression) && canExtractSideEffect(subExpression)) { subExpression = ensureCodeBlock(project, subExpression); if (subExpression == null) { LOG.error("ensureCodeBlock returned null", new Attachment("subExpression.txt", getSubExpression().getText())); diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterAssignCallResult.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterAssignCallResult.java new file mode 100644 index 000000000000..649f0e495e85 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterAssignCallResult.java @@ -0,0 +1,10 @@ +// "Simplify 'Objects.nonNull(...)' to true extracting side effects" "true" +import java.util.Set; +import java.util.Objects; + +class X { + void test(Set set) { + set.add("foo"); + boolean b = true; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterDoWhileWithSideEffects.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterDoWhileWithSideEffects.java new file mode 100644 index 000000000000..eeeefc380c54 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterDoWhileWithSideEffects.java @@ -0,0 +1,13 @@ +// "Unwrap 'do-while' statement (may change semantics)" "true" +import org.jetbrains.annotations.Contract; + +class X { + @Contract("_ -> true") + boolean test(Object obj) { + return true; + } + + void doSmth(Object obj) { + System.out.println("aaahh"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterWhileWithSideEffectsNonShortCircuit.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterWhileWithSideEffectsNonShortCircuit.java new file mode 100644 index 000000000000..ebaf3b021495 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/afterWhileWithSideEffectsNonShortCircuit.java @@ -0,0 +1,23 @@ +// "Simplify 'pureConsumer(...)' to true extracting side effects" "true" +import org.jetbrains.annotations.Contract; + +public class Main { + private static int counter = 0; + + public static void main(String[] args) { + while (true) { + sideEffect(); + if (!(counter < 5)) break; + System.out.println(counter); + } + } + + @Contract(value = "!null->true;null->false", pure = true) + private static boolean pureConsumer(Object consumed) { + return consumed != null; + } + + private static int sideEffect() { + return counter++; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeAssignCallResult.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeAssignCallResult.java new file mode 100644 index 000000000000..8e9319a4a6b1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeAssignCallResult.java @@ -0,0 +1,9 @@ +// "Simplify 'Objects.nonNull(...)' to true extracting side effects" "true" +import java.util.Set; +import java.util.Objects; + +class X { + void test(Set set) { + boolean b = Objects.nonNull(set.add("foo")); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeDoWhileWithSideEffects.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeDoWhileWithSideEffects.java new file mode 100644 index 000000000000..671b6afa40cb --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeDoWhileWithSideEffects.java @@ -0,0 +1,16 @@ +// "Unwrap 'do-while' statement (may change semantics)" "true" +import org.jetbrains.annotations.Contract; + +class X { + @Contract("_ -> true") + boolean test(Object obj) { + return true; + } + + void doSmth(Object obj) { + do { + System.out.println("aaahh"); + } + while(obj != null && test(obj) && obj == null); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeWhileWithSideEffectsNonShortCircuit.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeWhileWithSideEffectsNonShortCircuit.java new file mode 100644 index 000000000000..af2da93c4133 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/unwrapIfStatement/beforeWhileWithSideEffectsNonShortCircuit.java @@ -0,0 +1,21 @@ +// "Simplify 'pureConsumer(...)' to true extracting side effects" "true" +import org.jetbrains.annotations.Contract; + +public class Main { + private static int counter = 0; + + public static void main(String[] args) { + while (pureConsumer(sideEffect()) & counter < 5) { + System.out.println(counter); + } + } + + @Contract(value = "!null->true;null->false", pure = true) + private static boolean pureConsumer(Object consumed) { + return consumed != null; + } + + private static int sideEffect() { + return counter++; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java b/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java index 7aca1a5952f7..ae3fc52434c5 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java @@ -48,7 +48,7 @@ class S { public void te6(boolean b){ Boolean c = Boolean.TRUE; boolean o = !c; - o |= c&b; + o |= c&b; if (o) { } }