[JS IR] Respect JsExport while assigning stable names

see https://youtrack.jetbrains.com/issue/KT-43404
2020-12-02 22:20:16 +01:00
parent 6b649d02d3
commit 8e5bcd349e
11 changed files with 152 additions and 16 deletions
@@ -397,7 +397,7 @@ fun IrClass.createImplicitParameterDeclarationWithWrappedDescriptor() {
@Suppress("UNCHECKED_CAST")
 fun isElseBranch(branch: IrBranch) = branch is IrElseBranch || ((branch.condition as? IrConst<Boolean>)?.value == true)

-fun IrSimpleFunction.isMethodOfAny() =
+fun IrFunction.isMethodOfAny() =
    ((valueParameters.size == 0 && name.asString().let { it == "hashCode" || it == "toString" }) ||
            (valueParameters.size == 1 && name.asString() == "equals" && valueParameters[0].type.isNullableAny()))

@@ -15,7 +15,7 @@ import org.jetbrains.kotlin.ir.ObsoleteDescriptorBasedAPI
 import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
 import org.jetbrains.kotlin.ir.backend.js.JsCommonBackendContext
 import org.jetbrains.kotlin.ir.backend.js.JsLoweredDeclarationOrigin
-import org.jetbrains.kotlin.ir.backend.js.utils.getJsName
+import org.jetbrains.kotlin.ir.backend.js.utils.hasStableJsName
 import org.jetbrains.kotlin.ir.backend.js.utils.realOverrideTarget
 import org.jetbrains.kotlin.ir.builders.*
 import org.jetbrains.kotlin.ir.builders.declarations.buildFun
@@ -118,7 +118,7 @@ abstract class BridgesConstruction(val context: JsCommonBackendContext) : Declar
    ): IrFunction {

        val origin =
-            if (bridge.isEffectivelyExternal() || bridge.getJsName() != null)
+            if (bridge.hasStableJsName())
                JsLoweredDeclarationOrigin.BRIDGE_TO_EXTERNAL_FUNCTION
            else
                IrDeclarationOrigin.BRIDGE
@@ -109,20 +109,13 @@ fun jsFunctionSignature(declaration: IrFunction): Signature {
    require(declaration.dispatchReceiverParameter != null)

    val declarationName = declaration.getJsNameOrKotlinName().asString()
-    val stableName = StableNameSignature(declarationName)

-    if (declaration.origin == JsLoweredDeclarationOrigin.BRIDGE_TO_EXTERNAL_FUNCTION) {
-        return stableName
-    }
-    if (declaration.isEffectivelyExternal()) {
-        return stableName
-    }
-    if (declaration.getJsName() != null) {
-        return stableName
-    }
-    // Handle names for special functions
-    if (declaration is IrSimpleFunction && declaration.isMethodOfAny()) {
-        return stableName
+    val needsStableName = declaration.origin == JsLoweredDeclarationOrigin.BRIDGE_TO_EXTERNAL_FUNCTION ||
+            declaration.hasStableJsName() ||
+            (declaration as? IrSimpleFunction)?.isMethodOfAny() == true // Handle names for special functions
+
+    if (needsStableName) {
+        return StableNameSignature(declarationName)
    }

    val nameBuilder = StringBuilder()
@@ -15,11 +15,29 @@ import org.jetbrains.kotlin.ir.expressions.impl.IrVarargImpl
 import org.jetbrains.kotlin.ir.types.IrType
 import org.jetbrains.kotlin.ir.types.isNullableAny
 import org.jetbrains.kotlin.ir.types.isUnit
+import org.jetbrains.kotlin.ir.util.isEffectivelyExternal
 import org.jetbrains.kotlin.ir.util.isTopLevelDeclaration
+import org.jetbrains.kotlin.ir.util.parentClassOrNull
 import org.jetbrains.kotlin.name.Name

 fun TODO(element: IrElement): Nothing = TODO(element::class.java.simpleName + " is not supported yet here")

+fun IrFunction.hasStableJsName(): Boolean {
+    val namedOrMissingGetter = when (this) {
+        is IrSimpleFunction -> {
+            val owner = correspondingPropertySymbol?.owner
+            if (owner == null) {
+                true
+            } else {
+                owner.getter?.getJsName() != null
+            }
+        }
+        else -> true
+    }
+
+    return (isEffectivelyExternal() || getJsName() != null || parentClassOrNull?.isJsExport() == true) && namedOrMissingGetter
+}
+
 fun IrFunction.isEqualsInheritedFromAny() =
    name == Name.identifier("equals") &&
            dispatchReceiverParameter != null &&
@@ -5167,6 +5167,16 @@ public class IrBoxJsES6TestGenerated extends AbstractIrBoxJsES6Test {
            KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("js/js.translator/testData/box/jsExport"), Pattern.compile("^([^_](.+))\\.kt$"), null, TargetBackend.JS_IR_ES6, true);
        }

+        @TestMetadata("dataClass.kt")
+        public void testDataClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/dataClass.kt");
+        }
+
+        @TestMetadata("jsExportInClass.kt")
+        public void testJsExportInClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/jsExportInClass.kt");
+        }
+
        @TestMetadata("recursiveExport.kt")
        public void testRecursiveExport() throws Exception {
            runTest("js/js.translator/testData/box/jsExport/recursiveExport.kt");
@@ -5167,6 +5167,16 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest {
            KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("js/js.translator/testData/box/jsExport"), Pattern.compile("^([^_](.+))\\.kt$"), null, TargetBackend.JS_IR, true);
        }

+        @TestMetadata("dataClass.kt")
+        public void testDataClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/dataClass.kt");
+        }
+
+        @TestMetadata("jsExportInClass.kt")
+        public void testJsExportInClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/jsExportInClass.kt");
+        }
+
        @TestMetadata("recursiveExport.kt")
        public void testRecursiveExport() throws Exception {
            runTest("js/js.translator/testData/box/jsExport/recursiveExport.kt");
@@ -5182,6 +5182,16 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
            KotlinTestUtils.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("js/js.translator/testData/box/jsExport"), Pattern.compile("^([^_](.+))\\.kt$"), null, TargetBackend.JS, true);
        }

+        @TestMetadata("dataClass.kt")
+        public void testDataClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/dataClass.kt");
+        }
+
+        @TestMetadata("jsExportInClass.kt")
+        public void testJsExportInClass() throws Exception {
+            runTest("js/js.translator/testData/box/jsExport/jsExportInClass.kt");
+        }
+
        @TestMetadata("recursiveExport.kt")
        public void testRecursiveExport() throws Exception {
            runTest("js/js.translator/testData/box/jsExport/recursiveExport.kt");
@@ -0,0 +1,13 @@
+$kotlin_test_internal$.beginModule();
+
+module.exports = function() {
+    var Point = require("JS_TESTS").api.Point;
+    var p = new Point(3, 7);
+
+    return {
+        "res": p.copy(13, 11).toString()
+    };
+};
+
+$kotlin_test_internal$.endModule("lib");
+
@@ -0,0 +1,30 @@
+// MODULE_KIND: COMMON_JS
+// SKIP_MINIFICATION
+
+// FILE: api.kt
+package api
+
+@JsExport
+data class Point(val x: Int, val y: Int) {
+    override fun toString(): String = "[${x}::${y}]"
+}
+
+// we need his class to make sure that there's more than one ping method in existence - due to peculiarities of current namer otherwise test can pass but JsExport won't be actually respected
+data class AltPoint(val x: Int, val y: Int)
+
+// FILE: main.kt
+external interface JsResult {
+    val res: String
+}
+
+@JsModule("lib")
+external fun jsBox(): JsResult
+
+fun box(): String {
+    val res = jsBox().res
+    if (res != "[13::11]") {
+        return "Fail1: ${res}"
+    }
+
+    return "OK"
+}
@@ -0,0 +1,13 @@
+$kotlin_test_internal$.beginModule();
+
+module.exports = function() {
+    var A = require("JS_TESTS").api.A;
+    var B = require("JS_TESTS").api.B;
+
+    return {
+        "res": (new A().ping()) + (new B().pong())
+    };
+};
+
+$kotlin_test_internal$.endModule("lib");
+
@@ -0,0 +1,39 @@
+// MODULE_KIND: COMMON_JS
+// SKIP_MINIFICATION
+
+// FILE: api.kt
+package api
+
+@JsExport
+class A() {
+    fun ping() = "ping"
+}
+
+@JsExport
+class B() {
+    @JsName("pong")
+    fun ping() = "pong"
+}
+
+// we need his class to make sure that there's more than one ping method in existence - due to peculiarities of current namer otherwise test can pass but JsExport won't be actually respected
+class C() {
+    fun ping() = "pong"
+}
+
+
+// FILE: main.kt
+external interface JsResult {
+    val res: String
+}
+
+@JsModule("lib")
+external fun jsBox(): JsResult
+
+fun box(): String {
+    val res = jsBox().res
+    if (res != "pingpong") {
+        return "Fail: ${res}"
+    }
+
+    return "OK"
+}