From 88bdc032be56c441669a142d82abf6b5e6079cac Mon Sep 17 00:00:00 2001
From: peter <peter@jetbrains.com>
Date: Thu, 11 May 2017 16:21:43 +0200
Subject: [PATCH] look for conflicts when replacing 'for' with its
 initialization (IDEA-CR-20896)

---
 .../SimplifyBooleanExpressionFix.java         | 72 +++++++++++++------
 .../FalseForConditionWithInitialization.java  |  9 +--
 ...eForConditionWithInitialization_after.java |  9 ++-
 3 files changed, 60 insertions(+), 30 deletions(-)

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 6d2eaa7c0a20..0e3bf2c4641c 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
@@ -31,6 +31,7 @@ import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.IncorrectOperationException;
+import com.intellij.util.containers.ContainerUtil;
 import com.siyeh.ig.psiutils.DeclarationSearchUtils;
 import com.siyeh.ig.psiutils.ParenthesesUtils;
 import com.siyeh.ig.psiutils.SideEffectChecker;
@@ -174,32 +175,57 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement {
       removeFollowingStatements(orig, (PsiCodeBlock)parent);
     }
 
-    if (statement instanceof PsiBlockStatement && parent instanceof PsiCodeBlock &&
-        !DeclarationSearchUtils.containsConflictingDeclarations(((PsiBlockStatement)statement).getCodeBlock(), (PsiCodeBlock)parent)) {
-      // See IDEADEV-24277
-      // Code block can only be inlined into another (parent) code block.
-      // Code blocks, which are if or loop statement branches should not be inlined.
-      PsiCodeBlock codeBlock = ((PsiBlockStatement)statement).getCodeBlock();
-      PsiJavaToken lBrace = codeBlock.getLBrace();
-      PsiJavaToken rBrace = codeBlock.getRBrace();
-      if (lBrace == null || rBrace == null) return;
-
-
-      final PsiElement[] children = codeBlock.getChildren();
-      if (children.length > 2) {
-        final PsiElement added =
-          parent.addRangeBefore(
-            children[1],
-            children[children.length - 2],
-            orig);
-        final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(orig.getManager());
-        codeStyleManager.reformat(added);
+    if (parent instanceof PsiCodeBlock) {
+      if (statement instanceof PsiBlockStatement &&
+          !DeclarationSearchUtils.containsConflictingDeclarations(((PsiBlockStatement)statement).getCodeBlock(), (PsiCodeBlock)parent)) {
+        inlineBlockStatements(orig, (PsiBlockStatement)statement, parent);
+        return;
+      }
+      if (hasConflictingDeclarations(statement, (PsiCodeBlock)parent)) {
+        orig.replace(wrapWithCodeBlock(statement));
+        return;
       }
-      orig.delete();
     }
-    else {
-      orig.replace(statement);
+    orig.replace(statement);
+  }
+
+  private static boolean hasConflictingDeclarations(@Nullable PsiStatement statement, PsiCodeBlock parent) {
+    return statement instanceof PsiDeclarationStatement &&
+           ContainerUtil.exists(((PsiDeclarationStatement)statement).getDeclaredElements(), e -> isConflictingLocalVariable(parent, e));
+  }
+
+  private static boolean isConflictingLocalVariable(PsiCodeBlock parent, PsiElement declaration) {
+    if (!(declaration instanceof PsiLocalVariable)) return false;
+    String name = ((PsiLocalVariable)declaration).getName();
+    return name != null && PsiResolveHelper.SERVICE.getInstance(declaration.getProject()).resolveAccessibleReferencedVariable(name, parent) != null;
+  }
+
+  private static PsiCodeBlock wrapWithCodeBlock(PsiStatement replacement) {
+    PsiCodeBlock newBlock = JavaPsiFacade.getElementFactory(replacement.getProject()).createCodeBlock();
+    newBlock.add(replacement);
+    return newBlock;
+  }
+
+  private static void inlineBlockStatements(@NotNull PsiStatement orig, @NotNull PsiBlockStatement statement, PsiElement parent) {
+    // See IDEADEV-24277
+    // Code block can only be inlined into another (parent) code block.
+    // Code blocks, which are if or loop statement branches should not be inlined.
+    PsiCodeBlock codeBlock = statement.getCodeBlock();
+    PsiJavaToken lBrace = codeBlock.getLBrace();
+    PsiJavaToken rBrace = codeBlock.getRBrace();
+    if (lBrace == null || rBrace == null) return;
+
+    final PsiElement[] children = codeBlock.getChildren();
+    if (children.length > 2) {
+      final PsiElement added =
+        parent.addRangeBefore(
+          children[1],
+          children[children.length - 2],
+          orig);
+      final CodeStyleManager codeStyleManager = CodeStyleManager.getInstance(orig.getManager());
+      codeStyleManager.reformat(added);
     }
+    orig.delete();
   }
 
   private static boolean blockAlwaysReturns(@NotNull PsiStatement statement) {
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization.java b/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization.java
index 4c4420a066c4..09031ed8e75c 100644
--- a/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization.java
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization.java
@@ -1,8 +1,9 @@
 class Test {
 
-  public static void test() {
-    for(int i = <error descr="Cannot resolve method 'launchMissiles()'">launchMissiles</error>(); (<warning descr="Condition is always false">fa<caret>lse</warning>);) {
-      System.out.println("Hello");
+    public static void test() {
+        for(int i = <error descr="Cannot resolve method 'launchMissiles()'">launchMissiles</error>(); (<warning descr="Condition is always false">fa<caret>lse</warning>);) {
+            System.out.println("Hello");
+        }
+        int i = 1;
     }
-  }
 }
\ No newline at end of file
diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization_after.java b/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization_after.java
index 3e717c149111..79a9f9e355bc 100644
--- a/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization_after.java
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/FalseForConditionWithInitialization_after.java
@@ -1,6 +1,9 @@
 class Test {
 
-  public static void test() {
-      int i = launchMissiles();
-  }
+    public static void test() {
+        {
+            int i = launchMissiles();
+        }
+        int i = 1;
+    }
 }
\ No newline at end of file