Fix move refactoring conflict window output

1) Refactoring 2) #KT-32496 Fixed
2019-08-16 18:42:32 +03:00
parent ae3f36ebde
commit 9c861a111f
2 changed files with 83 additions and 69 deletions
@@ -42,11 +42,11 @@ import org.jetbrains.kotlin.idea.project.TargetPlatformDetector
 import org.jetbrains.kotlin.idea.project.forcedTargetPlatform
 import org.jetbrains.kotlin.idea.refactoring.getUsageContext
 import org.jetbrains.kotlin.idea.refactoring.move.KotlinMoveUsage
+import org.jetbrains.kotlin.idea.refactoring.pullUp.renderForConflicts
 import org.jetbrains.kotlin.idea.search.and
 import org.jetbrains.kotlin.idea.search.not
 import org.jetbrains.kotlin.idea.util.projectStructure.getModule
 import org.jetbrains.kotlin.idea.util.projectStructure.module
-import org.jetbrains.kotlin.js.resolve.diagnostics.findPsi
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.load.java.JavaVisibilities
 import org.jetbrains.kotlin.name.FqName
@@ -191,8 +191,10 @@ class MoveConflictChecker(

    private fun render(declaration: PsiElement) = RefactoringUIUtil.getDescription(declaration, false)

+    private fun render(descriptor: DeclarationDescriptor) = CommonRefactoringUtil.htmlEmphasize(descriptor.renderForConflicts())
+
    // Based on RefactoringConflictsUtil.analyzeModuleConflicts
-    fun analyzeModuleConflictsInUsages(
+    private fun analyzeModuleConflictsInUsages(
        project: Project,
        usages: Collection<UsageInfo>,
        targetScope: VirtualFile,
@@ -231,7 +233,7 @@ class MoveConflictChecker(
        }
    }

-    fun checkModuleConflictsInUsages(externalUsages: MutableSet<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
+    private fun checkModuleConflictsInUsages(externalUsages: MutableSet<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
        val newConflicts = MultiMap<PsiElement, String>()
        val targetScope = moveTarget.targetScope ?: return

@@ -363,7 +365,7 @@ class MoveConflictChecker(
        internalUsages.removeIf { it.reference?.element?.let { element -> element in referencesToSkip } ?: false }
    }

-    fun checkVisibilityInUsages(usages: Collection<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
+    private fun checkVisibilityInUsages(usages: Collection<UsageInfo>, conflicts: MultiMap<PsiElement, String>) {
        val declarationToContainers = HashMap<KtNamedDeclaration, MutableSet<PsiElement>>()
        for (usage in usages) {
            val element = usage.element
@@ -578,12 +580,14 @@ class MoveConflictChecker(
        fun walkDeclarations(
            currentScopeDeclaration: DeclarationDescriptor,
            declaration: DeclarationDescriptor,
-            report: (String, String) -> Unit
+            report: (DeclarationDescriptor, DeclarationDescriptor) -> Unit
        ) {
-            fun descrText(d: DeclarationDescriptor) = d.findPsi()?.text ?: "unknown"
            when (currentScopeDeclaration) {
                is PackageFragmentDescriptor -> {
-                    fun getPackage(decl: PackageFragmentDescriptor) = decl.containingDeclaration.getPackage(decl.fqName)
+
+                    fun getPackage(declaration: PackageFragmentDescriptor) =
+                        declaration.containingDeclaration.getPackage(declaration.fqName)
+
                    val packageDescriptor = getPackage(currentScopeDeclaration)
                    if ((declaration.containingDeclaration)?.fqNameOrNull() == currentScopeDeclaration.fqNameOrNull()) {
                        return
@@ -592,7 +596,7 @@ class MoveConflictChecker(
                        .memberScope
                        .getContributedDescriptors { it == declaration.name }
                        .filter { equivalent(it, declaration) }
-                        .forEach { report(descrText(it), packageDescriptor.fqName.asString()) }
+                        .forEach { report(it, packageDescriptor) }
                    return
                }
                is ClassDescriptor -> {
@@ -601,7 +605,7 @@ class MoveConflictChecker(
                            .unsubstitutedMemberScope
                            .getContributedDescriptors { it == declaration.name }
                            .filter { equivalent(it, declaration) }
-                            .forEach { report(descrText(it), descrText(currentScopeDeclaration)) }
+                            .forEach { report(it, currentScopeDeclaration) }
                    }
                }

@@ -615,15 +619,15 @@ class MoveConflictChecker(
                val declarationDescriptor =
                    (declaration as KtElement).analyze().get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration)
                if (declarationDescriptor is DeclarationDescriptor) {
-                    val baseDescriptor = moveTarget.getContainerDescriptor()
-                    if (baseDescriptor != null) {
-                        walkDeclarations(baseDescriptor, declarationDescriptor) { conflictingDecl, scopeName ->
-                            conflicts.putValue(
-                                declaration,
-                                "Following declarations would clash: \"${declaration.getText()}\" (element to move) " +
-                                        "and \"\n$conflictingDecl\" declared in scope " +
-                                        "$scopeName (move destination)"
-                            )
+
+                    moveTarget.getContainerDescriptor()?.let { baseDescriptor ->
+                        walkDeclarations(baseDescriptor, declarationDescriptor) { conflictingDeclaration, conflictingScope ->
+                            val message =
+                                "Following declarations would clash: to move ${render(declarationDescriptor)} " +
+                                        "and destination ${render(conflictingDeclaration)} declared in scope " +
+                                        render(conflictingScope)
+
+                            conflicts.putValue(declaration, message)
                        }
                    }
                }
@@ -49,12 +49,14 @@ import org.jetbrains.kotlin.types.Variance
 import org.jetbrains.kotlin.types.substitutions.getTypeSubstitutor
 import org.jetbrains.kotlin.util.findCallableMemberBySignature

-fun checkConflicts(project: Project,
-                   sourceClass: KtClassOrObject,
-                   targetClass: PsiNamedElement,
-                   memberInfos: List<KotlinMemberInfo>,
-                   onShowConflicts: () -> Unit = {},
-                   onAccept: () -> Unit) {
+fun checkConflicts(
+    project: Project,
+    sourceClass: KtClassOrObject,
+    targetClass: PsiNamedElement,
+    memberInfos: List<KotlinMemberInfo>,
+    onShowConflicts: () -> Unit = {},
+    onAccept: () -> Unit
+) {
    val conflicts = MultiMap<PsiElement, String>()

    val pullUpData = KotlinPullUpData(sourceClass,
@@ -127,9 +129,12 @@ private val CALLABLE_RENDERER = IdeDescriptorRenderers.SOURCE_CODE_SHORT_NAMES_N

 fun DeclarationDescriptor.renderForConflicts(): String {
    return when (this) {
-        is ClassDescriptor -> "${DescriptorRenderer.getClassifierKindPrefix(this)} ${IdeDescriptorRenderers.SOURCE_CODE.renderClassifierName(this)}"
+        is ClassDescriptor -> "${DescriptorRenderer.getClassifierKindPrefix(this)} " +
+                IdeDescriptorRenderers.SOURCE_CODE.renderClassifierName(this)
        is FunctionDescriptor -> "function '${CALLABLE_RENDERER.render(this)}'"
        is PropertyDescriptor -> "property '${CALLABLE_RENDERER.render(this)}'"
+        is PackageFragmentDescriptor -> fqName.asString()
+        is PackageViewDescriptor -> fqName.asString()
        else -> ""
    }
 }
@@ -140,9 +145,9 @@ internal fun KotlinPullUpData.getClashingMemberInTargetClass(memberDescriptor: C
 }

 private fun KotlinPullUpData.checkClashWithSuperDeclaration(
-        member: KtNamedDeclaration,
-        memberDescriptor: DeclarationDescriptor,
-        conflicts: MultiMap<PsiElement, String>
+    member: KtNamedDeclaration,
+    memberDescriptor: DeclarationDescriptor,
+    conflicts: MultiMap<PsiElement, String>
 ) {
    val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}"

@@ -169,40 +174,44 @@ private fun PsiClass.isSourceOrTarget(data: KotlinPullUpData): Boolean {
 }

 private fun KotlinPullUpData.checkAccidentalOverrides(
-        member: KtNamedDeclaration,
-        memberDescriptor: DeclarationDescriptor,
-        conflicts: MultiMap<PsiElement, String>) {
+    member: KtNamedDeclaration,
+    memberDescriptor: DeclarationDescriptor,
+    conflicts: MultiMap<PsiElement, String>
+) {
    if (memberDescriptor is CallableDescriptor && !member.hasModifier(KtTokens.PRIVATE_KEYWORD)) {
        val memberDescriptorInTargetClass = memberDescriptor.substitute(sourceToTargetClassSubstitutor)
        if (memberDescriptorInTargetClass != null) {
            HierarchySearchRequest<PsiElement>(targetClass, targetClass.useScope)
-                    .searchInheritors()
-                    .asSequence()
-                    .filterNot { it.isSourceOrTarget(this) }
-                    .mapNotNull { it.unwrapped as? KtClassOrObject }
-                    .forEach {
-                        val subClassDescriptor = it.resolveToDescriptorWrapperAware(resolutionFacade) as ClassDescriptor
-                        val substitutor = getTypeSubstitutor(targetClassDescriptor.defaultType,
-                                                             subClassDescriptor.defaultType) ?: TypeSubstitutor.EMPTY
-                        val memberDescriptorInSubClass =
-                                memberDescriptorInTargetClass.substitute(substitutor) as? CallableMemberDescriptor
-                        val clashingMemberDescriptor =
-                                memberDescriptorInSubClass?.let { subClassDescriptor.findCallableMemberBySignature(it) } ?: return
-                        val clashingMember = clashingMemberDescriptor.source.getPsi() ?: return
+                .searchInheritors()
+                .asSequence()
+                .filterNot { it.isSourceOrTarget(this) }
+                .mapNotNull { it.unwrapped as? KtClassOrObject }
+                .forEach {
+                    val subClassDescriptor = it.resolveToDescriptorWrapperAware(resolutionFacade) as ClassDescriptor
+                    val substitutor = getTypeSubstitutor(
+                        targetClassDescriptor.defaultType,
+                        subClassDescriptor.defaultType
+                    ) ?: TypeSubstitutor.EMPTY
+                    val memberDescriptorInSubClass =
+                        memberDescriptorInTargetClass.substitute(substitutor) as? CallableMemberDescriptor
+                    val clashingMemberDescriptor =
+                        memberDescriptorInSubClass?.let { subClassDescriptor.findCallableMemberBySignature(it) } ?: return
+                    val clashingMember = clashingMemberDescriptor.source.getPsi() ?: return

-                        val message = memberDescriptor.renderForConflicts() +
-                                      " in super class would clash with existing member of " +
-                                      it.resolveToDescriptorWrapperAware(resolutionFacade).renderForConflicts()
-                        conflicts.putValue(clashingMember, message.capitalize())
-                    }
+                    val message = memberDescriptor.renderForConflicts() +
+                            " in super class would clash with existing member of " +
+                            it.resolveToDescriptorWrapperAware(resolutionFacade).renderForConflicts()
+                    conflicts.putValue(clashingMember, message.capitalize())
+                }
        }
    }
 }

 private fun KotlinPullUpData.checkInnerClassToInterface(
-        member: KtNamedDeclaration,
-        memberDescriptor: DeclarationDescriptor,
-        conflicts: MultiMap<PsiElement, String>) {
+    member: KtNamedDeclaration,
+    memberDescriptor: DeclarationDescriptor,
+    conflicts: MultiMap<PsiElement, String>
+) {
    if (isInterfaceTarget && memberDescriptor is ClassDescriptor && memberDescriptor.isInner) {
        val message = "${memberDescriptor.renderForConflicts()} is an inner class. It can not be moved to the interface"
        conflicts.putValue(member, message.capitalize())
@@ -210,19 +219,20 @@ private fun KotlinPullUpData.checkInnerClassToInterface(
 }

 private fun KotlinPullUpData.checkVisibility(
-        memberInfo: KotlinMemberInfo,
-        memberDescriptor: DeclarationDescriptor,
-        conflicts: MultiMap<PsiElement, String>
+    memberInfo: KotlinMemberInfo,
+    memberDescriptor: DeclarationDescriptor,
+    conflicts: MultiMap<PsiElement, String>
 ) {
    fun reportConflictIfAny(targetDescriptor: DeclarationDescriptor) {
        if (targetDescriptor in memberDescriptors.values) return
        val target = (targetDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() ?: return
        if (targetDescriptor is DeclarationDescriptorWithVisibility
-            && !Visibilities.isVisibleIgnoringReceiver(targetDescriptor, targetClassDescriptor)) {
+            && !Visibilities.isVisibleIgnoringReceiver(targetDescriptor, targetClassDescriptor)
+        ) {
            val message = RefactoringBundle.message(
-                    "0.uses.1.which.is.not.accessible.from.the.superclass",
-                    memberDescriptor.renderForConflicts(),
-                    targetDescriptor.renderForConflicts()
+                "0.uses.1.which.is.not.accessible.from.the.superclass",
+                memberDescriptor.renderForConflicts(),
+                targetDescriptor.renderForConflicts()
            )
            conflicts.putValue(target, message.capitalize())
        }
@@ -242,19 +252,19 @@ private fun KotlinPullUpData.checkVisibility(
        }
    }

-    childrenToCheck.forEach {
-        it.accept(
-                object : KtTreeVisitorVoid() {
-                    override fun visitReferenceExpression(expression: KtReferenceExpression) {
-                        super.visitReferenceExpression(expression)
+    childrenToCheck.forEach { children ->
+        children.accept(
+            object : KtTreeVisitorVoid() {
+                override fun visitReferenceExpression(expression: KtReferenceExpression) {
+                    super.visitReferenceExpression(expression)

-                        val context = resolutionFacade.analyze(expression)
-                        expression.references
-                                .flatMap { (it as? KtReference)?.resolveToDescriptors(context) ?: emptyList() }
-                                .forEach(::reportConflictIfAny)
+                    val context = resolutionFacade.analyze(expression)
+                    expression.references
+                        .flatMap { (it as? KtReference)?.resolveToDescriptors(context) ?: emptyList() }
+                        .forEach(::reportConflictIfAny)

-                    }
                }
+            }
        )
    }
 }