From 94c4b426f508381f747a2fc1c46a6f022fe8a111 Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Mon, 15 Aug 2016 18:46:54 +0300 Subject: [PATCH] Pull Up: Skip super members without explicit declarations #KT-13124 Fixed --- ChangeLog.md | 1 + .../idea/refactoring/pullUp/markingUtils.kt | 9 +++++++-- .../refactoring/pullUp/pullUpConflictsUtils.kt | 14 +++++++------- .../refactoring/pullUp/k2k/skipFakeOverrides.kt | 17 +++++++++++++++++ .../pullUp/k2k/skipFakeOverrides.kt.after | 17 +++++++++++++++++ .../refactoring/pullUp/PullUpTestGenerated.java | 6 ++++++ 6 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt create mode 100644 idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt.after diff --git a/ChangeLog.md b/ChangeLog.md index 002714d3ed9..a6c9f00fb95 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -269,6 +269,7 @@ Using 'this' as function argument in constructor of non-final class - [`KT-13240`](https://youtrack.jetbrains.com/issue/KT-13240) Rename: Do not report shadowing conflict if redeclaration is detected - [`KT-13253`](https://youtrack.jetbrains.com/issue/KT-13253) Rename: Report conflicts for constructor parameters - [`KT-12971`](https://youtrack.jetbrains.com/issue/KT-12971) Push Down: Do not specifiy visibility on generated overriding members +- [`KT-13124`](https://youtrack.jetbrains.com/issue/KT-13124) Pull Up: Skip super members without explicit declarations ##### New features diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/markingUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/markingUtils.kt index 92076166ec3..130a346055e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/markingUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/markingUtils.kt @@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.refactoring.pullUp import com.intellij.openapi.util.Key import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet import org.jetbrains.kotlin.idea.imports.importableFqName @@ -27,9 +28,11 @@ import org.jetbrains.kotlin.idea.core.ShortenReferences import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.allChildren +import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiverOrThis import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorUtils +import org.jetbrains.kotlin.resolve.calls.callUtil.getCalleeExpressionIfAny import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.resolvedCallUtil.getExplicitReceiverValue import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue @@ -53,8 +56,10 @@ fun markElements( private fun visitSuperOrThis(expression: KtInstanceExpressionWithLabel) { if (targetClassDescriptor == null) return - val referenceTarget = context[BindingContext.REFERENCE_TARGET, expression.instanceReference] - if (referenceTarget == targetClassDescriptor) { + val callee = expression.getQualifiedExpressionForReceiver()?.selectorExpression?.getCalleeExpressionIfAny() ?: return + val calleeTarget = callee.getResolvedCall(context)?.resultingDescriptor ?: return + if ((calleeTarget as? CallableMemberDescriptor)?.kind != CallableMemberDescriptor.Kind.DECLARATION) return + if (calleeTarget.containingDeclaration == targetClassDescriptor) { expression.replaceWithTargetThis = true affectedElements.add(expression) } diff --git a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt index c3c3a7684a2..7fb8bf4d2bb 100644 --- a/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/refactoring/pullUp/pullUpConflictsUtils.kt @@ -91,13 +91,13 @@ private fun KotlinPullUpData.checkClashWithSuperDeclaration( member: KtNamedDeclaration, memberDescriptor: DeclarationDescriptor, conflicts: MultiMap) { - if (memberDescriptor is CallableMemberDescriptor) { - val clashingSuper = getClashingMemberInTargetClass(memberDescriptor) - if (clashingSuper != null && clashingSuper.modality != Modality.ABSTRACT) { - val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}" - conflicts.putValue(member, message.capitalize()) - } - } + if (memberDescriptor !is CallableMemberDescriptor) return + + val clashingSuper = getClashingMemberInTargetClass(memberDescriptor) ?: return + if (clashingSuper.modality == Modality.ABSTRACT) return + if (clashingSuper.kind != CallableMemberDescriptor.Kind.DECLARATION) return + val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}" + conflicts.putValue(member, message.capitalize()) } private fun KotlinPullUpData.checkAccidentalOverrides( diff --git a/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt b/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt new file mode 100644 index 00000000000..b407e0cc65e --- /dev/null +++ b/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt @@ -0,0 +1,17 @@ +// TARGET_CLASS: B +open class A { + open fun callSuper() { + // Something important + } +} + +open class B : A() { + +} + +class C : B() { + // INFO: {"checked": "true"} + override fun callSuper() { + super.callSuper() // We simply call up to the base class + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt.after b/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt.after new file mode 100644 index 00000000000..8567440a26c --- /dev/null +++ b/idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt.after @@ -0,0 +1,17 @@ +// TARGET_CLASS: B +open class A { + open fun callSuper() { + // Something important + } +} + +open class B : A() { + // INFO: {"checked": "true"} + override fun callSuper() { + super.callSuper() // We simply call up to the base class + } + +} + +class C : B() { +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java index 7cadea536f4..749fa94cffb 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/pullUp/PullUpTestGenerated.java @@ -223,6 +223,12 @@ public class PullUpTestGenerated extends AbstractPullUpTest { doKotlinTest(fileName); } + @TestMetadata("skipFakeOverrides.kt") + public void testSkipFakeOverrides() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/skipFakeOverrides.kt"); + doKotlinTest(fileName); + } + @TestMetadata("spaceAfterModifier.kt") public void testSpaceAfterModifier() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/pullUp/k2k/spaceAfterModifier.kt");