ConvertToLocalInspection: CR fixes (IDEA-CR-42222)

1. localVariableIsCopy -> isLocalVariableCopy
2. added more precise convert to local quick fix message (with info about to where field will be inlined)
3. fixed tests for new convert to local quick fix messages
4. removed inlineVariable(PsiVariable, PsiExpression)

java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/BaseConvertToLocalQuickFix.java

@@ -33,19 +33,14 @@ import com.intellij.refactoring.util.InlineUtil;
 import com.intellij.util.IJSwingUtilities;
 import com.intellij.util.IncorrectOperationException;
 import com.intellij.util.NotNullFunction;
+import com.intellij.util.ObjectUtils;
 import com.siyeh.ig.psiutils.CommentTracker;
 import com.siyeh.ig.psiutils.ParenthesesUtils;
 import com.siyeh.ig.psiutils.VariableAccessUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Objects;
-import java.util.Set;
-import java.util.stream.Collectors;
+import java.util.*;
 
 /**
  * refactored from {@link FieldCanBeLocalInspection}
@@ -68,38 +63,33 @@ public abstract class BaseConvertToLocalQuickFix<V extends PsiVariable> implemen
     final PsiFile myFile = variable.getContainingFile();
 
     try {
-      final List<PsiElement> newDeclarations = moveDeclaration(project, variable).stream()
-        .map(declaration -> inlineRedundant(declaration))
-        .filter(Objects::nonNull)
-        .collect(Collectors.toList());
-
+      final List<PsiElement> newDeclarations = moveDeclaration(project, variable);
       if (newDeclarations.isEmpty()) return;
 
       positionCaretToDeclaration(project, myFile, newDeclarations.get(newDeclarations.size() - 1));
+      newDeclarations.forEach(declaration -> inlineRedundant(declaration));
     }
     catch (IncorrectOperationException e) {
       LOG.error(e);
     }
   }
 
-  @Nullable
-  private static PsiElement inlineRedundant(@Nullable PsiElement declaration) {
-    if (declaration == null) return null;
+  private static void inlineRedundant(@Nullable PsiElement declaration) {
+    if (declaration == null) return;
 
     final PsiLocalVariable newVariable = extractDeclared(declaration);
     if (newVariable != null) {
       final PsiExpression initializer = ParenthesesUtils.stripParentheses(newVariable.getInitializer());
 
-      if (VariableAccessUtils.localVariableIsCopy(newVariable, initializer)) {
-        WriteAction.run(() -> {
-          InlineUtil.inlineVariable(newVariable, initializer);
+      WriteAction.run(() -> {
+        if (VariableAccessUtils.isLocalVariableCopy(newVariable, initializer)) {
+          for (PsiReference reference : ReferencesSearch.search(newVariable).findAll()) {
+            InlineUtil.inlineVariable(newVariable, initializer, (PsiJavaCodeReferenceElement)reference);
+          }
           declaration.delete();
-        });
-        return null;
-      }
+        }
+      });
     }
-
-    return declaration;
   }
 
   @Nullable
@@ -109,11 +99,7 @@ public abstract class BaseConvertToLocalQuickFix<V extends PsiVariable> implemen
     final PsiElement[] declaredElements = ((PsiDeclarationStatement)declaration).getDeclaredElements();
     if (declaredElements.length != 1) return null;
 
-    final PsiElement declared = declaredElements[0];
-
-    if (!(declared instanceof PsiLocalVariable)) return null;
-
-    return (PsiLocalVariable)declared;
+    return ObjectUtils.tryCast(declaredElements[0], PsiLocalVariable.class);
   }
 
   @Nullable

java/java-impl/src/com/intellij/codeInspection/varScopeCanBeNarrowed/FieldCanBeLocalInspection.java

@@ -26,6 +26,7 @@ import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.refactoring.util.RefactoringUtil;
+import com.intellij.util.Query;
 import com.siyeh.InspectionGadgetsBundle;
 import gnu.trove.THashSet;
 import org.jdom.Element;
@@ -37,6 +38,7 @@ import javax.swing.*;
 import java.awt.*;
 import java.util.List;
 import java.util.*;
+import java.util.stream.Collectors;
 
 public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTool {
   @NonNls public static final String SHORT_NAME = "FieldCanBeLocal";
@@ -68,15 +70,19 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
     if (candidates.isEmpty()) return;
     final List<ImplicitUsageProvider> implicitUsageProviders = ImplicitUsageProvider.EP_NAME.getExtensionList();
 
+    FieldLoop:
     for (final PsiField field : candidates) {
       if (usedFields.contains(field) && !hasImplicitReadOrWriteUsage(field, implicitUsageProviders)) {
-        if (ReferencesSearch.search(field, new LocalSearchScope(aClass)).anyMatch(reference -> {
+        final Query<PsiReference> references = ReferencesSearch.search(field, new LocalSearchScope(aClass));
+        final Map<PsiCodeBlock, Collection<PsiReference>> refs = new HashMap<>();
+        for (PsiReference reference : references.findAll()) {
           final PsiElement element = reference.getElement();
-          if (!(element instanceof PsiReferenceExpression)) return false;
+          if (!(element instanceof PsiReferenceExpression)) break;
           final PsiElement qualifier = ((PsiReferenceExpression)element).getQualifier();
-          return qualifier != null && (!(qualifier instanceof PsiThisExpression) || ((PsiThisExpression)qualifier).getQualifier() != null);
-        })) {
-          continue;
+          if (qualifier != null && (!(qualifier instanceof PsiThisExpression) || ((PsiThisExpression)qualifier).getQualifier() != null) ||
+              !groupReferenceByCodeBlocks(refs, reference)) {
+            continue FieldLoop;
+          }
         }
         final String message = InspectionsBundle.message("inspection.field.can.be.local.problem.descriptor");
         final ArrayList<LocalQuickFix> fixes = new ArrayList<>();
@@ -88,7 +94,7 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
           fixes.add(quickFix);
           return true;
         });
-        final LocalQuickFix fix = createFix();
+        final LocalQuickFix fix = createFix(refs);
         if (fix != null) {
           fixes.add(fix);
         }
@@ -97,8 +103,8 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
     }
   }
 
-  protected LocalQuickFix createFix() {
-    return new ConvertFieldToLocalQuickFix();
+  protected LocalQuickFix createFix(@NotNull Map<PsiCodeBlock, Collection<PsiReference>> refs) {
+    return new ConvertFieldToLocalQuickFix(refs);
   }
 
   @Nullable
@@ -118,6 +124,7 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
     root.accept(new JavaRecursiveElementWalkingVisitor() {
       @Override
       public void visitMethod(PsiMethod method) {
+
         if (method.isConstructor()) {
           final PsiCodeBlock body = method.getBody();
           if (body != null) {
@@ -279,6 +286,50 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
     return false;
   }
 
+  private static boolean groupByCodeBlocks(final Collection<? extends PsiReference> allReferences,
+                                           final Map<PsiCodeBlock, Collection<PsiReference>> refs) {
+    for (PsiReference psiReference : allReferences) {
+      if (!groupReferenceByCodeBlocks(refs, psiReference)) return false;
+    }
+    return true;
+  }
+
+  private static boolean groupReferenceByCodeBlocks(Map<PsiCodeBlock, Collection<PsiReference>> refs, PsiReference psiReference) {
+    final PsiElement element = psiReference.getElement();
+    final PsiCodeBlock block = PsiTreeUtil.getTopmostParentOfType(element, PsiCodeBlock.class);
+    if (block == null) {
+      return false;
+    }
+
+    Collection<PsiReference> references = refs.get(block);
+    if (references == null) {
+      references = new ArrayList<>();
+      if (findExistentBlock(refs, psiReference, block, references)) return true;
+      refs.put(block, references);
+    }
+    references.add(psiReference);
+    return true;
+  }
+
+  private static boolean findExistentBlock(Map<PsiCodeBlock, Collection<PsiReference>> refs,
+                                           PsiReference psiReference,
+                                           PsiCodeBlock block,
+                                           Collection<? super PsiReference> references) {
+    for (Iterator<PsiCodeBlock> iterator = refs.keySet().iterator(); iterator.hasNext(); ) {
+      PsiCodeBlock codeBlock = iterator.next();
+      if (PsiTreeUtil.isAncestor(codeBlock, block, false)) {
+        refs.get(codeBlock).add(psiReference);
+        return true;
+      }
+      else if (PsiTreeUtil.isAncestor(block, codeBlock, false)) {
+        references.addAll(refs.get(codeBlock));
+        iterator.remove();
+        break;
+      }
+    }
+    return false;
+  }
+
   @Override
   public boolean isEnabledByDefault() {
     return true;
@@ -323,6 +374,32 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
   }
 
   private static class ConvertFieldToLocalQuickFix extends BaseConvertToLocalQuickFix<PsiField> {
+    private final String myName;
+
+    private ConvertFieldToLocalQuickFix(Map<PsiCodeBlock, Collection<PsiReference>> refs) {
+      Set<PsiMethod> methods = refs.keySet().stream()
+        .filter(block -> block.getParent() instanceof PsiMethod)
+        .map(block -> (PsiMethod)block.getParent())
+        .collect(Collectors.toSet());
+
+      myName = determineName(methods, refs);
+    }
+
+    @NotNull
+    private String determineName(Set<PsiMethod> methods, Map<PsiCodeBlock, Collection<PsiReference>> refs) {
+      if (methods.isEmpty() || methods.size() != refs.size()) return getFamilyName();
+      if (methods.size() > 1) return InspectionsBundle.message("inspection.field.can.be.local.quickfix.multiple.methods", methods.size());
+
+      final String methodName = methods.toArray(PsiMethod.EMPTY_ARRAY)[0].getName();
+      return InspectionsBundle.message("inspection.field.can.be.local.quickfix.one.method", methodName);
+    }
+
+    @NotNull
+    @Override
+    public String getName() {
+      return myName;
+    }
+
     @NotNull
     @Override
     protected List<PsiElement> moveDeclaration(@NotNull final Project project, @NotNull final PsiField variable) {
@@ -343,44 +420,6 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
       return newDeclarations;
     }
 
-    private static boolean groupByCodeBlocks(final Collection<? extends PsiReference> allReferences, Map<PsiCodeBlock, Collection<PsiReference>> refs) {
-      for (PsiReference psiReference : allReferences) {
-        final PsiElement element = psiReference.getElement();
-        final PsiCodeBlock block = PsiTreeUtil.getTopmostParentOfType(element, PsiCodeBlock.class);
-        if (block == null) {
-          return false;
-        }
-
-        Collection<PsiReference> references = refs.get(block);
-        if (references == null) {
-          references = new ArrayList<>();
-          if (findExistentBlock(refs, psiReference, block, references)) continue;
-          refs.put(block, references);
-        }
-        references.add(psiReference);
-      }
-      return true;
-    }
-
-    private static boolean findExistentBlock(Map<PsiCodeBlock, Collection<PsiReference>> refs,
-                                             PsiReference psiReference,
-                                             PsiCodeBlock block,
-                                             Collection<? super PsiReference> references) {
-      for (Iterator<PsiCodeBlock> iterator = refs.keySet().iterator(); iterator.hasNext(); ) {
-        PsiCodeBlock codeBlock = iterator.next();
-        if (PsiTreeUtil.isAncestor(codeBlock, block, false)) {
-          refs.get(codeBlock).add(psiReference);
-          return true;
-        }
-        else if (PsiTreeUtil.isAncestor(block, codeBlock, false)) {
-          references.addAll(refs.get(codeBlock));
-          iterator.remove();
-          break;
-        }
-      }
-      return false;
-    }
-
     @Override
     @Nullable
     protected PsiField getVariable(@NotNull ProblemDescriptor descriptor) {

java/java-impl/src/com/intellij/refactoring/util/InlineUtil.java

@@ -49,24 +49,6 @@ public class InlineUtil {
 
   private InlineUtil() {}
 
-  /**
-   * Replace variable with its initializer for every variable reference.
-   *
-   * @return references after replacement
-   */
-  @NotNull
-  public static Collection<PsiElement> inlineVariable(@NotNull PsiVariable variable, @NotNull PsiExpression initializer) {
-    final Collection<PsiElement> replacedElements = new ArrayList<>();
-    final Collection<PsiReference> references = ReferencesSearch.search(variable).findAll();
-
-    for (PsiReference reference : references) {
-      final PsiExpression expression = inlineVariable(variable, initializer, (PsiJavaCodeReferenceElement)reference);
-      replacedElements.add(expression);
-    }
-
-    return replacedElements;
-  }
-
   @NotNull
   public static PsiExpression inlineVariable(PsiVariable variable, PsiExpression initializer, PsiJavaCodeReferenceElement ref) throws IncorrectOperationException {
     return inlineVariable(variable, initializer, ref, null);

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterAssignmentAndFinalByDefault.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'getFoo1'" "true"
 class Test {
 
     int getFoo1(boolean f) {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterBlocksInSameMethod.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'IntelliJBugConvertToLocal'" "true"
 import java.util.ArrayList;
 
 class ITest {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterFinalFieldIsCopy.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'Test'" "true"
 class Test {
 
     public Test(String param) {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterInSwitchCases.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'someMethod'" "true"
 class TestFieldConversion
 {
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterInitializer.java

@@ -0,0 +1,9 @@
+// "Convert to local" "true"
+class TestInitializer {
+
+    {
+        boolean field = true;
+    System.out.println(field);
+  }
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterMultipleMethods.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variables in 2 methods" "true"
 class Test {
 
     int getFoo1() {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterMultipleMethodsMultipleFields.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'setEditable1'" "true"
 class MyClassTest {
 
   private boolean editable = false;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/afterSetterAfterSuperConstructorCall.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'FieldCanBeLocalTest'" "true"
 
 import javax.swing.*;
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeAssignmentAndFinalByDefault.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'getFoo1'" "true"
 class Test {
   private int my<caret>Foo;
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeBlocksInSameMethod.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'IntelliJBugConvertToLocal'" "true"
 import java.util.ArrayList;
 
 class ITest {

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeFinalFieldIsCopy.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'Test'" "true"
 class Test {
 
   private final String f<caret>ield;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeInSwitchCases.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'someMethod'" "true"
 class TestFieldConversion
 {
   /**

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeInitializer.java

@@ -0,0 +1,11 @@
+// "Convert to local" "true"
+class TestInitializer {
+
+  private boolean fie<caret>ld;
+
+  {
+    field = true;
+    System.out.println(field);
+  }
+
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeMultipleMethods.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variables in 2 methods" "true"
 class Test {
   private int my<caret>Foo;
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeMultipleMethodsMultipleFields.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'setEditable1'" "true"
 class MyClassTest {
 
   private boolean editable = false;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/convert2Local/beforeSetterAfterSuperConstructorCall.java

@@ -1,4 +1,4 @@
-// "Convert to local" "true"
+// "Convert field to local variable in method 'FieldCanBeLocalTest'" "true"
 
 import javax.swing.*;
 

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -196,6 +196,8 @@ inspection.field.can.be.local.problem.descriptor=Field can be converted to a loc
 inspection.parameter.can.be.local.display.name=Parameter can be local
 inspection.parameter.can.be.local.problem.descriptor=Parameter can be converted to a local variable
 inspection.convert.to.local.quickfix=Convert to local
+inspection.field.can.be.local.quickfix.multiple.methods=Convert field to local variables in {0} methods
+inspection.field.can.be.local.quickfix.one.method=Convert field to local variable in method ''{0}''
 
 inspection.unused.return.value.display.name=Method can be void
 inspection.unused.return.value.problem.descriptor=Return value of the method is never used

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/VariableAccessUtils.java

@@ -403,14 +403,14 @@ public class VariableAccessUtils {
   /**
    * Check if local variable has the same behavior as its initializer.
    */
-  public static boolean localVariableIsCopy(@NotNull PsiLocalVariable variable) {
-    return localVariableIsCopy(variable, ParenthesesUtils.stripParentheses(variable.getInitializer()));
+  public static boolean isLocalVariableCopy(@NotNull PsiLocalVariable variable) {
+    return isLocalVariableCopy(variable, ParenthesesUtils.stripParentheses(variable.getInitializer()));
   }
 
   /**
    * Check if local variable has the same behavior as given expression.
    */
-  public static boolean localVariableIsCopy(@NotNull PsiLocalVariable variable, @Nullable PsiExpression expression) {
+  public static boolean isLocalVariableCopy(@NotNull PsiLocalVariable variable, @Nullable PsiExpression expression) {
     if (expression instanceof PsiTypeCastExpression) {
       PsiExpression operand = ((PsiTypeCastExpression)expression).getOperand();
       if (operand instanceof PsiReferenceExpression && RedundantCastUtil.isCastRedundant((PsiTypeCastExpression)expression)) {

plugins/InspectionGadgets/src/com/siyeh/ig/dataflow/UnnecessaryLocalVariableInspection.java

@@ -111,7 +111,7 @@ public class UnnecessaryLocalVariableInspection extends BaseInspection {
           }
         }
       }
-      if (VariableAccessUtils.localVariableIsCopy(variable)) {
+      if (VariableAccessUtils.isLocalVariableCopy(variable)) {
         registerVariableError(variable);
       }
       else if (!m_ignoreImmediatelyReturnedVariables && isImmediatelyReturned(variable)) {

plugins/InspectionGadgets/src/com/siyeh/ig/fixes/InlineVariableFix.java

@@ -18,12 +18,14 @@ package com.siyeh.ig.fixes;
 import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
+import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.refactoring.util.InlineUtil;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.InspectionGadgetsFix;
 import com.siyeh.ig.psiutils.HighlightUtils;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.ArrayList;
 import java.util.Collection;
 
 public class InlineVariableFix extends InspectionGadgetsFix {
@@ -43,7 +45,12 @@ public class InlineVariableFix extends InspectionGadgetsFix {
       return;
     }
 
-    final Collection<PsiElement> replacedElements = InlineUtil.inlineVariable(variable, initializer);
+    final Collection<PsiReference> references = ReferencesSearch.search(variable).findAll();
+    final Collection<PsiElement> replacedElements = new ArrayList<>();
+    for (PsiReference reference : references) {
+      final PsiExpression expression = InlineUtil.inlineVariable(variable, initializer, (PsiJavaCodeReferenceElement)reference);
+      replacedElements.add(expression);
+    }
 
     if (isOnTheFly()) {
       HighlightUtils.highlightElements(replacedElements);