From dca23f871cc22acee9258c3d58b40d71e3693858 Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Wed, 29 May 2019 14:32:39 +0300 Subject: [PATCH] Allow to force-require syntetic SAM-adapters even in NI This is needed for some IDE clients, particularly, completion: even though presenting only non-converted member (e.g., 'foo(Consumer') is nominally OK, as resolution with NI is smart enough to accept 'foo { }' for such a call, it is inconvenient for users (for example, hitting enter would insert round brackets instead of a figure brackets) This commit adds very-very narrow API (borderline hacky) in JavaSyntheticScopes, to allow clients explicitly ask for a scopes with force-enabled synthetic conversions. It fixes several tests, which had started to fail after corresponding commit about NI and SAM-adapters (fe5976d7f465d1fc58ed07c1fc0829632032da20), e.g.: - Java8BasicCompletionTestGenerated.testCollectionMethods - Java8BasicCompletionTestGenerated.testStreamMethods - JvmBasicCompletionTestGenerated$Common$StaticMembers.testJavaStaticMethods - JvmBasicCompletionTestGenerated$Java.testSAMAdaptersStatic - JvmWithLibBasicCompletionTestGenerated.testSamAdapter - JvmWithLibBasicCompletionTestGenerated.testSamAdapterAndGenerics Note that changes are made in ReferenceVariantsHelper, which is used by several other clients in IDE. Presumably, those changes are needed for them too. --- .../kotlin/synthetic/JavaSyntheticScopes.kt | 62 ++++++++++++++----- .../synthetic/SamAdapterFunctionsScope.kt | 5 +- .../synthetic/syntheticExtensionsUtils.kt | 18 ++++++ .../codeInsight/ReferenceVariantsHelper.kt | 19 +++++- .../kotlin/idea/core/KotlinIndicesHelper.kt | 3 +- .../kotlin/idea/core/psiModificationUtils.kt | 59 +++++++++++++++--- .../inspections/JavaMapForEachInspection.kt | 5 +- .../RedundantSamConstructorInspection.kt | 11 +++- 8 files changed, 146 insertions(+), 36 deletions(-) diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticScopes.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticScopes.kt index 598e0edf969..2fdfb97b8e5 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticScopes.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/JavaSyntheticScopes.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.synthetic import com.intellij.openapi.project.Project +import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.descriptors.ModuleDescriptor import org.jetbrains.kotlin.extensions.ProjectExtensionDescriptor @@ -28,34 +29,61 @@ import org.jetbrains.kotlin.resolve.scopes.SyntheticScopes import org.jetbrains.kotlin.storage.StorageManager class JavaSyntheticScopes( - private val project: Project, - private val moduleDescriptor: ModuleDescriptor, - storageManager: StorageManager, - lookupTracker: LookupTracker, - languageVersionSettings: LanguageVersionSettings, - samConventionResolver: SamConversionResolver, - deprecationResolver: DeprecationResolver -): SyntheticScopes { - override val scopes = run { - val javaSyntheticPropertiesScope = JavaSyntheticPropertiesScope(storageManager, lookupTracker) + private val project: Project, + private val moduleDescriptor: ModuleDescriptor, + storageManager: StorageManager, + lookupTracker: LookupTracker, + languageVersionSettings: LanguageVersionSettings, + samConventionResolver: SamConversionResolver, + deprecationResolver: DeprecationResolver +) : SyntheticScopes { + override val scopes: Collection + // New Inference disables SAM-adapters scope, because it knows how to perform SAM-conversion in resolution + // However, some outer clients (mostly in IDE) sometimes would like to look at synthetic SAM-produced descriptors + // (e.g., completion) + val scopesWithForceEnabledSamAdapters: Collection + + init { + val newInferenceEnabled = languageVersionSettings.supportsFeature(LanguageFeature.NewInference) + + val javaSyntheticPropertiesScope = JavaSyntheticPropertiesScope(storageManager, lookupTracker) val scopesFromExtensions = SyntheticScopeProviderExtension .getInstances(project) .flatMap { it.getScopes(moduleDescriptor, javaSyntheticPropertiesScope) } - listOf( - javaSyntheticPropertiesScope, - SamAdapterFunctionsScope( - storageManager, languageVersionSettings, samConventionResolver, deprecationResolver, - lookupTracker + + val samAdapterFunctionsScope = SamAdapterFunctionsScope( + storageManager, + samConventionResolver, + deprecationResolver, + lookupTracker, + samViaSyntheticScopeDisabled = newInferenceEnabled + ) + + scopes = listOf(javaSyntheticPropertiesScope, samAdapterFunctionsScope) + scopesFromExtensions + + if (newInferenceEnabled) { + val forceEnabledSamAdapterFunctionsScope = SamAdapterFunctionsScope( + storageManager, + samConventionResolver, + deprecationResolver, + lookupTracker, + samViaSyntheticScopeDisabled = false ) - ) + scopesFromExtensions + + scopesWithForceEnabledSamAdapters = + listOf(javaSyntheticPropertiesScope, forceEnabledSamAdapterFunctionsScope) + scopesFromExtensions + } else { + scopesWithForceEnabledSamAdapters = scopes + } } } interface SyntheticScopeProviderExtension { companion object : ProjectExtensionDescriptor( - "org.jetbrains.kotlin.syntheticScopeProviderExtension", SyntheticScopeProviderExtension::class.java) + "org.jetbrains.kotlin.syntheticScopeProviderExtension", SyntheticScopeProviderExtension::class.java + ) fun getScopes(moduleDescriptor: ModuleDescriptor, javaSyntheticPropertiesScope: JavaSyntheticPropertiesScope): List } \ No newline at end of file diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/SamAdapterFunctionsScope.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/SamAdapterFunctionsScope.kt index acea1170180..fcdb97f46b8 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/SamAdapterFunctionsScope.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/SamAdapterFunctionsScope.kt @@ -55,12 +55,11 @@ val SAM_LOOKUP_NAME = Name.special("") class SamAdapterFunctionsScope( storageManager: StorageManager, - private val languageVersionSettings: LanguageVersionSettings, private val samResolver: SamConversionResolver, private val deprecationResolver: DeprecationResolver, - private val lookupTracker: LookupTracker + private val lookupTracker: LookupTracker, + private val samViaSyntheticScopeDisabled: Boolean ) : SyntheticScope.Default() { - private val samViaSyntheticScopeDisabled = languageVersionSettings.supportsFeature(LanguageFeature.NewInference) private val extensionForFunction = storageManager.createMemoizedFunctionWithNullableValues { function -> diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/syntheticExtensionsUtils.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/syntheticExtensionsUtils.kt index f02f4b14019..bc8dc1263f6 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/syntheticExtensionsUtils.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/synthetic/syntheticExtensionsUtils.kt @@ -17,8 +17,14 @@ package org.jetbrains.kotlin.synthetic import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.descriptors.synthetic.SyntheticMemberDescriptor import org.jetbrains.kotlin.load.java.descriptors.JavaCallableMemberDescriptor import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor +import org.jetbrains.kotlin.load.java.sam.SamAdapterDescriptor +import org.jetbrains.kotlin.load.java.sam.SamConstructorDescriptor +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCallImpl +import org.jetbrains.kotlin.resolve.calls.tower.NewResolvedCallImpl import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue fun FunctionDescriptor.hasJavaOriginInHierarchy(): Boolean { @@ -55,3 +61,15 @@ fun syntheticVisibility(originalDescriptor: DeclarationDescriptorWithVisibility, } } + +fun ResolvedCall.isResolvedWithSamConversions(): Boolean { + return if (this is NewResolvedCallImpl) { + // New inference + this.resolvedCallAtom.argumentsWithConversion.isNotEmpty() + } else { + // Old Inference + this.resultingDescriptor is SamAdapterDescriptor<*> || + this.resultingDescriptor is SamConstructorDescriptor || + this.resultingDescriptor is SamAdapterExtensionFunctionDescriptor + } +} \ No newline at end of file diff --git a/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt b/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt index 0ba086b8268..27cc9bdf9df 100644 --- a/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt +++ b/idea/ide-common/src/org/jetbrains/kotlin/idea/codeInsight/ReferenceVariantsHelper.kt @@ -44,6 +44,7 @@ import org.jetbrains.kotlin.resolve.scopes.utils.collectAllFromMeAndParent import org.jetbrains.kotlin.resolve.scopes.utils.collectDescriptorsFiltered import org.jetbrains.kotlin.resolve.scopes.utils.memberScopeAsImportingScope import org.jetbrains.kotlin.resolve.source.getPsi +import org.jetbrains.kotlin.synthetic.JavaSyntheticScopes import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.expressions.DoubleColonLHS @@ -442,7 +443,7 @@ class ReferenceVariantsHelper( process(descriptor as CallableDescriptor) } - val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java) + val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java).forceEnableSamAdapters() if (kindFilter.acceptsKinds(DescriptorKindFilter.VARIABLES_MASK)) { val lookupLocation = (scope.ownerDescriptor.toSourceElement.getPsi() as? KtElement)?.let { KotlinLookupLocation(it) } ?: NoLookupLocation.FROM_IDE @@ -477,7 +478,21 @@ fun ResolutionScope.collectSyntheticStaticMembersAndConstructors( kindFilter: DescriptorKindFilter, nameFilter: (Name) -> Boolean ): List { - val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java) + val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java).forceEnableSamAdapters() return (syntheticScopes.collectSyntheticStaticFunctions(this) + syntheticScopes.collectSyntheticConstructors(this)) .filter { kindFilter.accepts(it) && nameFilter(it.name) } } + +// New Inference disables scope with synthetic SAM-adapters because it uses conversions for resolution +// However, sometimes we need to pretend that we have those synthetic members, for example: +// - to show both option (with SAM-conversion signature, and without) in completion +// - for various intentions and checks (see RedundantSamConstructorInspection, ConflictingExtensionPropertyIntention and other) +// TODO(dsavvinov): review clients, rewrite them to not rely on synthetic adapetrs +fun SyntheticScopes.forceEnableSamAdapters(): SyntheticScopes { + return if (this !is JavaSyntheticScopes) + this + else + object : SyntheticScopes { + override val scopes: Collection = this@forceEnableSamAdapters.scopesWithForceEnabledSamAdapters + } +} \ No newline at end of file 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 a10c00b21e5..a1e0affba02 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 @@ -30,6 +30,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.resolveImportReference import org.jetbrains.kotlin.idea.caches.resolve.unsafeResolveToDescriptor import org.jetbrains.kotlin.idea.caches.resolve.util.getJavaMemberDescriptor import org.jetbrains.kotlin.idea.caches.resolve.util.resolveToDescriptor +import org.jetbrains.kotlin.idea.codeInsight.forceEnableSamAdapters import org.jetbrains.kotlin.idea.core.extension.KotlinIndicesHelperExtension import org.jetbrains.kotlin.idea.resolve.ResolutionFacade import org.jetbrains.kotlin.idea.resolve.frontendService @@ -466,7 +467,7 @@ class KotlinIndicesHelper( processor(descriptor) // SAM-adapter - val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java) + val syntheticScopes = resolutionFacade.getFrontendService(SyntheticScopes::class.java).forceEnableSamAdapters() syntheticScopes.collectSyntheticStaticFunctions(container.staticScope, descriptor.name, NoLookupLocation.FROM_IDE) .filterIsInstance>() .firstOrNull { it.baseDescriptorForSynthetic.original == descriptor.original } diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt index 250ad17d245..2b717ab6389 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/psiModificationUtils.kt @@ -22,11 +22,15 @@ import com.intellij.psi.impl.source.codeStyle.CodeEditUtil import com.intellij.psi.tree.IElementType import com.intellij.psi.util.PsiTreeUtil import org.jetbrains.kotlin.builtins.isFunctionOrSuspendFunctionType +import org.jetbrains.kotlin.config.LanguageFeature +import org.jetbrains.kotlin.config.LanguageVersionSettings import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.extensions.DeclarationAttributeAltererExtension import org.jetbrains.kotlin.idea.caches.resolve.analyze +import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.references.mainReference +import org.jetbrains.kotlin.idea.resolve.frontendService import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers import org.jetbrains.kotlin.idea.util.hasJvmFieldAnnotation import org.jetbrains.kotlin.idea.util.isExpectDeclaration @@ -41,6 +45,7 @@ import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.OverridingUtil import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.callUtil.getValueArgumentsInParentheses +import org.jetbrains.kotlin.resolve.calls.components.SamConversionTransformer import org.jetbrains.kotlin.resolve.calls.model.ArgumentMatch import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.types.KotlinType @@ -131,25 +136,61 @@ fun KtCallExpression.canMoveLambdaOutsideParentheses(): Boolean { if (callee is KtNameReferenceExpression) { val lambdaArgumentCount = valueArguments.count { it.getArgumentExpression()?.unpackFunctionLiteral() != null } val referenceArgumentCount = valueArguments.count { it.getArgumentExpression() is KtCallableReferenceExpression } - val bindingContext = analyze(BodyResolveMode.PARTIAL) + + val resolutionFacade = getResolutionFacade() + val samConversionTransformer = resolutionFacade.frontendService() + val languageVersionSettings = resolutionFacade.frontendService() + + val bindingContext = analyze(resolutionFacade, BodyResolveMode.PARTIAL) val targets = bindingContext[BindingContext.REFERENCE_TARGET, callee]?.let { listOf(it) } ?: bindingContext[BindingContext.AMBIGUOUS_REFERENCE_TARGET, callee] ?: listOf() + val candidates = targets.filterIsInstance() + // if there are functions among candidates but none of them have last function parameter then not show the intention - if (candidates.isNotEmpty() && candidates.none { candidate -> - val params = candidate.valueParameters - val lastParamType = params.lastOrNull()?.type - (lastParamType?.isFunctionOrSuspendFunctionType == true || lastParamType?.isTypeParameter() == true) && params.count { - it.type.let { type -> type.isFunctionOrSuspendFunctionType || type.isTypeParameter() } - } == lambdaArgumentCount + referenceArgumentCount - } - ) return false + val areAllCandidatesWithoutLastFunctionParameter = candidates.none { + it.allowsMoveOfLastParameterOutsideParentheses( + lambdaArgumentCount + referenceArgumentCount, + samConversionTransformer, + languageVersionSettings.supportsFeature(LanguageFeature.NewInference) + ) + } + + if (candidates.isNotEmpty() && areAllCandidatesWithoutLastFunctionParameter) return false } return true } +private fun FunctionDescriptor.allowsMoveOfLastParameterOutsideParentheses( + lambdaAndCallableReferencesInOriginalCallCount: Int, + samConversionTransformer: SamConversionTransformer, + newInferenceEnabled: Boolean +): Boolean { + fun KotlinType.allowsMoveOutsideParentheses(): Boolean { + // Fast-path + if (isFunctionOrSuspendFunctionType || isTypeParameter()) return true + + // Also check if it can be SAM-converted + // Note that it is not necessary in OI, where we provide synthetic candidate descriptors with already + // converted types, but in NI it is performed by conversions, so we check it explicitly + // Also note that 'newInferenceEnabled' is essentially a micro-optimization, as there are no + // harm in just calling 'samConversionTransformer' on all candidates. + return newInferenceEnabled && samConversionTransformer.getFunctionTypeForPossibleSamType(this.unwrap()) != null + } + + val params = valueParameters + val lastParamType = params.lastOrNull()?.type ?: return false + + if (!lastParamType.allowsMoveOutsideParentheses()) return false + + val movableParametersOfCandidateCount = params.count { it.type.allowsMoveOutsideParentheses() } + return movableParametersOfCandidateCount == lambdaAndCallableReferencesInOriginalCallCount +} + + + fun KtCallExpression.moveFunctionLiteralOutsideParentheses() { assert(lambdaArguments.isEmpty()) val argumentList = valueArgumentList!! diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/JavaMapForEachInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/JavaMapForEachInspection.kt index 4bbabd54dca..5d360098e63 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/JavaMapForEachInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/JavaMapForEachInspection.kt @@ -22,7 +22,7 @@ import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.callUtil.getType import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode -import org.jetbrains.kotlin.synthetic.SamAdapterExtensionFunctionDescriptor +import org.jetbrains.kotlin.synthetic.isResolvedWithSamConversions class JavaMapForEachInspection : AbstractApplicabilityBasedInspection( KtDotQualifiedExpression::class.java @@ -37,7 +37,8 @@ class JavaMapForEachInspection : AbstractApplicabilityBasedInspection) + resolutionResults.resultingDescriptor + else + SamCodegenUtil.getOriginalIfSamAdapter(resolutionResults.resultingDescriptor) ?: return false + return samAdapterOriginalDescriptor.original == originalCall.resultingDescriptor.original } @@ -176,7 +183,7 @@ class RedundantSamConstructorInspection : AbstractKotlinInspection() { val originalFunctionDescriptor = functionResolvedCall.resultingDescriptor.original as? FunctionDescriptor ?: return emptyList() val containingClass = originalFunctionDescriptor.containingDeclaration as? ClassDescriptor ?: return emptyList() - val syntheticScopes = functionCall.getResolutionFacade().getFrontendService(SyntheticScopes::class.java) + val syntheticScopes = functionCall.getResolutionFacade().getFrontendService(SyntheticScopes::class.java).forceEnableSamAdapters() // SAM adapters for static functions val contributedFunctions = syntheticScopes.collectSyntheticStaticFunctions(containingClass.staticScope)