From e6669b51d4450cb9be0b2d3f1f36d41ebf692800 Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Wed, 6 Sep 2023 18:25:21 -0500 Subject: [PATCH] [FIR] Check standalone companion object qualifiers are visible --- .../fir/analysis/checkers/FirHelpers.kt | 13 ++++++++ .../FirStandaloneQualifierChecker.kt | 8 ++--- .../FirVisibilityQualifierChecker.kt | 33 ++++++++++++++++--- .../classObjects/ClassObjectVisibility.fir.kt | 12 +++++-- .../classObjects/ClassObjectVisibility.kt | 8 +++++ .../classObjects/ClassObjectVisibility.txt | 28 ++++++++++++++++ .../invisibleClassObjects.fir.diag.txt | 2 ++ .../classObjects/invisibleClassObjects.fir.kt | 2 +- .../enum/classObjectInEnumPrivate.fir.kt | 2 +- .../resolve/constructorVsCompanion.fir.kt | 2 +- .../typealias/typeAliasInvisibleObject.fir.kt | 4 +-- 11 files changed, 97 insertions(+), 17 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt index 8c1245292eb..4ec58813447 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/FirHelpers.kt @@ -833,8 +833,21 @@ fun ConeKotlinType.leastUpperBound(session: FirSession): ConeKotlinType { fun ConeKotlinType.fullyExpandedClassId(session: FirSession): ClassId? { return fullyExpandedType(session).classId } + @OptIn(ExperimentalContracts::class) fun ConeKotlinType.hasDiagnosticKind(kind: DiagnosticKind): Boolean { contract { returns(true) implies (this@hasDiagnosticKind is ConeErrorType) } return this is ConeErrorType && (diagnostic as? ConeSimpleDiagnostic)?.kind == kind } + +fun FirResolvedQualifier.isStandalone( + context: CheckerContext, +): Boolean { + val lastQualifiedAccess = context.callsOrAssignments.lastOrNull() as? FirQualifiedAccessExpression + // Note: qualifier isn't standalone when it's in receiver (SomeClass.foo) or getClass (SomeClass::class) position + if (lastQualifiedAccess?.explicitReceiver === this || lastQualifiedAccess?.dispatchReceiver === this) return false + val lastGetClass = context.getClassCalls.lastOrNull() + if (lastGetClass?.argument === this) return false + + return true +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirStandaloneQualifierChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirStandaloneQualifierChecker.kt index baa690634bd..f59da480d1b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirStandaloneQualifierChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirStandaloneQualifierChecker.kt @@ -10,9 +10,9 @@ import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.diagnostics.reportOn import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.isStandalone import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.declarations.fullyExpandedClass -import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol @@ -22,11 +22,7 @@ import org.jetbrains.kotlin.fir.types.resolvedType object FirStandaloneQualifierChecker : FirResolvedQualifierChecker() { override fun check(expression: FirResolvedQualifier, context: CheckerContext, reporter: DiagnosticReporter) { - val lastQualifiedAccess = context.callsOrAssignments.lastOrNull() as? FirQualifiedAccessExpression - // Note: qualifier isn't standalone when it's in receiver (SomeClass.foo) or getClass (SomeClass::class) position - if (lastQualifiedAccess?.explicitReceiver === expression || lastQualifiedAccess?.dispatchReceiver === expression) return - val lastGetClass = context.getClassCalls.lastOrNull() - if (lastGetClass?.argument === expression) return + if (!expression.isStandalone(context)) return // Note: if it's real Unit, it will be filtered by ClassKind.OBJECT check below in reportErrorOn if (!expression.resolvedType.isUnit) return diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirVisibilityQualifierChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirVisibilityQualifierChecker.kt index 53deaa9d20e..30672ba7cfa 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirVisibilityQualifierChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/expression/FirVisibilityQualifierChecker.kt @@ -8,8 +8,10 @@ package org.jetbrains.kotlin.fir.analysis.checkers.expression import org.jetbrains.kotlin.KtFakeSourceElementKind import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext +import org.jetbrains.kotlin.fir.analysis.checkers.isStandalone import org.jetbrains.kotlin.fir.analysis.diagnostics.toInvisibleReferenceDiagnostic import org.jetbrains.kotlin.fir.declarations.FirTypeAlias +import org.jetbrains.kotlin.fir.declarations.fullyExpandedClass import org.jetbrains.kotlin.fir.declarations.utils.expandedConeType import org.jetbrains.kotlin.fir.expressions.FirErrorResolvedQualifier import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier @@ -18,20 +20,22 @@ import org.jetbrains.kotlin.fir.resolve.diagnostics.ConeVisibilityError import org.jetbrains.kotlin.fir.resolve.toSymbol import org.jetbrains.kotlin.fir.symbols.SymbolInternals import org.jetbrains.kotlin.fir.symbols.impl.FirClassLikeSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.fir.types.toSymbol import org.jetbrains.kotlin.fir.visibilityChecker object FirVisibilityQualifierChecker : FirResolvedQualifierChecker() { override fun check(expression: FirResolvedQualifier, context: CheckerContext, reporter: DiagnosticReporter) { - checkClassLikeSymbol(expression.symbol ?: return, expression, context, reporter) + checkClassLikeSymbol(expression.symbol ?: return, expression, expression.isStandalone(context), context, reporter) } @OptIn(SymbolInternals::class) private fun checkClassLikeSymbol( symbol: FirClassLikeSymbol<*>, expression: FirResolvedQualifier, + isStandalone: Boolean, context: CheckerContext, - reporter: DiagnosticReporter + reporter: DiagnosticReporter, ) { val firFile = context.containingFile ?: return val firClassLikeDeclaration = symbol.fir @@ -50,14 +54,35 @@ object FirVisibilityQualifierChecker : FirResolvedQualifierChecker() { return } + // Validate standalone references to companion objects are visible. Qualified use is validated + // by call resolution cone diagnostics in coneDiagnosticToFirDiagnostic. + if (isStandalone) { + val invisibleCompanion = expression.symbol?.fullyExpandedClass(context.session)?.toInvisibleCompanion(context) + if (invisibleCompanion != null) { + if (expression !is FirErrorResolvedQualifier || expression.diagnostic !is ConeVisibilityError) { + reporter.report(invisibleCompanion.toInvisibleReferenceDiagnostic(expression.source), context) + } + + return + } + } + if (firClassLikeDeclaration is FirTypeAlias) { firClassLikeDeclaration.expandedConeType?.toSymbol(context.session)?.let { - checkClassLikeSymbol(it, expression, context, reporter) + checkClassLikeSymbol(it, expression, isStandalone, context, reporter) } } symbol.getOwnerLookupTag()?.toSymbol(context.session)?.let { - checkClassLikeSymbol(it, expression, context, reporter) + checkClassLikeSymbol(it, expression, isStandalone = false, context, reporter) + } + } + + @OptIn(SymbolInternals::class) + private fun FirRegularClassSymbol.toInvisibleCompanion(context: CheckerContext): FirRegularClassSymbol? { + val firFile = context.containingFile ?: return null + return companionObjectSymbol?.takeIf { + !context.session.visibilityChecker.isClassLikeVisible(it.fir, context.session, firFile, context.containingDeclarations) } } } diff --git a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.fir.kt b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.fir.kt index 4e786195c46..171ee8f2a0b 100644 --- a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.fir.kt +++ b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.fir.kt @@ -3,9 +3,10 @@ package foo fun test() { A.d A.Companion.f - B.D - CCC + B.D + CCC CCC.classObjectVar + E.F.G } class A() { @@ -28,3 +29,10 @@ class CCC() { val classObjectVar = 3 } } + +class E { + class F { + companion object G + } + private companion object +} diff --git a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.kt b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.kt index b4a10ef21a4..4825a5c864b 100644 --- a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.kt +++ b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.kt @@ -6,6 +6,7 @@ fun test() { B.D CCC CCC.classObjectVar + E.F.G } class A() { @@ -28,3 +29,10 @@ class CCC() { val classObjectVar = 3 } } + +class E { + class F { + companion object G + } + private companion object +} diff --git a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.txt b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.txt index bc4071738c0..3b454622ad1 100644 --- a/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.txt +++ b/compiler/testData/diagnostics/tests/classObjects/ClassObjectVisibility.txt @@ -60,4 +60,32 @@ package foo { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } } + + public final class E { + public constructor E() + 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 + + private companion object Companion { + private constructor Companion() + 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 + } + + public final class F { + public constructor F() + 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 + + public companion object G { + private constructor G() + 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 + } + } + } } diff --git a/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.diag.txt b/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.diag.txt index 022d2e922c7..d31164e6f50 100644 --- a/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.diag.txt +++ b/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.diag.txt @@ -12,6 +12,8 @@ /b.kt:(225,226): error: Cannot access 'class B : Any': it is private in file. +/b.kt:(234,235): error: Cannot access 'companion object Companion : Any': it is private in 'a/C'. + /b.kt:(243,244): error: Cannot access 'class D : Any': it is private in file. /b.kt:(263,264): error: Cannot access 'class B : Any': it is private in file. diff --git a/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.kt b/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.kt index cd6a65634dd..52f521b18da 100644 --- a/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.kt +++ b/compiler/testData/diagnostics/tests/classObjects/invisibleClassObjects.fir.kt @@ -41,7 +41,7 @@ import a.D.Companion.quux fun test() { f(A) f(B) - f(C) + f(C) f(D) A.foo() diff --git a/compiler/testData/diagnostics/tests/enum/classObjectInEnumPrivate.fir.kt b/compiler/testData/diagnostics/tests/enum/classObjectInEnumPrivate.fir.kt index b13ef37b74e..be44574c8f4 100644 --- a/compiler/testData/diagnostics/tests/enum/classObjectInEnumPrivate.fir.kt +++ b/compiler/testData/diagnostics/tests/enum/classObjectInEnumPrivate.fir.kt @@ -7,4 +7,4 @@ enum class E { fun foo() = E.values() fun bar() = E.valueOf("ENTRY") fun baz() = E.ENTRY -fun quux() = E \ No newline at end of file +fun quux() = E diff --git a/compiler/testData/diagnostics/tests/resolve/constructorVsCompanion.fir.kt b/compiler/testData/diagnostics/tests/resolve/constructorVsCompanion.fir.kt index a5e60aa0f3c..cc83eb1b1fe 100644 --- a/compiler/testData/diagnostics/tests/resolve/constructorVsCompanion.fir.kt +++ b/compiler/testData/diagnostics/tests/resolve/constructorVsCompanion.fir.kt @@ -17,7 +17,7 @@ class E private constructor() { } val a = A -val b = B +val b = B val c = C val d = D val e = E(42) diff --git a/compiler/testData/diagnostics/tests/typealias/typeAliasInvisibleObject.fir.kt b/compiler/testData/diagnostics/tests/typealias/typeAliasInvisibleObject.fir.kt index 1fffd688b40..40716b45a4d 100644 --- a/compiler/testData/diagnostics/tests/typealias/typeAliasInvisibleObject.fir.kt +++ b/compiler/testData/diagnostics/tests/typealias/typeAliasInvisibleObject.fir.kt @@ -4,5 +4,5 @@ class C { typealias CAlias = C -val test1 = CAlias -val test1a = C \ No newline at end of file +val test1 = CAlias +val test1a = C