From 248ee0e44b8f8dd794e35ecc01d8997d9ae1264b Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Wed, 19 Oct 2016 23:58:46 +0300 Subject: [PATCH] PY-17265 Do not search for a movable function or a reference to it under the caret manually It was necessary when "Make ... Top-Level" was a general refactoring action, but now by MoveHandler's machinery takes care of it. --- .../move/PyMoveSymbolDelegate.java | 30 ++++------------ .../PyMakeFunctionTopLevelTest.java | 34 +++++++++++++++++-- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/python/src/com/jetbrains/python/refactoring/move/PyMoveSymbolDelegate.java b/python/src/com/jetbrains/python/refactoring/move/PyMoveSymbolDelegate.java index 13e78f63a9c4..cea549b75595 100644 --- a/python/src/com/jetbrains/python/refactoring/move/PyMoveSymbolDelegate.java +++ b/python/src/com/jetbrains/python/refactoring/move/PyMoveSymbolDelegate.java @@ -64,7 +64,7 @@ public class PyMoveSymbolDelegate extends MoveHandlerDelegate { return false; } // Local function or method - if (findMovableLocalFunctionOrMethod(elements[0]) != null) { + if (isMovableLocalFunctionOrMethod(elements[0])) { return true; } @@ -88,8 +88,8 @@ public class PyMoveSymbolDelegate extends MoveHandlerDelegate { } final BaseRefactoringProcessor processor; - final PyFunction function = findMovableLocalFunctionOrMethod(elements[0]); - if (function != null) { + if (isMovableLocalFunctionOrMethod(elements[0])) { + final PyFunction function = (PyFunction)elements[0]; final PyMakeFunctionTopLevelDialog dialog = new PyMakeFunctionTopLevelDialog(project, function, initialPath, initialPath); if (!dialog.showAndGet()) { return; @@ -156,7 +156,7 @@ public class PyMoveSymbolDelegate extends MoveHandlerDelegate { // Fallback to the old way to select single element to move final PsiNamedElement e = PyMoveModuleMembersHelper.extractNamedElement(element); if (e != null && PyMoveModuleMembersHelper.hasMovableElementType(e)) { - if (PyMoveModuleMembersHelper.isMovableModuleMember(e) || findMovableLocalFunctionOrMethod(e) != null) { + if (PyMoveModuleMembersHelper.isMovableModuleMember(e) || isMovableLocalFunctionOrMethod(e)) { doMove(project, new PsiElement[]{e}, targetContainer, null); } else { @@ -191,27 +191,11 @@ public class PyMoveSymbolDelegate extends MoveHandlerDelegate { } @VisibleForTesting - @Nullable - public static PyFunction findMovableLocalFunctionOrMethod(@NotNull PsiElement element) { - if (isLocalFunction(element) || isSuitableInstanceMethod(element)) { - return (PyFunction)element; - } - // e.g. caret is on "def" keyword - if (isLocalFunction(element.getParent()) || isSuitableInstanceMethod(element.getParent())) { - return (PyFunction)element.getParent(); - } - final PyReferenceExpression refExpr = PsiTreeUtil.getParentOfType(element, PyReferenceExpression.class); - if (refExpr == null) { - return null; - } - final PsiElement resolved = refExpr.getReference().resolve(); - if (isLocalFunction(resolved) || isSuitableInstanceMethod(resolved)) { - return (PyFunction)resolved; - } - return null; + public static boolean isMovableLocalFunctionOrMethod(@NotNull PsiElement element) { + return isLocalFunction(element) || isSuitableInstanceMethod(element); } - public static boolean isSuitableInstanceMethod(@Nullable PsiElement element) { + private static boolean isSuitableInstanceMethod(@Nullable PsiElement element) { final PyFunction function = as(element, PyFunction.class); if (function == null || function.getContainingClass() == null) { return false; diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyMakeFunctionTopLevelTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyMakeFunctionTopLevelTest.java index 753dc5230257..f3f5b9c6f179 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyMakeFunctionTopLevelTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyMakeFunctionTopLevelTest.java @@ -17,11 +17,17 @@ package com.jetbrains.python.refactoring; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.roots.ModuleRootManager; +import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiReference; +import com.intellij.refactoring.RefactoringBundle; +import com.intellij.refactoring.actions.MoveAction; +import com.intellij.refactoring.util.CommonRefactoringUtil; import com.intellij.testFramework.PlatformTestUtil; +import com.intellij.testFramework.TestActionEvent; import com.intellij.util.IncorrectOperationException; import com.jetbrains.python.PyBundle; import com.jetbrains.python.PyTokenTypes; @@ -99,8 +105,30 @@ public class PyMakeFunctionTopLevelTest extends PyTestCase { } private boolean isActionEnabled() { - final PsiElement elementUnderCaret = myFixture.getFile().findElementAt(myFixture.getCaretOffset()); - return PyMoveSymbolDelegate.findMovableLocalFunctionOrMethod(elementUnderCaret) != null; + final int offset = myFixture.getCaretOffset(); + PsiElement element = myFixture.getFile().findElementAt(offset); + + // Duplicates the logic of MoveHandler#invoke(), since it doesn't allow to check + // whether MoveHandlerDelegate#tryMove() returns true or false without actually + // invoking it. + while (element != null) { + if (PyMoveSymbolDelegate.isMovableLocalFunctionOrMethod(element)) { + return true; + } + final TextRange range = element.getTextRange(); + if (range != null) { + final int relative = offset - range.getStartOffset(); + final PsiReference reference = element.findReferenceAt(relative); + if (reference != null) { + final PsiElement refElement = reference.resolve(); + if (refElement != null && PyMoveSymbolDelegate.isMovableLocalFunctionOrMethod(myFixture.getElementAtCaret())) { + return true; + } + } + } + element = element.getParent(); + } + return false; } // PY-6637 @@ -122,7 +150,7 @@ public class PyMakeFunctionTopLevelTest extends PyTestCase { myFixture.getEditor().getCaretModel().moveCaretRelatively(-3, 0, false, false, false); final PsiElement tokenAtCaret = file.findElementAt(myFixture.getCaretOffset()); assertNotNull(tokenAtCaret); - assertEquals(tokenAtCaret.getNode().getElementType(), PyTokenTypes.DEF_KEYWORD); + assertEquals(PyTokenTypes.DEF_KEYWORD, tokenAtCaret.getNode().getElementType()); assertTrue(isActionEnabled()); moveByText("method");