From 16dbc6fb4d06c0bffa586d9dbb467d8bb1a37b3e Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 7 Jun 2018 14:57:49 +0300 Subject: [PATCH] Extract Function: Forbid inappropriate declarations inside extractable fragment #KT-20467 Fixed #KT-20469 Fixed --- .../ExtractKotlinFunctionHandler.kt | 5 +++-- .../KotlinIntroduceParameterHandler.kt | 1 + .../KotlinIntroducePropertyHandler.kt | 2 ++ .../IntroduceTypeAliasHandler.kt | 1 + .../KotlinIntroduceTypeParameterHandler.kt | 1 + .../idea/refactoring/introduce/introduceUtil.kt | 16 +++++++++++++++- .../extractFunction/basic/selectedConstructor.kt | 4 ++++ .../basic/selectedConstructor.kt.conflicts | 1 + .../extractFunction/basic/selectedParameter.kt | 2 ++ .../basic/selectedParameter.kt.conflicts | 1 + .../script/NotExpression.kts.conflicts | 2 +- .../script/TopLevelExpression.kts.conflicts | 2 +- .../introduce/ExtractionTestGenerated.java | 10 ++++++++++ 13 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt create mode 100644 idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt.conflicts create mode 100644 idea/testData/refactoring/extractFunction/basic/selectedParameter.kt create mode 100644 idea/testData/refactoring/extractFunction/basic/selectedParameter.kt.conflicts diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractFunction/ExtractKotlinFunctionHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractFunction/ExtractKotlinFunctionHandler.kt index adcdca1ee45..05d53869580 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractFunction/ExtractKotlinFunctionHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/extractFunction/ExtractKotlinFunctionHandler.kt @@ -28,9 +28,9 @@ import org.jetbrains.kotlin.idea.refactoring.getExtractionContainers import org.jetbrains.kotlin.idea.refactoring.introduce.extractFunction.ui.KotlinExtractFunctionDialog import org.jetbrains.kotlin.idea.refactoring.introduce.extractionEngine.* import org.jetbrains.kotlin.idea.refactoring.introduce.selectElementsWithTargetSibling +import org.jetbrains.kotlin.idea.refactoring.introduce.validateExpressionElements import org.jetbrains.kotlin.idea.util.psi.patternMatching.toRange -import org.jetbrains.kotlin.psi.KtBlockExpression -import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.* class ExtractKotlinFunctionHandler( private val allContainersEnabled: Boolean = false, @@ -69,6 +69,7 @@ class ExtractKotlinFunctionHandler( file, "Select target code block", listOf(CodeInsightUtils.ElementKind.EXPRESSION), + ::validateExpressionElements, { elements, parent -> parent.getExtractionContainers(elements.size == 1, allContainersEnabled) }, continuation ) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt index b8de6938381..709275ce764 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceParameter/KotlinIntroduceParameterHandler.kt @@ -187,6 +187,7 @@ fun selectNewParameterContext( file = file, title = "Introduce parameter to declaration", elementKinds = listOf(CodeInsightUtils.ElementKind.EXPRESSION), + elementValidator = ::validateExpressionElements, getContainers = { _, parent -> val parents = parent.parents val stopAt = (parent.parents.zip(parent.parents.drop(1))) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceProperty/KotlinIntroducePropertyHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceProperty/KotlinIntroducePropertyHandler.kt index 8a082a1b4b8..1521fefea33 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceProperty/KotlinIntroducePropertyHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceProperty/KotlinIntroducePropertyHandler.kt @@ -31,6 +31,7 @@ import org.jetbrains.kotlin.idea.refactoring.introduce.extractionEngine.* import org.jetbrains.kotlin.idea.refactoring.introduce.selectElementsWithTargetSibling import org.jetbrains.kotlin.idea.refactoring.introduce.showErrorHint import org.jetbrains.kotlin.idea.refactoring.introduce.showErrorHintByKey +import org.jetbrains.kotlin.idea.refactoring.introduce.validateExpressionElements import org.jetbrains.kotlin.idea.util.psi.patternMatching.toRange import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtClassBody @@ -73,6 +74,7 @@ class KotlinIntroducePropertyHandler( file, "Select target code block", listOf(CodeInsightUtils.ElementKind.EXPRESSION), + ::validateExpressionElements, { _, parent -> parent.getExtractionContainers(strict = true, includeAll = true).filter { it is KtClassBody || (it is KtFile && !it.isScript()) } }, diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeAlias/IntroduceTypeAliasHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeAlias/IntroduceTypeAliasHandler.kt index 92e822bdd57..fb5086fe257 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeAlias/IntroduceTypeAliasHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeAlias/IntroduceTypeAliasHandler.kt @@ -55,6 +55,7 @@ open class KotlinIntroduceTypeAliasHandler : RefactoringActionHandler { file, "Select target code block", listOf(TYPE_ELEMENT, TYPE_CONSTRUCTOR), + { null }, { _, parent -> listOf(parent.containingFile) }, continuation ) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeParameter/KotlinIntroduceTypeParameterHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeParameter/KotlinIntroduceTypeParameterHandler.kt index 65d6dd230c3..4e73a91aa8b 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeParameter/KotlinIntroduceTypeParameterHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceTypeParameter/KotlinIntroduceTypeParameterHandler.kt @@ -74,6 +74,7 @@ object KotlinIntroduceTypeParameterHandler : RefactoringActionHandler { file, "Introduce type parameter to declaration", listOf(CodeInsightUtils.ElementKind.TYPE_ELEMENT), + { null }, { _, parent -> getPossibleTypeParameterContainers(parent) }, continuation ) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt index f568bc647e0..7bb768b5a23 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/introduce/introduceUtil.kt @@ -47,6 +47,7 @@ fun selectElementsWithTargetSibling( file: KtFile, title: String, elementKinds: Collection, + elementValidator: (List) -> String?, getContainers: (elements: List, commonParent: PsiElement) -> List, continuation: (elements: List, targetSibling: PsiElement) -> Unit ) { @@ -69,7 +70,7 @@ fun selectElementsWithTargetSibling( continuation(elements, outermostParent) } - selectElementsWithTargetParent(operationName, editor, file, title, elementKinds, getContainers, ::onSelectionComplete) + selectElementsWithTargetParent(operationName, editor, file, title, elementKinds, elementValidator, getContainers, ::onSelectionComplete) } fun selectElementsWithTargetParent( @@ -78,6 +79,7 @@ fun selectElementsWithTargetParent( file: KtFile, title: String, elementKinds: Collection, + elementValidator: (List) -> String?, getContainers: (elements: List, commonParent: PsiElement) -> List, continuation: (elements: List, targetParent: PsiElement) -> Unit ) { @@ -86,6 +88,11 @@ fun selectElementsWithTargetParent( } fun selectTargetContainer(elements: List) { + elementValidator(elements)?.let { + showErrorHint(file.project, editor, it, operationName) + return + } + val physicalElements = elements.map { it.substringContextOrThis } val parent = PsiTreeUtil.findCommonParent(physicalElements) ?: throw AssertionError("Should have at least one parent: ${physicalElements.joinToString("\n")}") @@ -236,4 +243,11 @@ fun insertDeclaration(declaration: T, targetSibling: PsiElem return (targetContainer.addBefore(declaration, anchor) as T).apply { targetContainer.addBefore(KtPsiFactory(declaration).createWhiteSpace("\n\n"), anchor) } +} + +internal fun validateExpressionElements(elements: List): String? { + if (elements.any { it is KtConstructor<*> || it is KtParameter || it is KtTypeAlias || it is KtPropertyAccessor }) { + return "Refactoring is not applicable to this code fragment" + } + return null } \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt b/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt new file mode 100644 index 00000000000..95088e6f0ba --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt @@ -0,0 +1,4 @@ +// SIBLING: +class Foo { + constructor(bar: Int) +} \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt.conflicts new file mode 100644 index 00000000000..256a14cd328 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt.conflicts @@ -0,0 +1 @@ +Refactoring is not applicable to this code fragment \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt b/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt new file mode 100644 index 00000000000..a247dee06a7 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt @@ -0,0 +1,2 @@ +// SIBLING: +class Foo(bar: String = "baz") \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt.conflicts b/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt.conflicts new file mode 100644 index 00000000000..256a14cd328 --- /dev/null +++ b/idea/testData/refactoring/extractFunction/basic/selectedParameter.kt.conflicts @@ -0,0 +1 @@ +Refactoring is not applicable to this code fragment \ No newline at end of file diff --git a/idea/testData/refactoring/extractFunction/script/NotExpression.kts.conflicts b/idea/testData/refactoring/extractFunction/script/NotExpression.kts.conflicts index 1568c6aea62..dee11733077 100644 --- a/idea/testData/refactoring/extractFunction/script/NotExpression.kts.conflicts +++ b/idea/testData/refactoring/extractFunction/script/NotExpression.kts.conflicts @@ -1 +1 @@ -Cannot refactor in this place +Cannot refactor in this place \ No newline at end of file diff --git a/idea/testData/refactoring/introduceParameter/script/TopLevelExpression.kts.conflicts b/idea/testData/refactoring/introduceParameter/script/TopLevelExpression.kts.conflicts index 1568c6aea62..dee11733077 100644 --- a/idea/testData/refactoring/introduceParameter/script/TopLevelExpression.kts.conflicts +++ b/idea/testData/refactoring/introduceParameter/script/TopLevelExpression.kts.conflicts @@ -1 +1 @@ -Cannot refactor in this place +Cannot refactor in this place \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java index c551da6f301..8a8c916d190 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/introduce/ExtractionTestGenerated.java @@ -986,6 +986,16 @@ public class ExtractionTestGenerated extends AbstractExtractionTest { runTest("idea/testData/refactoring/extractFunction/basic/refInReturn.kt"); } + @TestMetadata("selectedConstructor.kt") + public void testSelectedConstructor() throws Exception { + runTest("idea/testData/refactoring/extractFunction/basic/selectedConstructor.kt"); + } + + @TestMetadata("selectedParameter.kt") + public void testSelectedParameter() throws Exception { + runTest("idea/testData/refactoring/extractFunction/basic/selectedParameter.kt"); + } + @TestMetadata("suspendCall.kt") public void testSuspendCall() throws Exception { runTest("idea/testData/refactoring/extractFunction/basic/suspendCall.kt");