From eb2feeeb165593714472100778d84950fd7f51c7 Mon Sep 17 00:00:00 2001 From: Nikolay Krasko Date: Mon, 19 Oct 2015 15:20:38 +0300 Subject: [PATCH] Fixes after review in imports for additional expressions --- .../kotlin/idea/quickfix/AutoImportFix.kt | 225 +++++++++--------- .../autoImports/delegateExtensionGet.test | 4 +- .../autoImports/delegateExtensionSet.test | 4 +- 3 files changed, 114 insertions(+), 119 deletions(-) diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt index d1b891d6e09..4022a58ca0b 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt @@ -31,7 +31,6 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.DiagnosticFactory -import org.jetbrains.kotlin.diagnostics.DiagnosticWithParameters2 import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.idea.JetBundle import org.jetbrains.kotlin.idea.actions.KotlinAddImportAction @@ -44,25 +43,24 @@ import org.jetbrains.kotlin.idea.core.getResolutionScope import org.jetbrains.kotlin.idea.core.isVisible import org.jetbrains.kotlin.idea.project.ProjectStructureUtil import org.jetbrains.kotlin.idea.util.CallTypeAndReceiver -import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.psiUtil.isImportDirectiveExpression import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.types.expressions.OperatorConventions +import org.jetbrains.kotlin.util.OperatorNameConventions import org.jetbrains.kotlin.utils.CachedValueProperty +import org.jetbrains.kotlin.utils.addToStdlib.singletonList +import org.jetbrains.kotlin.utils.addToStdlib.singletonOrEmptyList import java.util.* /** * Check possibility and perform fix for unresolved references. */ -abstract class AutoImportFixBase public constructor( - expression: KtExpression, - val diagnostics: Collection = emptyList()) : KotlinQuickFixAction(expression), HighPriorityAction, HintAction { +abstract class AutoImportFixBase(expression: T, val diagnostics: Collection) : + KotlinQuickFixAction(expression), HighPriorityAction, HintAction { - protected constructor( - expression: KtExpression, - diagnostic: Diagnostic? = null) : this(expression, if (diagnostic == null) emptyList() else listOf(diagnostic)) + protected constructor(expression: T, diagnostic: Diagnostic? = null) : this(expression, diagnostic.singletonOrEmptyList()) private val modificationCountOnCreate = PsiModificationTracker.SERVICE.getInstance(element.getProject()).getModificationCount() @@ -76,6 +74,10 @@ abstract class AutoImportFixBase public constructor( }, { PsiModificationTracker.SERVICE.getInstance(element.getProject()).getModificationCount() }) + protected abstract fun getSupportedErrors(): Collection> + protected abstract fun getCallTypeAndReceiver(): CallTypeAndReceiver<*, *> + protected abstract fun getImportNames(): Collection + override fun showHint(editor: Editor): Boolean { if (!element.isValid() || isOutdated()) return false @@ -107,98 +109,94 @@ abstract class AutoImportFixBase public constructor( private fun isOutdated() = modificationCountOnCreate != PsiModificationTracker.SERVICE.getInstance(element.getProject()).getModificationCount() - protected open fun createAction(project: Project, editor: Editor) = KotlinAddImportAction(project, editor, element as KtSimpleNameExpression, suggestions) + protected open fun createAction(project: Project, editor: Editor) = KotlinAddImportAction(project, editor, element, suggestions) protected fun computeSuggestions(): Collection { if (!element.isValid()) return listOf() + if (element.getContainingFile() !is KtFile) return emptyList() - val file = element.getContainingFile() as? KtFile ?: return emptyList() - - val callTypeAndReceiver = getTypeAndReceiver() + val callTypeAndReceiver = getCallTypeAndReceiver() if (callTypeAndReceiver is CallTypeAndReceiver.UNKNOWN) return emptyList() - var referenceNames = getImportNames(diagnostics, element).filter { it.isNotEmpty() } + var referenceNames = getImportNames() if (referenceNames.isEmpty()) return emptyList() return referenceNames.flatMapTo(LinkedHashSet()) { - Helper.computeSuggestionsForName(callTypeAndReceiver, element, file, it, getSupportedErrors()) + computeSuggestionsForName(it, callTypeAndReceiver) } } - protected abstract fun getSupportedErrors(): Collection> - protected abstract fun getTypeAndReceiver(): CallTypeAndReceiver<*, *> - protected abstract fun getImportNames(diagnostics: Collection, element: KtExpression): Collection + public fun computeSuggestionsForName(name: Name, callTypeAndReceiver: CallTypeAndReceiver<*, *>): + Collection { + val nameStr = name.asString() + if (nameStr.isEmpty()) return emptyList() - private object Helper { - public fun computeSuggestionsForName( - callTypeAndReceiver: CallTypeAndReceiver, - expression: KtExpression, file: KtFile, referenceName: String, - supportedErrors: Collection>): Collection { - fun filterByCallType(descriptor: DeclarationDescriptor) = callTypeAndReceiver.callType.descriptorKindFilter.accepts(descriptor) + val file = element.getContainingFile() as KtFile - val searchScope = getResolveScope(file) + fun filterByCallType(descriptor: DeclarationDescriptor) = callTypeAndReceiver.callType.descriptorKindFilter.accepts(descriptor) - val bindingContext = expression.analyze(BodyResolveMode.PARTIAL) + val searchScope = getResolveScope(file) - val diagnostics = bindingContext.getDiagnostics().forElement(expression) + val bindingContext = element.analyze(BodyResolveMode.PARTIAL) - if (!diagnostics.any { it.getFactory() in supportedErrors }) return emptyList() + val diagnostics = bindingContext.getDiagnostics().forElement(element) - val resolutionScope = expression.getResolutionScope(bindingContext, file.getResolutionFacade()) - val containingDescriptor = resolutionScope.ownerDescriptor + if (!diagnostics.any { it.getFactory() in getSupportedErrors() }) return emptyList() - fun isVisible(descriptor: DeclarationDescriptor): Boolean { - if (descriptor is DeclarationDescriptorWithVisibility) { - return descriptor.isVisible(containingDescriptor, bindingContext, expression as? KtSimpleNameExpression) - } + val resolutionScope = element.getResolutionScope(bindingContext, file.getResolutionFacade()) + val containingDescriptor = resolutionScope.ownerDescriptor - return true + fun isVisible(descriptor: DeclarationDescriptor): Boolean { + if (descriptor is DeclarationDescriptorWithVisibility) { + return descriptor.isVisible(containingDescriptor, bindingContext, element as? KtSimpleNameExpression) } - val result = ArrayList() - - val indicesHelper = KotlinIndicesHelper(expression.getResolutionFacade(), searchScope, ::isVisible, true) - - if (expression is KtSimpleNameExpression) { - if (!expression.isImportDirectiveExpression() && !KtPsiUtil.isSelectorInQualified(expression)) { - if (ProjectStructureUtil.isJsKotlinModule(file)) { - indicesHelper.getKotlinClasses({ it == referenceName }, { true }).filterTo(result, ::filterByCallType) - - } - else { - indicesHelper.getJvmClassesByName(referenceName).filterTo(result, ::filterByCallType) - } - - indicesHelper.getTopLevelCallablesByName(referenceName).filterTo(result, ::filterByCallType) - } - } - - result.addAll(indicesHelper.getCallableTopLevelExtensions({ it == referenceName }, callTypeAndReceiver, expression, bindingContext)) - - return if (result.size > 1) - reduceCandidatesBasedOnDependencyRuleViolation(result, file) - else - result + return true } - private fun reduceCandidatesBasedOnDependencyRuleViolation( - candidates: Collection, file: PsiFile): Collection { - val project = file.project - val validationManager = DependencyValidationManager.getInstance(project) - return candidates.filter { - val targetFile = DescriptorToSourceUtilsIde.getAnyDeclaration(project, it)?.containingFile ?: return@filter true - validationManager.getViolatorDependencyRules(file, targetFile).isEmpty() + val result = ArrayList() + + val indicesHelper = KotlinIndicesHelper(element.getResolutionFacade(), searchScope, ::isVisible, true) + + val expression = element + if (expression is KtSimpleNameExpression) { + if (!expression.isImportDirectiveExpression() && !KtPsiUtil.isSelectorInQualified(expression)) { + if (ProjectStructureUtil.isJsKotlinModule(file)) { + indicesHelper.getKotlinClasses({ it == nameStr }, { true }).filterTo(result, ::filterByCallType) + } + else { + indicesHelper.getJvmClassesByName(nameStr).filterTo(result, ::filterByCallType) + } + + indicesHelper.getTopLevelCallablesByName(nameStr).filterTo(result, ::filterByCallType) } } + + result.addAll(indicesHelper.getCallableTopLevelExtensions({ it == nameStr }, callTypeAndReceiver, element, bindingContext)) + + return if (result.size > 1) + reduceCandidatesBasedOnDependencyRuleViolation(result, file) + else + result + } + + private fun reduceCandidatesBasedOnDependencyRuleViolation( + candidates: Collection, file: PsiFile): Collection { + val project = file.project + val validationManager = DependencyValidationManager.getInstance(project) + return candidates.filter { + val targetFile = DescriptorToSourceUtilsIde.getAnyDeclaration(project, it)?.containingFile ?: return@filter true + validationManager.getViolatorDependencyRules(file, targetFile).isEmpty() + } } } -class AutoImportFix(expression: KtSimpleNameExpression, diagnostic: Diagnostic? = null) : AutoImportFixBase(expression, diagnostic) { - override fun getTypeAndReceiver(): CallTypeAndReceiver<*, *> = CallTypeAndReceiver.detect(element as JetSimpleNameExpression) - override fun getImportNames(diagnostics: Collection, element: KtExpression): Collection { - element as KtSimpleNameExpression +class AutoImportFix(expression: KtSimpleNameExpression, diagnostic: Diagnostic? = null) : + AutoImportFixBase(expression, diagnostic) { + override fun getCallTypeAndReceiver() = CallTypeAndReceiver.detect(element) + override fun getImportNames(): Collection { if (element.getIdentifier() == null) { val conventionName = KtPsiUtil.getConventionName(element) if (conventionName != null) { @@ -208,16 +206,16 @@ class AutoImportFix(expression: KtSimpleNameExpression, diagnostic: Diagnostic? val conterpart = OperatorConventions.ASSIGNMENT_OPERATION_COUNTERPARTS.get(elementType) val counterpartName = OperatorConventions.BINARY_OPERATION_NAMES.get(conterpart) if (counterpartName != null) { - return listOf(conventionName.asString(), counterpartName.asString()) + return listOf(conventionName, counterpartName) } } } - return listOf(conventionName.asString()) + return conventionName.singletonOrEmptyList() } } - else { - return listOf(element.getReferencedName()) + else if (Name.isValidIdentifier(element.getReferencedName())) { + return Name.identifier(element.getReferencedName()).singletonOrEmptyList() } return emptyList() @@ -226,15 +224,8 @@ class AutoImportFix(expression: KtSimpleNameExpression, diagnostic: Diagnostic? override fun getSupportedErrors() = ERRORS companion object : JetSingleIntentionActionFactory() { - override fun createAction(diagnostic: Diagnostic): KotlinQuickFixAction? { - // There could be different psi elements (i.e. JetArrayAccessExpression), but we can fix only JetSimpleNameExpression case - val psiElement = diagnostic.getPsiElement() - if (psiElement is KtSimpleNameExpression) { - return AutoImportFix(psiElement, diagnostic) - } - - return null - } + override fun createAction(diagnostic: Diagnostic) = + (diagnostic.getPsiElement() as? KtSimpleNameExpression)?.let { AutoImportFix(it, diagnostic) } override fun isApplicableForCodeFragment() = true @@ -242,40 +233,43 @@ class AutoImportFix(expression: KtSimpleNameExpression, diagnostic: Diagnostic? } } -class MissingInvokeAutoImportFix(expression: KtExpression, diagnostic: Diagnostic) : AutoImportFixBase(expression, diagnostic) { - override fun getImportNames(diagnostics: Collection, element: KtExpression) = setOf("invoke") +class MissingInvokeAutoImportFix(expression: KtExpression, diagnostic: Diagnostic) : + AutoImportFixBase(expression, diagnostic) { + override fun getImportNames() = OperatorNameConventions.INVOKE.singletonList() - override fun getTypeAndReceiver() = CallTypeAndReceiver.OPERATOR(element as KtExpression) + override fun getCallTypeAndReceiver() = CallTypeAndReceiver.OPERATOR(element) override fun getSupportedErrors() = ERRORS companion object : JetSingleIntentionActionFactory() { - override fun createAction(diagnostic: Diagnostic): KotlinQuickFixAction? { - val element = diagnostic.psiElement - if (element is KtExpression) { - return MissingInvokeAutoImportFix(element, diagnostic) - } - - return null - } + override fun createAction(diagnostic: Diagnostic) = + (diagnostic.psiElement as? KtExpression)?.let { MissingInvokeAutoImportFix(it, diagnostic) } private val ERRORS by lazy(LazyThreadSafetyMode.PUBLICATION) { QuickFixes.getInstance().getDiagnostics(this) } } } -class MissingArrayAccessorAutoImportFix(element: KtArrayAccessExpression, diagnostic: Diagnostic) : AutoImportFixBase(element, diagnostic) { - override fun getImportNames(diagnostics: Collection, element: KtExpression): Set { - val s = if ((element.parent as? KtBinaryExpression)?.operationToken == KtTokens.EQ) "set" else "get" - return setOf(s) +class MissingArrayAccessorAutoImportFix(element: KtArrayAccessExpression, diagnostic: Diagnostic) : + AutoImportFixBase(element, diagnostic) { + override fun getImportNames(): Collection { + val name = if (diagnostics.first().factory == Errors.NO_GET_METHOD) { + OperatorNameConventions.GET + } + else { + OperatorNameConventions.SET + } + return name.singletonList() } - override fun getTypeAndReceiver() = - CallTypeAndReceiver.OPERATOR((element as KtArrayAccessExpression).arrayExpression!!) + override fun getCallTypeAndReceiver() = + CallTypeAndReceiver.OPERATOR(element.arrayExpression!!) override fun getSupportedErrors() = ERRORS companion object : JetSingleIntentionActionFactory() { - override fun createAction(diagnostic: Diagnostic): KotlinQuickFixAction? { + override fun createAction(diagnostic: Diagnostic): KotlinQuickFixAction? { + assert(diagnostic.factory == Errors.NO_GET_METHOD || diagnostic.factory == Errors.NO_SET_METHOD) + val element = diagnostic.psiElement if (element is KtArrayAccessExpression && element.arrayExpression != null) { return MissingArrayAccessorAutoImportFix(element, diagnostic) @@ -288,16 +282,23 @@ class MissingArrayAccessorAutoImportFix(element: KtArrayAccessExpression, diagno } } -class MissingDelegateAccessorsAutoImportFix(element: KtExpression, diagnostics: Collection) : AutoImportFixBase(element, diagnostics) { +class MissingDelegateAccessorsAutoImportFix(element: KtExpression, diagnostics: Collection) : + AutoImportFixBase(element, diagnostics) { override fun createAction(project: Project, editor: Editor): KotlinAddImportAction { return KotlinAddImportAction(project, editor, element, suggestions) } - override fun getImportNames(diagnostics: Collection, element: KtExpression): Set { - return diagnostics.mapTo(LinkedHashSet()) { if (it.toString().contains("setValue")) "setValue" else "getValue" } + override fun getImportNames(): Collection { + return diagnostics.map { + val missingMethodSignature = Errors.DELEGATE_SPECIAL_FUNCTION_MISSING.cast(it).a + if (missingMethodSignature.startsWith(OperatorNameConventions.GET_VALUE.identifier)) + OperatorNameConventions.GET_VALUE + else + OperatorNameConventions.SET_VALUE + }.distinct() } - override fun getTypeAndReceiver() = CallTypeAndReceiver.DELEGATE(element as KtExpression) + override fun getCallTypeAndReceiver() = CallTypeAndReceiver.DELEGATE(element) override fun getSupportedErrors() = ERRORS @@ -311,19 +312,13 @@ class MissingDelegateAccessorsAutoImportFix(element: KtExpression, diagnostics: } } -class MissingComponentsAutoImportFix(element: KtExpression, diagnostics: Collection) : AutoImportFixBase(element, diagnostics) { - override fun createAction(project: Project, editor: Editor): KotlinAddImportAction { - return KotlinAddImportAction(project, editor, element, suggestions) - } +class MissingComponentsAutoImportFix(element: KtExpression, diagnostics: Collection) : + AutoImportFixBase(element, diagnostics) { + override fun createAction(project: Project, editor: Editor) = KotlinAddImportAction(project, editor, element, suggestions) - override fun getImportNames(diagnostics: Collection, element: KtExpression): List { - return diagnostics.map { - @Suppress("UNCHECKED_CAST") - (it as DiagnosticWithParameters2<*, Name, *>).a.identifier - } - } + override fun getImportNames() = diagnostics.map { Name.identifier(Errors.COMPONENT_FUNCTION_MISSING.cast(it).a.identifier) } - override fun getTypeAndReceiver() = CallTypeAndReceiver.OPERATOR(element as KtExpression) + override fun getCallTypeAndReceiver() = CallTypeAndReceiver.OPERATOR(element) override fun getSupportedErrors() = ERRORS diff --git a/idea/testData/quickfix/autoImports/delegateExtensionGet.test b/idea/testData/quickfix/autoImports/delegateExtensionGet.test index 0a54d0a7d01..c5b31802465 100644 --- a/idea/testData/quickfix/autoImports/delegateExtensionGet.test +++ b/idea/testData/quickfix/autoImports/delegateExtensionGet.test @@ -1,6 +1,6 @@ // FILE: first.before.kt // "Import" "true" -// ERROR: Missing 'getValue(testing.BigTest, kotlin.PropertyMetadata)' method on delegate of type 'some.DelegateImpl' +// ERROR: Missing 'getValue(testing.BigTest, kotlin.reflect.KProperty<*>)' method on delegate of type 'some.DelegateImpl' package testing @@ -25,7 +25,7 @@ public fun DelegateImpl.getValue(thisRef: Any?, property: PropertyMetadat // FILE: first.after.kt // "Import" "true" -// ERROR: Missing 'getValue(testing.BigTest, kotlin.PropertyMetadata)' method on delegate of type 'some.DelegateImpl' +// ERROR: Missing 'getValue(testing.BigTest, kotlin.reflect.KProperty<*>)' method on delegate of type 'some.DelegateImpl' package testing diff --git a/idea/testData/quickfix/autoImports/delegateExtensionSet.test b/idea/testData/quickfix/autoImports/delegateExtensionSet.test index d9873fe93c5..6eab383a7c0 100644 --- a/idea/testData/quickfix/autoImports/delegateExtensionSet.test +++ b/idea/testData/quickfix/autoImports/delegateExtensionSet.test @@ -1,6 +1,6 @@ // FILE: first.before.kt // "Import" "true" -// ERROR: Missing 'setValue(testing.BigTest, kotlin.PropertyMetadata, kotlin.Int)' method on delegate of type 'some.DelegateImpl' +// ERROR: Missing 'setValue(testing.BigTest, kotlin.reflect.KProperty<*>, kotlin.Int)' method on delegate of type 'some.DelegateImpl' package testing @@ -26,7 +26,7 @@ public fun DelegateImpl.setValue(thisRef: Any, propertyMetadata: Property // FILE: first.after.kt // "Import" "true" -// ERROR: Missing 'setValue(testing.BigTest, kotlin.PropertyMetadata, kotlin.Int)' method on delegate of type 'some.DelegateImpl' +// ERROR: Missing 'setValue(testing.BigTest, kotlin.reflect.KProperty<*>, kotlin.Int)' method on delegate of type 'some.DelegateImpl' package testing