K2: fix internal visibility checks for overrides #KT-53197 Fixed

analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java

@@ -22866,6 +22866,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag
                 runTest("compiler/testData/diagnostics/tests/override/Generics.kt");
             }
 
+            @Test
+            @TestMetadata("InternalPotentialOverride.kt")
+            public void testInternalPotentialOverride() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/override/InternalPotentialOverride.kt");
+            }
+
             @Test
             @TestMetadata("InvisiblePotentialOverride.kt")
             public void testInvisiblePotentialOverride() throws Exception {

compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java

@@ -22866,6 +22866,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
                 runTest("compiler/testData/diagnostics/tests/override/Generics.kt");
             }
 
+            @Test
+            @TestMetadata("InternalPotentialOverride.kt")
+            public void testInternalPotentialOverride() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/override/InternalPotentialOverride.kt");
+            }
+
             @Test
             @TestMetadata("InvisiblePotentialOverride.kt")
             public void testInvisiblePotentialOverride() throws Exception {

compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java

@@ -22866,6 +22866,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
                 runTest("compiler/testData/diagnostics/tests/override/Generics.kt");
             }
 
+            @Test
+            @TestMetadata("InternalPotentialOverride.kt")
+            public void testInternalPotentialOverride() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/override/InternalPotentialOverride.kt");
+            }
+
             @Test
             @TestMetadata("InvisiblePotentialOverride.kt")
             public void testInvisiblePotentialOverride() throws Exception {

compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/generators/FakeOverrideGenerator.kt

@@ -205,7 +205,7 @@ class FakeOverrideGenerator(
         // But they are treated differently in IR (real declarations have already been declared before) and such methods are present among realDeclarationSymbols
         if (originalSymbol in realDeclarationSymbols) return
 
-        if (originalDeclaration.visibility == Visibilities.Private) return
+        if (!originalDeclaration.isVisibleFromDerivedClass(klass.moduleData)) return
 
         val origin = IrDeclarationOrigin.FAKE_OVERRIDE
         val baseSymbol = originalSymbol.unwrapSubstitutionAndIntersectionOverrides() as S

compiler/fir/java/src/org/jetbrains/kotlin/fir/resolve/scopes/JvmMappedScopes.kt

@@ -80,7 +80,7 @@ private fun wrapSubstitutionScopeIfNeed(
         FirClassSubstitutionScope(
             session, useSiteMemberScope, PLATFORM_TYPE_PARAMETERS_SUBSTITUTION_SCOPE_KEY, substitutor,
             dispatchReceiverTypeForSubstitutedMembers = derivedClass.defaultType(),
-            skipPrivateMembers = true,
+            skipInvisibleMembers = true,
         )
     }
 }

compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt

@@ -484,3 +484,16 @@ fun FirClassLikeSymbol<*>.getContainingClassLookupTag(): ConeClassLikeLookupTag?
 fun FirBasedSymbol<*>.isVariableOrNamedFunction(): Boolean {
     return this is FirVariableSymbol || this is FirNamedFunctionSymbol || this is FirPropertyAccessorSymbol
 }
+
+
+fun FirCallableDeclaration.isVisibleFromDerivedClass(derivedClassModuleData: FirModuleData): Boolean {
+    return when (visibility) {
+        Visibilities.Private, Visibilities.PrivateToThis ->
+            false
+        Visibilities.Internal ->
+            derivedClassModuleData == this@isVisibleFromDerivedClass.moduleData
+        else ->
+            true
+    }
+}
+

compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/FirKotlinScopeProvider.kt

@@ -10,7 +10,6 @@ import org.jetbrains.kotlin.fir.FirSession
 import org.jetbrains.kotlin.fir.FirSessionComponent
 import org.jetbrains.kotlin.fir.declarations.FirClass
 import org.jetbrains.kotlin.fir.declarations.FirRegularClass
-import org.jetbrains.kotlin.fir.declarations.utils.classId
 import org.jetbrains.kotlin.fir.declarations.utils.delegateFields
 import org.jetbrains.kotlin.fir.declarations.utils.isExpect
 import org.jetbrains.kotlin.fir.resolve.*
@@ -112,7 +111,7 @@ fun FirClass.scopeForClass(
     scopeSession: ScopeSession
 ): FirTypeScope = scopeForClassImpl(
     substitutor, useSiteSession, scopeSession,
-    skipPrivateMembers = false,
+    skipInvisibleMembers = false,
     classFirDispatchReceiver = this,
     // TODO: why it's always false?
     isFromExpectClass = false
@@ -131,7 +130,7 @@ fun ConeKotlinType.scopeForSupertype(
             substitutor(symbol, this, useSiteSession),
             useSiteSession,
             scopeSession,
-            skipPrivateMembers = true,
+            skipInvisibleMembers = true,
             classFirDispatchReceiver = subClass,
             isFromExpectClass = (subClass as? FirRegularClass)?.isExpect == true
         )
@@ -150,7 +149,7 @@ private fun FirClass.scopeForClassImpl(
     substitutor: ConeSubstitutor,
     useSiteSession: FirSession,
     scopeSession: ScopeSession,
-    skipPrivateMembers: Boolean,
+    skipInvisibleMembers: Boolean,
     classFirDispatchReceiver: FirClass,
     isFromExpectClass: Boolean
 ): FirTypeScope {
@@ -166,7 +165,7 @@ private fun FirClass.scopeForClassImpl(
             basicScope,
             key, substitutor,
             substitutor.substituteOrSelf(classFirDispatchReceiver.defaultType()) as ConeClassLikeType,
-            skipPrivateMembers,
+            skipInvisibleMembers,
             makeExpect = isFromExpectClass
         )
     }

compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirClassSubstitutionScope.kt

@@ -5,15 +5,10 @@
 
 package org.jetbrains.kotlin.fir.scopes.impl
 
-import org.jetbrains.kotlin.descriptors.Visibilities
-import org.jetbrains.kotlin.fir.FirSession
-import org.jetbrains.kotlin.fir.FirSessionComponent
+import org.jetbrains.kotlin.fir.*
 import org.jetbrains.kotlin.fir.caches.*
 import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.declarations.synthetic.FirSyntheticProperty
-import org.jetbrains.kotlin.fir.declarations.utils.visibility
-import org.jetbrains.kotlin.fir.dispatchReceiverClassOrNull
-import org.jetbrains.kotlin.fir.originalForSubstitutionOverride
 import org.jetbrains.kotlin.fir.resolve.ScopeSessionKey
 import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor
 import org.jetbrains.kotlin.fir.resolve.substitution.chain
@@ -33,7 +28,7 @@ class FirClassSubstitutionScope(
     key: ScopeSessionKey<*, *>,
     private val substitutor: ConeSubstitutor,
     private val dispatchReceiverTypeForSubstitutedMembers: ConeClassLikeType,
-    private val skipPrivateMembers: Boolean,
+    private val skipInvisibleMembers: Boolean,
     private val makeExpect: Boolean = false
 ) : FirTypeScope() {
     companion object {
@@ -124,7 +119,7 @@ class FirClassSubstitutionScope(
     fun createSubstitutionOverrideFunction(original: FirNamedFunctionSymbol): FirNamedFunctionSymbol {
         if (substitutor == ConeSubstitutor.Empty) return original
         val member = original.fir
-        if (skipPrivateMembers && member.visibility == Visibilities.Private) return original
+        if (skipInvisibleMembers && !member.isVisibleInThisScope()) return original
 
         val symbolForOverride = FirFakeOverrideGenerator.createSymbolForSubstitutionOverride(original, newOwnerClassId)
 
@@ -224,7 +219,7 @@ class FirClassSubstitutionScope(
     fun createSubstitutionOverrideProperty(original: FirPropertySymbol): FirPropertySymbol {
         if (substitutor == ConeSubstitutor.Empty) return original
         val member = original.fir
-        if (skipPrivateMembers && member.visibility == Visibilities.Private) return original
+        if (skipInvisibleMembers && !member.isVisibleInThisScope()) return original
 
         val symbolForOverride = FirFakeOverrideGenerator.createSymbolForSubstitutionOverride(original, newOwnerClassId)
 
@@ -310,7 +305,7 @@ class FirClassSubstitutionScope(
     fun createSubstitutionOverrideField(original: FirFieldSymbol): FirFieldSymbol {
         if (substitutor == ConeSubstitutor.Empty) return original
         val member = original.fir
-        if (skipPrivateMembers && member.visibility == Visibilities.Private) return original
+        if (skipInvisibleMembers && !member.isVisibleInThisScope()) return original
 
         member.symbol.ensureResolved(FirResolvePhase.STATUS)
         val returnType = member.returnTypeRef.coneTypeSafe<ConeKotlinType>()
@@ -323,7 +318,7 @@ class FirClassSubstitutionScope(
     fun createSubstitutionOverrideSyntheticProperty(original: FirSyntheticPropertySymbol): FirSyntheticPropertySymbol {
         if (substitutor == ConeSubstitutor.Empty) return original
         val member = original.fir as FirSyntheticProperty
-        if (skipPrivateMembers && member.visibility == Visibilities.Private) return original
+        if (skipInvisibleMembers && !member.isVisibleInThisScope()) return original
 
         member.symbol.ensureResolved(FirResolvePhase.STATUS)
         val returnType = member.returnTypeRef.coneTypeSafe<ConeKotlinType>()
@@ -380,6 +375,10 @@ class FirClassSubstitutionScope(
     override fun toString(): String {
         return "Substitution scope for [$useSiteMemberScope] for type $dispatchReceiverTypeForSubstitutedMembers"
     }
+
+    private fun FirCallableDeclaration.isVisibleInThisScope(): Boolean {
+        return isVisibleFromDerivedClass(this@FirClassSubstitutionScope.session.moduleData)
+    }
 }
 
 class FirSubstitutionOverrideStorage(val session: FirSession) : FirSessionComponent {

compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirStandardOverrideChecker.kt

@@ -5,10 +5,9 @@
 
 package org.jetbrains.kotlin.fir.scopes.impl
 
-import org.jetbrains.kotlin.descriptors.Visibilities
 import org.jetbrains.kotlin.fir.FirSession
 import org.jetbrains.kotlin.fir.declarations.*
-import org.jetbrains.kotlin.fir.declarations.utils.visibility
+import org.jetbrains.kotlin.fir.isVisibleFromDerivedClass
 import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor
 import org.jetbrains.kotlin.fir.resolve.transformers.ensureResolvedTypeDeclaration
 import org.jetbrains.kotlin.fir.symbols.ensureResolved
@@ -122,8 +121,7 @@ class FirStandardOverrideChecker(private val session: FirSession) : FirAbstractO
     }
 
     override fun isOverriddenFunction(overrideCandidate: FirSimpleFunction, baseDeclaration: FirSimpleFunction): Boolean {
-        if (Visibilities.isPrivate(baseDeclaration.visibility)) return false
-
+        if (!baseDeclaration.isVisibleFromDerivedClass(overrideCandidate.moduleData)) return false
         if (overrideCandidate.valueParameters.size != baseDeclaration.valueParameters.size) return false
 
         val substitutor = buildTypeParametersSubstitutorIfCompatible(overrideCandidate, baseDeclaration) ?: return false
@@ -141,8 +139,7 @@ class FirStandardOverrideChecker(private val session: FirSession) : FirAbstractO
         overrideCandidate: FirCallableDeclaration,
         baseDeclaration: FirProperty
     ): Boolean {
-        if (Visibilities.isPrivate(baseDeclaration.visibility)) return false
-
+        if (!baseDeclaration.isVisibleFromDerivedClass(overrideCandidate.moduleData)) return false
         if (overrideCandidate !is FirProperty) return false
         val substitutor = buildTypeParametersSubstitutorIfCompatible(overrideCandidate, baseDeclaration) ?: return false
         overrideCandidate.ensureResolved(FirResolvePhase.TYPES)

compiler/testData/diagnostics/tests/override/InternalPotentialOverride.kt

@@ -0,0 +1,22 @@
+// FIR_IDENTICAL
+// MODULE: m1
+// FILE: A.kt
+
+open class A {
+    internal open fun foo() : Int = 1
+}
+
+open class AG<T> {
+    internal open fun bar(arg: T) = arg
+}
+
+// MODULE: m2(m1)
+// FILE: B.kt
+
+class B : A() {
+    fun foo() : String = ""
+}
+
+class BG : AG<String>() {
+    fun bar(arg: Int) = arg
+}

compiler/testData/diagnostics/tests/override/InternalPotentialOverride.txt

@@ -0,0 +1,39 @@
+// -- Module: <m1> --
+package
+
+public open class A {
+    public constructor A()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    internal open fun foo(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public open class AG</*0*/ T> {
+    public constructor AG</*0*/ T>()
+    internal open fun bar(/*0*/ arg: T): T
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+// -- Module: <m2> --
+package
+
+public final class B : A {
+    public constructor B()
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public final fun foo(): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+
+public final class BG : AG<kotlin.String> {
+    public constructor BG()
+    public final fun bar(/*0*/ arg: kotlin.Int): kotlin.Int
+    invisible_fake open override /*1*/ /*fake_override*/ fun bar(/*0*/ arg: kotlin.String): kotlin.String
+    public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
+    public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
+    public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
+}
+

compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.fir.kt

@@ -0,0 +1,47 @@
+// !LANGUAGE: +ProhibitInvisibleAbstractMethodsInSuperclasses
+// MODULE: base
+// FILE: Base.kt
+package base
+
+abstract class Base {
+    fun foo(): String {
+        return internalFoo()
+    }
+    internal abstract fun internalFoo(): String
+}
+
+open class BaseWithOverride : Base() {
+    override fun internalFoo(): String = ""
+}
+
+// MODULE: intermediate(base)
+// FILE: Intermediate.kt
+package intermediate
+import base.*
+
+abstract class Intermediate : Base()
+
+// MODULE: impl(base, intermediate)
+// FILE: Impl.kt
+package impl
+import base.*
+import intermediate.*
+
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>class ImplDirectFromBase<!> : Base()
+
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>object ImplObjDirectFromBase<!> : Base()
+
+class ImplDirectFromBaseWithOverride : BaseWithOverride()
+
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>class ImplDirectFromBaseWithOverrid<!> : Base() {
+    <!NOTHING_TO_OVERRIDE!>override<!> fun internalFoo(): String = ""
+}
+
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>class ImplViaIntermediate<!> : Intermediate()
+
+fun foo() {
+    ImplDirectFromBase().foo()
+    ImplObjDirectFromBase.foo()
+    ImplDirectFromBaseWithOverride().foo()
+    ImplViaIntermediate().foo()
+}

compiler/testData/diagnostics/tests/visibility/abstractInvisibleMemberFromKotlin.kt

@@ -1,4 +1,3 @@
-// FIR_IDENTICAL
 // !LANGUAGE: +ProhibitInvisibleAbstractMethodsInSuperclasses
 // MODULE: base
 // FILE: Base.kt

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java

@@ -22872,6 +22872,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
                 runTest("compiler/testData/diagnostics/tests/override/Generics.kt");
             }
 
+            @Test
+            @TestMetadata("InternalPotentialOverride.kt")
+            public void testInternalPotentialOverride() throws Exception {
+                runTest("compiler/testData/diagnostics/tests/override/InternalPotentialOverride.kt");
+            }
+
             @Test
             @TestMetadata("InvisiblePotentialOverride.kt")
             public void testInvisiblePotentialOverride() throws Exception {

compiler/tests-spec/testData/diagnostics/linked/declarations/classifier-declaration/class-declaration/abstract-classes/p-2/neg/1.7.fir.kt

@@ -26,16 +26,16 @@ import base.*
 
 // TESTCASE NUMBER: 1
 
-class Case1 : BaseKotlin() {
-    <!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>class Case1<!> : BaseKotlin() {
+    <!NOTHING_TO_OVERRIDE!>override<!> fun foo(b: Boolean?) {}
 }
 
 fun case1() {
     val v = Case1()
     v.boo(true)
 
-    val o = object :  BaseKotlin() {
-        <!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
+    val o = <!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>object<!> :  BaseKotlin() {
+        <!NOTHING_TO_OVERRIDE!>override<!> fun foo(b: Boolean?) {}
     }
 }
 
@@ -44,16 +44,16 @@ fun case1() {
 */
 abstract class AbstractClassCase2 : BaseKotlin() {}
 
-class Case2: AbstractClassCase2() {
-    <!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
+<!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>class Case2<!>: AbstractClassCase2() {
+    <!NOTHING_TO_OVERRIDE!>override<!> fun foo(b: Boolean?) {}
 }
 
 fun case2() {
     val v = Case2()
     v.boo(true)
 
-    val o = object : AbstractClassCase2() {
-        <!CANNOT_OVERRIDE_INVISIBLE_MEMBER!>override<!> fun foo(b: Boolean?) {}
+    val o = <!INVISIBLE_ABSTRACT_MEMBER_FROM_SUPER_ERROR!>object<!> : AbstractClassCase2() {
+        <!NOTHING_TO_OVERRIDE!>override<!> fun foo(b: Boolean?) {}
     }
 }