[FIR] Properly check for visibility in FirTypeIntersectionScopeContext

This fixes a false positive OVERRIDING_FINAL_MEMBER caused by a
package-private member in a different package being added to the
list of overridden symbols. However, in Java, package-private members
from different packages are effectively like private members in that
they cannot be overridden.

#KT-61696 Fixed
This commit is contained in:
Kirill Rakhman
2023-09-07 17:04:46 +02:00
committed by Space Team
parent d16f33cf5b
commit 2bc25d4f6e
10 changed files with 97 additions and 11 deletions
@@ -24825,6 +24825,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt"); runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt");
} }
@Test
@TestMetadata("packagePrivateAndPublic.kt")
public void testPackagePrivateAndPublic() throws Exception {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/packagePrivateAndPublic.kt");
}
@Test @Test
@TestMetadata("returnTypeMismatch.kt") @TestMetadata("returnTypeMismatch.kt")
public void testReturnTypeMismatch() throws Exception { public void testReturnTypeMismatch() throws Exception {
@@ -24825,6 +24825,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt"); runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt");
} }
@Test
@TestMetadata("packagePrivateAndPublic.kt")
public void testPackagePrivateAndPublic() throws Exception {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/packagePrivateAndPublic.kt");
}
@Test @Test
@TestMetadata("returnTypeMismatch.kt") @TestMetadata("returnTypeMismatch.kt")
public void testReturnTypeMismatch() throws Exception { public void testReturnTypeMismatch() throws Exception {
@@ -24825,6 +24825,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt"); runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt");
} }
@Test
@TestMetadata("packagePrivateAndPublic.kt")
public void testPackagePrivateAndPublic() throws Exception {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/packagePrivateAndPublic.kt");
}
@Test @Test
@TestMetadata("returnTypeMismatch.kt") @TestMetadata("returnTypeMismatch.kt")
public void testReturnTypeMismatch() throws Exception { public void testReturnTypeMismatch() throws Exception {
@@ -24831,6 +24831,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt"); runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt");
} }
@Test
@TestMetadata("packagePrivateAndPublic.kt")
public void testPackagePrivateAndPublic() throws Exception {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/packagePrivateAndPublic.kt");
}
@Test @Test
@TestMetadata("returnTypeMismatch.kt") @TestMetadata("returnTypeMismatch.kt")
public void testReturnTypeMismatch() throws Exception { public void testReturnTypeMismatch() throws Exception {
@@ -23,6 +23,7 @@ import org.jetbrains.kotlin.library.metadata.resolver.KotlinResolvedLibrary
import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.name.Name
object FirNativeSessionFactory : FirAbstractSessionFactory() { object FirNativeSessionFactory : FirAbstractSessionFactory() {
@OptIn(SessionConfiguration::class)
fun createLibrarySession( fun createLibrarySession(
mainModuleName: Name, mainModuleName: Name,
resolvedLibraries: List<KotlinResolvedLibrary>, resolvedLibraries: List<KotlinResolvedLibrary>,
@@ -39,6 +40,7 @@ object FirNativeSessionFactory : FirAbstractSessionFactory() {
languageVersionSettings, languageVersionSettings,
extensionRegistrars, extensionRegistrars,
registerExtraComponents = { session -> registerExtraComponents = { session ->
session.register(FirVisibilityChecker::class, FirVisibilityChecker.Default)
registerExtraComponents(session) registerExtraComponents(session)
}, },
createKotlinScopeProvider = { FirKotlinScopeProvider() }, createKotlinScopeProvider = { FirKotlinScopeProvider() },
@@ -18,7 +18,6 @@ import org.jetbrains.kotlin.fir.declarations.utils.visibility
import org.jetbrains.kotlin.fir.expressions.FirExpression import org.jetbrains.kotlin.fir.expressions.FirExpression
import org.jetbrains.kotlin.fir.resolve.SupertypeSupplier import org.jetbrains.kotlin.fir.resolve.SupertypeSupplier
import org.jetbrains.kotlin.fir.resolve.calls.FirSimpleSyntheticPropertySymbol import org.jetbrains.kotlin.fir.resolve.calls.FirSimpleSyntheticPropertySymbol
import org.jetbrains.kotlin.fir.resolve.calls.ReceiverValue
import org.jetbrains.kotlin.fir.resolve.isSubclassOf import org.jetbrains.kotlin.fir.resolve.isSubclassOf
import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol
@@ -76,12 +75,12 @@ object FirJavaVisibilityChecker : FirVisibilityChecker() {
} }
override fun platformOverrideVisibilityCheck( override fun platformOverrideVisibilityCheck(
candidateInDerivedClass: FirBasedSymbol<*>, packageNameOfDerivedClass: FqName,
symbolInBaseClass: FirBasedSymbol<*>, symbolInBaseClass: FirBasedSymbol<*>,
visibilityInBaseClass: Visibility, visibilityInBaseClass: Visibility,
): Boolean = when (visibilityInBaseClass) { ): Boolean = when (visibilityInBaseClass) {
JavaVisibilities.ProtectedAndPackage, JavaVisibilities.ProtectedStaticVisibility -> true JavaVisibilities.ProtectedAndPackage, JavaVisibilities.ProtectedStaticVisibility -> true
JavaVisibilities.PackageVisibility -> symbolInBaseClass.isInPackage(candidateInDerivedClass.packageFqName()) JavaVisibilities.PackageVisibility -> symbolInBaseClass.isInPackage(packageNameOfDerivedClass)
else -> true else -> true
} }
@@ -68,7 +68,7 @@ abstract class FirVisibilityChecker : FirSessionComponent {
} }
override fun platformOverrideVisibilityCheck( override fun platformOverrideVisibilityCheck(
candidateInDerivedClass: FirBasedSymbol<*>, packageNameOfDerivedClass: FqName,
symbolInBaseClass: FirBasedSymbol<*>, symbolInBaseClass: FirBasedSymbol<*>,
visibilityInBaseClass: Visibility, visibilityInBaseClass: Visibility,
): Boolean { ): Boolean {
@@ -151,13 +151,20 @@ abstract class FirVisibilityChecker : FirSessionComponent {
fun isVisibleForOverriding( fun isVisibleForOverriding(
candidateInDerivedClass: FirMemberDeclaration, candidateInDerivedClass: FirMemberDeclaration,
candidateInBaseClass: FirMemberDeclaration candidateInBaseClass: FirMemberDeclaration,
): Boolean = isVisibleForOverriding(candidateInDerivedClass.moduleData, candidateInDerivedClass.symbol, candidateInBaseClass) ): Boolean =
isVisibleForOverriding(candidateInDerivedClass.moduleData, candidateInDerivedClass.symbol.packageFqName(), candidateInBaseClass)
fun isVisibleForOverriding( fun isVisibleForOverriding(
derivedClassModuleData: FirModuleData, derivedClassModuleData: FirModuleData,
symbolFromDerivedClass: FirBasedSymbol<*>, symbolFromDerivedClass: FirBasedSymbol<*>,
candidateInBaseClass: FirMemberDeclaration, candidateInBaseClass: FirMemberDeclaration,
): Boolean = isVisibleForOverriding(derivedClassModuleData, symbolFromDerivedClass.packageFqName(), candidateInBaseClass)
fun isVisibleForOverriding(
derivedClassModuleData: FirModuleData,
packageNameOfDerivedClass: FqName,
candidateInBaseClass: FirMemberDeclaration,
): Boolean = when (candidateInBaseClass.visibility) { ): Boolean = when (candidateInBaseClass.visibility) {
Visibilities.Internal -> { Visibilities.Internal -> {
candidateInBaseClass.moduleData == derivedClassModuleData || candidateInBaseClass.moduleData == derivedClassModuleData ||
@@ -166,7 +173,14 @@ abstract class FirVisibilityChecker : FirSessionComponent {
Visibilities.Private, Visibilities.PrivateToThis -> false Visibilities.Private, Visibilities.PrivateToThis -> false
Visibilities.Protected -> true Visibilities.Protected -> true
else -> platformOverrideVisibilityCheck(symbolFromDerivedClass, candidateInBaseClass.symbol, candidateInBaseClass.visibility) else -> {
platformOverrideVisibilityCheck(
packageNameOfDerivedClass,
candidateInBaseClass.symbol,
candidateInBaseClass.visibility
)
}
} }
private fun isSpecificDeclarationVisible( private fun isSpecificDeclarationVisible(
@@ -249,7 +263,7 @@ abstract class FirVisibilityChecker : FirSessionComponent {
): Boolean ): Boolean
protected abstract fun platformOverrideVisibilityCheck( protected abstract fun platformOverrideVisibilityCheck(
candidateInDerivedClass: FirBasedSymbol<*>, packageNameOfDerivedClass: FqName,
symbolInBaseClass: FirBasedSymbol<*>, symbolInBaseClass: FirBasedSymbol<*>,
visibilityInBaseClass: Visibility, visibilityInBaseClass: Visibility,
): Boolean ): Boolean
@@ -39,7 +39,8 @@ class FirTypeIntersectionScopeContext(
) { ) {
private val overrideService = session.overrideService private val overrideService = session.overrideService
private val isReceiverClassExpect = dispatchReceiverType.toRegularClassSymbol(session)?.isExpect == true private val dispatchClassSymbol: FirRegularClassSymbol? = dispatchReceiverType.toRegularClassSymbol(session)
private val isReceiverClassExpect = dispatchClassSymbol?.isExpect == true
val intersectionOverrides: FirCache<FirCallableSymbol<*>, MemberWithBaseScope<FirCallableSymbol<*>>, ResultOfIntersection.NonTrivial<*>> = val intersectionOverrides: FirCache<FirCallableSymbol<*>, MemberWithBaseScope<FirCallableSymbol<*>>, ResultOfIntersection.NonTrivial<*>> =
session.intersectionOverrideStorage.cacheByScope.getValue(dispatchReceiverType) session.intersectionOverrideStorage.cacheByScope.getValue(dispatchReceiverType)
@@ -150,9 +151,9 @@ class FirTypeIntersectionScopeContext(
val result = mutableListOf<ResultOfIntersection<D>>() val result = mutableListOf<ResultOfIntersection<D>>()
while (allMembersWithScope.size > 1) { while (allMembersWithScope.size > 1) {
val groupWithPrivate = val groupWithInvisible =
overrideService.extractBothWaysOverridable(allMembersWithScope.maxByVisibility(), allMembersWithScope, overrideChecker) overrideService.extractBothWaysOverridable(allMembersWithScope.maxByVisibility(), allMembersWithScope, overrideChecker)
val group = groupWithPrivate.filter { !Visibilities.isPrivate(it.member.fir.visibility) }.ifEmpty { groupWithPrivate } val group = groupWithInvisible.filter { it.isVisible() }.ifEmpty { groupWithInvisible }
val nonSubsumed = if (forClassUseSiteScope) group.nonSubsumed() else group val nonSubsumed = if (forClassUseSiteScope) group.nonSubsumed() else group
val mostSpecific = overrideService.selectMostSpecificMembers(nonSubsumed, ReturnTypeCalculatorForFullBodyResolve.Default) val mostSpecific = overrideService.selectMostSpecificMembers(nonSubsumed, ReturnTypeCalculatorForFullBodyResolve.Default)
val nonTrivial = if (forClassUseSiteScope) { val nonTrivial = if (forClassUseSiteScope) {
@@ -186,6 +187,17 @@ class FirTypeIntersectionScopeContext(
return result return result
} }
private fun MemberWithBaseScope<*>.isVisible(): Boolean {
// Checking for private is not enough because package-private declarations can be hidden, too, if they're in a different package.
val dispatchClassSymbol = dispatchClassSymbol ?: return true
return session.visibilityChecker.isVisibleForOverriding(
dispatchClassSymbol.moduleData,
dispatchClassSymbol.classId.packageFqName,
member.fir
)
}
fun <D : FirCallableSymbol<*>> createIntersectionOverride( fun <D : FirCallableSymbol<*>> createIntersectionOverride(
mostSpecific: List<MemberWithBaseScope<D>>, mostSpecific: List<MemberWithBaseScope<D>>,
extractedOverrides: List<MemberWithBaseScope<D>>, extractedOverrides: List<MemberWithBaseScope<D>>,
@@ -0,0 +1,29 @@
// FIR_IDENTICAL
// FILE: ViewModel.java
package viewmodel;
public class ViewModel {
final void clear() {
}
}
// FILE: samePackage.kt
package viewmodel
interface IMyViewModel {
fun clear()
}
class MyViewModel: ViewModel(), IMyViewModel {
<!OVERRIDING_FINAL_MEMBER!>override<!> fun clear() = Unit
}
// FILE: differentPackage.kt
package different
import viewmodel.IMyViewModel
import viewmodel.ViewModel
class MyViewModel: ViewModel(), IMyViewModel {
override fun clear() = Unit
}
@@ -26571,6 +26571,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt"); runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/kt9550.kt");
} }
@Test
@TestMetadata("packagePrivateAndPublic.kt")
public void testPackagePrivateAndPublic() throws Exception {
runTest("compiler/testData/diagnostics/tests/override/clashesOnInheritance/packagePrivateAndPublic.kt");
}
@Test @Test
@TestMetadata("returnTypeMismatch.kt") @TestMetadata("returnTypeMismatch.kt")
public void testReturnTypeMismatch() throws Exception { public void testReturnTypeMismatch() throws Exception {