From 461d91a10e3ce9fff599b95857e78bb5860a39bf Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Mon, 30 May 2022 19:05:47 +0300 Subject: [PATCH] FIR: Rework checking dispatch receivers applicability Using `ownerLookupTag` might be wrong in case of private constuctors of inner classes: their owner is an Inner class, but expected dispach receiver is Outer --- ...CompilerTestFE10TestdataTestGenerated.java | 6 +++ ...irOldFrontendDiagnosticsTestGenerated.java | 6 +++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ .../fir/java/FirJavaVisibilityChecker.kt | 5 +- .../kotlin/fir/FirVisibilityChecker.kt | 46 +++++++++++++------ .../org/jetbrains/kotlin/fir/ClassMembers.kt | 8 +++- ...ClassDelegatingConstructorCallToPrivate.kt | 10 ++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ 8 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index 39ff8e112bf..36595dac596 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -33496,6 +33496,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/visibility"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); } + @Test + @TestMetadata("innerClassDelegatingConstructorCallToPrivate.kt") + public void testInnerClassDelegatingConstructorCallToPrivate() throws Exception { + runTest("compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt"); + } + @Test @TestMetadata("innerConstructorThroughSubclass.kt") public void testInnerConstructorThroughSubclass() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index ff34a085675..7273154eb5a 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -33496,6 +33496,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/visibility"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); } + @Test + @TestMetadata("innerClassDelegatingConstructorCallToPrivate.kt") + public void testInnerClassDelegatingConstructorCallToPrivate() throws Exception { + runTest("compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt"); + } + @Test @TestMetadata("innerConstructorThroughSubclass.kt") public void testInnerConstructorThroughSubclass() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index 7397667e784..75fa79ae9e9 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -33496,6 +33496,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/visibility"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); } + @Test + @TestMetadata("innerClassDelegatingConstructorCallToPrivate.kt") + public void testInnerClassDelegatingConstructorCallToPrivate() throws Exception { + runTest("compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt"); + } + @Test @TestMetadata("innerConstructorThroughSubclass.kt") public void testInnerConstructorThroughSubclass() throws Exception { diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt index 622a7408cc1..4492c068da6 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/FirJavaVisibilityChecker.kt @@ -17,9 +17,6 @@ import org.jetbrains.kotlin.fir.resolve.SupertypeSupplier import org.jetbrains.kotlin.fir.resolve.calls.FirSimpleSyntheticPropertySymbol import org.jetbrains.kotlin.fir.resolve.calls.ReceiverValue import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirNamedFunctionSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirPropertyAccessorSymbol -import org.jetbrains.kotlin.fir.symbols.impl.FirVariableSymbol @NoMutableState object FirJavaVisibilityChecker : FirVisibilityChecker() { @@ -40,7 +37,7 @@ object FirJavaVisibilityChecker : FirVisibilityChecker() { } else { val ownerLookupTag = symbol.getOwnerLookupTag() ?: return false if (canSeeProtectedMemberOf( - containingDeclarations, dispatchReceiver, ownerLookupTag, session, + symbol, containingDeclarations, dispatchReceiver, ownerLookupTag, session, isVariableOrNamedFunction = symbol.isVariableOrNamedFunction(), isSyntheticProperty = symbol.fir is FirSyntheticPropertyAccessor, supertypeSupplier diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt index 78cee0354d5..0befc4c0eaa 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt @@ -175,6 +175,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { else -> { // Member: visible inside parent class, including all its member classes canSeePrivateMemberOf( + symbol, containingDeclarations, ownerLookupTag, dispatchReceiver, @@ -191,7 +192,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { Visibilities.Protected -> { val ownerId = symbol.getOwnerLookupTag() ownerId != null && canSeeProtectedMemberOf( - containingDeclarations, dispatchReceiver, ownerId, session, + symbol, containingDeclarations, dispatchReceiver, ownerId, session, isVariableOrNamedFunction = symbol.isVariableOrNamedFunction(), symbol.fir is FirSyntheticPropertyAccessor, supertypeSupplier @@ -223,6 +224,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { ): Boolean private fun canSeePrivateMemberOf( + symbol: FirBasedSymbol<*>, containingDeclarationOfUseSite: List, ownerLookupTag: ConeClassLikeLookupTag, dispatchReceiver: ReceiverValue?, @@ -231,6 +233,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { ): Boolean { ownerLookupTag.ownerIfCompanion(session)?.let { companionOwnerLookupTag -> return canSeePrivateMemberOf( + symbol, containingDeclarationOfUseSite, companionOwnerLookupTag, dispatchReceiver, @@ -239,16 +242,25 @@ abstract class FirVisibilityChecker : FirSessionComponent { ) } - val ownerSymbol = ownerLookupTag.toSymbol(session) ?: return true + if (dispatchReceiver != null) { + val dispatchReceiverParameterClassSymbol = + (symbol.fir as? FirCallableDeclaration) + ?.propertyIfAccessor?.propertyIfBackingField + ?.dispatchReceiverClassOrNull()?.toSymbol(session) + ?: return true - val dispatchReceiverValueOwnerLookupTag = - dispatchReceiver.ownerIfCompanion(session) - ?: dispatchReceiver?.type?.findClassRepresentation( - ownerLookupTag.constructClassType(Array(ownerSymbol.fir.typeParameters.size) { ConeStarProjection }, isNullable = true), + val dispatchReceiverParameterClassLookupTag = dispatchReceiverParameterClassSymbol.toLookupTag() + val dispatchReceiverValueOwnerLookupTag = + dispatchReceiver.type.findClassRepresentation( + dispatchReceiverParameterClassLookupTag.constructClassType( + Array(dispatchReceiverParameterClassSymbol.fir.typeParameters.size) { ConeStarProjection }, + isNullable = true + ), session, ) - if (dispatchReceiverValueOwnerLookupTag != null && ownerLookupTag != dispatchReceiverValueOwnerLookupTag) return false + if (dispatchReceiverParameterClassLookupTag != dispatchReceiverValueOwnerLookupTag) return false + } for (declaration in containingDeclarationOfUseSite) { if (declaration !is FirClass) continue @@ -373,6 +385,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { } protected fun canSeeProtectedMemberOf( + usedSymbol: FirBasedSymbol<*>, containingDeclarationOfUseSite: List, dispatchReceiver: ReceiverValue?, ownerLookupTag: ConeClassLikeLookupTag, @@ -382,6 +395,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { supertypeSupplier: SupertypeSupplier ): Boolean { if (canSeePrivateMemberOf( + usedSymbol, containingDeclarationOfUseSite, ownerLookupTag, dispatchReceiver, @@ -431,19 +445,21 @@ val FirSession.visibilityChecker: FirVisibilityChecker by FirSession.sessionComp fun FirBasedSymbol<*>.getOwnerLookupTag(): ConeClassLikeLookupTag? { return when (this) { is FirBackingFieldSymbol -> fir.propertySymbol.getOwnerLookupTag() - is FirClassLikeSymbol<*> -> { - if (classId.isLocal) { - (fir as? FirRegularClass)?.containingClassForLocal() - } else { - val ownerId = classId.outerClassId - ownerId?.let { ConeClassLikeLookupTagImpl(it) } - } - } + is FirClassLikeSymbol<*> -> getContainingClassLookupTag() is FirCallableSymbol<*> -> containingClass() else -> error("Unsupported owner search for ${fir.javaClass}: ${fir.render()}") } } +fun FirClassLikeSymbol<*>.getContainingClassLookupTag(): ConeClassLikeLookupTag? { + return if (classId.isLocal) { + (fir as? FirRegularClass)?.containingClassForLocal() + } else { + val ownerId = classId.outerClassId + ownerId?.let { ConeClassLikeLookupTagImpl(it) } + } +} + fun FirBasedSymbol<*>.isVariableOrNamedFunction(): Boolean { return this is FirVariableSymbol || this is FirNamedFunctionSymbol || this is FirPropertyAccessorSymbol } diff --git a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/ClassMembers.kt b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/ClassMembers.kt index 4205c39e287..18b258c1dee 100644 --- a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/ClassMembers.kt +++ b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/ClassMembers.kt @@ -5,10 +5,8 @@ package org.jetbrains.kotlin.fir -import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.isLocal -import org.jetbrains.kotlin.fir.declarations.utils.modality import org.jetbrains.kotlin.fir.symbols.ConeClassLikeLookupTag import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.types.ConeClassLikeType @@ -127,3 +125,9 @@ private object CorrespondingProperty : FirDeclarationDataKey() * The corresponding [FirProperty] if the current value parameter is a `val` or `var` declared inside the primary constructor. */ var FirValueParameter.correspondingProperty: FirProperty? by FirDeclarationDataRegistry.data(CorrespondingProperty) + +val FirCallableDeclaration.propertyIfAccessor: FirCallableDeclaration + get() = (this as? FirPropertyAccessor)?.propertySymbol?.fir ?: this + +val FirCallableDeclaration.propertyIfBackingField: FirCallableDeclaration + get() = (this as? FirBackingField)?.propertySymbol?.fir ?: this diff --git a/compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt b/compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt new file mode 100644 index 00000000000..a16f75d919e --- /dev/null +++ b/compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt @@ -0,0 +1,10 @@ +// FIR_IDENTICAL +// SKIP_TXT + +val w: Int = 2 + +class Outer { + private inner class Inner private constructor(x: Int) { + constructor() : this(w) + } +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index a94ba349961..37c8767b1eb 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -33586,6 +33586,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/visibility"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); } + @Test + @TestMetadata("innerClassDelegatingConstructorCallToPrivate.kt") + public void testInnerClassDelegatingConstructorCallToPrivate() throws Exception { + runTest("compiler/testData/diagnostics/tests/visibility/innerClassDelegatingConstructorCallToPrivate.kt"); + } + @Test @TestMetadata("innerConstructorThroughSubclass.kt") public void testInnerConstructorThroughSubclass() throws Exception {