Fix move refactoring conflict window output

1) Refactoring
2) #KT-32496 Fixed
This commit is contained in:
Igor Yakovlev
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.project.forcedTargetPlatform
import org.jetbrains.kotlin.idea.refactoring.getUsageContext import org.jetbrains.kotlin.idea.refactoring.getUsageContext
import org.jetbrains.kotlin.idea.refactoring.move.KotlinMoveUsage 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.and
import org.jetbrains.kotlin.idea.search.not import org.jetbrains.kotlin.idea.search.not
import org.jetbrains.kotlin.idea.util.projectStructure.getModule import org.jetbrains.kotlin.idea.util.projectStructure.getModule
import org.jetbrains.kotlin.idea.util.projectStructure.module 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.lexer.KtTokens
import org.jetbrains.kotlin.load.java.JavaVisibilities import org.jetbrains.kotlin.load.java.JavaVisibilities
import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.name.FqName
@@ -191,8 +191,10 @@ class MoveConflictChecker(
private fun render(declaration: PsiElement) = RefactoringUIUtil.getDescription(declaration, false) private fun render(declaration: PsiElement) = RefactoringUIUtil.getDescription(declaration, false)
private fun render(descriptor: DeclarationDescriptor) = CommonRefactoringUtil.htmlEmphasize(descriptor.renderForConflicts())
// Based on RefactoringConflictsUtil.analyzeModuleConflicts // Based on RefactoringConflictsUtil.analyzeModuleConflicts
fun analyzeModuleConflictsInUsages( private fun analyzeModuleConflictsInUsages(
project: Project, project: Project,
usages: Collection<UsageInfo>, usages: Collection<UsageInfo>,
targetScope: VirtualFile, 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 newConflicts = MultiMap<PsiElement, String>()
val targetScope = moveTarget.targetScope ?: return val targetScope = moveTarget.targetScope ?: return
@@ -363,7 +365,7 @@ class MoveConflictChecker(
internalUsages.removeIf { it.reference?.element?.let { element -> element in referencesToSkip } ?: false } 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>>() val declarationToContainers = HashMap<KtNamedDeclaration, MutableSet<PsiElement>>()
for (usage in usages) { for (usage in usages) {
val element = usage.element val element = usage.element
@@ -578,12 +580,14 @@ class MoveConflictChecker(
fun walkDeclarations( fun walkDeclarations(
currentScopeDeclaration: DeclarationDescriptor, currentScopeDeclaration: DeclarationDescriptor,
declaration: DeclarationDescriptor, declaration: DeclarationDescriptor,
report: (String, String) -> Unit report: (DeclarationDescriptor, DeclarationDescriptor) -> Unit
) { ) {
fun descrText(d: DeclarationDescriptor) = d.findPsi()?.text ?: "unknown"
when (currentScopeDeclaration) { when (currentScopeDeclaration) {
is PackageFragmentDescriptor -> { is PackageFragmentDescriptor -> {
fun getPackage(decl: PackageFragmentDescriptor) = decl.containingDeclaration.getPackage(decl.fqName)
fun getPackage(declaration: PackageFragmentDescriptor) =
declaration.containingDeclaration.getPackage(declaration.fqName)
val packageDescriptor = getPackage(currentScopeDeclaration) val packageDescriptor = getPackage(currentScopeDeclaration)
if ((declaration.containingDeclaration)?.fqNameOrNull() == currentScopeDeclaration.fqNameOrNull()) { if ((declaration.containingDeclaration)?.fqNameOrNull() == currentScopeDeclaration.fqNameOrNull()) {
return return
@@ -592,7 +596,7 @@ class MoveConflictChecker(
.memberScope .memberScope
.getContributedDescriptors { it == declaration.name } .getContributedDescriptors { it == declaration.name }
.filter { equivalent(it, declaration) } .filter { equivalent(it, declaration) }
.forEach { report(descrText(it), packageDescriptor.fqName.asString()) } .forEach { report(it, packageDescriptor) }
return return
} }
is ClassDescriptor -> { is ClassDescriptor -> {
@@ -601,7 +605,7 @@ class MoveConflictChecker(
.unsubstitutedMemberScope .unsubstitutedMemberScope
.getContributedDescriptors { it == declaration.name } .getContributedDescriptors { it == declaration.name }
.filter { equivalent(it, declaration) } .filter { equivalent(it, declaration) }
.forEach { report(descrText(it), descrText(currentScopeDeclaration)) } .forEach { report(it, currentScopeDeclaration) }
} }
} }
@@ -615,15 +619,15 @@ class MoveConflictChecker(
val declarationDescriptor = val declarationDescriptor =
(declaration as KtElement).analyze().get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration) (declaration as KtElement).analyze().get(BindingContext.DECLARATION_TO_DESCRIPTOR, declaration)
if (declarationDescriptor is DeclarationDescriptor) { if (declarationDescriptor is DeclarationDescriptor) {
val baseDescriptor = moveTarget.getContainerDescriptor()
if (baseDescriptor != null) { moveTarget.getContainerDescriptor()?.let { baseDescriptor ->
walkDeclarations(baseDescriptor, declarationDescriptor) { conflictingDecl, scopeName -> walkDeclarations(baseDescriptor, declarationDescriptor) { conflictingDeclaration, conflictingScope ->
conflicts.putValue( val message =
declaration, "Following declarations would clash: to move ${render(declarationDescriptor)} " +
"Following declarations would clash: \"${declaration.getText()}\" (element to move) " + "and destination ${render(conflictingDeclaration)} declared in scope " +
"and \"\n$conflictingDecl\" declared in scope " + render(conflictingScope)
"$scopeName (move destination)"
) 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.types.substitutions.getTypeSubstitutor
import org.jetbrains.kotlin.util.findCallableMemberBySignature import org.jetbrains.kotlin.util.findCallableMemberBySignature
fun checkConflicts(project: Project, fun checkConflicts(
sourceClass: KtClassOrObject, project: Project,
targetClass: PsiNamedElement, sourceClass: KtClassOrObject,
memberInfos: List<KotlinMemberInfo>, targetClass: PsiNamedElement,
onShowConflicts: () -> Unit = {}, memberInfos: List<KotlinMemberInfo>,
onAccept: () -> Unit) { onShowConflicts: () -> Unit = {},
onAccept: () -> Unit
) {
val conflicts = MultiMap<PsiElement, String>() val conflicts = MultiMap<PsiElement, String>()
val pullUpData = KotlinPullUpData(sourceClass, val pullUpData = KotlinPullUpData(sourceClass,
@@ -127,9 +129,12 @@ private val CALLABLE_RENDERER = IdeDescriptorRenderers.SOURCE_CODE_SHORT_NAMES_N
fun DeclarationDescriptor.renderForConflicts(): String { fun DeclarationDescriptor.renderForConflicts(): String {
return when (this) { 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 FunctionDescriptor -> "function '${CALLABLE_RENDERER.render(this)}'"
is PropertyDescriptor -> "property '${CALLABLE_RENDERER.render(this)}'" is PropertyDescriptor -> "property '${CALLABLE_RENDERER.render(this)}'"
is PackageFragmentDescriptor -> fqName.asString()
is PackageViewDescriptor -> fqName.asString()
else -> "" else -> ""
} }
} }
@@ -140,9 +145,9 @@ internal fun KotlinPullUpData.getClashingMemberInTargetClass(memberDescriptor: C
} }
private fun KotlinPullUpData.checkClashWithSuperDeclaration( private fun KotlinPullUpData.checkClashWithSuperDeclaration(
member: KtNamedDeclaration, member: KtNamedDeclaration,
memberDescriptor: DeclarationDescriptor, memberDescriptor: DeclarationDescriptor,
conflicts: MultiMap<PsiElement, String> conflicts: MultiMap<PsiElement, String>
) { ) {
val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}" val message = "${targetClassDescriptor.renderForConflicts()} already contains ${memberDescriptor.renderForConflicts()}"
@@ -169,40 +174,44 @@ private fun PsiClass.isSourceOrTarget(data: KotlinPullUpData): Boolean {
} }
private fun KotlinPullUpData.checkAccidentalOverrides( private fun KotlinPullUpData.checkAccidentalOverrides(
member: KtNamedDeclaration, member: KtNamedDeclaration,
memberDescriptor: DeclarationDescriptor, memberDescriptor: DeclarationDescriptor,
conflicts: MultiMap<PsiElement, String>) { conflicts: MultiMap<PsiElement, String>
) {
if (memberDescriptor is CallableDescriptor && !member.hasModifier(KtTokens.PRIVATE_KEYWORD)) { if (memberDescriptor is CallableDescriptor && !member.hasModifier(KtTokens.PRIVATE_KEYWORD)) {
val memberDescriptorInTargetClass = memberDescriptor.substitute(sourceToTargetClassSubstitutor) val memberDescriptorInTargetClass = memberDescriptor.substitute(sourceToTargetClassSubstitutor)
if (memberDescriptorInTargetClass != null) { if (memberDescriptorInTargetClass != null) {
HierarchySearchRequest<PsiElement>(targetClass, targetClass.useScope) HierarchySearchRequest<PsiElement>(targetClass, targetClass.useScope)
.searchInheritors() .searchInheritors()
.asSequence() .asSequence()
.filterNot { it.isSourceOrTarget(this) } .filterNot { it.isSourceOrTarget(this) }
.mapNotNull { it.unwrapped as? KtClassOrObject } .mapNotNull { it.unwrapped as? KtClassOrObject }
.forEach { .forEach {
val subClassDescriptor = it.resolveToDescriptorWrapperAware(resolutionFacade) as ClassDescriptor val subClassDescriptor = it.resolveToDescriptorWrapperAware(resolutionFacade) as ClassDescriptor
val substitutor = getTypeSubstitutor(targetClassDescriptor.defaultType, val substitutor = getTypeSubstitutor(
subClassDescriptor.defaultType) ?: TypeSubstitutor.EMPTY targetClassDescriptor.defaultType,
val memberDescriptorInSubClass = subClassDescriptor.defaultType
memberDescriptorInTargetClass.substitute(substitutor) as? CallableMemberDescriptor ) ?: TypeSubstitutor.EMPTY
val clashingMemberDescriptor = val memberDescriptorInSubClass =
memberDescriptorInSubClass?.let { subClassDescriptor.findCallableMemberBySignature(it) } ?: return memberDescriptorInTargetClass.substitute(substitutor) as? CallableMemberDescriptor
val clashingMember = clashingMemberDescriptor.source.getPsi() ?: return val clashingMemberDescriptor =
memberDescriptorInSubClass?.let { subClassDescriptor.findCallableMemberBySignature(it) } ?: return
val clashingMember = clashingMemberDescriptor.source.getPsi() ?: return
val message = memberDescriptor.renderForConflicts() + val message = memberDescriptor.renderForConflicts() +
" in super class would clash with existing member of " + " in super class would clash with existing member of " +
it.resolveToDescriptorWrapperAware(resolutionFacade).renderForConflicts() it.resolveToDescriptorWrapperAware(resolutionFacade).renderForConflicts()
conflicts.putValue(clashingMember, message.capitalize()) conflicts.putValue(clashingMember, message.capitalize())
} }
} }
} }
} }
private fun KotlinPullUpData.checkInnerClassToInterface( private fun KotlinPullUpData.checkInnerClassToInterface(
member: KtNamedDeclaration, member: KtNamedDeclaration,
memberDescriptor: DeclarationDescriptor, memberDescriptor: DeclarationDescriptor,
conflicts: MultiMap<PsiElement, String>) { conflicts: MultiMap<PsiElement, String>
) {
if (isInterfaceTarget && memberDescriptor is ClassDescriptor && memberDescriptor.isInner) { if (isInterfaceTarget && memberDescriptor is ClassDescriptor && memberDescriptor.isInner) {
val message = "${memberDescriptor.renderForConflicts()} is an inner class. It can not be moved to the interface" val message = "${memberDescriptor.renderForConflicts()} is an inner class. It can not be moved to the interface"
conflicts.putValue(member, message.capitalize()) conflicts.putValue(member, message.capitalize())
@@ -210,19 +219,20 @@ private fun KotlinPullUpData.checkInnerClassToInterface(
} }
private fun KotlinPullUpData.checkVisibility( private fun KotlinPullUpData.checkVisibility(
memberInfo: KotlinMemberInfo, memberInfo: KotlinMemberInfo,
memberDescriptor: DeclarationDescriptor, memberDescriptor: DeclarationDescriptor,
conflicts: MultiMap<PsiElement, String> conflicts: MultiMap<PsiElement, String>
) { ) {
fun reportConflictIfAny(targetDescriptor: DeclarationDescriptor) { fun reportConflictIfAny(targetDescriptor: DeclarationDescriptor) {
if (targetDescriptor in memberDescriptors.values) return if (targetDescriptor in memberDescriptors.values) return
val target = (targetDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() ?: return val target = (targetDescriptor as? DeclarationDescriptorWithSource)?.source?.getPsi() ?: return
if (targetDescriptor is DeclarationDescriptorWithVisibility if (targetDescriptor is DeclarationDescriptorWithVisibility
&& !Visibilities.isVisibleIgnoringReceiver(targetDescriptor, targetClassDescriptor)) { && !Visibilities.isVisibleIgnoringReceiver(targetDescriptor, targetClassDescriptor)
) {
val message = RefactoringBundle.message( val message = RefactoringBundle.message(
"0.uses.1.which.is.not.accessible.from.the.superclass", "0.uses.1.which.is.not.accessible.from.the.superclass",
memberDescriptor.renderForConflicts(), memberDescriptor.renderForConflicts(),
targetDescriptor.renderForConflicts() targetDescriptor.renderForConflicts()
) )
conflicts.putValue(target, message.capitalize()) conflicts.putValue(target, message.capitalize())
} }
@@ -242,19 +252,19 @@ private fun KotlinPullUpData.checkVisibility(
} }
} }
childrenToCheck.forEach { childrenToCheck.forEach { children ->
it.accept( children.accept(
object : KtTreeVisitorVoid() { object : KtTreeVisitorVoid() {
override fun visitReferenceExpression(expression: KtReferenceExpression) { override fun visitReferenceExpression(expression: KtReferenceExpression) {
super.visitReferenceExpression(expression) super.visitReferenceExpression(expression)
val context = resolutionFacade.analyze(expression) val context = resolutionFacade.analyze(expression)
expression.references expression.references
.flatMap { (it as? KtReference)?.resolveToDescriptors(context) ?: emptyList() } .flatMap { (it as? KtReference)?.resolveToDescriptors(context) ?: emptyList() }
.forEach(::reportConflictIfAny) .forEach(::reportConflictIfAny)
}
} }
}
) )
} }
} }