diff --git a/source/com/intellij/refactoring/move/moveInstanceMethod/InternalUsageInfo.java b/source/com/intellij/refactoring/move/moveInstanceMethod/InternalUsageInfo.java index 2baeb937eb4a..53d9544a1f16 100644 --- a/source/com/intellij/refactoring/move/moveInstanceMethod/InternalUsageInfo.java +++ b/source/com/intellij/refactoring/move/moveInstanceMethod/InternalUsageInfo.java @@ -2,19 +2,28 @@ package com.intellij.refactoring.move.moveInstanceMethod; import com.intellij.usageView.UsageInfo; import com.intellij.psi.PsiReferenceExpression; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiNewExpression; +import com.intellij.openapi.diagnostic.Logger; /** * @author ven */ public class InternalUsageInfo extends UsageInfo { - private final PsiReferenceExpression myReferenceExpression; - - public InternalUsageInfo(final PsiReferenceExpression referenceExpression) { - super(referenceExpression); - myReferenceExpression = referenceExpression; + private static final Logger LOG = Logger.getInstance("#com.intellij.refactoring.move.moveInstanceMethod.InternalUsageInfo"); + public InternalUsageInfo(final PsiElement referenceElement) { + super(referenceElement); + LOG.assertTrue(referenceElement instanceof PsiReferenceExpression || referenceElement instanceof PsiNewExpression); } - public PsiReferenceExpression getReferenceExpression() { - return myReferenceExpression; + PsiExpression getQualifier () { + PsiElement element = getElement(); + if (element instanceof PsiReferenceExpression) { + return ((PsiReferenceExpression)element).getQualifierExpression(); + } + else if (element instanceof PsiNewExpression) return ((PsiNewExpression)element).getQualifier(); + + return null; } } diff --git a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodDialog.java b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodDialog.java index 06e5d650529a..72ebb13be224 100644 --- a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodDialog.java +++ b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodDialog.java @@ -56,7 +56,8 @@ public class MoveInstanceMethodDialog extends MoveInstanceMethodDialogBase { protected void doAction() { final PsiVariable targetVariable = (PsiVariable)myList.getSelectedValue(); final String parameterName = myOldClassParameterNameField.getText().trim(); - if (!myMethod.getManager().getNameHelper().isIdentifier(parameterName)) { + if (!myMethod.getManager().getNameHelper().isIdentifier(parameterName) && + MoveMethodUtil.isOldThisNeeded (myMethod)) { Messages.showErrorDialog(getProject(), "Please Enter a Valid name for Parameter", myRefactoringName); return; } diff --git a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodHandler.java b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodHandler.java index 60a14d18ac7f..120a1cece718 100644 --- a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodHandler.java +++ b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodHandler.java @@ -25,7 +25,7 @@ import java.util.List; * @author ven */ public class MoveInstanceMethodHandler implements RefactoringActionHandler { - private static final Logger LOG = Logger.getInstance("#com.intellij.refactoring.convertToInstanceMethod.ConvertToInstanceMethodHandler"); + private static final Logger LOG = Logger.getInstance("#com.intellij.refactoring.move.moveInstanceMethod.MoveInstanceMethodHandler"); static final String REFACTORING_NAME = "Move Instance Method"; public void invoke(Project project, Editor editor, PsiFile file, DataContext dataContext) { diff --git a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodProcessor.java b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodProcessor.java index 9cefd5d15bc2..7ed43321111f 100644 --- a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodProcessor.java +++ b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodProcessor.java @@ -26,6 +26,7 @@ import java.util.*; */ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ private static final Logger LOG = Logger.getInstance("#com.intellij.refactoring.move.moveInstanceMethod.MoveInstanceMethodProcessor"); + private final boolean myOldThisNeeded; public PsiMethod getMethod() { return myMethod; @@ -57,6 +58,7 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ final PsiClass targetClass = ((PsiClassType) type).resolve(); myTargetClass = targetClass; myNewVisibility = newVisibility; + myOldThisNeeded = MoveMethodUtil.isOldThisNeeded(myMethod); } protected UsageViewDescriptor createUsageViewDescriptor(UsageInfo[] usages, FindUsagesCommand refreshCommand) { @@ -134,13 +136,19 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ final PsiCodeBlock body = myMethod.getBody(); if (body != null) { body.accept(new PsiRecursiveElementVisitor() { + public void visitNewExpression(PsiNewExpression expression) { + if (MoveMethodUtil.isClassInstanceReference(expression, myMethod.getContainingClass())) { + usages.add(new InternalUsageInfo(expression)); + } + } + public void visitReferenceExpression(PsiReferenceExpression expression) { - if (!expression.isQualified()) { + if (MoveMethodUtil.isClassInstanceReference(expression, myMethod.getContainingClass())) { + usages.add(new InternalUsageInfo(expression)); + } else if (!expression.isQualified()) { final PsiElement resolved = expression.resolve(); if (myTargetVariable.equals(resolved)) { usages.add(new InternalUsageInfo(expression)); - } else if (resolved instanceof PsiMember && ((PsiMember)resolved).getContainingClass().equals(myMethod.getContainingClass())) { - usages.add(new InternalUsageInfo(expression)); } } @@ -192,7 +200,7 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ final PsiClass inheritor = ((InheritorUsageInfo)usage).getInheritor(); addMethodToClass(inheritor, patternMethod); } else if (usage instanceof MethodCallUsageInfo) { - correctMethodCall (((MethodCallUsageInfo)usage).getMethodCallExpression()); + correctMethodCall (((MethodCallUsageInfo)usage).getMethodCallExpression(), false); } else if (usage instanceof JavadocUsageInfo) { docRefs.add(usage.getElement().getReference()); } @@ -213,17 +221,23 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ } } - private void correctMethodCall(final PsiMethodCallExpression expression) { + private void correctMethodCall(final PsiMethodCallExpression expression, boolean isInternalCall) { try { final PsiManager manager = myMethod.getManager(); - PsiExpression qualifier = expression.getMethodExpression().getQualifierExpression(); - if (qualifier == null) qualifier = manager.getElementFactory().createExpressionFromText("this", null); + final PsiExpression oldQualifier = expression.getMethodExpression().getQualifierExpression(); PsiExpression newQualifier = null; if (myTargetVariable instanceof PsiParameter) { final int index = myMethod.getParameterList().getParameterIndex((PsiParameter)myTargetVariable); final PsiExpression[] arguments = expression.getArgumentList().getExpressions(); if (index < arguments.length) { - newQualifier = (PsiExpression)arguments[index].copy(); + if (isInternalCall && (oldQualifier == null || isTrivialThisQualifier(oldQualifier))) { + //See MoveInstanceMethodTest.testRecursive + LOG.assertTrue(myOldThisNeeded); + newQualifier = manager.getElementFactory().createExpressionFromText(myOldClassParameterName, null); + } + else { + newQualifier = (PsiExpression)arguments[index].copy(); + } arguments[index].delete(); } } else { @@ -231,11 +245,15 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ newQualifier = manager.getElementFactory().createExpressionFromText(myTargetVariable.getName(), null); } - expression.getArgumentList().add(qualifier); + if (myOldThisNeeded) { + PsiExpression qualifier = expression.getMethodExpression().getQualifierExpression(); + if (qualifier == null) qualifier = manager.getElementFactory().createExpressionFromText("this", null); + expression.getArgumentList().add(qualifier); + } + if (newQualifier != null) { - if ((newQualifier instanceof PsiThisExpression && ((PsiThisExpression)newQualifier).getQualifier() == null)) { + if (isTrivialThisQualifier(newQualifier)) { //Remove redundant 'this' qualifier - final PsiExpression oldQualifier = expression.getMethodExpression().getQualifierExpression(); if (oldQualifier != null) oldQualifier.delete(); } else { @@ -250,6 +268,10 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ } } + private boolean isTrivialThisQualifier(final PsiExpression newQualifier) { + return newQualifier instanceof PsiThisExpression && ((PsiThisExpression)newQualifier).getQualifier() == null; + } + private PsiMethod addMethodToClass(final PsiClass aClass, final PsiMethod patternMethod) { try { final PsiMethod method = (PsiMethod)aClass.add(patternMethod); @@ -279,18 +301,21 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ public void visitReferenceExpression(PsiReferenceExpression expression) { try { final PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier == null || qualifier instanceof PsiThisExpression) { + if (qualifier instanceof PsiReferenceExpression && ((PsiReferenceExpression)qualifier).isReferenceTo(variableCopy)) { + //Target is a field, replace target.m -> m + qualifier.delete(); + } else { final PsiElement resolved = expression.resolve(); if (variableCopy.equals(resolved)) { PsiThisExpression thisExpression = (PsiThisExpression)factory.createExpressionFromText("this", null); expression.replace(thisExpression); - } else if (resolved instanceof PsiMember && ((PsiMember)resolved).getContainingClass().equals(methodCopy.getContainingClass())) { + } else if (MoveMethodUtil.isClassInstanceReference(expression, methodCopy.getContainingClass())) { PsiReferenceExpression qualified = (PsiReferenceExpression)factory.createExpressionFromText(myOldClassParameterName + ".f", null); qualified.getReferenceNameElement().replace(expression.getReferenceNameElement()); expression.replace(qualified); } - } else if (qualifier instanceof PsiReferenceExpression && ((PsiReferenceExpression)qualifier).isReferenceTo(variableCopy)) { - qualifier.delete(); } + } + super.visitReferenceExpression(expression); } catch (IncorrectOperationException e) { @@ -298,10 +323,29 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ } } + public void visitNewExpression(PsiNewExpression expression) { + try { + final PsiExpression qualifier = expression.getQualifier(); + if (qualifier instanceof PsiReferenceExpression && ((PsiReferenceExpression)qualifier).isReferenceTo(variableCopy)) { + //Target is a field, replace target.new A() -> new A() + qualifier.delete(); + } else { + if (MoveMethodUtil.isClassInstanceReference(expression, methodCopy.getContainingClass())) { + if (qualifier != null) qualifier.delete(); + final PsiExpression newExpression = factory.createExpressionFromText(myOldClassParameterName + "." + expression.getText(), null); + expression.replace(newExpression); + } + } + super.visitNewExpression(expression); + } + catch (IncorrectOperationException e) { + LOG.error(e); + } + } public void visitMethodCallExpression(PsiMethodCallExpression expression) { if (expression.getMethodExpression().isReferenceTo(myMethod)) { - correctMethodCall(expression); + correctMethodCall(expression, true); } super.visitMethodCallExpression(expression); @@ -315,9 +359,11 @@ public class MoveInstanceMethodProcessor extends BaseRefactoringProcessor{ methodCopy.getParameterList().getParameters()[index].delete(); } - final PsiClassType type = factory.createType(myMethod.getContainingClass()); - final PsiParameter parameter = factory.createParameter(myOldClassParameterName, type); - methodCopy.getParameterList().add(parameter); + if (myOldThisNeeded) { + final PsiClassType type = factory.createType(myMethod.getContainingClass()); + final PsiParameter parameter = factory.createParameter(myOldClassParameterName, type); + methodCopy.getParameterList().add(parameter); + } final List newParameters = Arrays.asList(methodCopy.getParameterList().getParameters()); RefactoringUtil.fixJavadocsForParams(methodCopy, new HashSet(newParameters)); diff --git a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodViewDescriptor.java b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodViewDescriptor.java index 3725fdb745ff..3e621dce41e6 100644 --- a/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodViewDescriptor.java +++ b/source/com/intellij/refactoring/move/moveInstanceMethod/MoveInstanceMethodViewDescriptor.java @@ -31,7 +31,7 @@ public class MoveInstanceMethodViewDescriptor extends UsageViewDescriptorAdapter } public String getProcessedElementsHeader() { - return "Convert to instance method"; + return "Move instance method"; } public boolean canRefresh() { diff --git a/testData/refactoring/moveInstanceMethod/Javadoc.java b/testData/refactoring/moveInstanceMethod/Javadoc.java index 939179184b11..248d5ca77b91 100644 --- a/testData/refactoring/moveInstanceMethod/Javadoc.java +++ b/testData/refactoring/moveInstanceMethod/Javadoc.java @@ -7,6 +7,7 @@ public abstract class Test1 { * @param f */ void foo (Foreign f) { + bar(); } /** diff --git a/testData/refactoring/moveInstanceMethod/Javadoc.java.after b/testData/refactoring/moveInstanceMethod/Javadoc.java.after index 57696496dbe6..8170dbd3e1f2 100644 --- a/testData/refactoring/moveInstanceMethod/Javadoc.java.after +++ b/testData/refactoring/moveInstanceMethod/Javadoc.java.after @@ -3,6 +3,7 @@ class Foreign { * @param test1 */ void foo(Test1 test1) { + test1.bar(); } } diff --git a/testData/refactoring/moveInstanceMethod/Recursive.java b/testData/refactoring/moveInstanceMethod/Recursive.java index b6a09371e9ea..0e81ef52c9bc 100644 --- a/testData/refactoring/moveInstanceMethod/Recursive.java +++ b/testData/refactoring/moveInstanceMethod/Recursive.java @@ -1,5 +1,6 @@ public class MoveMethodTest { void foo (MoveMethodTest f) { + foo(f); f.foo(f); } } \ No newline at end of file diff --git a/testData/refactoring/moveInstanceMethod/Recursive.java.after b/testData/refactoring/moveInstanceMethod/Recursive.java.after index e815af618e7e..6d45cd0904d4 100644 --- a/testData/refactoring/moveInstanceMethod/Recursive.java.after +++ b/testData/refactoring/moveInstanceMethod/Recursive.java.after @@ -1,6 +1,7 @@ public class MoveMethodTest { void foo(MoveMethodTest moveMethodTest) { + moveMethodTest.foo(this); foo(this); } } \ No newline at end of file diff --git a/testData/refactoring/moveInstanceMethod/WithInner.java b/testData/refactoring/moveInstanceMethod/WithInner.java index 415ce95e2872..e0dc3d40ccf5 100644 --- a/testData/refactoring/moveInstanceMethod/WithInner.java +++ b/testData/refactoring/moveInstanceMethod/WithInner.java @@ -5,6 +5,7 @@ class Foreign { public abstract class Test1 { void foo (Foreign f, Inner i) { + new Inner(); } class Inner {} diff --git a/testData/refactoring/moveInstanceMethod/WithInner.java.after b/testData/refactoring/moveInstanceMethod/WithInner.java.after index efa97cfb8450..ac07170a6d38 100644 --- a/testData/refactoring/moveInstanceMethod/WithInner.java.after +++ b/testData/refactoring/moveInstanceMethod/WithInner.java.after @@ -1,5 +1,6 @@ class Foreign { void foo(Test1.Inner i, Test1 test1) { + test1.new Inner(); } class Inner {}