From 95248256627ca793df1e1e20b58480c3b8d7a991 Mon Sep 17 00:00:00 2001 From: Jinseong Jeon Date: Mon, 11 Oct 2021 10:58:18 -0700 Subject: [PATCH] FIR LC: approximate local type only if it's necessary and out of scope --- .../components/KtFe10PsiTypeProvider.kt | 6 +- analysis/analysis-api-fir/build.gradle.kts | 2 +- .../fir/components/KtFirPsiTypeProvider.kt | 101 ++++++++++++++++-- .../components/AbstractPsiTypeProviderTest.kt | 53 +++++++-- .../api/components/KtPsiTypeProvider.kt | 18 +++- .../localClass_localFunctionInSameScope.txt | 2 +- .../localClass_localPropertyInSameScope.txt | 2 +- ...calPropertyInSampeScope_functionalType.txt | 2 +- .../localClass_memberFunction.txt | 2 +- 9 files changed, 159 insertions(+), 29 deletions(-) diff --git a/analysis/analysis-api-fe10/src/org/jetbrains/kotlin/analysis/api/descriptors/components/KtFe10PsiTypeProvider.kt b/analysis/analysis-api-fe10/src/org/jetbrains/kotlin/analysis/api/descriptors/components/KtFe10PsiTypeProvider.kt index 6850f8c5885..fa72ea7e082 100644 --- a/analysis/analysis-api-fe10/src/org/jetbrains/kotlin/analysis/api/descriptors/components/KtFe10PsiTypeProvider.kt +++ b/analysis/analysis-api-fe10/src/org/jetbrains/kotlin/analysis/api/descriptors/components/KtFe10PsiTypeProvider.kt @@ -30,14 +30,14 @@ internal class KtFe10PsiTypeProvider(override val analysisSession: KtFe10Analysi private val typeMapper by lazy { KtFe10JvmTypeMapperContext(analysisSession.resolveSession) } - override fun asPsiType(type: KtType, context: PsiElement, mode: TypeMappingMode): PsiType? = withValidityAssertion { + override fun asPsiType(type: KtType, useSitePosition: PsiElement, mode: TypeMappingMode): PsiType? = withValidityAssertion { val kotlinType = (type as KtFe10Type).type if (kotlinType.isError || kotlinType.arguments.any { !it.isStarProjection && it.type.isError }) { return null } - return asPsiType(simplifyType(kotlinType), context, mode) + return asPsiType(simplifyType(kotlinType), useSitePosition, mode) } private fun simplifyType(type: UnwrappedType): KotlinType { @@ -72,4 +72,4 @@ internal class KtFe10PsiTypeProvider(override val analysisSession: KtFe10Analysi val typeElement = ClsTypeElementImpl(context, typeText, '\u0000') return typeElement.type } -} \ No newline at end of file +} diff --git a/analysis/analysis-api-fir/build.gradle.kts b/analysis/analysis-api-fir/build.gradle.kts index c871b5f3c1e..b4e792543ec 100644 --- a/analysis/analysis-api-fir/build.gradle.kts +++ b/analysis/analysis-api-fir/build.gradle.kts @@ -31,7 +31,7 @@ dependencies { testApi(project(":kotlin-test:kotlin-test-junit")) testApi(toolsJar()) testApiJUnit5() - testRuntimeOnly(project(":analysis:symbol-light-classes")) + testApi(project(":analysis:symbol-light-classes")) testRuntimeOnly(intellijDep()) { includeJars( diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirPsiTypeProvider.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirPsiTypeProvider.kt index 910d649124a..6c3ee31c409 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirPsiTypeProvider.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirPsiTypeProvider.kt @@ -5,8 +5,8 @@ package org.jetbrains.kotlin.analysis.api.fir.components -import com.intellij.psi.PsiElement -import com.intellij.psi.PsiType +import com.intellij.lang.java.JavaLanguage +import com.intellij.psi.* import com.intellij.psi.impl.cache.TypeInfo import com.intellij.psi.impl.compiled.ClsTypeElementImpl import com.intellij.psi.impl.compiled.SignatureParsing @@ -19,13 +19,20 @@ import org.jetbrains.kotlin.analysis.api.types.KtType import org.jetbrains.kotlin.analysis.api.withValidityAssertion import org.jetbrains.kotlin.analysis.low.level.api.fir.api.FirModuleResolveState import org.jetbrains.kotlin.analysis.low.level.api.fir.api.withFirDeclaration +import org.jetbrains.kotlin.asJava.classes.KtLightClassForFacade +import org.jetbrains.kotlin.asJava.elements.KtLightElement import org.jetbrains.kotlin.codegen.signature.BothSignatureWriter import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.descriptors.Visibility +import org.jetbrains.kotlin.descriptors.java.JavaVisibilities import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.analysis.checkers.toRegularClassSymbol import org.jetbrains.kotlin.fir.backend.jvm.jvmTypeMapper import org.jetbrains.kotlin.fir.declarations.FirClass import org.jetbrains.kotlin.fir.declarations.FirResolvePhase import org.jetbrains.kotlin.fir.declarations.utils.superConeTypes +import org.jetbrains.kotlin.fir.psi import org.jetbrains.kotlin.fir.resolve.fullyExpandedType import org.jetbrains.kotlin.fir.resolve.substitution.AbstractConeSubstitutor import org.jetbrains.kotlin.fir.resolve.toSymbol @@ -33,6 +40,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirClassSymbol import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.load.kotlin.TypeMappingMode import org.jetbrains.kotlin.name.SpecialNames +import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.types.model.SimpleTypeMarker import java.text.StringCharacterIterator @@ -43,33 +51,108 @@ internal class KtFirPsiTypeProvider( override fun asPsiType( type: KtType, - context: PsiElement, + useSitePosition: PsiElement, mode: TypeMappingMode, ): PsiType? = withValidityAssertion { - type.coneType.asPsiType(rootModuleSession, analysisSession.firResolveState, mode, context) + type.coneType.asPsiType(rootModuleSession, analysisSession.firResolveState, mode, useSitePosition) } } -private fun ConeKotlinType.simplifyType(session: FirSession, state: FirModuleResolveState): ConeKotlinType { +private fun ConeKotlinType.simplifyType( + session: FirSession, + state: FirModuleResolveState, + useSitePosition: PsiElement, +): ConeKotlinType { val substitutor = AnonymousTypesSubstitutor(session, state) + val visibilityForApproximation = useSitePosition.visibilityForApproximation + // TODO: See if the given [useSitePosition] is an `inline` method + val isInlineFunction = false var currentType = this do { val oldType = currentType currentType = currentType.fullyExpandedType(session) currentType = currentType.upperBoundIfFlexible() currentType = substitutor.substituteOrSelf(currentType) - currentType = PublicTypeApproximator.approximateTypeToPublicDenotable(currentType, session) ?: currentType + if (shouldHideLocalType(visibilityForApproximation, isInlineFunction)) { + val localTypes: List = if (isLocal(session)) listOf(this) else { + typeArguments.mapNotNull { + if (it is ConeKotlinTypeProjection && it.type.isLocal(session)) { + it.type + } else null + } + } + val unavailableLocalTypes = localTypes.filterNot { it.isLocalButAvailableAtPosition(session, useSitePosition) } + // Need to approximate if there are local types that are not available in this scope + val needsApproximation = localTypes.isNotEmpty() && unavailableLocalTypes.isNotEmpty() + if (needsApproximation) { + // TODO: can we approximate local types in type arguments *selectively* ? + currentType = PublicTypeApproximator.approximateTypeToPublicDenotable(currentType, session) ?: currentType + } + } } while (oldType !== currentType) return currentType } +// Mimic FirDeclaration.visibilityForApproximation +private val PsiElement.visibilityForApproximation: Visibility + get() { + if (this !is PsiMember) return Visibilities.Local + val containerVisibility = + if (parent is KtLightClassForFacade) Visibilities.Public + else (parent as? PsiClass)?.visibility ?: Visibilities.Local + if (containerVisibility == Visibilities.Local || visibility == Visibilities.Local) return Visibilities.Local + if (containerVisibility == Visibilities.Private) return Visibilities.Private + return visibility + } + +// Mimic JavaElementUtil#getVisibility +private val PsiModifierListOwner.visibility: Visibility + get() { + if (hasModifierProperty(PsiModifier.PUBLIC)) { + return Visibilities.Public + } + if (hasModifierProperty(PsiModifier.PRIVATE) || hasModifierProperty(PsiModifier.PACKAGE_LOCAL)) { + return Visibilities.Private + } + return if (language == JavaLanguage.INSTANCE) { + when { + hasModifierProperty(PsiModifier.PROTECTED) && hasModifierProperty(PsiModifier.STATIC) -> + JavaVisibilities.ProtectedStaticVisibility + hasModifierProperty(PsiModifier.PROTECTED) -> + JavaVisibilities.ProtectedAndPackage + else -> + JavaVisibilities.PackageVisibility + } + } else Visibilities.DEFAULT_VISIBILITY + } + +private fun ConeKotlinType.isLocal(session: FirSession): Boolean { + return with(session.typeContext) { + this@isLocal.typeConstructor().isLocalType() + } +} + +private fun ConeKotlinType.isLocalButAvailableAtPosition( + session: FirSession, + useSitePosition: PsiElement, +): Boolean { + val localClassSymbol = this.toRegularClassSymbol(session) ?: return false + val localPsi = localClassSymbol.source?.psi ?: return false + val context = (useSitePosition as? KtLightElement<*, *>)?.kotlinOrigin ?: useSitePosition + // Local type is available if it's inside the same context (containing declaration) + // or containing declaration is inside the local type, e.g., a member of the local class + return localPsi == context || + localPsi.parents.any { it == context } || + context.parents.any { it == localPsi } +} + internal fun ConeKotlinType.asPsiType( session: FirSession, state: FirModuleResolveState, mode: TypeMappingMode, - psiContext: PsiElement, + useSitePosition: PsiElement, ): PsiType? { - val correctedType = simplifyType(session, state) + val correctedType = simplifyType(session, state, useSitePosition) if (correctedType is ConeClassErrorType || correctedType !is SimpleTypeMarker) return null @@ -91,7 +174,7 @@ internal fun ConeKotlinType.asPsiType( val typeInfo = TypeInfo.fromString(javaType, false) val typeText = TypeInfo.createTypeText(typeInfo) ?: return null - val typeElement = ClsTypeElementImpl(psiContext, typeText, '\u0000') + val typeElement = ClsTypeElementImpl(useSitePosition, typeText, '\u0000') return typeElement.type } diff --git a/analysis/analysis-api-fir/tests/org/jetbrains/kotlin/analysis/api/fir/components/AbstractPsiTypeProviderTest.kt b/analysis/analysis-api-fir/tests/org/jetbrains/kotlin/analysis/api/fir/components/AbstractPsiTypeProviderTest.kt index edf911eee97..49802ae1364 100644 --- a/analysis/analysis-api-fir/tests/org/jetbrains/kotlin/analysis/api/fir/components/AbstractPsiTypeProviderTest.kt +++ b/analysis/analysis-api-fir/tests/org/jetbrains/kotlin/analysis/api/fir/components/AbstractPsiTypeProviderTest.kt @@ -5,30 +5,69 @@ package org.jetbrains.kotlin.analysis.api.fir.components +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiElementVisitor import org.jetbrains.kotlin.analysis.api.analyse -import org.jetbrains.kotlin.analysis.api.fir.test.framework.AbstractHLApiSingleModuleTest -import org.jetbrains.kotlin.analysis.low.level.api.fir.test.base.expressionMarkerProvider +import org.jetbrains.kotlin.analysis.api.fir.FirFrontendApiTestConfiguratorService +import org.jetbrains.kotlin.analysis.api.impl.barebone.test.expressionMarkerProvider +import org.jetbrains.kotlin.analysis.api.impl.base.test.test.framework.AbstractHLApiSingleFileTest +import org.jetbrains.kotlin.asJava.classes.KtLightClass +import org.jetbrains.kotlin.asJava.elements.KtLightElement +import org.jetbrains.kotlin.light.classes.symbol.classes.getOrCreateFirLightClass +import org.jetbrains.kotlin.light.classes.symbol.classes.getOrCreateFirLightFacade +import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.psi.KtFile +import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.test.model.TestModule import org.jetbrains.kotlin.test.services.TestServices import org.jetbrains.kotlin.test.services.assertions -abstract class AbstractPsiTypeProviderTest : AbstractHLApiSingleModuleTest() { +abstract class AbstractPsiTypeProviderTest : AbstractHLApiSingleFileTest(FirFrontendApiTestConfiguratorService) { - override fun doTestByFileStructure(ktFiles: List, module: TestModule, testServices: TestServices) { - val mainKtFile = ktFiles.singleOrNull() ?: ktFiles.first { it.name == "main.kt" } - val declaration = testServices.expressionMarkerProvider.getElementOfTypAtCaret(mainKtFile) + override fun doTestByFileStructure(ktFile: KtFile, module: TestModule, testServices: TestServices) { + val mainKtFileFqName = ktFile.packageFqName.child(Name.identifier(ktFile.name)) + val declaration = testServices.expressionMarkerProvider.getElementOfTypAtCaret(ktFile) + val containingClass = + declaration.parents.firstOrNull { it is KtClassOrObject }?.let { getOrCreateFirLightClass(it as KtClassOrObject) } + ?: getOrCreateFirLightFacade(listOf(ktFile), mainKtFileFqName) + ?: error("Can't get or create containing KtLightClass for $declaration") + val psiContext = containingClass.findLightDeclarationContext(declaration) + ?: error("Can't find psi context for $declaration") val actual = buildString { executeOnPooledThreadInReadAction { analyse(declaration) { val ktType = declaration.getReturnKtType() appendLine("KtType: ${ktType.render()}") - appendLine("PsiType: ${ktType.asPsiType(declaration)}") + appendLine("PsiType: ${ktType.asPsiType(psiContext)}") } } } testServices.assertions.assertEqualsToFile(testDataFileSibling(".txt"), actual) } + private fun KtLightClass.findLightDeclarationContext(ktDeclaration: KtDeclaration): KtLightElement<*, *>? { + val selfOrParents = listOf(ktDeclaration) + ktDeclaration.parents.filterIsInstance() + var result: KtLightElement<*, *>? = null + val visitor = object : PsiElementVisitor() { + override fun visitElement(element: PsiElement) { + if (element !is KtLightElement<*, *>) return + // NB: intentionally visit members first so that `self` can be found first if matched + if (element is PsiClass) { + element.fields.forEach { it.accept(this) } + element.methods.forEach { it.accept(this) } + element.innerClasses.forEach { it.accept(this) } + } + if (result == null && element.kotlinOrigin in selfOrParents) { + result = element + return + } + } + } + accept(visitor) + return result + } + } diff --git a/analysis/analysis-api/src/org/jetbrains/kotlin/analysis/api/components/KtPsiTypeProvider.kt b/analysis/analysis-api/src/org/jetbrains/kotlin/analysis/api/components/KtPsiTypeProvider.kt index 8ff94885417..7743b3acb2d 100644 --- a/analysis/analysis-api/src/org/jetbrains/kotlin/analysis/api/components/KtPsiTypeProvider.kt +++ b/analysis/analysis-api/src/org/jetbrains/kotlin/analysis/api/components/KtPsiTypeProvider.kt @@ -11,21 +11,29 @@ import org.jetbrains.kotlin.analysis.api.types.KtType import org.jetbrains.kotlin.load.kotlin.TypeMappingMode public abstract class KtPsiTypeProvider : KtAnalysisSessionComponent() { - public abstract fun asPsiType(type: KtType, context: PsiElement, mode: TypeMappingMode): PsiType? + public abstract fun asPsiType(type: KtType, useSitePosition: PsiElement, mode: TypeMappingMode): PsiType? } public interface KtPsiTypeProviderMixIn : KtAnalysisSessionMixIn { /** - * Converts the given [KtType] to [PsiType]. + * Converts the given [KtType] to [PsiType] under [useSitePosition] context. + * + * [useSitePosition] is used as the parent of the resulting [PsiType], + * which is in turn used to resolve [PsiType]. + * + * [useSitePosition] is also used to determine if the given [KtType] needs to be approximated. + * For example, if the given type is local yet available in the same scope of use site, we can + * still use such local type. Otherwise, e.g., exposed to public as a return type, the resulting + * type will be approximated accordingly. * * Returns `null` if the conversion encounters any erroneous cases, e.g., errors in type arguments. * A client can handle such case in its own way. For instance, * * UAST will return `UastErrorType` as a default error type. - * * LC will return `NonExistentClass` from the [context]. + * * LC will return `NonExistentClass` created from the [useSitePosition]. */ public fun KtType.asPsiType( - context: PsiElement, + useSitePosition: PsiElement, mode: TypeMappingMode = TypeMappingMode.DEFAULT, ): PsiType? = - analysisSession.psiTypeProvider.asPsiType(this, context, mode) + analysisSession.psiTypeProvider.asPsiType(this, useSitePosition, mode) } diff --git a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localFunctionInSameScope.txt b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localFunctionInSameScope.txt index 931753ae243..4ecb78baceb 100644 --- a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localFunctionInSameScope.txt +++ b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localFunctionInSameScope.txt @@ -1,2 +1,2 @@ KtType: Local -PsiType: PsiType:Object +PsiType: PsiType:Local diff --git a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSameScope.txt b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSameScope.txt index 931753ae243..4ecb78baceb 100644 --- a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSameScope.txt +++ b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSameScope.txt @@ -1,2 +1,2 @@ KtType: Local -PsiType: PsiType:Object +PsiType: PsiType:Local diff --git a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSampeScope_functionalType.txt b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSampeScope_functionalType.txt index 5ea0abe1e41..352c2825381 100644 --- a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSampeScope_functionalType.txt +++ b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_localPropertyInSampeScope_functionalType.txt @@ -1,2 +1,2 @@ KtType: () -> Local -PsiType: PsiType:Function0 +PsiType: PsiType:Function0 diff --git a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_memberFunction.txt b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_memberFunction.txt index 931753ae243..4ecb78baceb 100644 --- a/analysis/analysis-api/testData/components/psiTypeProvider/localClass_memberFunction.txt +++ b/analysis/analysis-api/testData/components/psiTypeProvider/localClass_memberFunction.txt @@ -1,2 +1,2 @@ KtType: Local -PsiType: PsiType:Object +PsiType: PsiType:Local