From 0072bf079d8bc9dcf17843dd4cb15d2f39522d76 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Wed, 17 Aug 2016 16:37:06 +0300 Subject: [PATCH] Changes on review of pull request --- .../kotlin/idea/KotlinBundle.properties | 1 - .../kotlin/idea/core/KotlinIndicesHelper.kt | 33 ++++---- .../kotlin/idea/quickfix/AutoImportFix.kt | 83 +++++++++++-------- .../noStaticImportForTopLevelFunction.test | 32 +++++++ .../QuickFixMultiFileTestGenerated.java | 12 ++- 5 files changed, 103 insertions(+), 58 deletions(-) create mode 100644 idea/testData/quickfix/autoImports/noStaticImportForTopLevelFunction.test diff --git a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/KotlinBundle.properties b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/KotlinBundle.properties index bfbb858fef4..d8cc16a7bd9 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/KotlinBundle.properties +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/KotlinBundle.properties @@ -16,7 +16,6 @@ remove.unnecessary.non.null.assertion=Remove unnecessary non-null assertion (!!) change.to.backing.field=Change reference to backing field implement.members=Implement members import.fix=Import -import.static.fix=Static import imports.chooser.title=Imports rename.kotlin.package.class.error=Can't rename Kotlin package facade class add.semicolon.after.invocation=Add semicolon after invocation of ''{0}'' diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/KotlinIndicesHelper.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/KotlinIndicesHelper.kt index a112cfc4e12..8d63316ca2b 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/KotlinIndicesHelper.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/KotlinIndicesHelper.kt @@ -205,32 +205,29 @@ class KotlinIndicesHelper( .toSet() } - - fun getJvmStatics(name: String): Collection { - return sequenceOf(PsiShortNamesCache.getInstance(project).getFieldsByName(name, scope), - PsiShortNamesCache.getInstance(project).getMethodsByName(name, scope)) - .flatMap { it.asSequence() } - .mapNotNull { (it as PsiMember).getJavaMemberDescriptor(resolutionFacade) } + fun getJvmCallablesByName(name: String): Collection { + val javaDeclarations = PsiShortNamesCache.getInstance(project).getFieldsByName(name, scope).asSequence() + + PsiShortNamesCache.getInstance(project).getMethodsByName(name, scope).asSequence() + return javaDeclarations + .mapNotNull { (it as PsiMember).getJavaMemberDescriptor(resolutionFacade) as? CallableDescriptor } .filter(descriptorFilter) .toSet() } - fun getKotlinStatics(name: String): Collection { - return sequenceOf(KotlinFunctionShortNameIndex.getInstance().get(name, project, scope), - KotlinPropertyShortNameIndex.getInstance().get(name, project, scope)) - .flatMap { it.asSequence() } - .mapNotNull { - when (it) { - is KtNamedFunction -> it.descriptor - is KtProperty -> it.descriptor - else -> null - } - }.filterNot { it.importableFqName == null } + fun getKotlinCallablesByName(name: String): Collection { + val functions = KotlinFunctionShortNameIndex.getInstance().get(name, project, scope) + .asSequence() + .map { it.descriptor as? CallableDescriptor } + val properties = KotlinPropertyShortNameIndex.getInstance().get(name, project, scope) + .asSequence() + .map { it.descriptor as? CallableDescriptor } + + return (functions + properties) + .filterNotNull() .filter(descriptorFilter) .toSet() } - fun getKotlinClasses(nameFilter: (String) -> Boolean, kindFilter: (ClassKind) -> Boolean): Collection { return KotlinFullClassNameIndex.getInstance().getAllKeys(project).asSequence() .map { FqName(it) } diff --git a/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt b/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt index 6f9f30dd3fa..f99c4e7fcf3 100644 --- a/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt +++ b/idea/src/org/jetbrains/kotlin/idea/quickfix/AutoImportFix.kt @@ -29,6 +29,7 @@ import com.intellij.psi.PsiFile import com.intellij.psi.util.PsiModificationTracker import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithVisibility +import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.DiagnosticFactory import org.jetbrains.kotlin.diagnostics.Errors @@ -42,6 +43,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.getResolveScope import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde import org.jetbrains.kotlin.idea.core.KotlinIndicesHelper import org.jetbrains.kotlin.idea.core.isVisible +import org.jetbrains.kotlin.idea.imports.canBeReferencedViaImport import org.jetbrains.kotlin.idea.project.ProjectStructureUtil import org.jetbrains.kotlin.idea.util.CallTypeAndReceiver import org.jetbrains.kotlin.name.Name @@ -147,8 +149,7 @@ internal abstract class AutoImportFixBase(expression: T) : val indicesHelper = KotlinIndicesHelper(resolutionFacade, searchScope, ::isVisible) - val result = fillCandidates(callTypeAndReceiver, file, bindingContext, - indicesHelper, nameStr, filterByCallType = { callTypeAndReceiver.callType.descriptorKindFilter.accepts(it) }) + val result = fillCandidates(nameStr, callTypeAndReceiver, bindingContext, indicesHelper) return if (result.size > 1) reduceCandidatesBasedOnDependencyRuleViolation(result, file) @@ -156,9 +157,12 @@ internal abstract class AutoImportFixBase(expression: T) : result } - abstract fun fillCandidates(callTypeAndReceiver: CallTypeAndReceiver<*, *>, file: KtFile, - bindingContext: BindingContext, indicesHelper: KotlinIndicesHelper, - nameStr: String, filterByCallType: (descriptor: DeclarationDescriptor) -> (Boolean)): List + abstract fun fillCandidates( + name: String, + callTypeAndReceiver: CallTypeAndReceiver<*, *>, + bindingContext: BindingContext, + indicesHelper: KotlinIndicesHelper + ): List private fun reduceCandidatesBasedOnDependencyRuleViolation( candidates: Collection, file: PsiFile): Collection { @@ -172,25 +176,32 @@ internal abstract class AutoImportFixBase(expression: T) : } internal abstract class AutoImportFixBaseDynamicContext(expression: T) : AutoImportFixBase(expression) { - override fun fillCandidates(callTypeAndReceiver: CallTypeAndReceiver<*, *>, file: KtFile, bindingContext: BindingContext, indicesHelper: KotlinIndicesHelper, nameStr: String, filterByCallType: (DeclarationDescriptor) -> Boolean): List { + override fun fillCandidates( + name: String, + callTypeAndReceiver: CallTypeAndReceiver<*, *>, + bindingContext: BindingContext, + indicesHelper: KotlinIndicesHelper + ): List { val result = ArrayList() 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) + val filterByCallType = { descriptor: DeclarationDescriptor -> callTypeAndReceiver.callType.descriptorKindFilter.accepts(descriptor) } + + if (ProjectStructureUtil.isJsKotlinModule(expression.getContainingKtFile())) { + indicesHelper.getKotlinClasses({ it == name }, { true }).filterTo(result, filterByCallType) } else { - indicesHelper.getJvmClassesByName(nameStr).filterTo(result, filterByCallType) + indicesHelper.getJvmClassesByName(name).filterTo(result, filterByCallType) } - indicesHelper.getTopLevelCallablesByName(nameStr).filterTo(result, filterByCallType) + indicesHelper.getTopLevelCallablesByName(name).filterTo(result, filterByCallType) } } - result.addAll(indicesHelper.getCallableTopLevelExtensions(callTypeAndReceiver, expression, bindingContext) { it == nameStr }) + result.addAll(indicesHelper.getCallableTopLevelExtensions(callTypeAndReceiver, expression, bindingContext) { it == name }) return result } @@ -366,16 +377,28 @@ internal class MissingComponentsAutoImportFix(element: KtExpression, override va internal class AutoImportStaticFix(expression: KtSimpleNameExpression) : AutoImportFixBase(expression) { - override fun getText() = KotlinBundle.message("import.static.fix") + override fun getText() = "Static import" - override fun fillCandidates(callTypeAndReceiver: CallTypeAndReceiver<*, *>, file: KtFile, bindingContext: BindingContext, indicesHelper: KotlinIndicesHelper, nameStr: String, filterByCallType: (DeclarationDescriptor) -> Boolean): List { + override fun fillCandidates( + name: String, + callTypeAndReceiver: CallTypeAndReceiver<*, *>, + bindingContext: BindingContext, + indicesHelper: KotlinIndicesHelper + ): List { val result = ArrayList() val expression = element if (expression is KtSimpleNameExpression) { if (!expression.isImportDirectiveExpression() && !KtPsiUtil.isSelectorInQualified(expression)) { - indicesHelper.getKotlinStatics(nameStr).filterTo(result, filterByCallType) - if (!(ProjectStructureUtil.isJsKotlinModule(file))) { - indicesHelper.getJvmStatics(nameStr).filterTo(result, filterByCallType) + val filterByCallType = { descriptor: DeclarationDescriptor -> callTypeAndReceiver.callType.descriptorKindFilter.accepts(descriptor) } + + indicesHelper.getKotlinCallablesByName(name) + .filter { it.canBeReferencedViaImport() && it.containingDeclaration !is PackageFragmentDescriptor } + .filterTo(result, filterByCallType) + + if (!ProjectStructureUtil.isJsKotlinModule(expression.getContainingKtFile())) { + indicesHelper.getJvmCallablesByName(name) + .filter { it.canBeReferencedViaImport() } + .filterTo(result, filterByCallType) } } } @@ -385,28 +408,18 @@ internal class AutoImportStaticFix(expression: KtSimpleNameExpression) : AutoImp override fun getCallTypeAndReceiver() = CallTypeAndReceiver.detect(element) override val importNames: Collection = run { - if (element.getIdentifier() == null) { - val conventionName = KtPsiUtil.getConventionName(element) - if (conventionName != null) { - if (element is KtOperationReferenceExpression) { - val elementType = element.firstChild.node.elementType - if (elementType in OperatorConventions.ASSIGNMENT_OPERATIONS) { - val counterpart = OperatorConventions.ASSIGNMENT_OPERATION_COUNTERPARTS[elementType] - val counterpartName = OperatorConventions.BINARY_OPERATION_NAMES[counterpart] - if (counterpartName != null) { - return@run listOf(conventionName, counterpartName) - } - } - } - - return@run conventionName.singletonOrEmptyList() + if (element.getIdentifier() != null) { + val name = element.getReferencedName() + if (Name.isValidIdentifier(name)) { + Name.identifier(name).singletonList() + } + else { + emptyList() } } - else if (Name.isValidIdentifier(element.getReferencedName())) { - return@run Name.identifier(element.getReferencedName()).singletonOrEmptyList() + else { + emptyList() } - - emptyList() } override fun getSupportedErrors(): Collection> = ERRORS diff --git a/idea/testData/quickfix/autoImports/noStaticImportForTopLevelFunction.test b/idea/testData/quickfix/autoImports/noStaticImportForTopLevelFunction.test new file mode 100644 index 00000000000..0c611a50297 --- /dev/null +++ b/idea/testData/quickfix/autoImports/noStaticImportForTopLevelFunction.test @@ -0,0 +1,32 @@ +// FILE: main.before.kt +// "Static import" "false" +// ERROR: Unresolved reference: foobar +// ACTION: Create function 'foobar' +// ACTION: Import +// ACTION: Rename reference + +//KT-9009 + + +fun f() { + foobar() +} + +//FILE: dependency.before.kt +package bar + +fun foobar() { +} + +//FILE: main.after.kt +// "Static import" "false" +// ERROR: Unresolved reference: foobar +// ACTION: Create function 'foobar' +// ACTION: Rename reference + +//KT-9009 + + +fun f() { + foobar() +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java index cbe901dd4e2..beb6b3131be 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixMultiFileTestGenerated.java @@ -367,15 +367,19 @@ public class QuickFixMultiFileTestGenerated extends AbstractQuickFixMultiFileTes @TestMetadata("noStaticImportForJavaNonStaticField.test") public void testNoStaticImportForJavaNonStaticField() throws Exception { - String fileName = - KotlinTestUtils.navigationMetadata("idea/testData/quickfix/autoImports/noStaticImportForJavaNonStaticField.test"); + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/autoImports/noStaticImportForJavaNonStaticField.test"); doTestWithExtraFile(fileName); } @TestMetadata("noStaticImportForJavaNonStaticMethod.test") public void testNoStaticImportForJavaNonStaticMethod() throws Exception { - String fileName = - KotlinTestUtils.navigationMetadata("idea/testData/quickfix/autoImports/noStaticImportForJavaNonStaticMethod.test"); + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/autoImports/noStaticImportForJavaNonStaticMethod.test"); + doTestWithExtraFile(fileName); + } + + @TestMetadata("noStaticImportForTopLevelFunction.test") + public void testNoStaticImportForTopLevelFunction() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/quickfix/autoImports/noStaticImportForTopLevelFunction.test"); doTestWithExtraFile(fileName); }