[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
2023-09-22 16:47:11 +02:00
parent 17b8d59e4b
commit ab717c25d7
6 changed files with 168 additions and 123 deletions
@@ -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<FirRegularClassSymbol, FirContainingNamesAwareScope>()

 private val JAVA_ENHANCEMENT_FOR_STATIC_DECLARED_MEMBERS = scopeSessionKey<FirRegularClassSymbol, FirContainingNamesAwareScope>()
+
+private val JAVA_ENHANCEMENT_FOR_ALL_DECLARED_MEMBERS = scopeSessionKey<FirRegularClassSymbol, FirContainingNamesAwareScope>()
@@ -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<Name> = emptySet()
+
+    override fun processClassifiersByNameWithSubstitution(name: Name, processor: (FirClassifierSymbol<*>, ConeSubstitutor) -> Unit) {
+    }
+}
@@ -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()
@@ -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
@@ -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()
@@ -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