diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java index f1913aa85275..62751716d51f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java @@ -59,9 +59,14 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { public String getText() { PsiExpression expression = getSubExpression(); assert expression != null; - if (PsiUtil.skipParenthesizedExprUp(expression.getParent()) instanceof PsiIfStatement) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + if (parent instanceof PsiIfStatement) { return mySubExpressionValue ? "Unwrap 'if' statement" : "Remove 'if' statement"; } + if (!mySubExpressionValue) { + if (parent instanceof PsiWhileStatement) return "Remove 'while' statement"; + if (parent instanceof PsiDoWhileStatement) return "Unwrap 'do-while' statement"; + } return QuickFixBundle.message("simplify.boolean.expression.text", expression.getText(), mySubExpressionValue); } @@ -102,13 +107,29 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { simplifyExpression(expression); } - public static boolean simplifyIfStatement(final PsiExpression expression) throws IncorrectOperationException { - PsiElement parent = expression.getParent(); - if (!(parent instanceof PsiIfStatement) || ((PsiIfStatement)parent).getCondition() != expression) return false; - if (!(expression instanceof PsiLiteralExpression) || !PsiType.BOOLEAN.equals(expression.getType())) return false; + public static boolean simplifyIfOrLoopStatement(final PsiExpression expression) throws IncorrectOperationException { boolean condition = Boolean.parseBoolean(expression.getText()); - PsiIfStatement ifStatement = (PsiIfStatement)parent; - if (condition) { + if (!(expression instanceof PsiLiteralExpression) || !PsiType.BOOLEAN.equals(expression.getType())) return false; + + PsiElement parent = expression.getParent(); + if (parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getCondition() == expression) { + simplifyIfStatement(condition, (PsiIfStatement)parent); + return true; + } + if (parent instanceof PsiWhileStatement && !condition) { + parent.delete(); + return true; + } + if (parent instanceof PsiDoWhileStatement && !condition) { + replaceWithStatements((PsiDoWhileStatement)parent, ((PsiDoWhileStatement)parent).getBody()); + return true; + } + + return false; + } + + private static void simplifyIfStatement(boolean conditionAlwaysTrue, PsiIfStatement ifStatement) { + if (conditionAlwaysTrue) { replaceWithStatements(ifStatement, ifStatement.getThenBranch()); } else { @@ -120,10 +141,9 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { replaceWithStatements(ifStatement, elseBranch); } } - return true; } - private static void replaceWithStatements(@NotNull PsiIfStatement orig, @Nullable PsiStatement statement) throws IncorrectOperationException { + private static void replaceWithStatements(@NotNull PsiStatement orig, @Nullable PsiStatement statement) throws IncorrectOperationException { if (statement == null) { orig.delete(); return; @@ -172,7 +192,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { } } - private static void removeFollowingStatements(@NotNull PsiIfStatement anchor, @NotNull PsiCodeBlock parentBlock) { + private static void removeFollowingStatements(@NotNull PsiStatement anchor, @NotNull PsiCodeBlock parentBlock) { PsiStatement[] siblingStatements = parentBlock.getStatements(); int ifIndex = Arrays.asList(siblingStatements).indexOf(anchor); if (ifIndex >= 0 && ifIndex < siblingStatements.length - 1) { @@ -226,7 +246,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { return; } } - if (!simplifyIfStatement(newExpression)) { + if (!simplifyIfOrLoopStatement(newExpression)) { ParenthesesUtils.removeParentheses(newExpression, false); } } 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 ea1f142b51c0..5db6c11a28a8 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 @@ -153,6 +153,21 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } } + @Override + public void visitWhileStatement(PsiWhileStatement statement) { + checkLoopCondition(statement.getCondition()); + } + + @Override + public void visitDoWhileStatement(PsiDoWhileStatement statement) { + checkLoopCondition(statement.getCondition()); + } + + private void checkLoopCondition(PsiExpression condition) { + if (condition != null && condition.textMatches(PsiKeyword.FALSE)) { + holder.registerProblem(condition, "Condition is always false", createSimplifyBooleanExpressionFix(condition, false)); + } + } }; } diff --git a/java/java-tests/testData/inspection/dataFlow/SCR13702/expected.xml b/java/java-tests/testData/inspection/dataFlow/SCR13702/expected.xml deleted file mode 100644 index 9ac879d78616..000000000000 --- a/java/java-tests/testData/inspection/dataFlow/SCR13702/expected.xml +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/java/java-tests/testData/inspection/dataFlow/SCR13702/src/Foo.java b/java/java-tests/testData/inspection/dataFlow/SCR13702/src/Foo.java deleted file mode 100644 index 84c31a6bf5b9..000000000000 --- a/java/java-tests/testData/inspection/dataFlow/SCR13702/src/Foo.java +++ /dev/null @@ -1,5 +0,0 @@ -public class Foo { - public void foo() { - do {} while(false); - } -} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition.java b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition.java new file mode 100644 index 000000000000..255e3e8c26b1 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition.java @@ -0,0 +1,8 @@ +class Test { + + public static void test() { + do { + System.out.println(); + } while (false); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition_after.java b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition_after.java new file mode 100644 index 000000000000..34ea45b07d31 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralDoWhileCondition_after.java @@ -0,0 +1,6 @@ +class Test { + + public static void test() { + System.out.println(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition.java b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition.java new file mode 100644 index 000000000000..9761ede96b0f --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition.java @@ -0,0 +1,8 @@ +class Test { + + public static void test() { + while (false) { + System.out.println(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition_after.java b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition_after.java new file mode 100644 index 000000000000..b936287299b5 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/LiteralWhileCondition_after.java @@ -0,0 +1,5 @@ +class Test { + + public static void test() { + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java index c9327755394d..7c6ee034dafa 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionAncientTest.java @@ -53,7 +53,6 @@ public class DataFlowInspectionAncientTest extends InspectionTestCase { public void testExceptionCFG() { doTest(true); } public void testInst() { doTest(true); } public void testWrongEqualTypes() { doTest(true); } - public void testSCR13702() { doTest(); } public void testSCR13626() { doTest(); } public void testSCR13871() { doTest(); } public void testInstanceof() { doTest(); } diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index ad654b1d46aa..6c7a49b75e83 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -157,14 +157,18 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testReportConstantReferences() { doTestReportConstantReferences(); - myFixture.launchAction(myFixture.findSingleIntention("Replace with 'null'")); + String hint = "Replace with 'null'"; + checkIntentionResult(hint); + } + + private void checkIntentionResult(String hint) { + myFixture.launchAction(myFixture.findSingleIntention(hint)); myFixture.checkResultByFile(getTestName(false) + "_after.java"); } public void testReportConstantReferences_OverloadedCall() { doTestReportConstantReferences(); - myFixture.launchAction(myFixture.findSingleIntention("Replace with 'null'")); - myFixture.checkResultByFile(getTestName(false) + "_after.java"); + checkIntentionResult("Replace with 'null'"); } public void testReportConstantReferencesAfterFinalFieldAccess() { doTestReportConstantReferences(); } @@ -394,8 +398,7 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testTrueOrEqualsSomething() { doTest(); - myFixture.launchAction(myFixture.findSingleIntention("Remove redundant assignment")); - myFixture.checkResultByFile(getTestName(false) + "_after.java"); + checkIntentionResult("Remove redundant assignment"); } public void testDontSimplifyAssignment() { @@ -422,6 +425,16 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { myFixture.findSingleIntention("Remove 'if' statement"); } + public void testLiteralWhileCondition() { + doTest(); + checkIntentionResult("Remove 'while' statement"); + } + + public void testLiteralDoWhileCondition() { + doTest(); + checkIntentionResult("Unwrap 'do-while' statement"); + } + //https://youtrack.jetbrains.com/issue/IDEA-162184 public void testNullLiteralAndInferredMethodContract() { doTest();