From ddd3a0a46e5cf2cb05fb890b7cda60414a4a28fb Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Thu, 26 Jul 2018 14:02:44 +0300 Subject: [PATCH] Implement liftToExpected correctly for primary constructor properties Before this commit, we always tried to find expect property in this case. However, there is at least one case when we should find parameter of expect primary constructor instead (safe delete). So #KT-25321 Fixed --- .../kotlin/idea/util/expectActualUtil.kt | 11 ++++++++-- .../safeDelete/KotlinSafeDeleteProcessor.kt | 2 +- .../KotlinSafeDeleteProcessor.kt.172 | 2 +- .../KotlinSafeDeleteProcessor.kt.173 | 2 +- .../after/Common/Common.iml | 19 +++++++++++++++++ .../after/Common/src/test/test.kt | 8 +++++++ .../after/JS/JS.iml | 21 +++++++++++++++++++ .../after/JS/src/test/test.kt | 12 +++++++++++ .../after/JVM/JVM.iml | 21 +++++++++++++++++++ .../after/JVM/src/test/test.kt | 14 +++++++++++++ .../before/Common/Common.iml | 19 +++++++++++++++++ .../before/Common/src/test/test.kt | 8 +++++++ .../before/JS/JS.iml | 21 +++++++++++++++++++ .../before/JS/src/test/test.kt | 12 +++++++++++ .../before/JVM/JVM.iml | 21 +++++++++++++++++++ .../before/JVM/src/test/test.kt | 14 +++++++++++++ .../conflicts.txt | 1 + ...ActualClassPrimaryConstructorProperty.test | 4 ++++ .../MultiModuleSafeDeleteTestGenerated.java | 5 +++++ 19 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/Common.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/JS.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/JVM.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/Common.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/JS.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/JVM.iml create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/src/test/test.kt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/conflicts.txt create mode 100644 idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/expectActualUtil.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/expectActualUtil.kt index 5e9a159e474..96a9556ff41 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/expectActualUtil.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/expectActualUtil.kt @@ -12,11 +12,12 @@ import org.jetbrains.kotlin.idea.caches.project.ModuleSourceInfo import org.jetbrains.kotlin.idea.caches.project.implementedDescriptors import org.jetbrains.kotlin.idea.caches.project.implementingDescriptors import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny -import org.jetbrains.kotlin.idea.core.setVisibility +import org.jetbrains.kotlin.idea.caches.resolve.resolveToParameterDescriptorIfAny import org.jetbrains.kotlin.idea.core.toDescriptor import org.jetbrains.kotlin.idea.search.usagesSearch.descriptor import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtDeclaration +import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject import org.jetbrains.kotlin.psi.psiUtil.hasActualModifier import org.jetbrains.kotlin.psi.psiUtil.hasExpectModifier @@ -60,13 +61,19 @@ fun KtDeclaration.liftToExpected(): KtDeclaration? { return DescriptorToSourceUtils.descriptorToDeclaration(expectedDescriptor) as? KtDeclaration } +fun KtParameter.liftToExpected(): KtParameter? { + val parameterDescriptor = resolveToParameterDescriptorIfAny() + val expectedDescriptor = parameterDescriptor?.liftToExpected() ?: return null + return DescriptorToSourceUtils.descriptorToDeclaration(expectedDescriptor) as? KtParameter +} + fun ModuleDescriptor.hasDeclarationOf(descriptor: MemberDescriptor) = declarationOf(descriptor) != null private fun ModuleDescriptor.declarationOf(descriptor: MemberDescriptor): DeclarationDescriptor? = with(ExpectedActualResolver) { val expectedCompatibilityMap = findExpectedForActual(descriptor, this@declarationOf) expectedCompatibilityMap?.get(ExpectedActualResolver.Compatibility.Compatible)?.firstOrNull() - ?: expectedCompatibilityMap?.values?.flatten()?.firstOrNull() + ?: expectedCompatibilityMap?.values?.flatten()?.firstOrNull() } fun ModuleDescriptor.hasActualsFor(descriptor: MemberDescriptor) = diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt index f2586677240..4e926397248 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt @@ -457,7 +457,7 @@ class KotlinSafeDeleteProcessor : JavaSafeDeleteProcessor() { ): Collection? { when (element) { is KtParameter -> { - val expectParameter = element.liftToExpected() as? KtParameter + val expectParameter = element.liftToExpected() if (expectParameter != null && expectParameter != element) { return if (shouldAllowPropagationToExpected(element)) { listOf(expectParameter) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.172 b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.172 index f946ad13cfe..93531ba2732 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.172 +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.172 @@ -449,7 +449,7 @@ class KotlinSafeDeleteProcessor : JavaSafeDeleteProcessor() { ): Collection? { when (element) { is KtParameter -> { - val expectParameter = element.liftToExpected() as? KtParameter + val expectParameter = element.liftToExpected() if (expectParameter != null && expectParameter != element) { if (shouldAllowPropagationToExpected(element)) { return listOf(expectParameter) diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.173 b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.173 index 810dd5bfc5b..d0d84956969 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.173 +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/safeDelete/KotlinSafeDeleteProcessor.kt.173 @@ -459,7 +459,7 @@ class KotlinSafeDeleteProcessor : JavaSafeDeleteProcessor() { ): Collection? { when (element) { is KtParameter -> { - val expectParameter = element.liftToExpected() as? KtParameter + val expectParameter = element.liftToExpected() if (expectParameter != null && expectParameter != element) { if (shouldAllowPropagationToExpected(element)) { return listOf(expectParameter) diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/Common.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/Common.iml new file mode 100644 index 00000000000..0ecf9949f6c --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/Common.iml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/src/test/test.kt new file mode 100644 index 00000000000..9cd26a44df0 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/Common/src/test/test.kt @@ -0,0 +1,8 @@ +package test + +expect class Foo() + +fun test() { + Foo() + Foo() +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/JS.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/JS.iml new file mode 100644 index 00000000000..3de270f43fa --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/JS.iml @@ -0,0 +1,21 @@ + + + + + + Common + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/src/test/test.kt new file mode 100644 index 00000000000..68578d1917d --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JS/src/test/test.kt @@ -0,0 +1,12 @@ +package test + +actual class Foo actual constructor() { + constructor(s: String): this(0) +} + +fun test() { + Foo("1") + Foo(s = "1") + Foo() + Foo() +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/JVM.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/JVM.iml new file mode 100644 index 00000000000..76d2b885834 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/JVM.iml @@ -0,0 +1,21 @@ + + + + + + Common + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/src/test/test.kt new file mode 100644 index 00000000000..909b9baf390 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/after/JVM/src/test/test.kt @@ -0,0 +1,14 @@ +package test + +actual class Foo(s: String) { + actual constructor(): this("") { + val x = n + 1 + } +} + +fun test() { + Foo("1") + Foo(s = "1") + Foo() + Foo() +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/Common.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/Common.iml new file mode 100644 index 00000000000..0ecf9949f6c --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/Common.iml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/src/test/test.kt new file mode 100644 index 00000000000..ab9a12757f5 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/Common/src/test/test.kt @@ -0,0 +1,8 @@ +package test + +expect class Foo(n: Int) + +fun test() { + Foo(1) + Foo(n = 1) +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/JS.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/JS.iml new file mode 100644 index 00000000000..3de270f43fa --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/JS.iml @@ -0,0 +1,21 @@ + + + + + + Common + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/src/test/test.kt new file mode 100644 index 00000000000..f17d8fc5ed2 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JS/src/test/test.kt @@ -0,0 +1,12 @@ +package test + +actual class Foo actual constructor(val n: Int) { + constructor(s: String): this(0) +} + +fun test() { + Foo("1") + Foo(s = "1") + Foo(1) + Foo(n = 1) +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/JVM.iml b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/JVM.iml new file mode 100644 index 00000000000..76d2b885834 --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/JVM.iml @@ -0,0 +1,21 @@ + + + + + + Common + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/src/test/test.kt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/src/test/test.kt new file mode 100644 index 00000000000..da426b1503c --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/before/JVM/src/test/test.kt @@ -0,0 +1,14 @@ +package test + +actual class Foo(s: String) { + actual constructor(n: Int): this("") { + val x = n + 1 + } +} + +fun test() { + Foo("1") + Foo(s = "1") + Foo(1) + Foo(n = 1) +} \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/conflicts.txt b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/conflicts.txt new file mode 100644 index 00000000000..8ee670db6dd --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/conflicts.txt @@ -0,0 +1 @@ +parameter n has 2 usages that are not safe to delete. \ No newline at end of file diff --git a/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test new file mode 100644 index 00000000000..75be21f843a --- /dev/null +++ b/idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test @@ -0,0 +1,4 @@ +{ + "mainFile": "JS/src/test/test.kt", + "elementClass": "org.jetbrains.kotlin.psi.KtParameter" +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/safeDelete/MultiModuleSafeDeleteTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/safeDelete/MultiModuleSafeDeleteTestGenerated.java index 0511ee89872..1bd47eeda7e 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/safeDelete/MultiModuleSafeDeleteTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/safeDelete/MultiModuleSafeDeleteTestGenerated.java @@ -54,6 +54,11 @@ public class MultiModuleSafeDeleteTestGenerated extends AbstractMultiModuleSafeD runTest("idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorParameterLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorParameter.test"); } + @TestMetadata("byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test") + public void testByActualClassPrimaryConstructorPropertyLiftingToExpect_ExpectsAndActualsByActualClassPrimaryConstructorProperty() throws Exception { + runTest("idea/testData/refactoring/safeDeleteMultiModule/byActualClassPrimaryConstructorPropertyLiftingToExpect/expectsAndActualsByActualClassPrimaryConstructorProperty.test"); + } + @TestMetadata("byActualClassSecondaryConstructor/byActualClassSecondaryConstructor.test") public void testByActualClassSecondaryConstructor_ByActualClassSecondaryConstructor() throws Exception { runTest("idea/testData/refactoring/safeDeleteMultiModule/byActualClassSecondaryConstructor/byActualClassSecondaryConstructor.test");