From ac1c7d9a6a1d1a22f5b913f85c34b693bf74ace7 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 1 Sep 2017 18:57:01 +0300 Subject: [PATCH] Drop Incompatible.NoImpl, fix no "missing impl" on nested class Header/impl declarations are now considered fully compatible even in the case when the impl declaration has no "impl" modifier. The error about no "impl" is now reported separately and only on the impl declaration, never on the header declaration #KT-20087 Fixed --- .../checkers/HeaderImplDeclarationChecker.kt | 59 +++++++------------ .../headerClass/nestedClassesWithErrors.kt | 8 +++ .../headerClass/nestedClassesWithErrors.txt | 28 +++++++++ .../highlighter/PlatformHeaderAnnotator.kt | 2 +- 4 files changed, 59 insertions(+), 38 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/HeaderImplDeclarationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/HeaderImplDeclarationChecker.kt index c6fc3a2b28a..eec0746fc34 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/HeaderImplDeclarationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/HeaderImplDeclarationChecker.kt @@ -59,13 +59,13 @@ object HeaderImplDeclarationChecker : DeclarationChecker { ) { if (!languageVersionSettings.supportsFeature(LanguageFeature.MultiPlatformProjects)) return - if (descriptor !is MemberDescriptor) return + if (descriptor !is MemberDescriptor || DescriptorUtils.isEnumEntry(descriptor)) return - val checkImpl = !languageVersionSettings.getFlag(AnalysisFlag.multiPlatformDoNotCheckImpl) if (descriptor.isHeader) { - checkHeaderDeclarationHasImplementation(declaration, descriptor, diagnosticHolder, descriptor.module, checkImpl) + checkHeaderDeclarationHasImplementation(declaration, descriptor, diagnosticHolder, descriptor.module) } else { + val checkImpl = !languageVersionSettings.getFlag(AnalysisFlag.multiPlatformDoNotCheckImpl) checkImplementationHasHeaderDeclaration(declaration, descriptor, diagnosticHolder, checkImpl) } } @@ -74,13 +74,12 @@ object HeaderImplDeclarationChecker : DeclarationChecker { reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink, - platformModule: ModuleDescriptor, - checkImpl: Boolean + platformModule: ModuleDescriptor ) { // Only look for implementations of top level members; class members will be handled as a part of that header class if (descriptor.containingDeclaration !is PackageFragmentDescriptor) return - val compatibility = findImplForHeader(descriptor, platformModule, checkImpl) ?: return + val compatibility = findImplForHeader(descriptor, platformModule) ?: return val shouldReportError = compatibility.isEmpty() || @@ -97,11 +96,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { } } - private fun findImplForHeader( - header: MemberDescriptor, - platformModule: ModuleDescriptor, - checkImpl: Boolean - ): Map>? { + private fun findImplForHeader(header: MemberDescriptor, platformModule: ModuleDescriptor): Map>? { return when (header) { is CallableMemberDescriptor -> { header.findNamesakesFromModule(platformModule).filter { impl -> @@ -109,7 +104,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { // TODO: support non-source definitions (e.g. from Java) DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement }.groupBy { impl -> - areCompatibleCallables(header, impl, checkImpl) + areCompatibleCallables(header, impl) } } is ClassDescriptor -> { @@ -117,7 +112,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { header != impl && !impl.isHeader && DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement }.groupBy { impl -> - areCompatibleClassifiers(header, impl, checkImpl) + areCompatibleClassifiers(header, impl) } } else -> null @@ -132,11 +127,13 @@ object HeaderImplDeclarationChecker : DeclarationChecker { // TODO: use common module here val compatibility = findHeaderForImpl(descriptor, descriptor.module) ?: return - if (!descriptor.isImpl || !reportOn.hasModifier(KtTokens.IMPL_KEYWORD)) { - if (checkImpl && (Compatible in compatibility || Incompatible.NoImpl in compatibility)) { + val hasImplModifier = descriptor.isImpl && reportOn.hasModifier(KtTokens.IMPL_KEYWORD) + if (!hasImplModifier) { + if (Compatible !in compatibility) return + + if (checkImpl) { diagnosticHolder.report(Errors.IMPL_MISSING.on(reportOn)) } - return } // 'firstOrNull' is needed because in diagnostic tests, common sources appear twice, so the same class is duplicated @@ -214,7 +211,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { Substitutor(headerClass.declaredTypeParameters, container.declaredTypeParameters) } else null - areCompatibleCallables(declaration, impl, checkImpl = true, parentSubstitutor = substitutor) + areCompatibleCallables(declaration, impl, parentSubstitutor = substitutor) } } is ClassifierDescriptorWithTypeParameters -> { @@ -222,7 +219,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { impl != declaration && declaration is ClassDescriptor && declaration.isHeader }.groupBy { header -> - areCompatibleClassifiers(header as ClassDescriptor, impl, checkImpl = true) + areCompatibleClassifiers(header as ClassDescriptor, impl) } } else -> null @@ -230,10 +227,10 @@ object HeaderImplDeclarationChecker : DeclarationChecker { } fun MemberDescriptor.findCompatibleImplForHeader(platformModule: ModuleDescriptor): List = - findImplForHeader(this, platformModule, false)?.get(Compatible).orEmpty() + findImplForHeader(this, platformModule)?.get(Compatible).orEmpty() fun MemberDescriptor.findAnyImplForHeader(platformModule: ModuleDescriptor): List { - val implsGroupedByCompatibility = findImplForHeader(this, platformModule, false) + val implsGroupedByCompatibility = findImplForHeader(this, platformModule) return implsGroupedByCompatibility?.get(Compatible) ?: implsGroupedByCompatibility?.values?.flatten() ?: emptyList() @@ -341,8 +338,6 @@ object HeaderImplDeclarationChecker : DeclarationChecker { object TypeParameterVariance : Incompatible("declaration-site variances of type parameters are different") object TypeParameterReified : Incompatible("some type parameter is reified in one declaration and non-reified in the other") - object NoImpl : Incompatible("the implementation is not marked with the 'impl' modifier (-Xno-check-impl)") - object Unknown : Incompatible(null) } @@ -353,7 +348,6 @@ object HeaderImplDeclarationChecker : DeclarationChecker { private fun areCompatibleCallables( a: CallableMemberDescriptor, b: CallableMemberDescriptor, - checkImpl: Boolean, platformModule: ModuleDescriptor = b.module, parentSubstitutor: Substitutor? = null ): Compatibility { @@ -404,8 +398,6 @@ object HeaderImplDeclarationChecker : DeclarationChecker { else -> throw AssertionError("Unsupported declarations: $a, $b") } - if (checkImpl && !b.isImpl && b.kind == CallableMemberDescriptor.Kind.DECLARATION) return Incompatible.NoImpl - return Compatible } @@ -489,17 +481,13 @@ object HeaderImplDeclarationChecker : DeclarationChecker { return Compatible } - private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor, checkImpl: Boolean): Compatibility { + private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor): Compatibility { // Can't check FQ names here because nested header class may be implemented via impl typealias's expansion with the other FQ name assert(a.name == other.name) { "This function should be invoked only for declarations with the same name: $a, $other" } - var implTypealias = false val b = when (other) { is ClassDescriptor -> other - is TypeAliasDescriptor -> { - implTypealias = true - other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias - } + is TypeAliasDescriptor -> other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias else -> throw AssertionError("Incorrect impl classifier for $a: $other") } @@ -527,9 +515,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker { bSupertypes.none { bSupertype -> areCompatibleTypes(aSupertype, bSupertype, platformModule) } }) return Incompatible.Supertypes - areCompatibleClassScopes(a, b, checkImpl && !implTypealias, platformModule, substitutor).let { if (it != Compatible) return it } - - if (checkImpl && !b.isImpl && !implTypealias) return Incompatible.NoImpl + areCompatibleClassScopes(a, b, platformModule, substitutor).let { if (it != Compatible) return it } return Compatible } @@ -538,7 +524,6 @@ object HeaderImplDeclarationChecker : DeclarationChecker { private fun areCompatibleClassScopes( a: ClassDescriptor, b: ClassDescriptor, - checkImpl: Boolean, platformModule: ModuleDescriptor, substitutor: Substitutor ): Compatibility { @@ -557,9 +542,9 @@ object HeaderImplDeclarationChecker : DeclarationChecker { val mapping = bMembers.keysToMap { bMember -> when (aMember) { is CallableMemberDescriptor -> - areCompatibleCallables(aMember, bMember as CallableMemberDescriptor, checkImpl, platformModule, substitutor) + areCompatibleCallables(aMember, bMember as CallableMemberDescriptor, platformModule, substitutor) is ClassDescriptor -> - areCompatibleClassifiers(aMember, bMember as ClassDescriptor, checkImpl) + areCompatibleClassifiers(aMember, bMember as ClassDescriptor) else -> throw UnsupportedOperationException("Unsupported declaration: $aMember ($bMembers)") } } diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.kt index b0381ebabcd..80b2d641826 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.kt @@ -19,6 +19,10 @@ header class D { class N } +header class E { + class N +} + // MODULE: m1-jvm(m1-common) // FILE: jvm.kt @@ -36,3 +40,7 @@ impl class C { } impl class D + +impl class E { + class N +} diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.txt index 42afe52215b..6ed6fe8c1c3 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.txt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/nestedClassesWithErrors.txt @@ -66,6 +66,20 @@ public final header class D { } } +public final header 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 + + public final header class N { + public constructor N() + 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 + } +} + // -- Module: -- package @@ -130,3 +144,17 @@ public final impl class D { public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } + +public final impl 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 + + public final class N { + public constructor N() + 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/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/PlatformHeaderAnnotator.kt b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/PlatformHeaderAnnotator.kt index 736d46a4785..796f0358dcc 100644 --- a/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/PlatformHeaderAnnotator.kt +++ b/idea/idea-analysis/src/org/jetbrains/kotlin/idea/highlighter/PlatformHeaderAnnotator.kt @@ -68,7 +68,7 @@ class PlatformHeaderAnnotator : Annotator { val trace = BindingTraceContext() for (module in implementingModules) { - HeaderImplDeclarationChecker.checkHeaderDeclarationHasImplementation(declaration, descriptor, trace, module, checkImpl = false) + HeaderImplDeclarationChecker.checkHeaderDeclarationHasImplementation(declaration, descriptor, trace, module) } val suppressionCache = KotlinCacheService.getInstance(declaration.project).getSuppressionCache()