diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java index 45a633a3ec37..e0599fa2b68f 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java @@ -318,7 +318,8 @@ final class VariableExtractor { PsiWhileStatement whileStatement = (PsiWhileStatement)anchor; PsiExpression condition = whileStatement.getCondition(); if (condition != null && allOccurrences.stream().allMatch(occurrence -> PsiTreeUtil.isAncestor(whileStatement, occurrence, true))) { - if (firstOccurrence != null && PsiTreeUtil.isAncestor(condition, firstOccurrence, false)) { + if (firstOccurrence != null && PsiTreeUtil.isAncestor(condition, firstOccurrence, false) && + !ExpressionUtils.isLoopInvariant(firstOccurrence, whileStatement)) { PsiPolyadicExpression polyadic = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(condition), PsiPolyadicExpression.class); if (polyadic != null && JavaTokenType.ANDAND.equals(polyadic.getOperationTokenType())) { PsiExpression operand = ContainerUtil.find(polyadic.getOperands(), op -> PsiTreeUtil.isAncestor(op, firstOccurrence, false)); diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue.java new file mode 100644 index 000000000000..e79e2e92dda3 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue.java @@ -0,0 +1,5 @@ +public class whileTrue { + void test() { + while(true) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue_after.java new file mode 100644 index 000000000000..d27a65a6209c --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/whileTrue_after.java @@ -0,0 +1,6 @@ +public class whileTrue { + void test() { + boolean b = true; + while(b) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.after.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.after.java index db68114a8c4d..5b8e6059a893 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.after.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.after.java @@ -5,7 +5,7 @@ class Test { while (x > 0) { boolean temp = z > 0; if (!(y < 0 || temp)) break; - + z++; } } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.java index 3c0523f8a36e..42a29859629d 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionAndOr.java @@ -3,7 +3,7 @@ import java.util.Arrays; class Test { void test(int x, int y, int z) { while (x > 0 && (y < 0 || z > 0)) { - + z++; } } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.after.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.after.java index 0ebd2d3351be..0391dcb11c74 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.after.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.after.java @@ -1,8 +1,10 @@ class Test { - void test(boolean foo) { + void test() { while (true) { - boolean temp = foo; + boolean temp = foo(); if (!temp) break; } } + + native boolean foo(); } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.java index fe5b06af3c4e..007e75d04307 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionIncomplete.java @@ -1,5 +1,7 @@ class Test { - void test(boolean foo) { - while(foo) + void test() { + while(foo()) } + + native boolean foo(); } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.after.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.after.java index 7e5536fa169d..2cf3ff60cc17 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.after.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.after.java @@ -1,6 +1,7 @@ class Test { + int[] a = new int[10]; + void foo() { - int[] a = new int[10]; int log = 0; while (true) { int temp = a.length; diff --git a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.java b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.java index c76c0395c132..acf836b9fa92 100644 --- a/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.java +++ b/java/java-tests/testData/refactoring/introduceVariable/WhileConditionNoBrace.java @@ -1,6 +1,7 @@ class Test { + int[] a = new int[10]; + void foo() { - int[] a = new int[10]; int log = 0; while (1 << log < a.length) log++; } diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java index ddeabd559478..2e472bd5eb7b 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java @@ -219,6 +219,10 @@ public class InplaceIntroduceVariableTest extends AbstractJavaInplaceIntroduceTe public void testLambdaParameterAddCast() { doTestReplaceChoice("Replace all 0 occurrences"); } + + public void testWhileTrue() { + doTest(null); + } private void doTestStopEditing(Consumer pass) { String name = getTestName(true); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java index d4ac1a869ba0..8d165abeef02 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ExpressionUtils.java @@ -4,6 +4,7 @@ package com.siyeh.ig.psiutils; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.CodeInsightUtilCore; import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; import com.intellij.codeInspection.dataFlow.ContractReturnValue; import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil; import com.intellij.openapi.project.Project; @@ -27,6 +28,7 @@ import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Collection; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -1674,4 +1676,29 @@ public final class ExpressionUtils { } return null; } + + /** + * @param expression expression to test + * @param loopStatement loop statement + * @return true if given expression is likely to be a loop invariant. False if it's not invariant, or not known. + */ + public static boolean isLoopInvariant(PsiExpression expression, @SuppressWarnings("unused") PsiLoopStatement loopStatement) { + if (PsiUtil.isConstantExpression(expression)) return true; + if (SideEffectChecker.mayHaveSideEffects(expression)) return false; + Collection refs = PsiTreeUtil.collectElementsOfType(expression, PsiReferenceExpression.class); + for (PsiReferenceExpression ref : refs) { + PsiElement target = ref.resolve(); + // TODO: more sophisticated analysis + if (target instanceof PsiField && ((PsiField)target).hasModifierProperty(PsiModifier.FINAL)) continue; + if (target instanceof PsiLocalVariable || target instanceof PsiParameter) { + PsiVariable var = (PsiVariable)target; + if (var.hasModifierProperty(PsiModifier.FINAL) || + HighlightControlFlowUtil.isEffectivelyFinal(var, PsiUtil.getVariableCodeBlock(var, null), null)) { + continue; + } + } + return false; + } + return true; + } } \ No newline at end of file