From c901aa2abd4cdece842a0ca7fb196baee2f9d92b Mon Sep 17 00:00:00 2001 From: anna Date: Wed, 23 Mar 2011 15:15:37 +0100 Subject: [PATCH] inplace introduce constant: append initializer; check init place (IDEA-66940) --- .../BaseExpressionToFieldHandler.java | 68 +++++++++++-------- .../InplaceIntroduceConstantPopup.java | 20 ++++-- .../VariableInplaceIntroducer.java | 2 +- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java b/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java index 22314744e177..af3944e140d3 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/BaseExpressionToFieldHandler.java @@ -35,7 +35,6 @@ import com.intellij.ide.util.PackageUtil; import com.intellij.ide.util.PsiClassListCellRenderer; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.Result; -import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Editor; @@ -230,7 +229,7 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase JavaCodeStyleManager.getInstance(field.getProject()).shortenClassReferences(field); } - private static PsiElement getPhysicalElement(final PsiExpression selectedExpr) { + public static PsiElement getPhysicalElement(final PsiExpression selectedExpr) { PsiElement element = selectedExpr.getUserData(ElementToWorkOn.PARENT); if (element == null) element = selectedExpr; return element; @@ -677,32 +676,8 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase createField(myFieldName, myType, initializer, initializerPlace == InitializationPlace.IN_FIELD_DECLARATION && initializer != null, myParentClass); - PsiElement finalAnchorElement = null; - if (destClass == myParentClass) { - for (finalAnchorElement = myAnchorElement; - finalAnchorElement != null && finalAnchorElement.getParent() != destClass; - finalAnchorElement = finalAnchorElement.getParent()) { - - } - } - PsiMember anchorMember = finalAnchorElement instanceof PsiMember ? (PsiMember)finalAnchorElement : null; setModifiers(myField, mySettings, mySettings.isDeclareStatic()); - if ((anchorMember instanceof PsiField) && - anchorMember.hasModifierProperty(PsiModifier.STATIC) == myField.hasModifierProperty(PsiModifier.STATIC)) { - myField = (PsiField)destClass.addBefore(myField, anchorMember); - } - else if (anchorMember instanceof PsiClassInitializer) { - myField = (PsiField)destClass.addBefore(myField, anchorMember); - destClass.addBefore(CodeEditUtil.createLineFeed(myField.getManager()), anchorMember); - } - else { - final PsiField forwardReference = checkForwardRefs(initializer); - if (forwardReference != null) { - myField = (PsiField)destClass.addAfter(myField, forwardReference); - } else { - myField = (PsiField)destClass.add(myField); - } - } + myField = appendField(initializer, destClass, myParentClass, myAnchorElement, myField); if (!mySettings.isIntroduceEnumConstant()) { VisibilityUtil.fixVisibility(myOccurrences, myField, mySettings.getFieldVisibility()); } @@ -800,7 +775,42 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase } } - private PsiField checkForwardRefs(PsiExpression initializer) { + static PsiField appendField(final PsiExpression initializer, + final PsiClass destClass, + final PsiClass parentClass, + final PsiElement anchorElement, + final PsiField psiField) { + PsiElement finalAnchorElement = null; + if (destClass == parentClass) { + for (finalAnchorElement = anchorElement; + finalAnchorElement != null && finalAnchorElement.getParent() != destClass; + finalAnchorElement = finalAnchorElement.getParent()) { + + } + } + PsiMember anchorMember = finalAnchorElement instanceof PsiMember ? (PsiMember)finalAnchorElement : null; + + if ((anchorMember instanceof PsiField) && + anchorMember.hasModifierProperty(PsiModifier.STATIC) == psiField.hasModifierProperty(PsiModifier.STATIC)) { + return (PsiField)destClass.addBefore(psiField, anchorMember); + } + else if (anchorMember instanceof PsiClassInitializer) { + + PsiField field = (PsiField)destClass.addBefore(psiField, anchorMember); + destClass.addBefore(CodeEditUtil.createLineFeed(field.getManager()), anchorMember); + return field; + } + else { + final PsiField forwardReference = checkForwardRefs(initializer, parentClass); + if (forwardReference != null) { + return (PsiField)destClass.addAfter(psiField, forwardReference); + } else { + return (PsiField)destClass.add(psiField); + } + } + } + + private static PsiField checkForwardRefs(PsiExpression initializer, final PsiClass parentClass) { final PsiField[] refConstantFields = new PsiField[1]; initializer.accept(new JavaRecursiveElementWalkingVisitor() { @Override @@ -809,7 +819,7 @@ public abstract class BaseExpressionToFieldHandler extends IntroduceHandlerBase final PsiElement resolve = expression.resolve(); if (resolve instanceof PsiField && ((PsiField)resolve).hasModifierProperty(PsiModifier.FINAL) && - PsiTreeUtil.isAncestor(myParentClass, resolve, false) && ((PsiField)resolve).hasInitializer()) { + PsiTreeUtil.isAncestor(parentClass, resolve, false) && ((PsiField)resolve).hasInitializer()) { if (refConstantFields[0] == null || refConstantFields[0].getTextOffset() < resolve.getTextOffset()) { refConstantFields[0] = (PsiField)resolve; } diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java b/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java index 1c0c933c15a3..86e12da2147f 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java @@ -62,9 +62,9 @@ public class InplaceIntroduceConstantPopup { private final PsiLocalVariable myLocalVariable; private final PsiExpression[] myOccurrences; private final TypeSelectorManagerImpl myTypeSelectorManager; - private final PsiElement myAnchorElement; + private PsiElement myAnchorElement; private int myAnchorIdx = -1; - private final PsiElement myAnchorElementIfAll; + private PsiElement myAnchorElementIfAll; private int myAnchorIdxIfAll = -1; private final OccurenceManager myOccurenceManager; @@ -215,10 +215,10 @@ public class InplaceIntroduceConstantPopup { return ApplicationManager.getApplication().runWriteAction(new Computable() { @Override public PsiField compute() { - PsiField field = elementFactory.createField(myConstantName != null ? myConstantName : names[0], psiType); - field = (PsiField)myParentClass.add(field); + PsiField field = elementFactory.createFieldFromText(psiType.getCanonicalText() + " " + (myConstantName != null ? myConstantName : names[0]) + " = " + myExprText + ";", myParentClass); PsiUtil.setModifierProperty(field, PsiModifier.FINAL, true); PsiUtil.setModifierProperty(field, PsiModifier.STATIC, true); + field = BaseExpressionToFieldHandler.ConvertToFieldRunnable.appendField(myExpr, myParentClass, myParentClass, myAnchorElementIfAll, field); return field; } }); @@ -330,9 +330,7 @@ public class InplaceIntroduceConstantPopup { final BaseExpressionToFieldHandler.ConvertToFieldRunnable convertToFieldRunnable = new BaseExpressionToFieldHandler.ConvertToFieldRunnable(myExpr, settings, settings.getForcedType(), myOccurrences, myOccurenceManager, - myAnchorIdxIfAll != -1? myOccurrences[myAnchorIdxIfAll].getParent() : myAnchorElementIfAll, - myAnchorIdx != -1 ? myOccurrences[myAnchorIdx].getParent() : myAnchorElement, myEditor, - myParentClass); + myAnchorElementIfAll, myAnchorElement, myEditor, myParentClass); convertToFieldRunnable.run(); } } @@ -410,6 +408,14 @@ public class InplaceIntroduceConstantPopup { myOccurrences[i] = psiExpression; } } + + if (myAnchorIdxIfAll != -1) { + myAnchorElementIfAll = myOccurrences[myAnchorIdxIfAll].getParent(); + } + + if (myAnchorIdx != -1) { + myAnchorElement = myOccurrences[myAnchorIdx].getParent(); + } myOccurrenceMarkers = null; if (psiField.isValid()) { psiField.delete(); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableInplaceIntroducer.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableInplaceIntroducer.java index 2e05d7203321..363393e2c7e5 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableInplaceIntroducer.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableInplaceIntroducer.java @@ -180,7 +180,7 @@ public class VariableInplaceIntroducer extends VariableInplaceRenamer { } saveSettings(psiVariable); adjustLine(psiVariable, document); - int startOffset = myExprMarker != null ? myExprMarker.getStartOffset() : psiVariable.getTextOffset(); + int startOffset = myExprMarker != null && myExprMarker.isValid() ? myExprMarker.getStartOffset() : psiVariable.getTextOffset(); final PsiFile file = psiVariable.getContainingFile(); final PsiReference referenceAt = file.findReferenceAt(startOffset); if (referenceAt != null && referenceAt.resolve() instanceof PsiLocalVariable) {