inplace introduce: correctly restore anchors to insert newly created field

java/java-impl/src/com/intellij/refactoring/introduceField/AbstractInplaceIntroduceFieldPopup.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright 2000-2011 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.refactoring.introduceField;
+
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.*;
+import com.intellij.refactoring.introduceParameter.AbstractJavaInplaceIntroducer;
+import com.intellij.refactoring.ui.TypeSelectorManagerImpl;
+import com.intellij.refactoring.util.occurences.OccurenceManager;
+
+/**
+ * User: anna
+ */
+public abstract class AbstractInplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
+  protected final PsiClass myParentClass;
+  protected final OccurenceManager myOccurenceManager;
+
+  private SmartPsiElementPointer<PsiElement> myAnchorElement;
+  private int myAnchorIdx = -1;
+  private SmartPsiElementPointer<PsiElement> myAnchorElementIfAll;
+  private int myAnchorIdxIfAll = -1;
+
+  private final SmartPointerManager mySmartPointerManager;
+
+  public AbstractInplaceIntroduceFieldPopup(Project project,
+                                            Editor editor,
+                                            PsiExpression expr,
+                                            PsiVariable localVariable,
+                                            PsiExpression[] occurrences,
+                                            TypeSelectorManagerImpl typeSelectorManager,
+                                            String title,
+                                            PsiClass parentClass,
+                                            final PsiElement anchorElement,
+                                            final OccurenceManager occurenceManager,
+                                            final PsiElement anchorElementIfAll) {
+    super(project, editor, expr, localVariable, occurrences, typeSelectorManager, title);
+    myParentClass = parentClass;
+    myOccurenceManager = occurenceManager;
+    mySmartPointerManager = SmartPointerManager.getInstance(project);
+    myAnchorElement = anchorElement != null ? mySmartPointerManager.createSmartPsiElementPointer(anchorElement) : null;
+    myAnchorElementIfAll = anchorElementIfAll != null ? mySmartPointerManager.createSmartPsiElementPointer(anchorElementIfAll) : null;
+    for (int i = 0, occurrencesLength = occurrences.length; i < occurrencesLength; i++) {
+      PsiExpression occurrence = occurrences[i];
+      PsiElement parent = occurrence.getParent();
+      if (parent == anchorElement) {
+        myAnchorIdx = i;
+      }
+      if (parent == anchorElementIfAll) {
+        myAnchorIdxIfAll = i;
+      }
+    }
+  }
+
+  @Override
+  protected PsiElement checkLocalScope() {
+    return myParentClass;
+  }
+
+  protected PsiElement getAnchorElement() {
+    if (myAnchorIdx != -1 && myOccurrences[myAnchorIdx] != null) {
+      return myOccurrences[myAnchorIdx].getParent();
+    }
+    else {
+      return myAnchorElement != null ? myAnchorElement.getElement() : null;
+    }
+  }
+
+  protected PsiElement getAnchorElementIfAll() {
+    if (myAnchorIdxIfAll != -1 && myOccurrences[myAnchorIdxIfAll] != null) {
+      return myOccurrences[myAnchorIdxIfAll].getParent();
+    }
+    else {
+      return myAnchorElementIfAll != null ? myAnchorElementIfAll.getElement() : null;
+    }
+  }
+}

java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java

@@ -51,12 +51,7 @@ import java.awt.event.ItemListener;
  * User: anna
  * Date: 3/18/11
  */
-public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer {
-  private PsiElement myAnchorElement;
-  private int myAnchorIdx = -1;
-  private PsiElement myAnchorElementIfAll;
-  private int myAnchorIdxIfAll = -1;
-  private final OccurenceManager myOccurenceManager;
+public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceFieldPopup {
 
   private final String myInitializerText;
 
@@ -65,8 +60,6 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
 
   private JCheckBox myMoveToAnotherClassCb;
 
-  private final PsiClass myParentClass;
-
   public InplaceIntroduceConstantPopup(Project project,
                                        Editor editor,
                                        PsiClass parentClass,
@@ -76,21 +69,8 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
                                        TypeSelectorManagerImpl typeSelectorManager,
                                        PsiElement anchorElement,
                                        PsiElement anchorElementIfAll, OccurenceManager occurenceManager) {
-    super(project, editor, expr, localVariable, occurrences, typeSelectorManager, IntroduceConstantHandler.REFACTORING_NAME);
-    myParentClass = parentClass;
-    myAnchorElement = anchorElement;
-    myAnchorElementIfAll = anchorElementIfAll;
-    for (int i = 0, occurrencesLength = occurrences.length; i < occurrencesLength; i++) {
-      PsiExpression occurrence = occurrences[i];
-      PsiElement parent = occurrence.getParent();
-      if (parent == myAnchorElement) {
-        myAnchorIdx = i;
-      }
-      if (parent == myAnchorElementIfAll) {
-        myAnchorIdxIfAll = i;
-      }
-    }
-    myOccurenceManager = occurenceManager;
+    super(project, editor, expr, localVariable, occurrences, typeSelectorManager, IntroduceConstantHandler.REFACTORING_NAME,
+          parentClass, anchorElement, occurenceManager, anchorElementIfAll);
 
     myInitializerText = getExprText(expr, localVariable);
 
@@ -176,7 +156,7 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
         }
         field = BaseExpressionToFieldHandler.ConvertToFieldRunnable
           .appendField(myExpr, BaseExpressionToFieldHandler.InitializationPlace.IN_FIELD_DECLARATION, myParentClass, myParentClass,
-                       myAnchorElementIfAll, field);
+                       getAnchorElementIfAll(), field);
         myFieldRangeStart = myEditor.getDocument().createRangeMarker(field.getTextRange());
         return field;
       }
@@ -199,11 +179,6 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
     myReplaceAllCb.setSelected(allOccurrences);
   }
 
-  @Override
-  protected PsiElement checkLocalScope() {
-    return myParentClass;
-  }
-
   @Override
   protected void saveSettings(PsiVariable psiVariable) {
     super.saveSettings(psiVariable);
@@ -220,10 +195,6 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
     return PsiTreeUtil.getParentOfType(element, PsiField.class, false);
   }
 
-  @Override
-  protected void performPostIntroduceTasks() {
-  }
-
   @Override
   protected void moveOffsetAfter(boolean success) {
     if (success) {
@@ -279,24 +250,13 @@ public class InplaceIntroduceConstantPopup extends AbstractJavaInplaceIntroducer
           final BaseExpressionToFieldHandler.ConvertToFieldRunnable convertToFieldRunnable =
             new BaseExpressionToFieldHandler.ConvertToFieldRunnable(myExpr, settings, settings.getForcedType(),
                                                                     myOccurrences, myOccurenceManager,
-                                                                    myAnchorElementIfAll, myAnchorElement, myEditor, myParentClass);
+                                                                    getAnchorElementIfAll(), getAnchorElement(), myEditor, myParentClass);
           convertToFieldRunnable.run();
         }
       }
     }.execute();
   }
 
-  @Override
-  protected void restoreAnchors() {
-    if (myAnchorIdxIfAll != -1 && myOccurrences[myAnchorIdxIfAll] != null) {
-      myAnchorElementIfAll = myOccurrences[myAnchorIdxIfAll].getParent();
-    }
-
-    if (myAnchorIdx != -1 && myOccurrences[myAnchorIdx] != null) {
-      myAnchorElement = myOccurrences[myAnchorIdx].getParent();
-    }
-  }
-
   @Override
   protected JComponent getComponent() {
     myReplaceAllCb.addItemListener(new ItemListener() {

java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceFieldPopup.java

@@ -45,18 +45,9 @@ import java.awt.event.ItemListener;
  * User: anna
  * Date: 3/15/11
  */
-public class InplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
+public class InplaceIntroduceFieldPopup extends AbstractInplaceIntroduceFieldPopup {
 
-  private final PsiClass myParentClass;
   private final boolean myStatic;
-  private final Editor myEditor;
-  private final Project myProject;
-
-  private PsiElement myAnchorElement;
-  private int myAnchorIdx = -1;
-  private PsiElement myAnchorElementIfAll;
-  private int myAnchorIdxIfAll = -1;
-  private final OccurenceManager myOccurenceManager;
 
   private final IntroduceFieldPopupPanel myIntroduceFieldPanel;
 
@@ -74,25 +65,8 @@ public class InplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
                                     final PsiElement anchorElementIfAll,
                                     final OccurenceManager occurenceManager, Project project) {
     super(project, editor, initializerExpression, localVariable, occurrences, typeSelectorManager,
-          IntroduceFieldHandler.REFACTORING_NAME);
-    myParentClass = parentClass;
+          IntroduceFieldHandler.REFACTORING_NAME, parentClass, anchorElement, occurenceManager, anchorElementIfAll);
     myStatic = aStatic;
-    myAnchorElement = anchorElement;
-    myAnchorElementIfAll = anchorElementIfAll;
-    for (int i = 0, occurrencesLength = occurrences.length; i < occurrencesLength; i++) {
-      PsiExpression occurrence = occurrences[i];
-      PsiElement parent = occurrence.getParent();
-      if (parent == myAnchorElement) {
-        myAnchorIdx = i;
-      }
-      if (parent == anchorElementIfAll) {
-        myAnchorIdxIfAll = i;
-      }
-    }
-    myOccurenceManager = occurenceManager;
-    myProject = myLocalVariable != null ? myLocalVariable.getProject() : initializerExpression.getProject();
-    myEditor = editor;
-
     myIntroduceFieldPanel =
       new IntroduceFieldPopupPanel(parentClass, initializerExpression, localVariable, currentMethodConstructor, localVariable != null, aStatic,
                                myOccurrences, allowInitInMethod, allowInitInMethodIfAll, typeSelectorManager);
@@ -182,12 +156,7 @@ public class InplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
       myIntroduceFieldPanel.saveFinalState();
     }
 
-    @Override
-    protected PsiElement checkLocalScope() {
-      return myParentClass;
-    }
-
-    @Override
+  @Override
     protected JComponent getComponent() {
       myIntroduceFieldPanel.addOccurrenceListener(new ItemListener() {
         @Override
@@ -233,18 +202,7 @@ public class InplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
       return PsiTreeUtil.getParentOfType(element, PsiField.class, false);
     }
 
-    @Override
-    protected void restoreAnchors() {
-      if (myAnchorIdxIfAll != -1 && myOccurrences[myAnchorIdxIfAll] != null) {
-        myAnchorElementIfAll = myOccurrences[myAnchorIdxIfAll].getParent();
-      }
-
-      if (myAnchorIdx != -1 && myOccurrences[myAnchorIdx] != null) {
-        myAnchorElement = myOccurrences[myAnchorIdx].getParent();
-      }
-    }
-
-    public BaseExpressionToFieldHandler.InitializationPlace getInitializerPlace() {
+  public BaseExpressionToFieldHandler.InitializationPlace getInitializerPlace() {
       return myIntroduceFieldPanel.getInitializerPlace();
     }
 
@@ -273,8 +231,8 @@ public class InplaceIntroduceFieldPopup extends AbstractJavaInplaceIntroducer {
             final BaseExpressionToFieldHandler.ConvertToFieldRunnable convertToFieldRunnable =
               new BaseExpressionToFieldHandler.ConvertToFieldRunnable(myExpr, settings, settings.getForcedType(),
                                                                       myOccurrences, myOccurenceManager,
-                                                                      myAnchorIdxIfAll != -1 && myOccurrences[myAnchorIdxIfAll] != null ? myOccurrences[myAnchorIdxIfAll].getParent() : myAnchorElementIfAll,
-                                                                      myAnchorIdx != -1 && myOccurrences[myAnchorIdx] != null ? myOccurrences[myAnchorIdx].getParent() : myAnchorElement, myEditor,
+                                                                      getAnchorElementIfAll(),
+                                                                      getAnchorElement(), myEditor,
                                                                       myParentClass);
             convertToFieldRunnable.run();
           }

java/java-tests/testData/refactoring/inplaceIntroduceConstant/correctFinalPosition.java

@@ -0,0 +1,3 @@
+class Test {
+    private static final int FOO = 60 * 6<caret>0;
+}

java/java-tests/testData/refactoring/inplaceIntroduceConstant/correctFinalPosition_after.java

@@ -0,0 +1,4 @@
+class Test {
+    public static final int SEC = 60;
+    private static final int FOO = 60 * <caret>SEC;
+}

java/java-tests/testSrc/com/intellij/refactoring/InplaceIntroduceConstantTest.java

@@ -52,6 +52,15 @@ public class InplaceIntroduceConstantTest extends AbstractInplaceIntroduceTest {
       }
     });
   }
+  public void testCorrectFinalPosition() throws Exception {
+
+    doTest(new Pass<AbstractInplaceIntroducer>() {
+      @Override
+      public void pass(AbstractInplaceIntroducer inplaceIntroduceFieldPopup) {
+        type("SEC");
+      }
+    });
+  }
 
   public void testEscapePosition() throws Exception {
     doTestEscape();

platform/lang-impl/src/com/intellij/refactoring/introduce/inplace/AbstractInplaceIntroducer.java

@@ -126,7 +126,6 @@ public abstract class AbstractInplaceIntroducer<V extends PsiNameIdentifierOwner
   protected abstract V getVariable();
 
   public abstract E restoreExpression(PsiFile containingFile, V variable, RangeMarker marker, String exprText);
-  protected void restoreAnchors() {}
 
   public boolean startInplaceIntroduceTemplate() {
     final boolean replaceAllOccurrences = isReplaceAllOccurrences();
@@ -365,7 +364,6 @@ public abstract class AbstractInplaceIntroducer<V extends PsiNameIdentifierOwner
           }
         }
 
-        restoreAnchors();
         myOccurrenceMarkers = null;
         if (psiField.isValid()) {
           psiField.delete();