Check platform<->impl compatibility for class members

When matching platform and impl classifiers, ensure that each declaration from the platform class scope is present in the impl class scope. Note that the presence of the 'impl' modifier is not checked yet
2016-11-22 18:18:13 +03:00
parent 8d3f6f1ce7
commit bbafb7c013
21 changed files with 294 additions and 27 deletions
@@ -16,28 +16,58 @@

 package org.jetbrains.kotlin.diagnostics.rendering

+import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
 import org.jetbrains.kotlin.descriptors.MemberDescriptor
 import org.jetbrains.kotlin.resolve.checkers.PlatformImplDeclarationChecker

 object PlatformIncompatibilityDiagnosticRenderer :
        DiagnosticParameterRenderer<Map<PlatformImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>> {
+    private val INDENTATION_UNIT = "    "
+
    override fun render(
            obj: Map<PlatformImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>,
            renderingContext: RenderingContext
    ): String {
        if (obj.isEmpty()) return ""

+        val renderDescriptor: (DeclarationDescriptor) -> String = { Renderers.COMPACT_WITH_MODIFIERS.render(it, renderingContext) }
+
        return buildString {
            appendln()
-            for ((incompatibility, descriptors) in obj) {
-                append("The following declaration")
-                if (descriptors.size == 1) append(" is") else append("s are")
-                append(" incompatible")
-                incompatibility.reason?.let { append(" because $it") }
+            render(obj, "", renderDescriptor)
+        }
+    }
+
+    private fun StringBuilder.render(
+            map: Map<PlatformImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>,
+            indent: String,
+            renderDescriptor: (DeclarationDescriptor) -> String
+    ) {
+        for ((incompatibility, descriptors) in map) {
+            append(indent)
+            append("The following declaration")
+            if (descriptors.size == 1) append(" is") else append("s are")
+            append(" incompatible")
+            incompatibility.reason?.let { append(" because $it") }
+
+            incompatibility.unimplemented?.let { unimplemented ->
+                appendln(".")
+                append(indent)
+                appendln("No implementations are found for members listed below:")
+                for ((descriptor, mapping) in unimplemented) {
+                    appendln()
+                    append(indent + "    ")
+                    appendln(renderDescriptor(descriptor))
+                    if (mapping.isNotEmpty()) {
+                        appendln()
+                    }
+                    render(mapping, indent + INDENTATION_UNIT, renderDescriptor)
+                }
+            } ?: run {
                appendln(":")
                for (descriptor in descriptors) {
-                    append("    ")
-                    appendln(Renderers.COMPACT_WITH_MODIFIERS.render(descriptor, renderingContext))
+                    append(indent + "    ")
+                    appendln(renderDescriptor(descriptor))
                }
            }
        }
@@ -23,6 +23,7 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticSink
 import org.jetbrains.kotlin.diagnostics.Errors
 import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.psi.KtDeclaration
 import org.jetbrains.kotlin.psi.KtElement
 import org.jetbrains.kotlin.resolve.BindingContext
@@ -33,11 +34,13 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
 import org.jetbrains.kotlin.resolve.descriptorUtil.module
 import org.jetbrains.kotlin.resolve.scopes.DescriptorKindFilter
+import org.jetbrains.kotlin.resolve.scopes.MemberScope
 import org.jetbrains.kotlin.resolve.scopes.getDescriptorsFiltered
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.TypeConstructorSubstitution
 import org.jetbrains.kotlin.types.TypeSubstitutor
 import org.jetbrains.kotlin.types.typeUtil.asTypeProjection
+import org.jetbrains.kotlin.utils.SmartList
 import org.jetbrains.kotlin.utils.keysToMap

 class PlatformImplDeclarationChecker : DeclarationChecker {
@@ -95,17 +98,28 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
    private fun checkImplementationHasPlatformDeclaration(
            reportOn: KtDeclaration, descriptor: MemberDescriptor, diagnosticHolder: DiagnosticSink
    ) {
+        fun ClassifierDescriptor.findDeclarationForClass(): ClassDescriptor? =
+                findClassifiersFromTheSameModule().firstOrNull { declaration ->
+                    this != declaration &&
+                    declaration is ClassDescriptor && declaration.isPlatform &&
+                    areCompatibleClassifiers(declaration, this) == Compatible
+                } as? ClassDescriptor
+
        val hasDeclaration = when (descriptor) {
-            is CallableMemberDescriptor -> descriptor.findNamesakesFromTheSameModule().any { declaration ->
-                descriptor != declaration &&
-                declaration.isPlatform &&
-                areCompatibleCallables(declaration, descriptor) == Compatible
-            }
-            is ClassifierDescriptor -> descriptor.findClassifiersFromTheSameModule().any { declaration ->
-                descriptor != declaration &&
-                declaration is ClassDescriptor && declaration.isPlatform &&
-                areCompatibleClassifiers(declaration, descriptor) == Compatible
+            is CallableMemberDescriptor -> {
+                val container = descriptor.containingDeclaration
+                val candidates = when (container) {
+                    is ClassDescriptor -> container.findDeclarationForClass()?.getMembers(descriptor.name).orEmpty()
+                    is PackageFragmentDescriptor -> descriptor.findNamesakesFromTheSameModule()
+                    else -> return // do not report anything for incorrect code, e.g. 'impl' local function
+                }
+                candidates.any { declaration ->
+                    descriptor != declaration &&
+                    declaration.isPlatform &&
+                    areCompatibleCallables(declaration, descriptor) == Compatible
+                }
            }
+            is ClassifierDescriptor -> descriptor.findDeclarationForClass() != null
            else -> false
        }

@@ -138,7 +152,10 @@ class PlatformImplDeclarationChecker : DeclarationChecker {

    sealed class Compatibility {
        // Note that the reason is used in the diagnostic output, see PlatformIncompatibilityDiagnosticRenderer
-        sealed class Incompatible(val reason: String?) : Compatibility() {
+        sealed class Incompatible(
+                val reason: String?,
+                val unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>? = null
+        ) : Compatibility() {
            // Callables

            object ParameterShape : Incompatible("parameter shapes are different (extension vs non-extension)")
@@ -172,6 +189,10 @@ class PlatformImplDeclarationChecker : DeclarationChecker {

            object Supertypes : Incompatible("some supertypes are missing in the implementation")

+            class ClassScopes(
+                    unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>
+            ) : Incompatible("some members are not implemented", unimplemented)
+
            // Common

            object Modality : Incompatible("modality is different")
@@ -188,7 +209,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
    }

    // a is the declaration in common code, b is the definition in the platform-specific code
-    private fun areCompatibleCallables(a: CallableMemberDescriptor, b: CallableMemberDescriptor): Compatibility {
+    private fun areCompatibleCallables(a: CallableMemberDescriptor, b: CallableMemberDescriptor, parentSubstitutor: Substitutor? = null): Compatibility {
        assert(a.name == b.name) { "This function should be invoked only for declarations with the same name: $a, $b" }
        assert(a.containingDeclaration is ClassDescriptor == b.containingDeclaration is ClassDescriptor) {
            "This function should be invoked only for declarations in the same kind of container (both members or both top level): $a, $b"
@@ -206,7 +227,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
        val bTypeParams = b.typeParameters
        if (aTypeParams.size != bTypeParams.size) return Incompatible.TypeParameterCount

-        val substitutor = Substitutor(aTypeParams, bTypeParams)
+        val substitutor = Substitutor(aTypeParams, bTypeParams, parentSubstitutor)

        if (aParams.map { substitutor(it.type) } != bParams.map { it.type } ||
            aExtensionReceiver?.type?.let(substitutor) != bExtensionReceiver?.type) return Incompatible.ParameterTypes
@@ -260,9 +281,22 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
    private fun areCompatibleClassifiers(a: ClassDescriptor, other: ClassifierDescriptor): Compatibility {
        assert(a.fqNameUnsafe == other.fqNameUnsafe) { "This function should be invoked only for declarations with the same name: $a, $other" }

+        var parentSubstitutor: Substitutor? = null
+
        val b = when (other) {
            is ClassDescriptor -> other
-            is TypeAliasDescriptor -> other.classDescriptor ?: return Incompatible.Unknown
+            is TypeAliasDescriptor -> {
+                val classDescriptor = other.classDescriptor ?: return Compatible // do not report extra error on erroneous typealias
+                // If a platform class test.C is implemented by a typealias test.C = test.CImpl, we must now state that any occurrence
+                // of the type "test.C" in the platform class scope should be replaced with "test.CImpl".
+                // Otherwise the types would not be equal and e.g. test.C's constructor is not going to be found in test.CImpl's scope.
+                // For this, we construct an additional substitutor with a single mapping test.C -> test.CImpl
+                // TODO: this looks like a dirty hack
+                parentSubstitutor = Substitutor(null, TypeSubstitutor.create(TypeConstructorSubstitution.createByConstructorsMap(
+                        mapOf(a.typeConstructor to classDescriptor.defaultType.asTypeProjection())
+                )))
+                classDescriptor
+            }
            else -> throw AssertionError("Incorrect impl classifier for $a: $other")
        }

@@ -272,7 +306,7 @@ class PlatformImplDeclarationChecker : DeclarationChecker {
        val bTypeParams = b.declaredTypeParameters
        if (aTypeParams.size != bTypeParams.size) return Incompatible.TypeParameterCount

-        val substitutor = Substitutor(aTypeParams, bTypeParams)
+        val substitutor = Substitutor(aTypeParams, bTypeParams, parentSubstitutor)

        if (a.modality != b.modality) return Incompatible.Modality
        if (a.visibility != b.visibility) return Incompatible.Visibility
@@ -283,12 +317,46 @@ class PlatformImplDeclarationChecker : DeclarationChecker {

        if (!b.typeConstructor.supertypes.containsAll(a.typeConstructor.supertypes.map(substitutor))) return Incompatible.Supertypes

-        // TODO: check scopes
+        areCompatibleClassScopes(a, b, substitutor).let { if (it != Compatible) return it }
+
        // TODO: check 'impl' modifier

        return Compatible
    }

+    private fun areCompatibleClassScopes(a: ClassDescriptor, b: ClassDescriptor, substitutor: Substitutor): Compatibility {
+        val unimplemented = arrayListOf<Pair<CallableMemberDescriptor, Map<Incompatible, MutableCollection<CallableMemberDescriptor>>>>()
+
+        val bMembersByName = b.getMembers().groupBy { it.name }
+
+        outer@ for (aMember in a.getMembers()) {
+            val mapping = bMembersByName[aMember.name].orEmpty().keysToMap { bMember -> areCompatibleCallables(aMember, bMember, substitutor) }
+            if (mapping.values.any { it == Compatible }) continue
+
+            val incompatibilityMap = mutableMapOf<Incompatible, MutableCollection<CallableMemberDescriptor>>()
+            for ((descriptor, compatibility) in mapping) {
+                when (compatibility) {
+                    Compatible -> continue@outer
+                    is Incompatible -> incompatibilityMap.getOrPut(compatibility) { SmartList() }.add(descriptor)
+                }
+            }
+
+            unimplemented.add(aMember to incompatibilityMap)
+        }
+
+        // TODO: check static scope, enum entries
+
+        if (unimplemented.isEmpty()) return Compatible
+
+        return Incompatible.ClassScopes(unimplemented)
+    }
+
+    private fun ClassDescriptor.getMembers(name: Name? = null): Collection<CallableMemberDescriptor> {
+        val nameFilter = if (name != null) { it -> it == name } else MemberScope.ALL_NAME_FILTER
+        return defaultType.memberScope.getDescriptorsFiltered(nameFilter = nameFilter).filterIsInstance<CallableMemberDescriptor>() +
+               constructors.filter { nameFilter(it.name) }
+    }
+
    private inline fun <T, K> equalBy(first: T, second: T, selector: (T) -> K): Boolean =
            selector(first) == selector(second)

@@ -302,14 +370,20 @@ class PlatformImplDeclarationChecker : DeclarationChecker {

    // This substitutor takes the type from A's signature and returns the type that should be in that place in B's signature
    private class Substitutor(
-            aTypeParams: List<TypeParameterDescriptor>,
-            bTypeParams: List<TypeParameterDescriptor>
+            private val parent: Substitutor?,
+            private val typeSubstitutor: TypeSubstitutor
    ) : (KotlinType?) -> KotlinType? {
-        val typeSubstitutor = TypeSubstitutor.create(TypeConstructorSubstitution.createByParametersMap(
-                aTypeParams.keysToMap { bTypeParams[it.index].defaultType.asTypeProjection() }
+        constructor(
+                aTypeParams: List<TypeParameterDescriptor>,
+                bTypeParams: List<TypeParameterDescriptor>,
+                parent: Substitutor? = null
+        ) : this(parent, TypeSubstitutor.create(
+                TypeConstructorSubstitution.createByParametersMap(aTypeParams.keysToMap {
+                    bTypeParams[it.index].defaultType.asTypeProjection()
+                })
        ))

        override fun invoke(type: KotlinType?): KotlinType? =
-                type?.asTypeProjection()?.let(typeSubstitutor::substitute)?.type
+                (parent?.invoke(type) ?: type)?.asTypeProjection()?.let(typeSubstitutor::substitute)?.type
    }
 }
@@ -0,0 +1,3 @@
+platform class Foo {
+    constructor(s: String)
+}
@@ -0,0 +1,3 @@
+impl class Foo {
+    impl constructor(s: Array<String>)
+}
@@ -0,0 +1,25 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: COMPILATION_ERROR
+Output:
+compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/common.kt:1:16: error: no definition is found for platform declaration 'Foo'
+The following declaration is incompatible because some members are not implemented.
+No implementations are found for members listed below:
+
+    public constructor Foo(s: String)
+
+    The following declaration is incompatible because parameter types are different:
+        public constructor Foo(s: Array<String>)
+
+platform class Foo {
+               ^
+compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/jvm.kt:1:1: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+impl class Foo {
+^
+compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/jvm.kt:2:5: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+    impl constructor(s: Array<String>)
+    ^
+
@@ -0,0 +1,3 @@
+platform class Foo {
+    fun function(b: ByteArray): Int
+}
@@ -0,0 +1,3 @@
+impl class Foo {
+    impl fun function(b: ByteArray): Long = b.size.toLong()
+}
@@ -0,0 +1,25 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: COMPILATION_ERROR
+Output:
+compiler/testData/multiplatform/classScopes/functionIncorrectSignature/common.kt:1:16: error: no definition is found for platform declaration 'Foo'
+The following declaration is incompatible because some members are not implemented.
+No implementations are found for members listed below:
+
+    public final platform fun function(b: ByteArray): Int
+
+    The following declaration is incompatible because return type is different:
+        public final impl fun function(b: ByteArray): Long
+
+platform class Foo {
+               ^
+compiler/testData/multiplatform/classScopes/functionIncorrectSignature/jvm.kt:1:1: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+impl class Foo {
+^
+compiler/testData/multiplatform/classScopes/functionIncorrectSignature/jvm.kt:2:5: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+    impl fun function(b: ByteArray): Long = b.size.toLong()
+    ^
+
@@ -0,0 +1,3 @@
+platform class Foo {
+    constructor(s: String)
+}
@@ -0,0 +1 @@
+impl class Foo
@@ -0,0 +1,22 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: COMPILATION_ERROR
+Output:
+compiler/testData/multiplatform/classScopes/missingConstructor/common.kt:1:16: error: no definition is found for platform declaration 'Foo'
+The following declaration is incompatible because some members are not implemented.
+No implementations are found for members listed below:
+
+    public constructor Foo(s: String)
+
+    The following declaration is incompatible because number of value parameters is different:
+        public constructor Foo()
+
+platform class Foo {
+               ^
+compiler/testData/multiplatform/classScopes/missingConstructor/jvm.kt:1:1: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+impl class Foo
+^
+
@@ -0,0 +1,3 @@
+platform class Foo {
+    fun function(s: String)
+}
@@ -0,0 +1 @@
+impl class Foo
@@ -0,0 +1,19 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: COMPILATION_ERROR
+Output:
+compiler/testData/multiplatform/classScopes/missingFunction/common.kt:1:16: error: no definition is found for platform declaration 'Foo'
+The following declaration is incompatible because some members are not implemented.
+No implementations are found for members listed below:
+
+    public final platform fun function(s: String): Unit
+
+platform class Foo {
+               ^
+compiler/testData/multiplatform/classScopes/missingFunction/jvm.kt:1:1: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+impl class Foo
+^
+
@@ -0,0 +1,5 @@
+platform class Foo(param: String) {
+    var property: Int
+
+    fun <T> function(p: List<T>): T
+}
@@ -0,0 +1,5 @@
+impl class Foo impl constructor(param: String) {
+    impl var property: Int = param.length
+
+    impl fun <T> function(p: List<T>): T = p.first()
+}
@@ -0,0 +1,8 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: OK
+Output:
+
@@ -0,0 +1 @@
+platform class Foo
@@ -0,0 +1,10 @@
+impl class Foo impl constructor() {
+    impl constructor(s: String) : this()
+
+    impl fun nonPlatformFun() {}
+
+    impl val nonPlatformVal = ""
+
+    private fun nonImplFun() {}
+    private val nonImplVal = ""
+}
@@ -0,0 +1,17 @@
+-- Common --
+Exit code: OK
+Output:
+
+-- JVM --
+Exit code: COMPILATION_ERROR
+Output:
+compiler/testData/multiplatform/incorrectImplInClass/jvm.kt:2:5: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+    impl constructor(s: String) : this()
+    ^
+compiler/testData/multiplatform/incorrectImplInClass/jvm.kt:4:5: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+    impl fun nonPlatformFun() {}
+    ^
+compiler/testData/multiplatform/incorrectImplInClass/jvm.kt:6:5: error: modifier 'impl' is only applicable to members that are initially declared in platform-independent code
+    impl val nonPlatformVal = ""
+    ^
+
@@ -72,6 +72,12 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform
        doTest(fileName);
    }

+    @TestMetadata("incorrectImplInClass")
+    public void testIncorrectImplInClass() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/incorrectImplInClass/");
+        doTest(fileName);
+    }
+
    @TestMetadata("simple")
    public void testSimple() throws Exception {
        String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/simple/");