From 490ef210ac04ede70a68af8a55625c06599dc848 Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Thu, 4 Feb 2021 12:17:37 +0300 Subject: [PATCH] [FIR] Support sealed class inheritors in multiple files --- .../resolve/diagnostics/sealedSupertype.kt | 2 +- .../declaration/FirSealedSupertypeChecker.kt | 43 ++----- .../kotlin/fir/FirVisibilityChecker.kt | 27 +++-- ...rLegacySealedClassInheritorsTransformer.kt | 38 ++++++ ...t => FirSealedClassInheritorsProcessor.kt} | 109 +++++++++--------- .../resolve/transformers/ResolvePhaseUtils.kt | 4 +- .../box/sealed/multipleFiles_enabled.kt | 1 - .../tests/sealed/DerivedTopLevel.fir.kt | 2 +- .../tests/sealed/ExhaustiveWithFreedom.fir.kt | 6 +- .../tests/sealed/MultipleFiles_enabled.fir.kt | 12 +- .../NestedSealedWithoutRestrictions.fir.kt | 39 ------- .../sealed/NestedSealedWithoutRestrictions.kt | 1 + .../sealed/NeverDerivedFromNested.fir.kt | 2 +- 13 files changed, 130 insertions(+), 156 deletions(-) create mode 100644 compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirLegacySealedClassInheritorsTransformer.kt rename compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/{FirSealedClassInheritorsTransformer.kt => FirSealedClassInheritorsProcessor.kt} (75%) delete mode 100644 compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolve/diagnostics/sealedSupertype.kt b/compiler/fir/analysis-tests/testData/resolve/diagnostics/sealedSupertype.kt index 6bd11376197..61185630aaa 100644 --- a/compiler/fir/analysis-tests/testData/resolve/diagnostics/sealedSupertype.kt +++ b/compiler/fir/analysis-tests/testData/resolve/diagnostics/sealedSupertype.kt @@ -29,7 +29,7 @@ sealed class P { class K : P() object B { - class I : P() + class I : P() } fun test() { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSealedSupertypeChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSealedSupertypeChecker.kt index 700b30b1a5b..35f1d502457 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSealedSupertypeChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirSealedSupertypeChecker.kt @@ -20,26 +20,18 @@ object FirSealedSupertypeChecker : FirMemberDeclarationChecker() { override fun check(declaration: FirMemberDeclaration, context: CheckerContext, reporter: DiagnosticReporter) { if (declaration is FirClass<*>) { // only the file declaration is present - when { - context.containingDeclarations.size == 1 -> { - checkTopLevelDeclaration(declaration, context, reporter) - } - declaration.classId.isLocal -> { - checkLocalDeclaration(declaration, context, reporter) - } - else -> { - checkInnerDeclaration(declaration, context, reporter) - } + if (declaration.classId.isLocal) { + checkLocalDeclaration(declaration, context, reporter) + } else { + checkGlobalDeclaration(declaration, context, reporter) } } else if (declaration is FirProperty) { - val initializer = declaration.initializer.safeAs>() - ?: return - + val initializer = declaration.initializer.safeAs>() ?: return checkLocalDeclaration(initializer, context, reporter) } } - private fun checkTopLevelDeclaration(declaration: FirClass<*>, context: CheckerContext, reporter: DiagnosticReporter) { + private fun checkGlobalDeclaration(declaration: FirClass<*>, context: CheckerContext, reporter: DiagnosticReporter) { for (it in declaration.superTypeRefs) { val classId = it.coneType.classId ?: continue @@ -51,9 +43,9 @@ object FirSealedSupertypeChecker : FirMemberDeclarationChecker() { ?.fir.safeAs() ?: continue - if (fir.status.modality == Modality.SEALED && classId.outerClassId != null) { + if (fir.status.modality == Modality.SEALED && declaration.classId.packageFqName != fir.classId.packageFqName) { reporter.reportOn(it.source, FirErrors.SEALED_SUPERTYPE, context) - return + continue } } } @@ -76,23 +68,4 @@ object FirSealedSupertypeChecker : FirMemberDeclarationChecker() { } } } - - private fun checkInnerDeclaration(declaration: FirClass<*>, context: CheckerContext, reporter: DiagnosticReporter) { - for (it in declaration.superTypeRefs) { - val classId = it.coneType.classId ?: continue - - if (classId.isLocal) { - continue - } - - val fir = context.session.symbolProvider.getClassLikeSymbolByFqName(classId) - ?.fir.safeAs() - ?: continue - - if (fir.status.modality == Modality.SEALED && !context.containingDeclarations.contains(fir)) { - reporter.reportOn(it.source, FirErrors.SEALED_SUPERTYPE, context) - return - } - } - } } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt index abc7816f2d9..d2531c45207 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/FirVisibilityChecker.kt @@ -65,17 +65,24 @@ abstract class FirVisibilityChecker : FirSessionComponent { Visibilities.Private, Visibilities.PrivateToThis -> { val ownerId = symbol.getOwnerId() if (declaration.session == session) { - if (ownerId == null || declaration is FirConstructor && declaration.isFromSealedClass) { - val candidateFile = when (symbol) { - is FirClassLikeSymbol<*> -> provider.getFirClassifierContainerFileIfAny(symbol) - is FirCallableSymbol<*> -> provider.getFirCallableContainerFile(symbol) - else -> null + when { + ownerId == null -> { + val candidateFile = when (symbol) { + is FirClassLikeSymbol<*> -> provider.getFirClassifierContainerFileIfAny(symbol) + is FirCallableSymbol<*> -> provider.getFirCallableContainerFile(symbol) + else -> null + } + // Top-level: visible in file + candidateFile == useSiteFile + } + declaration is FirConstructor && declaration.isFromSealedClass -> { + // Sealed class constructor: visible in same package + declaration.symbol.callableId.packageName == useSiteFile.packageFqName + } + else -> { + // Member: visible inside parent class, including all its member classes + canSeePrivateMemberOf(containingDeclarations, ownerId, session) } - // Top-level: visible in file - candidateFile == useSiteFile - } else { - // Member: visible inside parent class, including all its member classes - canSeePrivateMemberOf(containingDeclarations, ownerId, session) } } else { declaration is FirSimpleFunction && declaration.isAllowedToBeAccessedFromOutside() diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirLegacySealedClassInheritorsTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirLegacySealedClassInheritorsTransformer.kt new file mode 100644 index 00000000000..bc813056e6d --- /dev/null +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirLegacySealedClassInheritorsTransformer.kt @@ -0,0 +1,38 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.fir.resolve.transformers + +import org.jetbrains.kotlin.fir.FirElement +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.resolve.ScopeSession +import org.jetbrains.kotlin.fir.visitors.CompositeTransformResult +import org.jetbrains.kotlin.fir.visitors.FirTransformer +import org.jetbrains.kotlin.fir.visitors.compose +import org.jetbrains.kotlin.name.ClassId + +/* + * This processor is needed only for IDE until there won't be proper IDE implementation + * for detecting sealed inheritors in multiple files + */ +class FirLegacySealedClassInheritorsProcessor(session: FirSession, scopeSession: ScopeSession) : FirTransformerBasedResolveProcessor(session, scopeSession) { + override val transformer = FirLegacySealedClassInheritorsTransformer() +} + +class FirLegacySealedClassInheritorsTransformer : FirTransformer() { + override fun transformElement(element: E, data: Nothing?): CompositeTransformResult { + throw IllegalStateException("Should not be there") + } + + override fun transformFile(file: FirFile, data: Nothing?): CompositeTransformResult { + val sealedClassInheritorsMap = mutableMapOf>() + file.accept(FirSealedClassInheritorsProcessor.InheritorsCollector, sealedClassInheritorsMap) + if (sealedClassInheritorsMap.isEmpty()) return file.compose() + return file.transform(FirSealedClassInheritorsProcessor.InheritorsTransformer(sealedClassInheritorsMap), null) + } +} diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsProcessor.kt similarity index 75% rename from compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsTransformer.kt rename to compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsProcessor.kt index 579c848f9cc..98d86b67a48 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirSealedClassInheritorsProcessor.kt @@ -1,5 +1,5 @@ /* - * Copyright 2010-2019 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ @@ -11,38 +11,69 @@ import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.expressions.FirStatement import org.jetbrains.kotlin.fir.resolve.ScopeSession -import org.jetbrains.kotlin.fir.resolve.symbolProvider -import org.jetbrains.kotlin.fir.resolve.providers.FirSymbolProvider +import org.jetbrains.kotlin.fir.resolve.firProvider import org.jetbrains.kotlin.fir.resolve.getSymbolByLookupTag +import org.jetbrains.kotlin.fir.resolve.providers.FirSymbolProvider +import org.jetbrains.kotlin.fir.resolve.symbolProvider import org.jetbrains.kotlin.fir.symbols.impl.FirClassifierSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirTypeAliasSymbol import org.jetbrains.kotlin.fir.types.ConeLookupTagBasedType import org.jetbrains.kotlin.fir.types.FirTypeRef import org.jetbrains.kotlin.fir.types.coneType -import org.jetbrains.kotlin.fir.visitors.CompositeTransformResult -import org.jetbrains.kotlin.fir.visitors.FirDefaultVisitor -import org.jetbrains.kotlin.fir.visitors.FirTransformer -import org.jetbrains.kotlin.fir.visitors.compose +import org.jetbrains.kotlin.fir.visitors.* import org.jetbrains.kotlin.name.ClassId -class FirSealedClassInheritorsProcessor(session: FirSession, scopeSession: ScopeSession) : FirTransformerBasedResolveProcessor(session, scopeSession) { - override val transformer = FirSealedClassInheritorsTransformer() -} - -class FirSealedClassInheritorsTransformer : FirTransformer() { - override fun transformElement(element: E, data: Nothing?): CompositeTransformResult { - throw IllegalStateException("Should not be there") - } - - override fun transformFile(file: FirFile, data: Nothing?): CompositeTransformResult { +class FirSealedClassInheritorsProcessor( + session: FirSession, + scopeSession: ScopeSession +) : FirGlobalResolveProcessor(session, scopeSession) { + override fun process(files: Collection) { val sealedClassInheritorsMap = mutableMapOf>() - file.accept(InheritorsCollector, sealedClassInheritorsMap) - if (sealedClassInheritorsMap.isEmpty()) return file.compose() - return file.transform(InheritorsTransformer(sealedClassInheritorsMap), null) + files.forEach { it.accept(InheritorsCollector, sealedClassInheritorsMap) } + files.forEach { it.transformSingle(InheritorsTransformer(sealedClassInheritorsMap), null) } } - private class InheritorsTransformer(private val inheritorsMap: MutableMap>) : FirTransformer() { + object InheritorsCollector : FirDefaultVisitor>>() { + override fun visitElement(element: FirElement, data: MutableMap>) {} + + override fun visitFile(file: FirFile, data: MutableMap>) { + file.declarations.forEach { it.accept(this, data) } + } + + override fun visitRegularClass(regularClass: FirRegularClass, data: MutableMap>) { + regularClass.declarations.forEach { it.accept(this, data) } + + if (regularClass.modality == Modality.SEALED) { + data.computeIfAbsent(regularClass) { mutableListOf() } + } + + val symbolProvider = regularClass.session.symbolProvider + + for (typeRef in regularClass.superTypeRefs) { + val parent = extractClassFromTypeRef(symbolProvider, typeRef).takeIf { it?.modality == Modality.SEALED } ?: continue + // Inheritors of sealed class are allowed only in same package + if (parent.classId.packageFqName != regularClass.classId.packageFqName) continue + val inheritors = data.computeIfAbsent(parent) { mutableListOf() } + inheritors += regularClass.symbol.classId + } + } + + private fun extractClassFromTypeRef(symbolProvider: FirSymbolProvider, typeRef: FirTypeRef): FirRegularClass? { + val lookupTag = (typeRef.coneType as? ConeLookupTagBasedType)?.lookupTag ?: return null + val classLikeSymbol: FirClassifierSymbol<*> = symbolProvider.getSymbolByLookupTag(lookupTag) ?: return null + return when (classLikeSymbol) { + is FirRegularClassSymbol -> classLikeSymbol.fir + is FirTypeAliasSymbol -> { + classLikeSymbol.ensureResolved(FirResolvePhase.SUPER_TYPES, symbolProvider.session) + extractClassFromTypeRef(symbolProvider, classLikeSymbol.fir.expandedTypeRef) + } + else -> null + } + } + } + + class InheritorsTransformer(private val inheritorsMap: MutableMap>) : FirTransformer() { override fun transformElement(element: E, data: Nothing?): CompositeTransformResult { return element.compose() } @@ -63,42 +94,6 @@ class FirSealedClassInheritorsTransformer : FirTransformer() { } } - private object InheritorsCollector : FirDefaultVisitor>>() { - override fun visitElement(element: FirElement, data: MutableMap>) {} - - override fun visitFile(file: FirFile, data: MutableMap>) { - file.declarations.forEach { it.accept(this, data) } - } - - override fun visitRegularClass(regularClass: FirRegularClass, data: MutableMap>) { - regularClass.declarations.forEach { it.accept(this, data) } - - if (regularClass.modality == Modality.SEALED) { - data.computeIfAbsent(regularClass) { mutableListOf() } - } - - val symbolProvider = regularClass.session.symbolProvider - - for (typeRef in regularClass.superTypeRefs) { - val parent = extractClassFromTypeRef(symbolProvider, typeRef).takeIf { it?.modality == Modality.SEALED } ?: continue - val inheritors = data.computeIfAbsent(parent) { mutableListOf() } - inheritors += regularClass.symbol.classId - } - } - - private fun extractClassFromTypeRef(symbolProvider: FirSymbolProvider, typeRef: FirTypeRef): FirRegularClass? { - val lookupTag = (typeRef.coneType as? ConeLookupTagBasedType)?.lookupTag ?: return null - val classLikeSymbol: FirClassifierSymbol<*> = symbolProvider.getSymbolByLookupTag(lookupTag) ?: return null - return when (classLikeSymbol) { - is FirRegularClassSymbol -> classLikeSymbol.fir - is FirTypeAliasSymbol -> { - classLikeSymbol.ensureResolved(FirResolvePhase.SUPER_TYPES, symbolProvider.session) - extractClassFromTypeRef(symbolProvider, classLikeSymbol.fir.expandedTypeRef) - } - else -> null - } - } - } } object SealedClassInheritorsKey : FirDeclarationDataKey() diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/ResolvePhaseUtils.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/ResolvePhaseUtils.kt index ab4467a3462..a4e76d1a194 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/ResolvePhaseUtils.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/ResolvePhaseUtils.kt @@ -50,7 +50,7 @@ fun FirResolvePhase.createTransformerBasedProcessorByPhase( CLASS_GENERATION -> FirDummyTransformerBasedProcessor(session, scopeSession) // TODO: remove IMPORTS -> FirImportResolveProcessor(session, scopeSession) SUPER_TYPES -> FirSupertypeResolverProcessor(session, scopeSession) - SEALED_CLASS_INHERITORS -> FirSealedClassInheritorsProcessor(session, scopeSession) + SEALED_CLASS_INHERITORS -> FirLegacySealedClassInheritorsProcessor(session, scopeSession) TYPES -> FirTypeResolveProcessor(session, scopeSession) ARGUMENTS_OF_PLUGIN_ANNOTATIONS -> FirAnnotationArgumentsResolveProcessor(session, scopeSession) EXTENSION_STATUS_UPDATE -> FirTransformerBasedExtensionStatusProcessor(session, scopeSession) @@ -74,4 +74,4 @@ private class FirDummyTransformerBasedProcessor( return element.compose() } } -} \ No newline at end of file +} diff --git a/compiler/testData/codegen/box/sealed/multipleFiles_enabled.kt b/compiler/testData/codegen/box/sealed/multipleFiles_enabled.kt index ff712206259..a54e80d1569 100644 --- a/compiler/testData/codegen/box/sealed/multipleFiles_enabled.kt +++ b/compiler/testData/codegen/box/sealed/multipleFiles_enabled.kt @@ -1,5 +1,4 @@ // ISSUE: KT-13495 -// IGNORE_BACKEND_FIR: JVM_IR // !LANGUAGE: +AllowSealedInheritorsInDifferentFilesOfSamePackage // FILE: Base.kt diff --git a/compiler/testData/diagnostics/tests/sealed/DerivedTopLevel.fir.kt b/compiler/testData/diagnostics/tests/sealed/DerivedTopLevel.fir.kt index 26c557d48cd..4b6d60300f8 100644 --- a/compiler/testData/diagnostics/tests/sealed/DerivedTopLevel.fir.kt +++ b/compiler/testData/diagnostics/tests/sealed/DerivedTopLevel.fir.kt @@ -1,7 +1,7 @@ sealed class Base class Derived: Base() { - class Derived2: Base() + class Derived2: Base() } fun test() { diff --git a/compiler/testData/diagnostics/tests/sealed/ExhaustiveWithFreedom.fir.kt b/compiler/testData/diagnostics/tests/sealed/ExhaustiveWithFreedom.fir.kt index f90edc7720d..302a7bd38d8 100644 --- a/compiler/testData/diagnostics/tests/sealed/ExhaustiveWithFreedom.fir.kt +++ b/compiler/testData/diagnostics/tests/sealed/ExhaustiveWithFreedom.fir.kt @@ -10,14 +10,14 @@ sealed class Base { // FILE: B.kt -class B : Base() +class B : Base() // FILE: Container.kt class Containter { - class C : Base() + class C : Base() - inner class D : Base() + inner class D : Base() } // FILE: main.kt diff --git a/compiler/testData/diagnostics/tests/sealed/MultipleFiles_enabled.fir.kt b/compiler/testData/diagnostics/tests/sealed/MultipleFiles_enabled.fir.kt index 3300a9cb6aa..455e7591dde 100644 --- a/compiler/testData/diagnostics/tests/sealed/MultipleFiles_enabled.fir.kt +++ b/compiler/testData/diagnostics/tests/sealed/MultipleFiles_enabled.fir.kt @@ -13,21 +13,21 @@ sealed class Base { package foo -class B : Base() +class B : Base() // FILE: c.kt package foo class Container { - class C : Base() + class C : Base() - inner class D : Base() + inner class D : Base() - val anon = object : Base() {} // Should be an error + val anon = object : Base() {} // Should be an error fun someFun() { - class LocalClass : Base() {} // Should be an error + class LocalClass : Base() {} // Should be an error } } @@ -37,4 +37,4 @@ package bar import foo.Base -class E : Base() +class E : Base() diff --git a/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.fir.kt b/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.fir.kt deleted file mode 100644 index ff3c72191d0..00000000000 --- a/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.fir.kt +++ /dev/null @@ -1,39 +0,0 @@ -// ISSUE: KT-13495 -// !LANGUAGE: +AllowSealedInheritorsInDifferentFilesOfSamePackage -// !DIAGNOSTICS: -UNUSED_VARIABLE - -// FILE: base.kt - -package foo - -class Container { - sealed class Base -} - -// FILE: a.kt - -package foo - -class A : Container.Base() - -// FILE: b.kt - -package foo - -class BContainer { - class B : Container.Base() - - inner class C : Container.Base() -} - -// FILE: test.kt - -package foo - -fun test(base: Container.Base) { - val x = when (base) { - is A -> 1 - is BContainer.B -> 2 - is BContainer.C -> 3 - } -} diff --git a/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.kt b/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.kt index c6da20d58a6..31716bacff5 100644 --- a/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.kt +++ b/compiler/testData/diagnostics/tests/sealed/NestedSealedWithoutRestrictions.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // ISSUE: KT-13495 // !LANGUAGE: +AllowSealedInheritorsInDifferentFilesOfSamePackage // !DIAGNOSTICS: -UNUSED_VARIABLE diff --git a/compiler/testData/diagnostics/tests/sealed/NeverDerivedFromNested.fir.kt b/compiler/testData/diagnostics/tests/sealed/NeverDerivedFromNested.fir.kt index f455a892688..b59a1dae1f6 100644 --- a/compiler/testData/diagnostics/tests/sealed/NeverDerivedFromNested.fir.kt +++ b/compiler/testData/diagnostics/tests/sealed/NeverDerivedFromNested.fir.kt @@ -2,7 +2,7 @@ class A { sealed class Base } -class Derived : A.Base() +class Derived : A.Base() fun test() { class DerivedLocal : A.Base()