From e96aeb77a6b09ccde0bb805dcb558164c9e95746 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Wed, 6 Nov 2019 15:19:42 +0300 Subject: [PATCH] FIR visibility check: support private class members --- .../jetbrains/kotlin/fir/FirCallResolver.kt | 3 + .../kotlin/fir/resolve/calls/Candidate.kt | 5 +- .../fir/resolve/calls/CandidateCollector.kt | 3 +- .../fir/resolve/calls/ConsumerFactories.kt | 3 +- .../kotlin/fir/resolve/calls/ResolverParts.kt | 179 +++++++++++------- .../transformers/FirSyntheticCallGenerator.kt | 3 +- .../expresssions/localWithBooleanNot.kt | 23 +++ .../expresssions/localWithBooleanNot.txt | 24 +++ .../resolve/expresssions/privateVisibility.kt | 57 ++++++ .../expresssions/privateVisibility.txt | 76 ++++++++ .../fir/FirDiagnosticsTestGenerated.java | 10 + .../expressions/objectAsCallable.fir.txt | 2 +- 12 files changed, 310 insertions(+), 78 deletions(-) create mode 100644 compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.kt create mode 100644 compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.txt create mode 100644 compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.kt create mode 100644 compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.txt diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt index 1a8c6d2180c..c9b82eb14a5 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirCallResolver.kt @@ -85,6 +85,7 @@ class FirCallResolver( typeArguments, session, file, + transformer.components.implicitReceiverStack, transformer.components.container ) { it.resultType } towerResolver.reset() @@ -143,6 +144,7 @@ class FirCallResolver( emptyList(), session, file, + transformer.components.implicitReceiverStack, transformer.components.container ) { it.resultType } towerResolver.reset() @@ -322,6 +324,7 @@ class FirCallResolver( emptyList(), session, file, + transformer.components.implicitReceiverStack, transformer.components.container, expectedType, outerConstraintSystemBuilder, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt index 32b705b96df..8defdce804d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Candidate.kt @@ -8,11 +8,13 @@ package org.jetbrains.kotlin.fir.resolve.calls import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.declarations.FirDeclaration import org.jetbrains.kotlin.fir.declarations.FirFile +import org.jetbrains.kotlin.fir.declarations.FirRegularClass import org.jetbrains.kotlin.fir.declarations.FirValueParameter import org.jetbrains.kotlin.fir.expressions.FirExpression import org.jetbrains.kotlin.fir.expressions.impl.FirNoReceiverExpression import org.jetbrains.kotlin.fir.resolve.BodyResolveComponents import org.jetbrains.kotlin.fir.resolve.DoubleColonLHS +import org.jetbrains.kotlin.fir.resolve.ImplicitReceiverStack import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol import org.jetbrains.kotlin.fir.types.ConeKotlinType @@ -34,7 +36,8 @@ class CallInfo( val typeArguments: List, val session: FirSession, val containingFile: FirFile, - val container: FirDeclaration, + val implicitReceiverStack: ImplicitReceiverStack, + val containingDeclaration: FirDeclaration, // Three properties for callable references only val expectedType: ConeKotlinType? = null, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt index 4210e713544..ff79f53bc10 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/CandidateCollector.kt @@ -98,7 +98,8 @@ class InvokeReceiverCandidateCollector( invokeCallInfo.typeArguments, session, invokeCallInfo.containingFile, - invokeCallInfo.container, + invokeCallInfo.implicitReceiverStack, + invokeCallInfo.containingDeclaration, invokeCallInfo.expectedType, invokeCallInfo.outerCSBuilder, invokeCallInfo.lhs, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConsumerFactories.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConsumerFactories.kt index b4452828326..c4378a0a60d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConsumerFactories.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ConsumerFactories.kt @@ -71,7 +71,8 @@ fun createFunctionConsumer( callInfo.typeArguments, bodyResolveComponents.session, callInfo.containingFile, - callInfo.container, + callInfo.implicitReceiverStack, + callInfo.containingDeclaration, callInfo.expectedType, callInfo.outerCSBuilder, callInfo.lhs, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolverParts.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolverParts.kt index 427114a315d..7a88ebee266 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolverParts.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolverParts.kt @@ -9,19 +9,18 @@ import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier import org.jetbrains.kotlin.fir.render -import org.jetbrains.kotlin.fir.resolve.DoubleColonLHS -import org.jetbrains.kotlin.fir.resolve.createFunctionalType -import org.jetbrains.kotlin.fir.resolve.createKPropertyType -import org.jetbrains.kotlin.fir.resolve.firProvider +import org.jetbrains.kotlin.fir.resolve.* import org.jetbrains.kotlin.fir.symbols.AbstractFirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirClassLikeSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.fir.types.ConeKotlinErrorType import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.fir.types.FirResolvedTypeRef import org.jetbrains.kotlin.fir.types.coneTypeSafe import org.jetbrains.kotlin.load.java.JavaVisibilities +import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.calls.inference.ConstraintSystemOperation import org.jetbrains.kotlin.resolve.calls.inference.model.SimpleConstraintSystemConstraintPosition @@ -275,81 +274,115 @@ internal object CheckVisibility : CheckerStage() { } } + private fun ImplicitReceiverStack.canSeePrivateMemberOf(ownerId: ClassId): Boolean { + for (implicitReceiverValue in receiversAsReversed()) { + if (implicitReceiverValue !is ImplicitDispatchReceiverValue) continue + if (implicitReceiverValue.boundSymbol.classId == ownerId) return true + } + return false + } + + private fun ClassId.asLocal(): ClassId = ClassId(packageFqName, relativeClassName, true) + + private suspend fun checkVisibility( + declaration: FirMemberDeclaration, + symbol: AbstractFirBasedSymbol<*>, + sink: CheckerSink, + callInfo: CallInfo + ): Boolean { + val useSiteFile = callInfo.containingFile + val implicitReceiverStack = callInfo.implicitReceiverStack + val visible = when (declaration.visibility) { + JavaVisibilities.PACKAGE_VISIBILITY -> { + symbol.packageFqName() == useSiteFile.packageFqName + } + Visibilities.PRIVATE, Visibilities.PRIVATE_TO_THIS -> { + if (declaration.session == callInfo.session) { + val provider = callInfo.session.firProvider + when (symbol) { + is FirClassLikeSymbol<*> -> { + val classId = symbol.classId + val ownerId = classId.outerClassId + when { + classId.isLocal -> { + // Normally should not be here + implicitReceiverStack.canSeePrivateMemberOf(ownerId?.asLocal() ?: classId) + } + ownerId == null -> { + // Top-level: visible in file + provider.getFirClassifierContainerFile(classId) == useSiteFile + } + else -> { + // Member: visible inside parent class, including all its member classes + implicitReceiverStack.canSeePrivateMemberOf(ownerId) + } + } + } + is FirCallableSymbol<*> -> { + val candidateFile = provider.getFirCallableContainerFile(symbol) + val ownerId = symbol.callableId.classId + when { + candidateFile == null -> { + // Local + ownerId != null && implicitReceiverStack.canSeePrivateMemberOf(ownerId.asLocal()) + } + ownerId == null -> { + // Top-level: visible in file + candidateFile == useSiteFile + } + else -> { + // Member: visible inside parent class, including all its member classes + implicitReceiverStack.canSeePrivateMemberOf(ownerId) + } + } + } + else -> { + throw AssertionError("Unsupported visibility check for ${declaration.javaClass}: ${declaration.render()}") + } + } + } else { + false + } + } + Visibilities.INTERNAL -> { + declaration.session == callInfo.session + } + Visibilities.PROTECTED -> { + true // TODO: Support protected visibility + } + JavaVisibilities.PROTECTED_AND_PACKAGE -> { + if (symbol.packageFqName() == useSiteFile.packageFqName) { + true + } else { + true // TODO: Support protected visibility + } + } + else -> true + } + + if (!visible) { + sink.yieldApplicability(CandidateApplicability.HIDDEN) + return false + } + return true + } + override suspend fun check(candidate: Candidate, sink: CheckerSink, callInfo: CallInfo) { val symbol = candidate.symbol val declaration = symbol.fir if (declaration is FirMemberDeclaration) { - val useSiteFile = callInfo.containingFile - val visible = when (declaration.visibility) { - JavaVisibilities.PACKAGE_VISIBILITY -> { - symbol.packageFqName() == useSiteFile.packageFqName - } - Visibilities.PRIVATE, Visibilities.PRIVATE_TO_THIS -> { - if (declaration.session == callInfo.session) { - val provider = callInfo.session.firProvider - when (symbol) { - is FirClassLikeSymbol<*> -> { - val classId = symbol.classId - when { - classId.isLocal -> { - // Normally should not be here - TODO() - } - !classId.isNestedClass -> { - // Top-level: visible in file - provider.getFirClassifierContainerFile(classId) == useSiteFile - } - else -> { - // Member: visible inside parent class, including all its member classes - TODO() - } - } - } - is FirCallableSymbol<*> -> { - val candidateFile = provider.getFirCallableContainerFile(symbol) - when { - candidateFile == null -> { - // Local - TODO() - } - symbol.callableId.classId == null -> { - // Top-level: visible in file - candidateFile == useSiteFile - } - else -> { - // Member: visible inside parent class, including all its member classes - TODO() - } - } - } - else -> { - throw AssertionError("Unsupported visibility check for ${declaration.javaClass}: ${declaration.render()}") - } - } - } else { - false - } - } - Visibilities.INTERNAL -> { - declaration.session == callInfo.session - } - Visibilities.PROTECTED -> { - // Support protected visibility... - TODO() - } - JavaVisibilities.PROTECTED_AND_PACKAGE -> { - if (symbol.packageFqName() == useSiteFile.packageFqName) { - true - } else { - // Support protected visibility... - TODO() - } - } - else -> true + if (!checkVisibility(declaration, symbol, sink, callInfo)) { + return } + } - if (!visible) { - sink.yieldApplicability(CandidateApplicability.HIDDEN) + if (declaration is FirConstructor) { + val ownerClassId = declaration.symbol.callableId.classId!! + val provider = declaration.session.firSymbolProvider + val classSymbol = provider.getClassLikeSymbolByFqName(ownerClassId) + + if (classSymbol is FirRegularClassSymbol) { + checkVisibility(classSymbol.fir, classSymbol, sink, callInfo) } } } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSyntheticCallGenerator.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSyntheticCallGenerator.kt index 612ddddd28b..dd81b0a1e67 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSyntheticCallGenerator.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSyntheticCallGenerator.kt @@ -131,7 +131,8 @@ class FirSyntheticCallGenerator( typeArguments = emptyList(), session = session, containingFile = file, - container = container + implicitReceiverStack = implicitReceiverStack, + containingDeclaration = container ) { it.resultType } private fun generateSyntheticSelectFunction(callableId: CallableId, isVararg: Boolean = true): FirSimpleFunctionImpl { diff --git a/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.kt b/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.kt new file mode 100644 index 00000000000..f3b9673fcce --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.kt @@ -0,0 +1,23 @@ +// FILE: Node.java + +public interface Node { + R result(); +} + +// FILE: foo.kt + +fun foo(): Boolean { + object : Node { + private var result = false + + fun bar(): Boolean { + return !result + } + + // Cannot see private member of local class / anonymous object + override fun result() = result + } + + val some = true + return !some +} \ No newline at end of file diff --git a/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.txt b/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.txt new file mode 100644 index 00000000000..a5278158304 --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.txt @@ -0,0 +1,24 @@ +FILE: foo.kt + public final fun foo(): R|kotlin/Boolean| { + object : R|Node| { + private constructor(): R|kotlin/Any| { + super() + } + + private final var result: R|kotlin/Boolean| = Boolean(false) + private get(): R|kotlin/Boolean| + private set(value: R|kotlin/Boolean|): R|kotlin/Unit| + + public final fun bar(): R|kotlin/Boolean| { + ^bar R|/anonymous.result|.R|kotlin/Boolean.not|() + } + + public final override fun result(): R|kotlin/Boolean| { + ^result R|/anonymous.result| + } + + } + + lval some: R|kotlin/Boolean| = Boolean(true) + ^foo R|/some|.R|kotlin/Boolean.not|() + } diff --git a/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.kt b/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.kt new file mode 100644 index 00000000000..77fb0bdb68f --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.kt @@ -0,0 +1,57 @@ +// FILE: first.kt + +private fun foo() {} + +private class Private { + private fun bar() {} + + fun baz() { + bar() + Nested() + } + + inner class Inner { + fun foo() { + bar() + } + } + + private class Nested +} + +fun withLocals() { + class Local { + private fun bar() + + fun baz() { + bar() + Inner() + } + + private inner class Inner { + fun foo() { + bar() + } + } + } + + Local().baz() + + Local().bar() // hidden +} + +fun test() { + foo() + Private().baz() + Private().Inner() + + Private().bar() // hidden + Private.Nested() // hidden +} + +// FILE: second.kt + +fun secondTest() { + foo() // hidden + Private() // hidden +} \ No newline at end of file diff --git a/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.txt b/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.txt new file mode 100644 index 00000000000..35613058477 --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.txt @@ -0,0 +1,76 @@ +FILE: first.kt + private final fun foo(): R|kotlin/Unit| { + } + private final class Private : R|kotlin/Any| { + public constructor(): R|Private| { + super() + } + + private final fun bar(): R|kotlin/Unit| { + } + + public final fun baz(): R|kotlin/Unit| { + this@R|/Private|.R|/Private.bar|() + R|/Private.Nested.Nested|() + } + + public final inner class Inner : R|kotlin/Any| { + public constructor(): R|Private.Inner| { + super() + } + + public final fun foo(): R|kotlin/Unit| { + this@R|/Private|.R|/Private.bar|() + } + + } + + private final class Nested : R|kotlin/Any| { + public constructor(): R|Private.Nested| { + super() + } + + } + + } + public final fun withLocals(): R|kotlin/Unit| { + local final class Local : R|kotlin/Any| { + public constructor(): R|Local| { + super() + } + + private final fun bar(): R|kotlin/Unit| + + public final fun baz(): R|kotlin/Unit| { + R|/Local.bar|() + R|/Local.Inner.Inner|() + } + + local final inner class Inner : R|kotlin/Any| { + public constructor(): R|Local.Inner| { + super() + } + + public final fun foo(): R|kotlin/Unit| { + R|/Local.bar|() + } + + } + + } + + R|/Local.Local|().R|/Local.baz|() + R|/Local.Local|().#() + } + public final fun test(): R|kotlin/Unit| { + R|/foo|() + R|/Private.Private|().R|/Private.baz|() + R|/Private.Private|().R|/Private.Inner.Inner|() + R|/Private.Private|().#() + Q|Private|.#() + } +FILE: second.kt + public final fun secondTest(): R|kotlin/Unit| { + #() + #() + } diff --git a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java index 9e1b3a00580..07830f33a38 100644 --- a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java +++ b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java @@ -414,6 +414,11 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { runTest("compiler/fir/resolve/testData/resolve/expresssions/localTypes.kt"); } + @TestMetadata("localWithBooleanNot.kt") + public void testLocalWithBooleanNot() throws Exception { + runTest("compiler/fir/resolve/testData/resolve/expresssions/localWithBooleanNot.kt"); + } + @TestMetadata("memberExtension.kt") public void testMemberExtension() throws Exception { runTest("compiler/fir/resolve/testData/resolve/expresssions/memberExtension.kt"); @@ -439,6 +444,11 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { runTest("compiler/fir/resolve/testData/resolve/expresssions/privateObjectLiteral.kt"); } + @TestMetadata("privateVisibility.kt") + public void testPrivateVisibility() throws Exception { + runTest("compiler/fir/resolve/testData/resolve/expresssions/privateVisibility.kt"); + } + @TestMetadata("qualifiedExpressions.kt") public void testQualifiedExpressions() throws Exception { runTest("compiler/fir/resolve/testData/resolve/expresssions/qualifiedExpressions.kt"); diff --git a/compiler/testData/ir/irText/expressions/objectAsCallable.fir.txt b/compiler/testData/ir/irText/expressions/objectAsCallable.fir.txt index f6d7b6f492d..af27e7a1761 100644 --- a/compiler/testData/ir/irText/expressions/objectAsCallable.fir.txt +++ b/compiler/testData/ir/irText/expressions/objectAsCallable.fir.txt @@ -100,7 +100,7 @@ FILE fqName: fileName:/objectAsCallable.kt PROPERTY name:test1 visibility:public modality:FINAL [val] FIELD PROPERTY_BACKING_FIELD name:test1 type:IrErrorType visibility:private [final,static] EXPRESSION_BODY - ERROR_CALL 'Unresolved reference: #' type=IrErrorType + ERROR_CALL 'Unresolved reference: #' type=IrErrorType CONST Int type=kotlin.Int value=42 FUN DEFAULT_PROPERTY_ACCESSOR name: visibility:public modality:FINAL <> () returnType:IrErrorType correspondingProperty: PROPERTY name:test1 visibility:public modality:FINAL [val]