#KT-22721: Fix move refactoring to make it check previous PsiElement before a whitespace

This commit is contained in:
Vadim Brilyantov
2019-02-01 15:32:09 +03:00
parent e413395cbf
commit 01200252f5
@@ -20,10 +20,7 @@ import com.intellij.openapi.actionSystem.DataContext
import com.intellij.openapi.actionSystem.LangDataKeys import com.intellij.openapi.actionSystem.LangDataKeys
import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.Editor
import com.intellij.openapi.project.Project import com.intellij.openapi.project.Project
import com.intellij.psi.PsiDirectory import com.intellij.psi.*
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiPackage
import com.intellij.psi.PsiReference
import com.intellij.refactoring.JavaRefactoringSettings import com.intellij.refactoring.JavaRefactoringSettings
import com.intellij.refactoring.RefactoringBundle import com.intellij.refactoring.RefactoringBundle
import com.intellij.refactoring.move.MoveCallback import com.intellij.refactoring.move.MoveCallback
@@ -69,14 +66,14 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
private fun KtNamedDeclaration.canMove() = if (this is KtClassOrObject) !isLocal else isTopLevelInFileOrScript(this) private fun KtNamedDeclaration.canMove() = if (this is KtClassOrObject) !isLocal else isTopLevelInFileOrScript(this)
private fun doMoveWithCheck( private fun doMoveWithCheck(
project: Project, elements: Array<out PsiElement>, targetContainer: PsiElement?, callback: MoveCallback?, editor: Editor? project: Project, elements: Array<out PsiElement>, targetContainer: PsiElement?, callback: MoveCallback?, editor: Editor?
): Boolean { ): Boolean {
if (!CommonRefactoringUtil.checkReadOnlyStatusRecursively(project, elements.toList(), true)) return false if (!CommonRefactoringUtil.checkReadOnlyStatusRecursively(project, elements.toList(), true)) return false
val container = getUniqueContainer(elements) val container = getUniqueContainer(elements)
if (container == null) { if (container == null) {
CommonRefactoringUtil.showErrorHint( CommonRefactoringUtil.showErrorHint(
project, editor, "All declarations must belong to the same directory or class", MOVE_DECLARATIONS, null project, editor, "All declarations must belong to the same directory or class", MOVE_DECLARATIONS, null
) )
return false return false
} }
@@ -97,7 +94,8 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
} }
if (elementsToSearch.any { !it.canMove() }) { if (elementsToSearch.any { !it.canMove() }) {
val message = RefactoringBundle.getCannotRefactorMessage("Move declaration is only supported for top-level declarations and nested classes") val message =
RefactoringBundle.getCannotRefactorMessage("Move declaration is only supported for top-level declarations and nested classes")
CommonRefactoringUtil.showErrorHint(project, editor, message, MOVE_DECLARATIONS, null) CommonRefactoringUtil.showErrorHint(project, editor, message, MOVE_DECLARATIONS, null)
return true return true
} }
@@ -129,15 +127,22 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
val targetPackageName = MoveClassesOrPackagesImpl.getInitialTargetPackageName(targetContainer, elements) val targetPackageName = MoveClassesOrPackagesImpl.getInitialTargetPackageName(targetContainer, elements)
val targetDirectory = if (targetContainer != null) { val targetDirectory = if (targetContainer != null) {
MoveClassesOrPackagesImpl.getInitialTargetDirectory(targetContainer, elements) MoveClassesOrPackagesImpl.getInitialTargetDirectory(targetContainer, elements)
} } else null
else null
val searchInComments = JavaRefactoringSettings.getInstance()!!.MOVE_SEARCH_IN_COMMENTS val searchInComments = JavaRefactoringSettings.getInstance()!!.MOVE_SEARCH_IN_COMMENTS
val searchInText = JavaRefactoringSettings.getInstance()!!.MOVE_SEARCH_FOR_TEXT val searchInText = JavaRefactoringSettings.getInstance()!!.MOVE_SEARCH_FOR_TEXT
val targetFile = targetContainer as? KtFile val targetFile = targetContainer as? KtFile
val moveToPackage = targetContainer !is KtFile val moveToPackage = targetContainer !is KtFile
MoveKotlinTopLevelDeclarationsDialog( MoveKotlinTopLevelDeclarationsDialog(
project, elementsToSearch, targetPackageName, targetDirectory, targetFile, moveToPackage, searchInComments, searchInText, callback project,
elementsToSearch,
targetPackageName,
targetDirectory,
targetFile,
moveToPackage,
searchInComments,
searchInText,
callback
).show() ).show()
} }
@@ -145,20 +150,25 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
if (elementsToSearch.size > 1) { if (elementsToSearch.size > 1) {
// todo: allow moving multiple classes to upper level // todo: allow moving multiple classes to upper level
if (targetContainer !is KtClassOrObject) { if (targetContainer !is KtClassOrObject) {
val message = RefactoringBundle.getCannotRefactorMessage("Moving multiple nested classes to top-level is not supported") val message =
RefactoringBundle.getCannotRefactorMessage("Moving multiple nested classes to top-level is not supported")
CommonRefactoringUtil.showErrorHint(project, editor, message, MOVE_DECLARATIONS, null) CommonRefactoringUtil.showErrorHint(project, editor, message, MOVE_DECLARATIONS, null)
return true return true
} }
@Suppress("UNCHECKED_CAST") @Suppress("UNCHECKED_CAST")
MoveKotlinNestedClassesDialog(project, MoveKotlinNestedClassesDialog(
elementsToSearch.filterIsInstance<KtClassOrObject>(), project,
container, elementsToSearch.filterIsInstance<KtClassOrObject>(),
targetContainer, container,
callback).show() targetContainer,
callback
).show()
return true return true
} }
KotlinSelectNestedClassRefactoringDialog.chooseNestedClassRefactoring(elementsToSearch.first() as KtClassOrObject, KotlinSelectNestedClassRefactoringDialog.chooseNestedClassRefactoring(
targetContainer) elementsToSearch.first() as KtClassOrObject,
targetContainer
)
} }
else -> throw AssertionError("Unexpected container: ${container.getElementTextWithContext()}") else -> throw AssertionError("Unexpected container: ${container.getElementTextWithContext()}")
@@ -188,17 +198,32 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
} }
} }
private fun tryToMoveImpl(
element: PsiElement, project: Project, dataContext: DataContext?, reference: PsiReference?, editor: Editor?
): Boolean {
val elementsToMove = element.unwrapped?.let { arrayOf(it) } ?: PsiElement.EMPTY_ARRAY
val targetContainer = dataContext?.let { dataContext -> LangDataKeys.TARGET_PSI_ELEMENT.getData(dataContext) }
return canMove(elementsToMove, targetContainer, true) && doMoveWithCheck(project, elementsToMove, targetContainer, null, editor)
}
private fun recursivelyTryToMove(
element: PsiElement, project: Project, dataContext: DataContext?, reference: PsiReference?, editor: Editor?
): Boolean {
return tryToMoveImpl(element, project, dataContext, reference, editor)
|| element.parent?.let { recursivelyTryToMove(it, project, dataContext, reference, editor) } ?: false
}
override fun canMove(elements: Array<out PsiElement>, targetContainer: PsiElement?): Boolean { override fun canMove(elements: Array<out PsiElement>, targetContainer: PsiElement?): Boolean {
return canMove(elements, targetContainer, false) return canMove(elements, targetContainer, false)
} }
override fun isValidTarget(psiElement: PsiElement?, sources: Array<out PsiElement>): Boolean { override fun isValidTarget(psiElement: PsiElement?, sources: Array<out PsiElement>): Boolean {
return psiElement is PsiPackage return psiElement is PsiPackage
|| (psiElement is PsiDirectory && psiElement.getPackage() != null) || (psiElement is PsiDirectory && psiElement.getPackage() != null)
|| psiElement is KtFile || psiElement is KtFile
|| (psiElement is KtClassOrObject || (psiElement is KtClassOrObject
&& !(psiElement.hasModifier(KtTokens.ANNOTATION_KEYWORD)) && !(psiElement.hasModifier(KtTokens.ANNOTATION_KEYWORD))
&& !sources.any { it.parent is KtFile }) && !sources.any { it.parent is KtFile })
} }
override fun doMove(project: Project, elements: Array<out PsiElement>, targetContainer: PsiElement?, callback: MoveCallback?) { override fun doMove(project: Project, elements: Array<out PsiElement>, targetContainer: PsiElement?, callback: MoveCallback?) {
@@ -206,11 +231,14 @@ class MoveKotlinDeclarationsHandler : MoveHandlerDelegate() {
} }
override fun tryToMove( override fun tryToMove(
element: PsiElement, project: Project, dataContext: DataContext?, reference: PsiReference?, editor: Editor? element: PsiElement, project: Project, dataContext: DataContext?, reference: PsiReference?, editor: Editor?
): Boolean { ): Boolean {
val elementsToMove = element.unwrapped?.let { arrayOf(it) } ?: PsiElement.EMPTY_ARRAY if (element is PsiWhiteSpace && element.textOffset > 0) {
val targetContainer = dataContext?.let { dataContext -> LangDataKeys.TARGET_PSI_ELEMENT.getData(dataContext) } val prevElement = element.containingFile.findElementAt(element.textOffset - 1)
return canMove(elementsToMove, targetContainer, true) && doMoveWithCheck(project, elementsToMove, targetContainer, null, editor) return prevElement != null
&& recursivelyTryToMove(prevElement, project, dataContext, reference, editor)
}
return tryToMoveImpl(element, project, dataContext, reference, editor)
} }
} }