From 52ffac3745921f9d984bfb20db6019afab954c56 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Thu, 11 Jan 2018 16:20:24 +0700 Subject: [PATCH] MoveConditionToLoopInspection inspection created: IDEA-117791 --- java/java-impl/src/META-INF/JavaPlugin.xml | 4 + .../MoveConditionToLoopInspection.java | 172 ++++++++++++++++++ .../moveConditionToLoop/afterForToWhile.java | 9 + .../moveConditionToLoop/afterToDoWhile.java | 9 + .../afterToDoWhileComments.java | 10 + .../moveConditionToLoop/afterToWhile.java | 15 ++ .../moveConditionToLoop/beforeForToWhile.java | 10 + .../moveConditionToLoop/beforeToDoWhile.java | 10 + .../beforeToDoWhileComments.java | 13 ++ .../beforeToDoWhileWithContinue.java | 11 ++ .../beforeToDoWhileWithVarsInLoop.java | 11 ++ .../moveConditionToLoop/beforeToWhile.java | 11 ++ .../MoveConditionToLoopInspectionTest.java | 26 +++ .../src/messages/InspectionsBundle.properties | 6 +- .../siyeh/ig/psiutils/ControlFlowUtils.java | 19 ++ .../MoveConditionToLoop.html | 19 ++ 16 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 java/java-impl/src/com/intellij/codeInspection/MoveConditionToLoopInspection.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/afterForToWhile.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhile.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhileComments.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/afterToWhile.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeForToWhile.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhile.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileComments.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithContinue.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithVarsInLoop.java create mode 100644 java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToWhile.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/MoveConditionToLoopInspectionTest.java create mode 100644 resources-en/src/inspectionDescriptions/MoveConditionToLoop.html diff --git a/java/java-impl/src/META-INF/JavaPlugin.xml b/java/java-impl/src/META-INF/JavaPlugin.xml index 3c416d95f474..525ee571eeae 100644 --- a/java/java-impl/src/META-INF/JavaPlugin.xml +++ b/java/java-impl/src/META-INF/JavaPlugin.xml @@ -489,6 +489,10 @@ groupPath="Java" groupBundle="messages.InspectionsBundle" groupKey="group.names.control.flow.issues" bundle="messages.InspectionsBundle" key="inspection.idempotent.loop.body" implementationClass="com.intellij.codeInspection.IdempotentLoopBodyInspection"/> + StreamEx.of(el.getChildren())) + .select(PsiBreakStatement.class) + .filter(stmt -> ControlFlowUtils.statementBreaksLoop(stmt, loopStatement)) + .count() != 1) { + return null; + } + PsiStatement first = statements[0]; + PsiExpression firstBreakCondition = extractBreakCondition(first, loopStatement); + if (firstBreakCondition != null) { + return new Context(loopStatement, body, firstBreakCondition, first, true); + } + if (noConversionToDoWhile) return null; + PsiStatement last = statements[statements.length - 1]; + PsiExpression lastBreakCondition = extractBreakCondition(last, loopStatement); + if (lastBreakCondition != null) { + if (StreamEx.of(statements) + .flatMap(statement -> StreamEx.ofTree((PsiElement)statement, el -> StreamEx.of(el.getChildren()))) + .anyMatch(e -> e instanceof PsiContinueStatement && + ((PsiContinueStatement)e).findContinuedStatement() == loopStatement)) { + return null; + } + boolean variablesInLoop = VariableAccessUtils.collectUsedVariables(lastBreakCondition).stream() + .anyMatch(var -> PsiTreeUtil.isAncestor(loopStatement, var, false)); + if (variablesInLoop) return null; + return new Context(loopStatement, body, lastBreakCondition, last, false); + } + return null; + } + + @Nullable + private static PsiExpression extractBreakCondition(@NotNull PsiStatement statement, @NotNull PsiLoopStatement loopStatement) { + PsiIfStatement ifStatement = tryCast(statement, PsiIfStatement.class); + if (ifStatement == null) return null; + if (ifStatement.getElseBranch() != null) return null; + PsiStatement thenBranch = ifStatement.getThenBranch(); + if (!ControlFlowUtils.statementBreaksLoop(ControlFlowUtils.stripBraces(thenBranch), loopStatement)) return null; + return ifStatement.getCondition(); + } + } + + private static class LoopTransformationFix implements LocalQuickFix { + @Nls + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.move.condition.to.loop"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiLoopStatement loop = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiLoopStatement.class); + if (loop == null) return; + Context context = Context.from(loop, false); + if (context == null) return; + CommentTracker ct = new CommentTracker(); + String negated = BoolUtils.getNegatedExpressionText(context.myCondition, ct); + ct.delete(context.myConditionStatement); + String loopText = context.myConditionInTheBeginning + ? "while(" + negated + ")" + ct.text(context.myLoopBody) + : "do" + ct.text(context.myLoopBody) + "while(" + negated + ");"; + ct.replaceAndRestoreComments(context.myLoopStatement, loopText); + } + } +} diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/afterForToWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterForToWhile.java new file mode 100644 index 000000000000..6a6ebaf1784c --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterForToWhile.java @@ -0,0 +1,9 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + while (i < 12) { + i++; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhile.java new file mode 100644 index 000000000000..1f6cf59f5954 --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhile.java @@ -0,0 +1,9 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + do { + i++; + } while (i < 12); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhileComments.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhileComments.java new file mode 100644 index 000000000000..02d382c7e2ba --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToDoWhileComments.java @@ -0,0 +1,10 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + // negative i is invalid - stop here + do { + i++; + } while (i >= 0); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToWhile.java new file mode 100644 index 000000000000..0654808577b2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/afterToWhile.java @@ -0,0 +1,15 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + /*1*/ + /*2*/ + /*3*/ + /*7*/ + /*8*/ + while (i < 12) { + /*4*/ + i =/*5*/ i + 1;/*6*/ + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeForToWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeForToWhile.java new file mode 100644 index 000000000000..90af34c95b8a --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeForToWhile.java @@ -0,0 +1,10 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + for(;;) { + if(i >= 12) break; + i++; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhile.java new file mode 100644 index 000000000000..f8f1c2d17778 --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhile.java @@ -0,0 +1,10 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + while(true) { + i++; + if(i >= 12) break; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileComments.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileComments.java new file mode 100644 index 000000000000..e0cc8f849531 --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileComments.java @@ -0,0 +1,13 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + while(true) { + i++; + if(i < 0) { + // negative i is invalid - stop here + break; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithContinue.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithContinue.java new file mode 100644 index 000000000000..57f91988cf7e --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithContinue.java @@ -0,0 +1,11 @@ +// "Move condition to loop" "false" +class Main { + public static void main(String[] args) { + int i = 0; + while((true) { + i++; + if(i == 4) continue; + if(i >= 12) break; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithVarsInLoop.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithVarsInLoop.java new file mode 100644 index 000000000000..7d8547969518 --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToDoWhileWithVarsInLoop.java @@ -0,0 +1,11 @@ +// "Move condition to loop" "false" +class Main { + public static void main(String[] args) { + int i = 0; + while(true) { + i++; + int j = i; + if(j >= 12) break; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToWhile.java b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToWhile.java new file mode 100644 index 000000000000..b58bc9ff402c --- /dev/null +++ b/java/java-tests/testData/codeInsight/moveConditionToLoop/beforeToWhile.java @@ -0,0 +1,11 @@ +// "Move condition to loop" "true" +class Main { + public static void main(String[] args) { + int i = 0; + while(true/*7*/) /*8*/ { + if(i >= 12/*1*/)/*2*/ break;/*3*/ + /*4*/ + i =/*5*/ i + 1;/*6*/ + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/MoveConditionToLoopInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/MoveConditionToLoopInspectionTest.java new file mode 100644 index 000000000000..dc7acba59d19 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/MoveConditionToLoopInspectionTest.java @@ -0,0 +1,26 @@ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. + +package com.intellij.java.codeInsight.daemon.quickFix; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.MoveConditionToLoopInspection; +import org.jetbrains.annotations.NotNull; + +public class MoveConditionToLoopInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new MoveConditionToLoopInspection()}; + } + + public void test() { + doAllTests(); + } + + + @Override + protected String getBasePath() { + return "/codeInsight/moveConditionToLoop/"; + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 6bb8eff1c1fb..af73962402ef 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -929,4 +929,8 @@ navigate.to.duplicate.fix=Navigate to duplicate inspection.idempotent.loop.body=Idempotent loop body inspection.undeclared.service.usage.name=Usage of service not declared in module-info -inspection.undeclared.service.usage.message=Usage of service ''{0}'' is not declared in module-info \ No newline at end of file +inspection.undeclared.service.usage.message=Usage of service ''{0}'' is not declared in module-info + +inspection.move.condition.to.loop=Move condition to loop +inspection.move.condition.to.loop.no.conversion.to.do.while=Don't suggest to replace to 'do while' +inspection.move.condition.to.loop.description=Conditional break inside infinite loop \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java index 81e71f1b2dc3..7a2abd87eaf0 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java @@ -127,6 +127,25 @@ public class ControlFlowUtils { } } + @Contract(value = "null -> false", pure = true) + public static boolean isEndlessLoop(@Nullable PsiLoopStatement loopStatement) { + if(loopStatement == null) return false; + if (loopStatement instanceof PsiWhileStatement) { + return BoolUtils.isTrue(((PsiWhileStatement)loopStatement).getCondition()); + } + if (loopStatement instanceof PsiDoWhileStatement) { + return BoolUtils.isTrue(((PsiDoWhileStatement)loopStatement).getCondition()); + } + if (loopStatement instanceof PsiForStatement) { + PsiForStatement forStatement = (PsiForStatement)loopStatement; + PsiExpression condition = forStatement.getCondition(); + if(condition != null && !BoolUtils.isTrue(condition)) return false; + return (forStatement.getInitialization() == null || forStatement.getInitialization() instanceof PsiEmptyStatement) + && (forStatement.getUpdate() == null || forStatement.getUpdate() instanceof PsiEmptyStatement); + } + return false; + } + private static boolean doWhileStatementMayCompleteNormally(@NotNull PsiDoWhileStatement loopStatement) { final PsiExpression condition = loopStatement.getCondition(); final Object value = ExpressionUtils.computeConstantExpression(condition); diff --git a/resources-en/src/inspectionDescriptions/MoveConditionToLoop.html b/resources-en/src/inspectionDescriptions/MoveConditionToLoop.html new file mode 100644 index 000000000000..e9a6f520ca22 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/MoveConditionToLoop.html @@ -0,0 +1,19 @@ + + +

This inspection detects conditional breaks at the end or beginning and suggests move condition to loop

+ +Example: +

+ while(true) { + if(i == 23) break; + i++; + } +

+Will be replaced with: +

+ while(i != 23) { + i++; + } +

+ + \ No newline at end of file