From ab717c25d77f85894b3f8a2fb18e0be3eda8bed1 Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Fri, 22 Sep 2023 16:47:11 +0200 Subject: [PATCH] [AA] Avoid duplicate inner classes in Java declared member scopes - Java combined declared member scopes are implemented as a composition of the non-static and static scope, so we have to exclude inner classes from the non-static scope to avoid duplicates. - This is not an issue for Kotlin combined declared member scopes, because the combined scope is already the base scope. ^KT-61800 --- .../api/fir/components/KtFirScopeProvider.kt | 102 ++++++++++------- .../api/fir/scopes/FirNoClassifiersScope.kt | 21 ++++ .../javaClass.pretty.txt | 8 +- .../combinedDeclaredMemberScope/javaClass.txt | 108 +++++++++--------- .../javaDeclaredEnhancementScope.pretty.txt | 4 +- .../javaDeclaredEnhancementScope.txt | 48 ++++---- 6 files changed, 168 insertions(+), 123 deletions(-) create mode 100644 analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/scopes/FirNoClassifiersScope.kt diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirScopeProvider.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirScopeProvider.kt index 485761fe9b7..6ceb16f857a 100644 --- a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirScopeProvider.kt +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/components/KtFirScopeProvider.kt @@ -87,7 +87,22 @@ internal class KtFirScopeProvider( override fun getStaticDeclaredMemberScope(classSymbol: KtSymbolWithMembers): KtScope = getDeclaredMemberScope(classSymbol, DeclaredMemberScopeKind.STATIC) - private enum class DeclaredMemberScopeKind { NON_STATIC, STATIC } + override fun getCombinedDeclaredMemberScope(classSymbol: KtSymbolWithMembers): KtScope = + getDeclaredMemberScope(classSymbol, DeclaredMemberScopeKind.COMBINED) + + private enum class DeclaredMemberScopeKind { + NON_STATIC, + + STATIC, + + /** + * A scope containing both non-static and static members. A smart combined scope (as opposed to a naive combination of [KtScope]s + * with [getCompositeScope]) avoids duplicate inner classes, as they are contained in non-static and static scopes. + * + * A proper combined declared member scope kind also makes it easier to cache combined scopes directly (if needed). + */ + COMBINED, + } private fun getDeclaredMemberScope(classSymbol: KtSymbolWithMembers, kind: DeclaredMemberScopeKind): KtScope { val firDeclaration = classSymbol.firSymbol.fir @@ -107,47 +122,10 @@ internal class KtFirScopeProvider( return when (kind) { DeclaredMemberScopeKind.NON_STATIC -> FirNonStaticMembersScope(combinedScope) DeclaredMemberScopeKind.STATIC -> FirStaticScope(combinedScope) + DeclaredMemberScopeKind.COMBINED -> combinedScope } } - private fun getFirJavaDeclaredMemberScope(firJavaClass: FirJavaClass, kind: DeclaredMemberScopeKind): FirContainingNamesAwareScope? { - val useSiteSession = analysisSession.useSiteSession - val scopeSession = getScopeSession() - - val firScope = when (kind) { - // `FirExcludingNonInnerClassesScope` is a workaround for non-static member scopes containing static classes (see KT-61900). - DeclaredMemberScopeKind.NON_STATIC -> FirExcludingNonInnerClassesScope( - JavaScopeProvider.getUseSiteMemberScope( - firJavaClass, - useSiteSession, - scopeSession, - memberRequiredPhase = FirResolvePhase.TYPES, - ) - ) - DeclaredMemberScopeKind.STATIC -> JavaScopeProvider.getStaticScope(firJavaClass, useSiteSession, scopeSession) ?: return null - } - - val cacheKey = when (kind) { - DeclaredMemberScopeKind.NON_STATIC -> JAVA_ENHANCEMENT_FOR_DECLARED_MEMBERS - DeclaredMemberScopeKind.STATIC -> JAVA_ENHANCEMENT_FOR_STATIC_DECLARED_MEMBERS - } - - return scopeSession.getOrBuild(firJavaClass.symbol, cacheKey) { - FirJavaDeclaredMembersOnlyScope(firScope, firJavaClass) - } - } - - override fun getCombinedDeclaredMemberScope(classSymbol: KtSymbolWithMembers): KtScope { - val firDeclaration = classSymbol.firSymbol.fir - if (firDeclaration is FirJavaClass) { - // Java enhancement scopes as provided by `JavaScopeProvider` are either use-site or static scopes, so we need to compose them - // to get the combined scope. A base declared member scope with Java enhancement doesn't exist, unfortunately. - return KtCompositeScope.create(listOf(getDeclaredMemberScope(classSymbol), getStaticDeclaredMemberScope(classSymbol)), token) - } - - return KtFirDelegatingNamesAwareScope(getCombinedFirKotlinDeclaredMemberScope(classSymbol), builder) - } - /** * Returns a declared member scope which contains both static and non-static callables, as well as all classifiers. Java classes need to * be handled specially, because [declaredMemberScope] doesn't handle Java enhancement properly. @@ -160,6 +138,50 @@ internal class KtFirScopeProvider( } } + private fun getFirJavaDeclaredMemberScope( + firJavaClass: FirJavaClass, + kind: DeclaredMemberScopeKind, + ): FirContainingNamesAwareScope? { + val useSiteSession = analysisSession.useSiteSession + val scopeSession = getScopeSession() + + fun getBaseUseSiteScope() = JavaScopeProvider.getUseSiteMemberScope( + firJavaClass, + useSiteSession, + scopeSession, + memberRequiredPhase = FirResolvePhase.TYPES, + ) + + fun getStaticScope() = JavaScopeProvider.getStaticScope(firJavaClass, useSiteSession, scopeSession) + + val firScope = when (kind) { + // `FirExcludingNonInnerClassesScope` is a workaround for non-static member scopes containing static classes (see KT-61900). + DeclaredMemberScopeKind.NON_STATIC -> FirExcludingNonInnerClassesScope(getBaseUseSiteScope()) + + DeclaredMemberScopeKind.STATIC -> getStaticScope() ?: return null + + // Java enhancement scopes as provided by `JavaScopeProvider` are either use-site or static scopes, so we need to compose them + // to get the combined scope. A base declared member scope with Java enhancement doesn't exist, unfortunately. + DeclaredMemberScopeKind.COMBINED -> { + // The static scope contains inner classes, so we need to exclude them from the non-static scope to avoid duplicates. + val nonStaticScope = FirNoClassifiersScope(getBaseUseSiteScope()) + getStaticScope() + ?.let { staticScope -> FirNameAwareCompositeScope(listOf(nonStaticScope, staticScope)) } + ?: nonStaticScope + } + } + + val cacheKey = when (kind) { + DeclaredMemberScopeKind.NON_STATIC -> JAVA_ENHANCEMENT_FOR_DECLARED_MEMBERS + DeclaredMemberScopeKind.STATIC -> JAVA_ENHANCEMENT_FOR_STATIC_DECLARED_MEMBERS + DeclaredMemberScopeKind.COMBINED -> JAVA_ENHANCEMENT_FOR_ALL_DECLARED_MEMBERS + } + + return scopeSession.getOrBuild(firJavaClass.symbol, cacheKey) { + FirJavaDeclaredMembersOnlyScope(firScope, firJavaClass) + } + } + override fun getDelegatedMemberScope(classSymbol: KtSymbolWithMembers): KtScope { val declaredScope = (getDeclaredMemberScope(classSymbol) as? KtFirDelegatingNamesAwareScope)?.firScope ?: return getEmptyScope() @@ -354,3 +376,5 @@ private class FirTypeScopeWithSyntheticProperties( private val JAVA_ENHANCEMENT_FOR_DECLARED_MEMBERS = scopeSessionKey() private val JAVA_ENHANCEMENT_FOR_STATIC_DECLARED_MEMBERS = scopeSessionKey() + +private val JAVA_ENHANCEMENT_FOR_ALL_DECLARED_MEMBERS = scopeSessionKey() diff --git a/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/scopes/FirNoClassifiersScope.kt b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/scopes/FirNoClassifiersScope.kt new file mode 100644 index 00000000000..7f96d7fe14e --- /dev/null +++ b/analysis/analysis-api-fir/src/org/jetbrains/kotlin/analysis/api/fir/scopes/FirNoClassifiersScope.kt @@ -0,0 +1,21 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.analysis.api.fir.scopes + +import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor +import org.jetbrains.kotlin.fir.scopes.FirContainingNamesAwareScope +import org.jetbrains.kotlin.fir.scopes.FirDelegatingContainingNamesAwareScope +import org.jetbrains.kotlin.fir.symbols.impl.FirClassifierSymbol +import org.jetbrains.kotlin.name.Name + +internal class FirNoClassifiersScope( + private val delegate: FirContainingNamesAwareScope, +) : FirDelegatingContainingNamesAwareScope(delegate) { + override fun getClassifierNames(): Set = emptySet() + + override fun processClassifiersByNameWithSubstitution(name: Name, processor: (FirClassifierSymbol<*>, ConeSubstitutor) -> Unit) { + } +} diff --git a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.pretty.txt b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.pretty.txt index b1dac59df6d..385411509b0 100644 --- a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.pretty.txt +++ b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.pretty.txt @@ -1,9 +1,9 @@ -open fun hello() - -constructor() - open fun bar(): kotlin.String! +open fun hello() + open var foo: kotlin.Int open class NestedClass + +constructor() \ No newline at end of file diff --git a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.txt b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.txt index 23da575016c..8caa6077fec 100644 --- a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.txt +++ b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaClass.txt @@ -1,3 +1,34 @@ +KtFunctionSymbol: + annotationsList: [] + callableIdIfNonLocal: /JavaClass.bar + contextReceivers: [] + contractEffects: [] + hasStableParameterNames: false + isActual: false + isBuiltinFunctionInvoke: false + isExpect: false + isExtension: false + isExternal: false + isInfix: false + isInline: false + isOperator: false + isOverride: false + isStatic: true + isSuspend: false + modality: OPEN + name: bar + origin: JAVA + receiverParameter: null + returnType: KtFlexibleType: + annotationsList: [] + type: kotlin/String! + symbolKind: CLASS_MEMBER + typeParameters: [] + valueParameters: [] + visibility: Public + getContainingModule: KtSourceModule "Sources of main" + deprecationStatus: null + KtFunctionSymbol: annotationsList: [] callableIdIfNonLocal: /JavaClass.hello @@ -34,60 +65,6 @@ KtFunctionSymbol: getContainingModule: KtSourceModule "Sources of main" deprecationStatus: null -KtConstructorSymbol: - annotationsList: [] - callableIdIfNonLocal: null - containingClassIdIfNonLocal: JavaClass - contextReceivers: [] - hasStableParameterNames: false - isActual: false - isExpect: false - isExtension: false - isPrimary: true - origin: JAVA - receiverParameter: null - returnType: KtUsualClassType: - annotationsList: [] - ownTypeArguments: [] - type: JavaClass - symbolKind: CLASS_MEMBER - typeParameters: [] - valueParameters: [] - visibility: Public - getContainingModule: KtSourceModule "Sources of main" - deprecationStatus: null - -KtFunctionSymbol: - annotationsList: [] - callableIdIfNonLocal: /JavaClass.bar - contextReceivers: [] - contractEffects: [] - hasStableParameterNames: false - isActual: false - isBuiltinFunctionInvoke: false - isExpect: false - isExtension: false - isExternal: false - isInfix: false - isInline: false - isOperator: false - isOverride: false - isStatic: true - isSuspend: false - modality: OPEN - name: bar - origin: JAVA - receiverParameter: null - returnType: KtFlexibleType: - annotationsList: [] - type: kotlin/String! - symbolKind: CLASS_MEMBER - typeParameters: [] - valueParameters: [] - visibility: Public - getContainingModule: KtSourceModule "Sources of main" - deprecationStatus: null - KtJavaFieldSymbol: annotationsList: [] callableIdIfNonLocal: /JavaClass.foo @@ -137,3 +114,26 @@ KtNamedClassOrObjectSymbol: getContainingModule: KtSourceModule "Sources of main" annotationApplicableTargets: null deprecationStatus: null + +KtConstructorSymbol: + annotationsList: [] + callableIdIfNonLocal: null + containingClassIdIfNonLocal: JavaClass + contextReceivers: [] + hasStableParameterNames: false + isActual: false + isExpect: false + isExtension: false + isPrimary: true + origin: JAVA + receiverParameter: null + returnType: KtUsualClassType: + annotationsList: [] + ownTypeArguments: [] + type: JavaClass + symbolKind: CLASS_MEMBER + typeParameters: [] + valueParameters: [] + visibility: Public + getContainingModule: KtSourceModule "Sources of main" + deprecationStatus: null \ No newline at end of file diff --git a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.pretty.txt b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.pretty.txt index 095c95a5a27..9898780b033 100644 --- a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.pretty.txt +++ b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.pretty.txt @@ -1,7 +1,7 @@ -open var field: kotlin.Int - open var field2: kotlin.String! +open var field: kotlin.Int + open fun foo(): kotlin.String! constructor() diff --git a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.txt b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.txt index 29dfe036772..c153d23ca1d 100644 --- a/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.txt +++ b/analysis/analysis-api/testData/components/scopeProvider/combinedDeclaredMemberScope/javaDeclaredEnhancementScope.txt @@ -1,3 +1,27 @@ +KtJavaFieldSymbol: + annotationsList: [] + callableIdIfNonLocal: /JavaClass.field2 + contextReceivers: [] + isExtension: false + isStatic: false + isVal: false + modality: OPEN + name: field2 + origin: JAVA + receiverParameter: null + returnType: KtFlexibleType: + annotationsList: [] + type: kotlin/String! + symbolKind: CLASS_MEMBER + typeParameters: [] + visibility: Public + getDispatchReceiver(): KtUsualClassType: + annotationsList: [] + ownTypeArguments: [] + type: JavaClass + getContainingModule: KtSourceModule "Sources of main" + deprecationStatus: null + KtJavaFieldSymbol: annotationsList: [] callableIdIfNonLocal: /JavaClass.field @@ -23,30 +47,6 @@ KtJavaFieldSymbol: getContainingModule: KtSourceModule "Sources of main" deprecationStatus: null -KtJavaFieldSymbol: - annotationsList: [] - callableIdIfNonLocal: /JavaClass.field2 - contextReceivers: [] - isExtension: false - isStatic: false - isVal: false - modality: OPEN - name: field2 - origin: JAVA - receiverParameter: null - returnType: KtFlexibleType: - annotationsList: [] - type: kotlin/String! - symbolKind: CLASS_MEMBER - typeParameters: [] - visibility: Public - getDispatchReceiver(): KtUsualClassType: - annotationsList: [] - ownTypeArguments: [] - type: JavaClass - getContainingModule: KtSourceModule "Sources of main" - deprecationStatus: null - KtFunctionSymbol: annotationsList: [] callableIdIfNonLocal: /JavaClass.foo