From 1d36c495376c09873731e67a3d4c2e4b5f397dbd Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 8 Jun 2017 19:35:19 +0300 Subject: [PATCH] Copy: Copy elements and process usages under the same write action In some cases doing these operations in separate write actions may lead to invalidation of copied elements #KT-18149 Fixed --- .../copy/CopyKotlinDeclarationsHandler.kt | 20 +- .../kt18149/after/refactor/ParentJava.java | 7 + .../copy/kt18149/after/refactor/copy/test.kt | 178 ++++++++++++++++++ .../copy/kt18149/after/refactor/copy2/test.kt | 178 ++++++++++++++++++ .../kt18149/before/refactor/ParentJava.java | 7 + .../copy/kt18149/before/refactor/copy/test.kt | 178 ++++++++++++++++++ .../refactoring/copy/kt18149/kt18149.test | 5 + .../refactoring/copy/CopyTestGenerated.java | 6 + 8 files changed, 570 insertions(+), 9 deletions(-) create mode 100644 idea/testData/refactoring/copy/kt18149/after/refactor/ParentJava.java create mode 100644 idea/testData/refactoring/copy/kt18149/after/refactor/copy/test.kt create mode 100644 idea/testData/refactoring/copy/kt18149/after/refactor/copy2/test.kt create mode 100644 idea/testData/refactoring/copy/kt18149/before/refactor/ParentJava.java create mode 100644 idea/testData/refactoring/copy/kt18149/before/refactor/copy/test.kt create mode 100644 idea/testData/refactoring/copy/kt18149/kt18149.test diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationsHandler.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationsHandler.kt index b815ca953b7..241b8499821 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationsHandler.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/copy/CopyKotlinDeclarationsHandler.kt @@ -245,23 +245,25 @@ class CopyKotlinDeclarationsHandler : CopyHandlerDelegateBase() { val targetFile: KtFile if (singleElementToCopy is KtFile) { - targetFile = runWriteAction { targetDirectory.copyFileFrom(targetFileName, singleElementToCopy) as KtFile } + targetFile = runWriteAction { + val copiedFile = targetDirectory.copyFileFrom(targetFileName, singleElementToCopy) as KtFile + performDelayedRefactoringRequests(project) + copiedFile + } } else { targetFile = getOrCreateTargetFile(originalFile, targetDirectory, targetFileName, commandName) ?: return@executeCommand runWriteAction { val newElements = elementsToCopy.map { targetFile.add(it.copy()) as KtNamedDeclaration } elementsToCopy.zip(newElements).toMap(oldToNewElementsMapping) - } - } - runWriteAction { - for (newElement in oldToNewElementsMapping.values) { - restoredInternalUsages += restoreInternalUsages(newElement as KtElement, oldToNewElementsMapping, true) - postProcessMoveUsages(restoredInternalUsages, oldToNewElementsMapping) - } + for (newElement in oldToNewElementsMapping.values) { + restoredInternalUsages += restoreInternalUsages(newElement as KtElement, oldToNewElementsMapping, true) + postProcessMoveUsages(restoredInternalUsages, oldToNewElementsMapping) + } - performDelayedRefactoringRequests(project) + performDelayedRefactoringRequests(project) + } } (oldToNewElementsMapping.values.singleOrNull() as? KtNamedDeclaration)?.let { newDeclaration -> diff --git a/idea/testData/refactoring/copy/kt18149/after/refactor/ParentJava.java b/idea/testData/refactoring/copy/kt18149/after/refactor/ParentJava.java new file mode 100644 index 00000000000..166ad8d506d --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/after/refactor/ParentJava.java @@ -0,0 +1,7 @@ +package refactor; + +public class ParentJava { + public interface FaceJava { + void inherit(); + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/copy/kt18149/after/refactor/copy/test.kt b/idea/testData/refactoring/copy/kt18149/after/refactor/copy/test.kt new file mode 100644 index 00000000000..7454609a8be --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/after/refactor/copy/test.kt @@ -0,0 +1,178 @@ +package refactor.copy + +import refactor.ParentJava +import kotlin.properties.Delegates + +enum class Possible { + NO, YES +} +data class Potable(val p1: String) +class Insider(val peace: String) + +class Init { + fun referred() = 0 + fun moved() = referred() +} + +interface FaceToFace { + fun extract() +} + +abstract class AbsToAbs { + abstract fun extract() +} + +class Simple { + fun extract() {} +} + +annotation class MemAnn + +class Variety { + // object + object ExtractedObject {} + // function + tailrec fun tailrecFun(p: Int) { if (p > 0) tailrecFun(p - 1) } + operator fun plus(other: Variety) {} + infix fun infixFun(other: Variety) {} + inline fun inlineFun(f: () -> Int) = f() + external fun externalFun() + internal fun internalFun() = 0 + protected fun protectedFun() = 0 + private fun privateFun() = 0 + fun genFunB(p: T): T = p + fun genFunC(p: T): C where T : C = p + @MemAnn fun annotatedFun() = 0 + final fun finalFun() = 0 + // property + var publicProp = 0 + internal var internalProp = 0 + protected var protectedProp = 0 + private var privateProp = 0 + var List.genVarJ: T + get() = last() + set(p) {} + var List.genVarL: T where T : C + get() = last() + set(p) {} + @MemAnn val annotatedVal = 0 + var byVar by Delegates.notNull() + lateinit var lateVal: String + final val finalVal = 0 + // class + class ExtractedClass {} + // inner class + inner class ExtractedInnerClass {} + // anonymousInitializer + init {} + // secondaryConstructor + constructor(p: Int) + + fun refer(p1: List) { + val v1 = ExtractedObject + val v2 = ExtractedClass() + val v3 = ExtractedInnerClass() + val v4 = tailrecFun(2) + val v5 = this + this + val v6 = this infixFun this + val v7 = inlineFun { 7 } + val vv = externalFun() + val v8 = internalFun() + val v9 = protectedFun() + val vA = privateFun() + val vB = genFunB(0) + val vD = annotatedFun() + val vw = finalFun() + publicProp = publicProp + 1 + internalProp = internalProp + 1 + protectedProp = protectedProp + 1 + privateProp = privateProp + 1 + p1.genVarJ = p1.genVarJ + "+" + val vK = annotatedVal + byVar = byVar + 1 + lateVal = lateVal + "+" + val vN = finalVal + } +} + +fun Variety.extend() {} + +object VarietyObject { + const val constProp = 0 + + fun refer() { + val vE = constProp + } +} + +abstract class AbstractVariety { + abstract fun abstractFun(): Int + abstract var abstractVar: Int + fun refer() = abstractFun() + abstractVar +} + +interface FaceVarietyObligation { + fun abstractToAbstract() + fun abstractToConcrete() + fun concreteToConcrete() +} + +interface FaceVariety : FaceVarietyObligation { + fun abstractFun() + var abstractProperty: String + fun concreteFun() {} + override fun abstractToAbstract() + override fun abstractToConcrete() {} + override fun concreteToConcrete() {} +} + +@Suppress("LeakingThis") +open class CtorParameter(pp: String, open val pv: String, open var pr: String) { + var vb = pp + pv + pr + init { vb += pp + pv + pr } + constructor() : this("p", "v", "r") + fun refer() { + pr += pv + vb += pr + } + companion object { + val instance = CtorParameter() + } +} + +class CtorParameterChild(val pvc: String, var prc: String) : CtorParameter(pvc, pvc, prc) +class CtorParameterChild2: CtorParameter { + constructor() : super("", "", "") +} +class CtorParameterChild3(override val pv: String, override var pr: String) : CtorParameter(pv, pv, pr) +data class CtorData(val pv: String, var pr: String) {} + +class Company { + companion object { + val companyVal = 0 + var companyVar = 0 + fun companyFun() = 0 + class CompanyClass {} + object CompanyObject {} + } + + fun refer() { + println(companyVal) + companyVar = companyVar + 1 + println(companyFun()) + val v1 = CompanyClass() + val v2 = CompanyObject + } +} + +fun referCompany() { + println(Company.companyVal) + Company.companyVar = Company.companyVar + 1 + println(Company.companyFun()) + val v1 = Company.Companion.CompanyClass() + val v2 = Company.Companion.CompanyObject +} + +class JavaChild : ParentJava.FaceJava { + override fun inherit() {} +} \ No newline at end of file diff --git a/idea/testData/refactoring/copy/kt18149/after/refactor/copy2/test.kt b/idea/testData/refactoring/copy/kt18149/after/refactor/copy2/test.kt new file mode 100644 index 00000000000..fe483c9b2ed --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/after/refactor/copy2/test.kt @@ -0,0 +1,178 @@ +package refactor.copy2 + +import refactor.ParentJava +import refactor.copy.Company +import kotlin.properties.Delegates + +enum class Possible { + NO, YES +} + +data class Potable(val p1: String) +class Insider(val peace: String) +class Init { + fun referred() = 0 + fun moved() = referred() +} + +interface FaceToFace { + fun extract() +} + +abstract class AbsToAbs { + abstract fun extract() +} + +class Simple { + fun extract() {} +} + +annotation class MemAnn +class Variety { + // object + object ExtractedObject {} + // function + tailrec fun tailrecFun(p: Int) { if (p > 0) tailrecFun(p - 1) } + operator fun plus(other: Variety) {} + infix fun infixFun(other: Variety) {} + inline fun inlineFun(f: () -> Int) = f() + external fun externalFun() + internal fun internalFun() = 0 + protected fun protectedFun() = 0 + private fun privateFun() = 0 + fun genFunB(p: T): T = p + fun genFunC(p: T): C where T : C = p + @MemAnn fun annotatedFun() = 0 + final fun finalFun() = 0 + // property + var publicProp = 0 + internal var internalProp = 0 + protected var protectedProp = 0 + private var privateProp = 0 + var List.genVarJ: T + get() = last() + set(p) {} + var List.genVarL: T where T : C + get() = last() + set(p) {} + @MemAnn val annotatedVal = 0 + var byVar by Delegates.notNull() + lateinit var lateVal: String + final val finalVal = 0 + // class + class ExtractedClass {} + // inner class + inner class ExtractedInnerClass {} + // anonymousInitializer + init {} + // secondaryConstructor + constructor(p: Int) + + fun refer(p1: List) { + val v1 = ExtractedObject + val v2 = ExtractedClass() + val v3 = ExtractedInnerClass() + val v4 = tailrecFun(2) + val v5 = this + this + val v6 = this infixFun this + val v7 = inlineFun { 7 } + val vv = externalFun() + val v8 = internalFun() + val v9 = protectedFun() + val vA = privateFun() + val vB = genFunB(0) + val vD = annotatedFun() + val vw = finalFun() + publicProp = publicProp + 1 + internalProp = internalProp + 1 + protectedProp = protectedProp + 1 + privateProp = privateProp + 1 + p1.genVarJ = p1.genVarJ + "+" + val vK = annotatedVal + byVar = byVar + 1 + lateVal = lateVal + "+" + val vN = finalVal + } +} + +fun Variety.extend() {} + +object VarietyObject { + const val constProp = 0 + + fun refer() { + val vE = constProp + } +} + +abstract class AbstractVariety { + abstract fun abstractFun(): Int + abstract var abstractVar: Int + fun refer() = abstractFun() + abstractVar +} + +interface FaceVarietyObligation { + fun abstractToAbstract() + fun abstractToConcrete() + fun concreteToConcrete() +} + +interface FaceVariety : FaceVarietyObligation { + fun abstractFun() + var abstractProperty: String + fun concreteFun() {} + override fun abstractToAbstract() + override fun abstractToConcrete() {} + override fun concreteToConcrete() {} +} + +@Suppress("LeakingThis") +open class CtorParameter(pp: String, open val pv: String, open var pr: String) { + var vb = pp + pv + pr + init { vb += pp + pv + pr } + constructor() : this("p", "v", "r") + fun refer() { + pr += pv + vb += pr + } + companion object { + val instance = CtorParameter() + } +} + +class CtorParameterChild(val pvc: String, var prc: String) : CtorParameter(pvc, pvc, prc) +class CtorParameterChild2: CtorParameter { + constructor() : super("", "", "") +} + +class CtorParameterChild3(override val pv: String, override var pr: String) : CtorParameter(pv, pv, pr) +data class CtorData(val pv: String, var pr: String) {} +class Company { + companion object { + val companyVal = 0 + var companyVar = 0 + fun companyFun() = 0 + class CompanyClass {} + object CompanyObject {} + } + + fun refer() { + println(companyVal) + companyVar = companyVar + 1 + println(companyFun()) + val v1 = CompanyClass() + val v2 = CompanyObject + } +} + +fun referCompany() { + println(Company.companyVal) + Company.companyVar = Company.companyVar + 1 + println(Company.companyFun()) + val v1 = Company.Companion.CompanyClass() + val v2 = Company.Companion.CompanyObject +} + +class JavaChild : ParentJava.FaceJava { + override fun inherit() {} +} \ No newline at end of file diff --git a/idea/testData/refactoring/copy/kt18149/before/refactor/ParentJava.java b/idea/testData/refactoring/copy/kt18149/before/refactor/ParentJava.java new file mode 100644 index 00000000000..166ad8d506d --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/before/refactor/ParentJava.java @@ -0,0 +1,7 @@ +package refactor; + +public class ParentJava { + public interface FaceJava { + void inherit(); + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/copy/kt18149/before/refactor/copy/test.kt b/idea/testData/refactoring/copy/kt18149/before/refactor/copy/test.kt new file mode 100644 index 00000000000..7454609a8be --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/before/refactor/copy/test.kt @@ -0,0 +1,178 @@ +package refactor.copy + +import refactor.ParentJava +import kotlin.properties.Delegates + +enum class Possible { + NO, YES +} +data class Potable(val p1: String) +class Insider(val peace: String) + +class Init { + fun referred() = 0 + fun moved() = referred() +} + +interface FaceToFace { + fun extract() +} + +abstract class AbsToAbs { + abstract fun extract() +} + +class Simple { + fun extract() {} +} + +annotation class MemAnn + +class Variety { + // object + object ExtractedObject {} + // function + tailrec fun tailrecFun(p: Int) { if (p > 0) tailrecFun(p - 1) } + operator fun plus(other: Variety) {} + infix fun infixFun(other: Variety) {} + inline fun inlineFun(f: () -> Int) = f() + external fun externalFun() + internal fun internalFun() = 0 + protected fun protectedFun() = 0 + private fun privateFun() = 0 + fun genFunB(p: T): T = p + fun genFunC(p: T): C where T : C = p + @MemAnn fun annotatedFun() = 0 + final fun finalFun() = 0 + // property + var publicProp = 0 + internal var internalProp = 0 + protected var protectedProp = 0 + private var privateProp = 0 + var List.genVarJ: T + get() = last() + set(p) {} + var List.genVarL: T where T : C + get() = last() + set(p) {} + @MemAnn val annotatedVal = 0 + var byVar by Delegates.notNull() + lateinit var lateVal: String + final val finalVal = 0 + // class + class ExtractedClass {} + // inner class + inner class ExtractedInnerClass {} + // anonymousInitializer + init {} + // secondaryConstructor + constructor(p: Int) + + fun refer(p1: List) { + val v1 = ExtractedObject + val v2 = ExtractedClass() + val v3 = ExtractedInnerClass() + val v4 = tailrecFun(2) + val v5 = this + this + val v6 = this infixFun this + val v7 = inlineFun { 7 } + val vv = externalFun() + val v8 = internalFun() + val v9 = protectedFun() + val vA = privateFun() + val vB = genFunB(0) + val vD = annotatedFun() + val vw = finalFun() + publicProp = publicProp + 1 + internalProp = internalProp + 1 + protectedProp = protectedProp + 1 + privateProp = privateProp + 1 + p1.genVarJ = p1.genVarJ + "+" + val vK = annotatedVal + byVar = byVar + 1 + lateVal = lateVal + "+" + val vN = finalVal + } +} + +fun Variety.extend() {} + +object VarietyObject { + const val constProp = 0 + + fun refer() { + val vE = constProp + } +} + +abstract class AbstractVariety { + abstract fun abstractFun(): Int + abstract var abstractVar: Int + fun refer() = abstractFun() + abstractVar +} + +interface FaceVarietyObligation { + fun abstractToAbstract() + fun abstractToConcrete() + fun concreteToConcrete() +} + +interface FaceVariety : FaceVarietyObligation { + fun abstractFun() + var abstractProperty: String + fun concreteFun() {} + override fun abstractToAbstract() + override fun abstractToConcrete() {} + override fun concreteToConcrete() {} +} + +@Suppress("LeakingThis") +open class CtorParameter(pp: String, open val pv: String, open var pr: String) { + var vb = pp + pv + pr + init { vb += pp + pv + pr } + constructor() : this("p", "v", "r") + fun refer() { + pr += pv + vb += pr + } + companion object { + val instance = CtorParameter() + } +} + +class CtorParameterChild(val pvc: String, var prc: String) : CtorParameter(pvc, pvc, prc) +class CtorParameterChild2: CtorParameter { + constructor() : super("", "", "") +} +class CtorParameterChild3(override val pv: String, override var pr: String) : CtorParameter(pv, pv, pr) +data class CtorData(val pv: String, var pr: String) {} + +class Company { + companion object { + val companyVal = 0 + var companyVar = 0 + fun companyFun() = 0 + class CompanyClass {} + object CompanyObject {} + } + + fun refer() { + println(companyVal) + companyVar = companyVar + 1 + println(companyFun()) + val v1 = CompanyClass() + val v2 = CompanyObject + } +} + +fun referCompany() { + println(Company.companyVal) + Company.companyVar = Company.companyVar + 1 + println(Company.companyFun()) + val v1 = Company.Companion.CompanyClass() + val v2 = Company.Companion.CompanyObject +} + +class JavaChild : ParentJava.FaceJava { + override fun inherit() {} +} \ No newline at end of file diff --git a/idea/testData/refactoring/copy/kt18149/kt18149.test b/idea/testData/refactoring/copy/kt18149/kt18149.test new file mode 100644 index 00000000000..148e0a34c3d --- /dev/null +++ b/idea/testData/refactoring/copy/kt18149/kt18149.test @@ -0,0 +1,5 @@ +{ + "mainFile": "refactor/copy/test.kt", + "targetPackage": "refactor.copy2", + "withRuntime": "true" +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/copy/CopyTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/copy/CopyTestGenerated.java index 03798a70972..f4cb8bba240 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/copy/CopyTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/copy/CopyTestGenerated.java @@ -174,6 +174,12 @@ public class CopyTestGenerated extends AbstractCopyTest { doTest(fileName); } + @TestMetadata("kt18149/kt18149.test") + public void testKt18149_Kt18149() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/copy/kt18149/kt18149.test"); + doTest(fileName); + } + @TestMetadata("refToImportJavaStaticField/refToImportedJavaStaticField.test") public void testRefToImportJavaStaticField_RefToImportedJavaStaticField() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/copy/refToImportJavaStaticField/refToImportedJavaStaticField.test");