From f070401bb5fbaa4b56625e8a4553d35e392b0a8f Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Thu, 27 Oct 2022 12:11:21 +0200 Subject: [PATCH] K2: fix visibility checks for static overrides via imported from static #KT-53441 In Progress --- .../resolve/arguments/kt41940.fir.txt | 2 +- .../j+k/FlexiblePrimitiveOverloading.fir.txt | 2 +- .../declaration/FirConflictsChecker.kt | 2 +- .../declaration/FirOverrideChecker.kt | 4 +- .../FirVisibilityQualifierChecker.kt | 3 +- .../kotlin/fir/FirVisibilityChecker.kt | 28 ++++++++++++- .../scopes/impl/FirAbstractImportingScope.kt | 22 ++++++++-- .../impl/FirObjectImportedCallableScope.kt | 41 ++++++++++--------- .../kotlin/fir/QualifiedNameResolution.kt | 4 +- .../fir/resolve/calls/VisibilityUtils.kt | 20 +++++---- .../fir/resolve/dfa/FirDataFlowAnalyzer.kt | 3 +- .../tests/visibility/packagePrivateStatic.kt | 14 +++++++ .../tests/visibility/packagePrivateStatic.txt | 5 +++ 13 files changed, 107 insertions(+), 43 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/arguments/kt41940.fir.txt b/compiler/fir/analysis-tests/testData/resolve/arguments/kt41940.fir.txt index 4bf790068f5..9a68a08246c 100644 --- a/compiler/fir/analysis-tests/testData/resolve/arguments/kt41940.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/arguments/kt41940.fir.txt @@ -1,4 +1,4 @@ FILE: main.kt public final fun test(list: R|kotlin/collections/List|, string: R|kotlin/String|): R|kotlin/Unit| { - R|/Matchers.assertThat|, kotlin/collections/List?>|>(R|/list|, R|/Matchers.hasItem|(R|/string|)) + Q|Matchers|.R|/Matchers.assertThat|, kotlin/collections/List?>|>(R|/list|, Q|Matchers|.R|/Matchers.hasItem|(R|/string|)) } diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/j+k/FlexiblePrimitiveOverloading.fir.txt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/j+k/FlexiblePrimitiveOverloading.fir.txt index c7939573405..bc9127a1f14 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/j+k/FlexiblePrimitiveOverloading.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/j+k/FlexiblePrimitiveOverloading.fir.txt @@ -1,4 +1,4 @@ FILE: FlexiblePrimitiveOverloading.kt public final fun foo(): R|kotlin/Unit| { - R|java/lang/Integer.getInteger|(String(text), Int(239)) + Q|java/lang/Integer|.R|java/lang/Integer.getInteger|(String(text), Int(239)) } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictsChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictsChecker.kt index 5bd63afadc4..34dc1cfed44 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictsChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirConflictsChecker.kt @@ -102,7 +102,7 @@ object FirConflictsChecker : FirBasicDeclarationChecker() { if (isExpectAndActual(declaration, conflicting)) return if ( conflicting is FirMemberDeclaration && - !session.visibilityChecker.isVisible(conflicting, session, containingFile, emptyList(), null) + !session.visibilityChecker.isVisible(conflicting, session, containingFile, emptyList(), dispatchReceiver = null) ) return val declarationIsLowPriority = hasLowPriorityAnnotation(declaration.annotations) val conflictingIsLowPriority = hasLowPriorityAnnotation(conflicting.annotations) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt index 4e41eabb9e3..42dec09c5fd 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirOverrideChecker.kt @@ -171,7 +171,7 @@ object FirOverrideChecker : FirClassChecker() { context.session, file, containingDeclarations, - null, + dispatchReceiver = null, skipCheckForContainingClassVisibility = true ) } @@ -281,7 +281,7 @@ object FirOverrideChecker : FirClassChecker() { context.session, file, containingDeclarations, - null, + dispatchReceiver = null, skipCheckForContainingClassVisibility = true ) }?.originalOrSelf() ?: 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 9b7851e4336..95cc24f01b7 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 @@ -35,9 +35,8 @@ object FirVisibilityQualifierChecker : FirResolvedQualifierChecker() { val firFile = context.containingDeclarations.firstOrNull() as? FirFile ?: return val firClassLikeDeclaration = symbol.fir - if (!context.session.visibilityChecker.isVisible( + if (!context.session.visibilityChecker.isClassLikeVisible( firClassLikeDeclaration, context.session, firFile, context.containingDeclarations, - dispatchReceiver = null, ) ) { reporter.reportOn(expression.source, FirErrors.INVISIBLE_REFERENCE, symbol, context) 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 1940cfa9521..525d45383c2 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt @@ -72,6 +72,25 @@ abstract class FirVisibilityChecker : FirSessionComponent { } } + fun isClassLikeVisible( + declaration: FirClassLikeDeclaration, + session: FirSession, + useSiteFile: FirFile, + containingDeclarations: List, + ): Boolean { + return isVisible( + declaration, + session, + useSiteFile, + containingDeclarations, + dispatchReceiver = null, + isCallToPropertySetter = false, + staticQualifierClassForCallable = null, + skipCheckForContainingClassVisibility = false, + supertypeSupplier = SupertypeSupplier.Default + ) + } + fun isVisible( declaration: FirMemberDeclaration, session: FirSession, @@ -79,6 +98,7 @@ abstract class FirVisibilityChecker : FirSessionComponent { containingDeclarations: List, dispatchReceiver: ReceiverValue?, isCallToPropertySetter: Boolean = false, + staticQualifierClassForCallable: FirRegularClass? = null, // There's no need to check if containing class is visible in case we check if a member might be overridden in a subclass // because visibility for its supertype that contain overridden member is being checked when resolving the type reference. // Such flag is not necessary in FE1.0, since there are full structure of fake overrides and containing declaration for overridden @@ -108,8 +128,14 @@ abstract class FirVisibilityChecker : FirSessionComponent { supertypeSupplier ) ?: return true return generateSequence(parentClass) { it.containingNonLocalClass(session) }.all { parent -> + val classLikeToCheck = + if (staticQualifierClassForCallable?.isSubClass(parent.symbol.toLookupTag(), session, supertypeSupplier) == true) { + staticQualifierClassForCallable + } else { + parent + } isSpecificDeclarationVisible( - parent, + classLikeToCheck, session, useSiteFile, containingDeclarations, diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractImportingScope.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractImportingScope.kt index 98d25bf9e00..9c7cf9f0485 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractImportingScope.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractImportingScope.kt @@ -61,9 +61,14 @@ abstract class FirAbstractImportingScope( for (import in imports) { val importedName = name ?: import.importedName ?: continue if (isExcluded(import, importedName)) continue - val staticsScope = import.resolvedParentClassId?.let(::getStaticsScope) + val parentClassId = import.resolvedParentClassId + val staticsScope = parentClassId?.let { getStaticsScope(it) } + if (staticsScope != null) { - staticsScope.processFunctionsByName(importedName, processor) + staticsScope.processFunctionsByName(importedName) { + if (it.isStatic) processor(it.fir.buildImportedCopy(parentClassId).symbol) + else processor(it) + } } else if (importedName.isSpecial || importedName.identifier.isNotEmpty()) { for (symbol in provider.getTopLevelFunctionSymbols(import.packageFqName, importedName)) { processor(symbol) @@ -76,9 +81,18 @@ abstract class FirAbstractImportingScope( for (import in imports) { val importedName = name ?: import.importedName ?: continue if (isExcluded(import, importedName)) continue - val staticsScope = import.resolvedParentClassId?.let(::getStaticsScope) + val parentClassId = import.resolvedParentClassId + val staticsScope = parentClassId?.let { getStaticsScope(it) } + if (staticsScope != null) { - staticsScope.processPropertiesByName(importedName, processor) + staticsScope.processPropertiesByName(importedName) { + if (it is FirPropertySymbol) { + if (it.isStatic) processor(it.fir.buildImportedCopy(parentClassId).symbol) + else processor(it) + } else { + processor(it) + } + } } else if (importedName.isSpecial || importedName.identifier.isNotEmpty()) { for (symbol in provider.getTopLevelPropertySymbols(import.packageFqName, importedName)) { processor(symbol) diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirObjectImportedCallableScope.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirObjectImportedCallableScope.kt index c23a12eb2bb..5fd59e78141 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirObjectImportedCallableScope.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/impl/FirObjectImportedCallableScope.kt @@ -5,10 +5,7 @@ package org.jetbrains.kotlin.fir.scopes.impl -import org.jetbrains.kotlin.fir.declarations.FirCallableDeclaration -import org.jetbrains.kotlin.fir.declarations.FirDeclarationDataKey -import org.jetbrains.kotlin.fir.declarations.FirDeclarationDataRegistry -import org.jetbrains.kotlin.fir.declarations.FirDeclarationOrigin +import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.builder.buildPropertyCopy import org.jetbrains.kotlin.fir.declarations.builder.buildSimpleFunctionCopy import org.jetbrains.kotlin.fir.scopes.FirContainingNamesAwareScope @@ -26,13 +23,7 @@ class FirObjectImportedCallableScope( ) : FirContainingNamesAwareScope() { override fun processFunctionsByName(name: Name, processor: (FirNamedFunctionSymbol) -> Unit) { objectUseSiteScope.processFunctionsByName(name) wrapper@{ symbol -> - val function = symbol.fir - val syntheticFunction = buildSimpleFunctionCopy(function) { - origin = FirDeclarationOrigin.ImportedFromObjectOrStatic - this.symbol = FirNamedFunctionSymbol(CallableId(importedClassId, name)) - }.apply { - importedFromObjectOrStaticData = ImportedFromObjectOrStaticData(importedClassId, function) - } + val syntheticFunction = symbol.fir.buildImportedCopy(importedClassId) processor(syntheticFunction.symbol) } } @@ -43,14 +34,7 @@ class FirObjectImportedCallableScope( processor(symbol) return@wrapper } - val property = symbol.fir - val syntheticProperty = buildPropertyCopy(property) { - origin = FirDeclarationOrigin.ImportedFromObjectOrStatic - this.symbol = FirPropertySymbol(CallableId(importedClassId, name)) - this.delegateFieldSymbol = null - }.apply { - importedFromObjectOrStaticData = ImportedFromObjectOrStaticData(importedClassId, property) - } + val syntheticProperty = symbol.fir.buildImportedCopy(importedClassId) processor(syntheticProperty.symbol) } } @@ -60,6 +44,25 @@ class FirObjectImportedCallableScope( override fun getClassifierNames(): Set = emptySet() } +internal fun FirSimpleFunction.buildImportedCopy(importedClassId: ClassId): FirSimpleFunction { + return buildSimpleFunctionCopy(this) { + origin = FirDeclarationOrigin.ImportedFromObjectOrStatic + this.symbol = FirNamedFunctionSymbol(CallableId(importedClassId, name)) + }.apply { + importedFromObjectOrStaticData = ImportedFromObjectOrStaticData(importedClassId, this@buildImportedCopy) + } +} + +internal fun FirProperty.buildImportedCopy(importedClassId: ClassId): FirProperty { + return buildPropertyCopy(this) { + origin = FirDeclarationOrigin.ImportedFromObjectOrStatic + this.symbol = FirPropertySymbol(CallableId(importedClassId, name)) + this.delegateFieldSymbol = null + }.apply { + importedFromObjectOrStaticData = ImportedFromObjectOrStaticData(importedClassId, this@buildImportedCopy) + } +} + private object ImportedFromObjectOrStaticClassIdKey : FirDeclarationDataKey() class ImportedFromObjectOrStaticData( diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/QualifiedNameResolution.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/QualifiedNameResolution.kt index e655d6a634b..2bce29f79ef 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/QualifiedNameResolution.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/QualifiedNameResolution.kt @@ -48,13 +48,11 @@ fun BodyResolveComponents.resolveRootPartOfQualifier( for (scope in createCurrentScopeList()) { scope.getSingleVisibleClassifier(session, this, name)?.let { if (it is FirRegularClassSymbol) { - val isVisible = session.visibilityChecker.isVisible( + val isVisible = session.visibilityChecker.isClassLikeVisible( it.fir, session, file, containingDeclarations, - null, - false, ) if (!isVisible) { return@let diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/VisibilityUtils.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/VisibilityUtils.kt index 89e6ea3ed23..7a547f8ee08 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/VisibilityUtils.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/VisibilityUtils.kt @@ -9,12 +9,11 @@ import org.jetbrains.kotlin.KtFakeSourceElementKind import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fakeElement import org.jetbrains.kotlin.fir.* -import org.jetbrains.kotlin.fir.declarations.FirBackingField -import org.jetbrains.kotlin.fir.declarations.FirCallableDeclaration -import org.jetbrains.kotlin.fir.declarations.FirMemberDeclaration -import org.jetbrains.kotlin.fir.declarations.FirProperty +import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.getExplicitBackingField +import org.jetbrains.kotlin.fir.declarations.utils.isStatic import org.jetbrains.kotlin.fir.declarations.utils.visibility +import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier import org.jetbrains.kotlin.fir.expressions.FirSmartCastExpression import org.jetbrains.kotlin.fir.expressions.FirVariableAssignment import org.jetbrains.kotlin.fir.expressions.builder.buildSmartCastExpression @@ -23,20 +22,27 @@ import org.jetbrains.kotlin.fir.types.coneType import org.jetbrains.kotlin.fir.types.isNullableNothing import org.jetbrains.kotlin.fir.types.makeConeTypeDefinitelyNotNullOrNotNull import org.jetbrains.kotlin.fir.types.typeContext +import org.jetbrains.kotlin.utils.addToStdlib.runIf fun FirVisibilityChecker.isVisible( declaration: FirMemberDeclaration, callInfo: CallInfo, dispatchReceiverValue: ReceiverValue? -): Boolean = - isVisible( +): Boolean { + val staticQualifierForCallable = runIf(declaration is FirCallableDeclaration && declaration.isStatic) { + val explicitReceiver = callInfo.explicitReceiver ?: (dispatchReceiverValue as? ExpressionReceiverValue)?.explicitReceiver + (explicitReceiver as? FirResolvedQualifier)?.symbol?.fir as? FirRegularClass + } + return isVisible( declaration, callInfo.session, callInfo.containingFile, callInfo.containingDeclarations, dispatchReceiverValue, - callInfo.callSite is FirVariableAssignment + staticQualifierClassForCallable = staticQualifierForCallable, + isCallToPropertySetter = callInfo.callSite is FirVariableAssignment ) +} fun FirVisibilityChecker.isVisible( declaration: FirMemberDeclaration, diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt index 75bfb6c9905..48925d5444a 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt @@ -132,12 +132,11 @@ abstract class FirDataFlowAnalyzer( val symbol = fullyExpandedType(components.session).lookupTag.toSymbol(components.session) ?: return false val declaration = symbol.fir as? FirRegularClass ?: return true - visibilityChecker.isVisible( + visibilityChecker.isClassLikeVisible( declaration, components.session, components.context.file, components.context.containers, - dispatchReceiver = null ) } is ConeTypeParameterType -> true diff --git a/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.kt b/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.kt index da7a242bb42..5c3460a6f9e 100644 --- a/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.kt +++ b/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.kt @@ -6,6 +6,8 @@ package foo; class Base { protected static void foo() {} protected void bar() {} + + public static void baz() {} } // FILE: foo/Derived.java @@ -24,3 +26,15 @@ class Impl : Derived() { bar() } } + +fun test() { + Derived.baz() +} + +// FILE: withImport.kt + +import foo.Derived.baz + +fun test() { + baz() +} diff --git a/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.txt b/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.txt index d089608c939..6532b8c373c 100644 --- a/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.txt +++ b/compiler/testData/diagnostics/tests/visibility/packagePrivateStatic.txt @@ -1,6 +1,9 @@ package +public fun test(): kotlin.Unit + package bar { + public fun test(): kotlin.Unit public final class Impl : foo.Derived { public constructor Impl() @@ -22,6 +25,7 @@ package foo { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String // Static members + public open fun baz(): kotlin.Unit protected/*protected static*/ open fun foo(): kotlin.Unit } @@ -33,6 +37,7 @@ package foo { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String // Static members + public open override /*1*/ /*fake_override*/ fun baz(): kotlin.Unit protected/*protected static*/ open override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit } }