From c74d04a7ca70de6a1e508da2cd34f434e0f43d38 Mon Sep 17 00:00:00 2001 From: Alexey Trilis Date: Thu, 23 Jul 2020 20:34:12 +0300 Subject: [PATCH 1/5] Resolve conflicts of var-properties with different types --- .../data/typescript/override/varConflict.d.kt | 30 +++ .../data/typescript/override/varConflict.d.ts | 11 ++ .../varConflictWithoutPropertyInChild.d.kt | 30 +++ .../varConflictWithoutPropertyInChild.d.ts | 11 ++ .../tests/extended/DescriptorTests.kt | 4 +- .../ModelContextAwareLowering.kt | 2 +- .../commonLowerings/OverrideTypeChecker.kt | 11 +- .../model/commonLowerings/lowerOverrides.kt | 49 ++--- .../overrides/VarConflictResolveLowering.kt | 180 ++++++++++++++++++ .../dukat/ts/translator/TypescriptLowerer.kt | 2 + 10 files changed, 293 insertions(+), 37 deletions(-) create mode 100644 compiler/test/data/typescript/override/varConflict.d.kt create mode 100644 compiler/test/data/typescript/override/varConflict.d.ts create mode 100644 compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.kt create mode 100644 compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.ts create mode 100644 model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt diff --git a/compiler/test/data/typescript/override/varConflict.d.kt b/compiler/test/data/typescript/override/varConflict.d.kt new file mode 100644 index 000000000..029bdd919 --- /dev/null +++ b/compiler/test/data/typescript/override/varConflict.d.kt @@ -0,0 +1,30 @@ +// [test] varConflict.kt +@file:Suppress("INTERFACE_WITH_SUPERCLASS", "OVERRIDING_FINAL_MEMBER", "RETURN_TYPE_MISMATCH_ON_OVERRIDE", "CONFLICTING_OVERLOADS") + +import kotlin.js.* +import kotlin.js.Json +import org.khronos.webgl.* +import org.w3c.dom.* +import org.w3c.dom.events.* +import org.w3c.dom.parsing.* +import org.w3c.dom.svg.* +import org.w3c.dom.url.* +import org.w3c.fetch.* +import org.w3c.files.* +import org.w3c.notifications.* +import org.w3c.performance.* +import org.w3c.workers.* +import org.w3c.xhr.* + +external open class B { + open val x: Boolean +} + +external interface C { + val x: Boolean? + get() = definedExternally +} + +external open class A : B, C { + override val x: Boolean +} \ No newline at end of file diff --git a/compiler/test/data/typescript/override/varConflict.d.ts b/compiler/test/data/typescript/override/varConflict.d.ts new file mode 100644 index 000000000..ec3b2429f --- /dev/null +++ b/compiler/test/data/typescript/override/varConflict.d.ts @@ -0,0 +1,11 @@ +declare class B { + x: boolean +} + +declare interface C { + x?: boolean +} + +declare class A extends B implements C { + x: boolean +} \ No newline at end of file diff --git a/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.kt b/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.kt new file mode 100644 index 000000000..04bbb124f --- /dev/null +++ b/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.kt @@ -0,0 +1,30 @@ +// [test] varConflictWithoutPropertyInChild.kt +@file:Suppress("INTERFACE_WITH_SUPERCLASS", "OVERRIDING_FINAL_MEMBER", "RETURN_TYPE_MISMATCH_ON_OVERRIDE", "CONFLICTING_OVERLOADS") + +import kotlin.js.* +import kotlin.js.Json +import org.khronos.webgl.* +import org.w3c.dom.* +import org.w3c.dom.events.* +import org.w3c.dom.parsing.* +import org.w3c.dom.svg.* +import org.w3c.dom.url.* +import org.w3c.fetch.* +import org.w3c.files.* +import org.w3c.notifications.* +import org.w3c.performance.* +import org.w3c.workers.* +import org.w3c.xhr.* + +external open class B { + open val x: Boolean +} + +external interface C { + val x: Boolean? + get() = definedExternally +} + +external open class A : B, C { + override val x: Boolean +} \ No newline at end of file diff --git a/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.ts b/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.ts new file mode 100644 index 000000000..06e0aad88 --- /dev/null +++ b/compiler/test/data/typescript/override/varConflictWithoutPropertyInChild.d.ts @@ -0,0 +1,11 @@ +declare class B { + x: boolean +} + +declare interface C { + x?: boolean +} + +declare class A extends B implements C { + +} \ No newline at end of file diff --git a/compiler/test/src/org/jetbrains/dukat/compiler/tests/extended/DescriptorTests.kt b/compiler/test/src/org/jetbrains/dukat/compiler/tests/extended/DescriptorTests.kt index 28a20d5d3..756133381 100644 --- a/compiler/test/src/org/jetbrains/dukat/compiler/tests/extended/DescriptorTests.kt +++ b/compiler/test/src/org/jetbrains/dukat/compiler/tests/extended/DescriptorTests.kt @@ -86,7 +86,9 @@ class DescriptorTests { "misc/stringTypeInAlias", "qualifiedNames/extendingEntityFromParentModule", "stdlib/convertTsStdlib", - "typePredicate/simple" + "typePredicate/simple", + "override/varConflict", + "override/varConflictWithoutPropertyInChild" ).map { it.replace("/", System.getProperty("file.separator")) } @JvmStatic diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/ModelContextAwareLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/ModelContextAwareLowering.kt index ff7bbd584..097239091 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/ModelContextAwareLowering.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/ModelContextAwareLowering.kt @@ -5,7 +5,7 @@ import org.jetbrains.dukat.astModel.SourceSetModel import org.jetbrains.dukat.graphs.Graph import org.jetbrains.dukat.model.commonLowerings.overrides.InheritanceContext -private fun ModelContext.buildInheritanceGraph(): Graph { +internal fun ModelContext.buildInheritanceGraph(): Graph { val graph = Graph() getClassLikeIterable().forEach { classLike -> diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/OverrideTypeChecker.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/OverrideTypeChecker.kt index fca6c830e..6380999ab 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/OverrideTypeChecker.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/OverrideTypeChecker.kt @@ -27,7 +27,7 @@ internal class OverrideTypeChecker( private val context: ModelContext, private val inheritanceContext: InheritanceContext, private val declaration: ClassLikeModel, - private val parent: ClassLikeModel + private val parent: ClassLikeModel? ) { private fun TypeModel.isDynamic(): Boolean { @@ -154,7 +154,7 @@ internal class OverrideTypeChecker( } } - private fun TypeModel.isOverridingReturnType(otherParameterType: TypeModel, box: TypeValueModel? = null): Boolean { + fun TypeModel.isOverridingReturnType(otherParameterType: TypeModel, box: TypeValueModel? = null): Boolean { val inbox = (box == null || box.value == IdentifierEntity("Array")) if (isEquivalent(otherParameterType) && inbox) { @@ -192,7 +192,7 @@ internal class OverrideTypeChecker( } } } else { - return fqName == otherParameterType.fqName + return fqName == otherParameterType.fqName && !(otherParameterType.nullable && !nullable) } } @@ -287,6 +287,9 @@ internal class OverrideTypeChecker( } private fun TypeParameterReferenceModel.isEquivalent(otherTypeParameter: TypeParameterReferenceModel): Boolean { + if (parent == null) { + return false + } val otherTypeParameterIndex = parent.typeParameters.indexOfFirst { val type = it.type type is TypeValueModel && type.value == otherTypeParameter.name @@ -297,7 +300,7 @@ internal class OverrideTypeChecker( return this == relevantHeritage?.typeParams?.getOrNull(otherTypeParameterIndex) } - private fun TypeModel.isEquivalent(otherParameterType: TypeModel): Boolean { + fun TypeModel.isEquivalent(otherParameterType: TypeModel): Boolean { if (this == otherParameterType) { return true } diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/lowerOverrides.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/lowerOverrides.kt index 6a820f547..ec02a756e 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/lowerOverrides.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/lowerOverrides.kt @@ -28,7 +28,7 @@ import org.jetbrains.dukat.stdlib.KLIBROOT import org.jetbrains.dukat.stdlib.TSLIBROOT import org.jetbrains.dukat.stdlibGenerator.generated.stdlibClassMethodsMap -private data class MemberData(val fqName: NameEntity?, val memberModel: MemberModel, val ownerModel: ClassLikeModel) +internal data class MemberData(val fqName: NameEntity?, val memberModel: MemberModel, val ownerModel: ClassLikeModel) internal enum class MemberOverrideStatus { IS_OVERRIDE, @@ -37,19 +37,28 @@ internal enum class MemberOverrideStatus { IS_IMPOSSIBLE } -private fun ModelContext.buildInheritanceGraph(): Graph { - val graph = Graph() +internal fun ClassLikeModel.getKnownParents(context: ModelContext): List> { + return context.getAllParents(this) +} + +internal fun ClassLikeModel.allParentMembers(context: ModelContext): Map> { + val memberMap = mutableMapOf>() - getClassLikeIterable().forEach { classLike -> - getAllParents(classLike).forEach { resolvedClassLike -> - graph.addEdge(classLike, resolvedClassLike.classLike) + getKnownParents(context).forEach { resolvedClassLike -> + resolvedClassLike.classLike.members.forEach { + when (it) { + is NamedModel -> { + if (!resolvedClassLike.existsOnlyInTsStdlib(it)) { + memberMap.getOrPut(it.name) { mutableListOf() }.add(MemberData(resolvedClassLike.fqName, it, resolvedClassLike.classLike)) + } + } + } } } - return graph + return memberMap } - private fun ResolvedClassLike.existsOnlyInTsStdlib(member: NamedModel): Boolean { return (fqName?.startsWith(TSLIBROOT) == true && (stdlibClassMethodsMap.containsKey(classLike.name)) && (stdlibClassMethodsMap[classLike.name]?.contains(member.name) == false)) } @@ -114,10 +123,6 @@ private class ClassLikeOverrideResolver( ) } - private fun ClassLikeModel.getKnownParents(): List> { - return context.getAllParents(this) - } - private fun MethodModel.removeDefaultParamValues(override: NameEntity?): MethodModel { return copy(override = override, parameters = parameters.map { it.copy(initializer = null) }) } @@ -236,24 +241,6 @@ private class ClassLikeOverrideResolver( } } - private fun ClassLikeModel.allParentMembers(): Map> { - val memberMap = mutableMapOf>() - - getKnownParents().forEach { resolvedClassLike -> - resolvedClassLike.classLike.members.forEach { - when (it) { - is NamedModel -> { - if (!resolvedClassLike.existsOnlyInTsStdlib(it)) { - memberMap.getOrPut(it.name) { mutableListOf() }.add(MemberData(resolvedClassLike.fqName, it, resolvedClassLike.classLike)) - } - } - } - } - } - - return memberMap - } - private fun MethodModel.isSpecialCase(): Boolean { val returnType = type @@ -277,7 +264,7 @@ private class ClassLikeOverrideResolver( } fun resolve(): ClassLikeModel { - val parentMembers = classLike.allParentMembers() + val parentMembers = classLike.allParentMembers(context) val membersLowered = classLike.members.flatMap { member -> member.lowerOverrides(parentMembers) diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt new file mode 100644 index 000000000..476cbf320 --- /dev/null +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt @@ -0,0 +1,180 @@ +package org.jetbrains.dukat.model.commonLowerings.overrides + +import org.jetbrains.dukat.astCommon.IdentifierEntity +import org.jetbrains.dukat.astCommon.NameEntity +import org.jetbrains.dukat.astCommon.appendLeft +import org.jetbrains.dukat.astCommon.shiftRight +import org.jetbrains.dukat.astModel.ClassLikeModel +import org.jetbrains.dukat.astModel.ClassModel +import org.jetbrains.dukat.astModel.InterfaceModel +import org.jetbrains.dukat.astModel.MemberModel +import org.jetbrains.dukat.astModel.ModuleModel +import org.jetbrains.dukat.astModel.PropertyModel +import org.jetbrains.dukat.astModel.SourceSetModel +import org.jetbrains.dukat.astModel.TypeModel +import org.jetbrains.dukat.model.commonLowerings.MemberData +import org.jetbrains.dukat.model.commonLowerings.ModelContext +import org.jetbrains.dukat.model.commonLowerings.ModelLowering +import org.jetbrains.dukat.model.commonLowerings.ModelWithOwnerTypeLowering +import org.jetbrains.dukat.model.commonLowerings.OverrideTypeChecker +import org.jetbrains.dukat.model.commonLowerings.allParentMembers +import org.jetbrains.dukat.model.commonLowerings.buildInheritanceGraph +import org.jetbrains.dukat.ownerContext.NodeOwner + +private fun PropertyModel.toVal(): PropertyModel { + return copy( + immutable = true, + setter = false + ) +} + +private class VarOverrideResolver( + private val modelContext: ModelContext, + private val inheritanceContext: InheritanceContext +) : ModelWithOwnerTypeLowering { + + val propertiesToChangeToVal: MutableList = mutableListOf() + + private fun findConflictingProperties( + parentMembers: Map>, + typeChecker: OverrideTypeChecker + ): List> { + return parentMembers.keys.mapNotNull { name -> + val properties = parentMembers[name].orEmpty().filter { it.memberModel is PropertyModel } + if (properties.isEmpty() || properties.all { + with(typeChecker) { + (it.memberModel as PropertyModel).type.isEquivalent((properties[0].memberModel as PropertyModel).type) + } + }) { + null + } else { + properties + } + } + } + + private fun findSuperType(types: List, typeChecker: OverrideTypeChecker): TypeModel? { + var superType: TypeModel? = types[0] + types.forEach { type -> + if (with(typeChecker) { superType?.isOverridingReturnType(type) } == true) { + superType = type + } else if (superType != null && with(typeChecker) { !type.isOverridingReturnType(superType!!) }) { + superType = null + } + } + return superType + } + + private fun createPropertyToAdd(properties: List, classLike: ClassLikeModel): PropertyModel? { + val name = (properties[0].memberModel as PropertyModel).name + return if (classLike.members.filterIsInstance().any { it.name == name }) { + null + } else { + val superType = findSuperType( + properties.map { (it.memberModel as PropertyModel).type }, + OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) + ) + superType?.let { type -> + (properties[0].memberModel as PropertyModel).copy( + type = type, + override = properties[0].fqName + ).toVal() + } + } + } + + private fun generateNewMembers(allConflictingProperties: List>, classLike: ClassLikeModel): List { + + val propertiesToAdd = allConflictingProperties.mapNotNull { createPropertyToAdd(it, classLike) } + + return classLike.members.map { member -> + if (member is PropertyModel && allConflictingProperties.any { properties -> + (properties[0].memberModel as PropertyModel).name == member.name + }) { + member.toVal() + } else { + member + } + } + propertiesToAdd + } + + override fun lowerClassLikeModel( + ownerContext: NodeOwner, + parentModule: ModuleModel + ): ClassLikeModel? { + val classLike = ownerContext.node + + val parentMembers = classLike.allParentMembers(modelContext) + + val allConflictingProperties = findConflictingProperties( + parentMembers, + OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) + ) + + allConflictingProperties.forEach { propertiesToChangeToVal += it } + + val newMembers = generateNewMembers(allConflictingProperties, classLike) + + return when (classLike) { + is InterfaceModel -> classLike.copy(members = newMembers) + is ClassModel -> classLike.copy(members = newMembers) + else -> classLike + } + } +} + +private class VarToValResolver(private val propertiesToChangeToVal: List) : ModelWithOwnerTypeLowering { + + private var currentFqName: NameEntity = IdentifierEntity("") + + override fun lowerPropertyModel(ownerContext: NodeOwner): PropertyModel { + val property = ownerContext.node + + return if (propertiesToChangeToVal.any { + (it.memberModel is PropertyModel) && (it.memberModel.name == property.name) && (it.fqName == currentFqName) + }) { + property.toVal() + } else { + property + } + } + + override fun lowerClassLikeModel( + ownerContext: NodeOwner, + parentModule: ModuleModel + ): ClassLikeModel? { + currentFqName = currentFqName.appendLeft(ownerContext.node.name) + val newClassLike = super.lowerClassLikeModel(ownerContext, parentModule) + currentFqName = currentFqName.shiftRight()!! + return newClassLike + } + + override fun lowerRoot(moduleModel: ModuleModel, ownerContext: NodeOwner): ModuleModel { + currentFqName = moduleModel.name + return super.lowerRoot(moduleModel, ownerContext) + } +} + +class VarConflictResolveLowering : ModelLowering { + + override fun lower(source: SourceSetModel): SourceSetModel { + val modelContext = ModelContext(source) + val inheritanceContext = InheritanceContext(modelContext.buildInheritanceGraph()) + val varOverrideResolver = VarOverrideResolver(modelContext, inheritanceContext) + val newSourceSet = source.copy( + sources = source.sources.map { + it.copy(root = varOverrideResolver.lowerRoot(it.root, NodeOwner(it.root, null))) + } + ) + val varToValResolver = VarToValResolver(varOverrideResolver.propertiesToChangeToVal) + return newSourceSet.copy( + sources = newSourceSet.sources.map { + it.copy(root = varToValResolver.lowerRoot(it.root, NodeOwner(it.root, null))) + } + ) + } + + override fun lower(module: ModuleModel): ModuleModel { + return module + } +} \ No newline at end of file diff --git a/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt b/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt index b90610715..6e2fc9cd0 100644 --- a/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt +++ b/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt @@ -23,6 +23,7 @@ import org.jetbrains.dukat.model.commonLowerings.RemoveConflictingOverloads import org.jetbrains.dukat.model.commonLowerings.RemoveKotlinBuiltIns import org.jetbrains.dukat.model.commonLowerings.RemoveRedundantTypeParams import org.jetbrains.dukat.model.commonLowerings.lower +import org.jetbrains.dukat.model.commonLowerings.overrides.VarConflictResolveLowering import org.jetbrains.dukat.moduleNameResolver.ModuleNameResolver import org.jetbrains.dukat.nodeIntroduction.IntroduceNodes import org.jetbrains.dukat.nodeIntroduction.ResolveModuleAnnotations @@ -119,6 +120,7 @@ open class TypescriptLowerer( .lower { context, inheritanceContext -> LowerOverrides(context, inheritanceContext) }, + VarConflictResolveLowering(), AddExplicitGettersAndSetters(), AnyfyUnresolvedTypes(), RemoveKotlinBuiltIns(), From 19566ad9fb2f84ba9cdce7c34b933422f21e72b9 Mon Sep 17 00:00:00 2001 From: Alexey Trilis Date: Mon, 27 Jul 2020 14:20:28 +0300 Subject: [PATCH 2/5] Transform property to val if type is specified when overriding --- .../class/inheritance/overrides.d.kt | 8 +-- .../overridesFromReferencedFile.d.kt | 4 +- .../class/inheritance/overridingStdLib.d.kt | 4 +- .../typescript/class/override/simple.d.kt | 4 +- .../typescript/interface/override/simple.d.kt | 4 +- .../overrides/VarConflictResolveLowering.kt | 60 ++++++++++++++++--- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/compiler/test/data/typescript/class/inheritance/overrides.d.kt b/compiler/test/data/typescript/class/inheritance/overrides.d.kt index 733539b85..64524d9f8 100644 --- a/compiler/test/data/typescript/class/inheritance/overrides.d.kt +++ b/compiler/test/data/typescript/class/inheritance/overrides.d.kt @@ -29,7 +29,7 @@ external interface BaseEvent { fun getDelegateTarget(): Shape fun getElement(): Element fun transform(shape: T = definedExternally): T - var prop: Any + val prop: Any fun queryByReturnType(query: String, parameters: Array = definedExternally): InvariantBox var thisIsNullable: String? } @@ -39,7 +39,7 @@ external open class BoxStringEvent : BaseEvent { override fun getDelegateTarget(): Box override fun getElement(): HTMLElement override fun transform(shape: T): T - override var prop: String + override val prop: String open fun queryByReturnType(query: String, parameters: Array = definedExternally): InvariantBox } @@ -49,12 +49,12 @@ external interface NumberEvent : BaseEvent { } external open class ParentClass { - open var prop: Any + open val prop: Any open fun ping(message: String) } external open class ChildClass : ParentClass { - override var prop: String + override val prop: String override fun ping(message: String /* "message" */) } diff --git a/compiler/test/data/typescript/class/inheritance/overridesFromReferencedFile.d.kt b/compiler/test/data/typescript/class/inheritance/overridesFromReferencedFile.d.kt index 16857a724..76b948f9e 100644 --- a/compiler/test/data/typescript/class/inheritance/overridesFromReferencedFile.d.kt +++ b/compiler/test/data/typescript/class/inheritance/overridesFromReferencedFile.d.kt @@ -22,7 +22,7 @@ external open class BoxStringEvent : BaseEvent { override fun getElement(): HTMLElement override fun transform(shape: T): T override fun getSortOfEventTarget(): SortOfElement - override var prop: String + override val prop: String } external interface NumberEvent : BaseEvent { @@ -65,5 +65,5 @@ external interface BaseEvent { fun getElement(): Element fun transform(shape: T = definedExternally): T fun getSortOfEventTarget(): SortOfEventTarget - var prop: Any + val prop: Any } \ No newline at end of file diff --git a/compiler/test/data/typescript/class/inheritance/overridingStdLib.d.kt b/compiler/test/data/typescript/class/inheritance/overridingStdLib.d.kt index 4ceae6136..739adbef9 100644 --- a/compiler/test/data/typescript/class/inheritance/overridingStdLib.d.kt +++ b/compiler/test/data/typescript/class/inheritance/overridingStdLib.d.kt @@ -17,8 +17,8 @@ import org.w3c.workers.* import org.w3c.xhr.* external interface AppEvent : Event { - override var currentTarget: Element? - override var target: Element? + override val currentTarget: Element? + override val target: Element? fun preventDefault(): Any } diff --git a/compiler/test/data/typescript/class/override/simple.d.kt b/compiler/test/data/typescript/class/override/simple.d.kt index d0d2b8feb..2f8ca9053 100644 --- a/compiler/test/data/typescript/class/override/simple.d.kt +++ b/compiler/test/data/typescript/class/override/simple.d.kt @@ -19,12 +19,12 @@ import org.w3c.xhr.* external open class Foo { open fun bar() open fun bar(a: Number) - open var baz: Any + open val baz: Any } external open class Boo : Foo { override fun bar() override fun bar(a: Number) open fun bar(a: String) - override var baz: Number + override val baz: Number } \ No newline at end of file diff --git a/compiler/test/data/typescript/interface/override/simple.d.kt b/compiler/test/data/typescript/interface/override/simple.d.kt index 2c1b95f6d..9a33b09f2 100644 --- a/compiler/test/data/typescript/interface/override/simple.d.kt +++ b/compiler/test/data/typescript/interface/override/simple.d.kt @@ -19,12 +19,12 @@ import org.w3c.xhr.* external interface Foo { fun bar() fun bar(a: Number) - var baz: Any + val baz: Any } external interface Boo : Foo { override fun bar() override fun bar(b: Number) fun bar(c: String) - override var baz: Number + override val baz: Number } \ No newline at end of file diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt index 476cbf320..73943c7f8 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt @@ -20,6 +20,7 @@ import org.jetbrains.dukat.model.commonLowerings.OverrideTypeChecker import org.jetbrains.dukat.model.commonLowerings.allParentMembers import org.jetbrains.dukat.model.commonLowerings.buildInheritanceGraph import org.jetbrains.dukat.ownerContext.NodeOwner +import org.jetbrains.dukat.ownerContext.wrap private fun PropertyModel.toVal(): PropertyModel { return copy( @@ -53,6 +54,23 @@ private class VarOverrideResolver( } } + private fun findVarPropertiesWithSpecifiedType( + member: PropertyModel, + parentMembers: Map>, + typeChecker: OverrideTypeChecker + ): MemberData? { + val parentProperty = parentMembers[member.name]?.firstOrNull { it.memberModel is PropertyModel } + if (parentProperty != null) { + if (with (typeChecker) { + val parentType = (parentProperty.memberModel as PropertyModel).type + member.type.isOverridingReturnType(parentType) && !member.type.isEquivalent(parentType) + }) { + return parentProperty + } + } + return null + } + private fun findSuperType(types: List, typeChecker: OverrideTypeChecker): TypeModel? { var superType: TypeModel? = types[0] types.forEach { type -> @@ -83,15 +101,29 @@ private class VarOverrideResolver( } } - private fun generateNewMembers(allConflictingProperties: List>, classLike: ClassLikeModel): List { + private fun generateNewMembers( + allConflictingProperties: List>, + varPropertiesWithSpecifiedType: List, + classLike: ClassLikeModel + ): List { val propertiesToAdd = allConflictingProperties.mapNotNull { createPropertyToAdd(it, classLike) } - return classLike.members.map { member -> - if (member is PropertyModel && allConflictingProperties.any { properties -> - (properties[0].memberModel as PropertyModel).name == member.name - }) { - member.toVal() + return classLike.members.mapNotNull { member -> + if (member is PropertyModel) { + when { + allConflictingProperties.any { properties -> + (properties[0].memberModel as PropertyModel).name == member.name + } -> { + member.toVal() + } + varPropertiesWithSpecifiedType.any { (it.memberModel as PropertyModel).name == member.name } -> { + member.toVal() + } + else -> { + member + } + } } else { member } @@ -102,7 +134,7 @@ private class VarOverrideResolver( ownerContext: NodeOwner, parentModule: ModuleModel ): ClassLikeModel? { - val classLike = ownerContext.node + val classLike = super.lowerClassLikeModel(ownerContext, parentModule) ?: return null val parentMembers = classLike.allParentMembers(modelContext) @@ -111,9 +143,18 @@ private class VarOverrideResolver( OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) ) + val varPropertiesWithSpecifiedType = classLike.members.filterIsInstance().mapNotNull { + findVarPropertiesWithSpecifiedType( + it, + parentMembers, + OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) + ) + } + allConflictingProperties.forEach { propertiesToChangeToVal += it } + varPropertiesWithSpecifiedType.forEach { propertiesToChangeToVal += it } - val newMembers = generateNewMembers(allConflictingProperties, classLike) + val newMembers = generateNewMembers(allConflictingProperties, varPropertiesWithSpecifiedType, classLike) return when (classLike) { is InterfaceModel -> classLike.copy(members = newMembers) @@ -166,7 +207,8 @@ class VarConflictResolveLowering : ModelLowering { it.copy(root = varOverrideResolver.lowerRoot(it.root, NodeOwner(it.root, null))) } ) - val varToValResolver = VarToValResolver(varOverrideResolver.propertiesToChangeToVal) + val varToValResolver = + VarToValResolver(varOverrideResolver.propertiesToChangeToVal) return newSourceSet.copy( sources = newSourceSet.sources.map { it.copy(root = varToValResolver.lowerRoot(it.root, NodeOwner(it.root, null))) From f52a44bcad8ba976c334a6ec640c744443b0b203 Mon Sep 17 00:00:00 2001 From: Alexey Trilis Date: Mon, 27 Jul 2020 15:22:54 +0300 Subject: [PATCH 3/5] Change list to set in val-to-var lowering to improve performance --- .../overrides/VarConflictResolveLowering.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt index 73943c7f8..eb8137fbb 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt @@ -34,7 +34,7 @@ private class VarOverrideResolver( private val inheritanceContext: InheritanceContext ) : ModelWithOwnerTypeLowering { - val propertiesToChangeToVal: MutableList = mutableListOf() + val propertiesToChangeToVal: MutableSet = mutableSetOf() private fun findConflictingProperties( parentMembers: Map>, @@ -151,8 +151,11 @@ private class VarOverrideResolver( ) } - allConflictingProperties.forEach { propertiesToChangeToVal += it } - varPropertiesWithSpecifiedType.forEach { propertiesToChangeToVal += it } + (allConflictingProperties + listOf(varPropertiesWithSpecifiedType)).forEach { propertiesToChangeToVal += it.mapNotNull { memberData -> + (memberData.memberModel as? PropertyModel)?.let { property -> + memberData.fqName?.appendLeft(property.name) + } + } } val newMembers = generateNewMembers(allConflictingProperties, varPropertiesWithSpecifiedType, classLike) @@ -164,16 +167,14 @@ private class VarOverrideResolver( } } -private class VarToValResolver(private val propertiesToChangeToVal: List) : ModelWithOwnerTypeLowering { +private class VarToValResolver(private val propertiesToChangeToVal: Set) : ModelWithOwnerTypeLowering { private var currentFqName: NameEntity = IdentifierEntity("") override fun lowerPropertyModel(ownerContext: NodeOwner): PropertyModel { val property = ownerContext.node - return if (propertiesToChangeToVal.any { - (it.memberModel is PropertyModel) && (it.memberModel.name == property.name) && (it.fqName == currentFqName) - }) { + return if (propertiesToChangeToVal.contains(currentFqName.appendLeft(property.name))) { property.toVal() } else { property From 649ec56d3eaccd70684bbb3f606960530732651d Mon Sep 17 00:00:00 2001 From: Alexey Trilis Date: Tue, 28 Jul 2020 17:35:28 +0300 Subject: [PATCH 4/5] Check if classes with conflicting properties are related (Then they are actually not conflicting in this case) --- model-lowerings-common/build.gradle | 1 + .../overrides/VarConflictResolveLowering.kt | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/model-lowerings-common/build.gradle b/model-lowerings-common/build.gradle index c90495328..39c82ccd5 100644 --- a/model-lowerings-common/build.gradle +++ b/model-lowerings-common/build.gradle @@ -6,6 +6,7 @@ dependencies { implementation(project(":ast-common")) implementation(project(":ast-model")) implementation(project(":graphs")) + implementation(project(":itertools")) implementation(project(":logging")) implementation(project(":ownerContext")) implementation(project(":panic")) diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt index eb8137fbb..998aa723b 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt @@ -1,5 +1,6 @@ package org.jetbrains.dukat.model.commonLowerings.overrides +import cartesian import org.jetbrains.dukat.astCommon.IdentifierEntity import org.jetbrains.dukat.astCommon.NameEntity import org.jetbrains.dukat.astCommon.appendLeft @@ -20,7 +21,6 @@ import org.jetbrains.dukat.model.commonLowerings.OverrideTypeChecker import org.jetbrains.dukat.model.commonLowerings.allParentMembers import org.jetbrains.dukat.model.commonLowerings.buildInheritanceGraph import org.jetbrains.dukat.ownerContext.NodeOwner -import org.jetbrains.dukat.ownerContext.wrap private fun PropertyModel.toVal(): PropertyModel { return copy( @@ -42,10 +42,10 @@ private class VarOverrideResolver( ): List> { return parentMembers.keys.mapNotNull { name -> val properties = parentMembers[name].orEmpty().filter { it.memberModel is PropertyModel } - if (properties.isEmpty() || properties.all { + if (properties.isEmpty() || cartesian(properties, properties).all { (first, second) -> with(typeChecker) { - (it.memberModel as PropertyModel).type.isEquivalent((properties[0].memberModel as PropertyModel).type) - } + (first.memberModel as PropertyModel).type.isEquivalent((second.memberModel as PropertyModel).type) + } || inheritanceContext.areRelated(first.ownerModel, second.ownerModel) }) { null } else { From acd92562d5e8fa712638b812d1260ff3aaa1781e Mon Sep 17 00:00:00 2001 From: Alexey Trilis Date: Tue, 28 Jul 2020 17:10:33 +0300 Subject: [PATCH 5/5] Change variance and nullability of property type in parent if needed to generate correct override --- .../data/typescript/override/variance.d.kt | 25 +++++++ .../data/typescript/override/variance.d.ts | 7 ++ ....kt => OverrideConflictResolveLowering.kt} | 65 +++++++++++++++---- .../OverrideNullabilityTransformer.kt | 36 ++++++++++ .../overrides/OverrideVarianceTransformer.kt | 47 ++++++++++++++ .../dukat/ts/translator/TypescriptLowerer.kt | 4 +- 6 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 compiler/test/data/typescript/override/variance.d.kt create mode 100644 compiler/test/data/typescript/override/variance.d.ts rename model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/{VarConflictResolveLowering.kt => OverrideConflictResolveLowering.kt} (76%) create mode 100644 model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideNullabilityTransformer.kt create mode 100644 model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideVarianceTransformer.kt diff --git a/compiler/test/data/typescript/override/variance.d.kt b/compiler/test/data/typescript/override/variance.d.kt new file mode 100644 index 000000000..510a06dee --- /dev/null +++ b/compiler/test/data/typescript/override/variance.d.kt @@ -0,0 +1,25 @@ +// [test] variance.kt +@file:Suppress("INTERFACE_WITH_SUPERCLASS", "OVERRIDING_FINAL_MEMBER", "RETURN_TYPE_MISMATCH_ON_OVERRIDE", "CONFLICTING_OVERLOADS") + +import kotlin.js.* +import kotlin.js.Json +import org.khronos.webgl.* +import org.w3c.dom.* +import org.w3c.dom.events.* +import org.w3c.dom.parsing.* +import org.w3c.dom.svg.* +import org.w3c.dom.url.* +import org.w3c.fetch.* +import org.w3c.files.* +import org.w3c.notifications.* +import org.w3c.performance.* +import org.w3c.workers.* +import org.w3c.xhr.* + +external open class Observable { + open val source: Observable +} + +external open class ConnectableObservable : Observable { + override val source: Observable +} \ No newline at end of file diff --git a/compiler/test/data/typescript/override/variance.d.ts b/compiler/test/data/typescript/override/variance.d.ts new file mode 100644 index 000000000..39216b55e --- /dev/null +++ b/compiler/test/data/typescript/override/variance.d.ts @@ -0,0 +1,7 @@ +export declare class Observable { + source: Observable +} + +export declare class ConnectableObservable extends Observable { + source: Observable +} \ No newline at end of file diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideConflictResolveLowering.kt similarity index 76% rename from model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt rename to model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideConflictResolveLowering.kt index 998aa723b..ec892e651 100644 --- a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/VarConflictResolveLowering.kt +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideConflictResolveLowering.kt @@ -29,12 +29,13 @@ private fun PropertyModel.toVal(): PropertyModel { ) } -private class VarOverrideResolver( +private class OverrideConflictResolver( private val modelContext: ModelContext, private val inheritanceContext: InheritanceContext ) : ModelWithOwnerTypeLowering { val propertiesToChangeToVal: MutableSet = mutableSetOf() + val propertiesToChangeType: MutableMap = mutableMapOf() private fun findConflictingProperties( parentMembers: Map>, @@ -71,6 +72,25 @@ private class VarOverrideResolver( return null } + private fun processParentPropertiesNeededToChangeType( + member: PropertyModel, + parentMembers: Map>, + typeChecker: OverrideTypeChecker + ) { + val parentProperty = parentMembers[member.name]?.firstOrNull { it.memberModel is PropertyModel } + if (parentProperty != null) { + val parentType = (parentProperty.memberModel as PropertyModel).type + val childType = member.type + val newType = parentType + .changeVarianceToMatchChild(childType, typeChecker) + .changeNullabilityToMatchChild(childType) + val fqName = parentProperty.fqName?.appendLeft(member.name) + if (newType != parentType && fqName != null) { + propertiesToChangeType[fqName] = newType + } + } + } + private fun findSuperType(types: List, typeChecker: OverrideTypeChecker): TypeModel? { var superType: TypeModel? = types[0] types.forEach { type -> @@ -109,7 +129,7 @@ private class VarOverrideResolver( val propertiesToAdd = allConflictingProperties.mapNotNull { createPropertyToAdd(it, classLike) } - return classLike.members.mapNotNull { member -> + return classLike.members.map { member -> if (member is PropertyModel) { when { allConflictingProperties.any { properties -> @@ -143,7 +163,9 @@ private class VarOverrideResolver( OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) ) - val varPropertiesWithSpecifiedType = classLike.members.filterIsInstance().mapNotNull { + val properties = classLike.members.filterIsInstance() + + val varPropertiesWithSpecifiedType = properties.mapNotNull { findVarPropertiesWithSpecifiedType( it, parentMembers, @@ -151,6 +173,14 @@ private class VarOverrideResolver( ) } + properties.forEach { + processParentPropertiesNeededToChangeType( + it, + parentMembers, + OverrideTypeChecker(modelContext, inheritanceContext, classLike, null) + ) + } + (allConflictingProperties + listOf(varPropertiesWithSpecifiedType)).forEach { propertiesToChangeToVal += it.mapNotNull { memberData -> (memberData.memberModel as? PropertyModel)?.let { property -> memberData.fqName?.appendLeft(property.name) @@ -167,17 +197,24 @@ private class VarOverrideResolver( } } -private class VarToValResolver(private val propertiesToChangeToVal: Set) : ModelWithOwnerTypeLowering { +private class ParentPropertyChanger( + private val propertiesToChangeToVal: Set, + private val propertiesToChangeType: MutableMap +) : ModelWithOwnerTypeLowering { private var currentFqName: NameEntity = IdentifierEntity("") override fun lowerPropertyModel(ownerContext: NodeOwner): PropertyModel { val property = ownerContext.node + val propertyFqName = currentFqName.appendLeft(property.name) - return if (propertiesToChangeToVal.contains(currentFqName.appendLeft(property.name))) { - property.toVal() + val newType = propertiesToChangeType[propertyFqName] ?: property.type + val newProperty = property.copy(type = newType) + + return if (propertiesToChangeToVal.contains(propertyFqName)) { + newProperty.toVal() } else { - property + newProperty } } @@ -197,22 +234,24 @@ private class VarToValResolver(private val propertiesToChangeToVal: Set { + this + } + childType is TypeParameterReferenceModel -> { + if (this.value == IdentifierEntity("Any")) { + this.copy(nullable = true) + } else { + this + } + } + childType is TypeValueModel -> { + if (params.size != childType.params.size) { + this + } else { + val newParams = params.zip(childType.params).map { (parentParam, childParam) -> + parentParam.type.changeNullabilityToMatchChild(childParam.type) + } + copy(params = newParams.mapIndexed { index, newParam -> + params[index].copy(type = newParam) + }) + } + } + else -> this + } +} \ No newline at end of file diff --git a/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideVarianceTransformer.kt b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideVarianceTransformer.kt new file mode 100644 index 000000000..0a44b9b16 --- /dev/null +++ b/model-lowerings-common/src/org/jetbrains/dukat/model/commonLowerings/overrides/OverrideVarianceTransformer.kt @@ -0,0 +1,47 @@ +package org.jetbrains.dukat.model.commonLowerings.overrides + +import org.jetbrains.dukat.astCommon.IdentifierEntity +import org.jetbrains.dukat.astModel.FunctionTypeModel +import org.jetbrains.dukat.astModel.TypeModel +import org.jetbrains.dukat.astModel.TypeParameterModel +import org.jetbrains.dukat.astModel.TypeParameterReferenceModel +import org.jetbrains.dukat.astModel.TypeValueModel +import org.jetbrains.dukat.astModel.Variance +import org.jetbrains.dukat.model.commonLowerings.OverrideTypeChecker + +private fun TypeParameterModel.toCovariant(): TypeParameterModel { + return copy(variance = Variance.COVARIANT) +} + +private fun TypeValueModel.toCovariant(indexes: Set): TypeValueModel { + return copy(params = params.mapIndexed { index, typeParameterModel -> + if (index in indexes) { + typeParameterModel.toCovariant() + } else { + typeParameterModel + } + }) +} + +internal fun TypeModel.changeVarianceToMatchChild( + childType: TypeModel, + typeChecker: OverrideTypeChecker +): TypeModel { + if (childType !is TypeValueModel || this !is TypeValueModel) { + return this + } + if (params.size != childType.params.size) { + return this + } + val indexesToChange = params.zip(childType.params).mapIndexedNotNull { index, (parentParam, childParam) -> + if (with(typeChecker) { childParam.type.isOverridingReturnType(parentParam.type) && !childParam.type.isEquivalent(parentParam.type) }) { + index + } else { + null + } + }.toSet() + if (indexesToChange.isEmpty()) { + return this + } + return toCovariant(indexesToChange) +} diff --git a/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt b/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt index 6e2fc9cd0..49cdd6b0d 100644 --- a/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt +++ b/typescript/ts-translator/src/org/jetbrains/dukat/ts/translator/TypescriptLowerer.kt @@ -23,7 +23,7 @@ import org.jetbrains.dukat.model.commonLowerings.RemoveConflictingOverloads import org.jetbrains.dukat.model.commonLowerings.RemoveKotlinBuiltIns import org.jetbrains.dukat.model.commonLowerings.RemoveRedundantTypeParams import org.jetbrains.dukat.model.commonLowerings.lower -import org.jetbrains.dukat.model.commonLowerings.overrides.VarConflictResolveLowering +import org.jetbrains.dukat.model.commonLowerings.overrides.OverrideConflictResolveLowering import org.jetbrains.dukat.moduleNameResolver.ModuleNameResolver import org.jetbrains.dukat.nodeIntroduction.IntroduceNodes import org.jetbrains.dukat.nodeIntroduction.ResolveModuleAnnotations @@ -120,7 +120,7 @@ open class TypescriptLowerer( .lower { context, inheritanceContext -> LowerOverrides(context, inheritanceContext) }, - VarConflictResolveLowering(), + OverrideConflictResolveLowering(), AddExplicitGettersAndSetters(), AnyfyUnresolvedTypes(), RemoveKotlinBuiltIns(),