Intrinsify String?.plus in JS BE

See KT-8020

compiler/testData/codegen/box/classes/kt508.kt

@@ -1,7 +1,3 @@
-// TODO: Enable for JS when it supports Java class library.
-// IGNORE_BACKEND: JS
-// fails on JS with TypeError: imported$plus is not a function, it is undefined.
-
 operator fun <K, V> MutableMap<K, V>.set(key : K, value : V) = put(key, value)
 
 fun box() : String {

compiler/testData/codegen/box/strings/kt894.kt

@@ -1,6 +1,3 @@
-// TODO: muted automatically, investigate should it be ran for JS or not
-// IGNORE_BACKEND: JS
-
 fun stringConcat(n : Int) : String? {
   var string : String? = ""
   for (i in 0..(n - 1))

compiler/testData/codegen/boxInline/defaultValues/defaultMethodInClass.kt

@@ -1,6 +1,3 @@
-// There's no String?.plus implementation in JS stdlib
-// IGNORE_BACKEND: JS
-
 // FILE: 1.kt
 
 package test

js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java

@@ -7657,6 +7657,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/standardClasses/stringBuilder.kt");
             doTest(fileName);
         }
+
+        @TestMetadata("stringPlus.kt")
+        public void testStringPlus() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/standardClasses/stringPlus.kt");
+            doTest(fileName);
+        }
     }
 
     @TestMetadata("js/js.translator/testData/box/superCall")

js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineDefaultValuesTestsGenerated.java

@@ -51,13 +51,7 @@ public class InlineDefaultValuesTestsGenerated extends AbstractInlineDefaultValu
     @TestMetadata("defaultMethodInClass.kt")
     public void testDefaultMethodInClass() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/boxInline/defaultValues/defaultMethodInClass.kt");
-        try {
+        doTest(fileName);
-            doTest(fileName);
-        }
-        catch (Throwable ignore) {
-            return;
-        }
-        throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
     }
 
     @TestMetadata("defaultParamRemapping.kt")

js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java

@@ -3965,13 +3965,7 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
         @TestMetadata("kt508.kt")
         public void testKt508() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/classes/kt508.kt");
-            try {
+            doTest(fileName);
-                doTest(fileName);
-            }
-            catch (Throwable ignore) {
-                return;
-            }
-            throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
         }
 
         @TestMetadata("kt5347.kt")
@@ -21539,13 +21533,7 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
         @TestMetadata("kt894.kt")
         public void testKt894() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/strings/kt894.kt");
-            try {
+            doTest(fileName);
-                doTest(fileName);
-            }
-            catch (Throwable ignore) {
-                return;
-            }
-            throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that.");
         }
 
         @TestMetadata("multilineStringsWithTemplates.kt")

js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/FunctionIntrinsics.java

@@ -23,6 +23,7 @@ import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.descriptors.FunctionDescriptor;
 import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.FunctionIntrinsic;
 import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.*;
+import org.jetbrains.kotlin.js.translate.intrinsic.operation.StringPlusCharFIF;
 
 import java.util.List;
 import java.util.Map;
@@ -42,6 +43,7 @@ public final class FunctionIntrinsics {
     private void registerFactories() {
         register(LongOperationFIF.INSTANCE);
         register(PrimitiveUnaryOperationFIF.INSTANCE);
+        register(StringPlusCharFIF.INSTANCE);
         register(PrimitiveBinaryOperationFIF.INSTANCE);
         register(ArrayFIF.INSTANCE);
         register(TopLevelFIF.INSTANCE);

js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/StringPlusCharBOIF.kt

@@ -1,73 +0,0 @@
-/*
- * Copyright 2010-2016 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.jetbrains.kotlin.js.translate.intrinsic.operation
-
-
-import com.google.common.collect.ImmutableSet
-import org.jetbrains.kotlin.builtins.KotlinBuiltIns
-import org.jetbrains.kotlin.descriptors.FunctionDescriptor
-import org.jetbrains.kotlin.js.backend.ast.JsBinaryOperation
-import org.jetbrains.kotlin.js.backend.ast.JsExpression
-import org.jetbrains.kotlin.js.translate.context.TranslationContext
-import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.TopLevelFIF
-import org.jetbrains.kotlin.js.translate.operation.OperatorTable
-import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
-import org.jetbrains.kotlin.js.translate.utils.PsiUtils.getOperationToken
-import org.jetbrains.kotlin.lexer.KtTokens
-import org.jetbrains.kotlin.psi.KtBinaryExpression
-import org.jetbrains.kotlin.types.KotlinType
-
-object StringPlusCharBOIF : BinaryOperationIntrinsicFactory {
-
-    private object StringPlusCharIntrinsic : AbstractBinaryOperationIntrinsic() {
-        override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression {
-            val operator = OperatorTable.getBinaryOperator(getOperationToken(expression))
-            return JsBinaryOperation(operator, left, JsAstUtils.charToString(right))
-        }
-    }
-
-    private object StringPlusAnyIntrinsic : AbstractBinaryOperationIntrinsic() {
-        override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression {
-            val operator = OperatorTable.getBinaryOperator(getOperationToken(expression))
-            return JsBinaryOperation(operator, left, TopLevelFIF.TO_STRING.apply(right, listOf(), context))
-        }
-    }
-
-    private object StringPlusStringIntrinsic : AbstractBinaryOperationIntrinsic() {
-        override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression {
-            val operator = OperatorTable.getBinaryOperator(getOperationToken(expression))
-            return JsBinaryOperation(operator, left, right)
-        }
-    }
-
-    override fun getSupportTokens() = ImmutableSet.of(KtTokens.PLUS)
-
-    override fun getIntrinsic(descriptor: FunctionDescriptor, leftType: KotlinType?, rightType: KotlinType?): BinaryOperationIntrinsic? {
-        if (KotlinBuiltIns.isString(leftType) && rightType != null) {
-            if (KotlinBuiltIns.isCharOrNullableChar(rightType)) {
-                return StringPlusCharIntrinsic
-            }
-            if (KotlinBuiltIns.isStringOrNullableString(rightType)) {
-                return StringPlusStringIntrinsic
-            }
-            if (KotlinBuiltIns.isAnyOrNullableAny(rightType)) {
-                return StringPlusAnyIntrinsic
-            }
-        }
-        return null
-    }
-}

js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/StringPlusCharFIF.kt

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.js.translate.intrinsic.operation
+
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
+import org.jetbrains.kotlin.descriptors.FunctionDescriptor
+import org.jetbrains.kotlin.js.backend.ast.JsBinaryOperation
+import org.jetbrains.kotlin.js.backend.ast.JsBinaryOperator
+import org.jetbrains.kotlin.js.backend.ast.JsExpression
+import org.jetbrains.kotlin.js.translate.callTranslator.CallInfo
+import org.jetbrains.kotlin.js.translate.context.TranslationContext
+import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.FunctionIntrinsic
+import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.FunctionIntrinsicFactory
+import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.TopLevelFIF
+import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
+import org.jetbrains.kotlin.types.TypeUtils
+
+object StringPlusCharFIF : FunctionIntrinsicFactory {
+    private class StringPlusAnyIntrinsic(private val leftTypeNullable: Boolean) : FunctionIntrinsic() {
+        override fun apply(callInfo: CallInfo, arguments: List<JsExpression>, context: TranslationContext): JsExpression {
+            val receiver = callInfo.dispatchReceiver ?: callInfo.extensionReceiver!!
+            val rightType = context.bindingContext().getType(callInfo.resolvedCall.call.valueArguments[0].getArgumentExpression()!!)
+                            ?: callInfo.resolvedCall.resultingDescriptor.valueParameters[0].type
+            val rightTypeNullable = TypeUtils.isNullableType(rightType)
+            val hasNonNullArg = !leftTypeNullable || !rightTypeNullable
+            val rightExpr = when {
+                KotlinBuiltIns.isChar(rightType) -> {
+                    JsAstUtils.charToString(arguments[0])
+                }
+                KotlinBuiltIns.isStringOrNullableString(rightType) && hasNonNullArg -> {
+                    arguments[0]
+                }
+                else -> {
+                    TopLevelFIF.TO_STRING.apply(arguments[0], listOf(), context)
+                }
+            }
+
+            return JsBinaryOperation(JsBinaryOperator.ADD, receiver, rightExpr)
+        }
+    }
+
+    override fun getIntrinsic(descriptor: FunctionDescriptor): FunctionIntrinsic? {
+        val fqName = descriptor.fqNameUnsafe.asString()
+        if (fqName != "kotlin.String.plus" && fqName != "kotlin.plus") return null
+
+        val leftType = (descriptor.dispatchReceiverParameter ?: descriptor.extensionReceiverParameter ?: return null).type
+
+        return if (KotlinBuiltIns.isStringOrNullableString(leftType)) {
+            StringPlusAnyIntrinsic(TypeUtils.isNullableType(leftType))
+        }
+        else {
+            null
+        }
+    }
+}

js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/binaryOperationIntrinsics.kt

@@ -16,7 +16,6 @@
 
 package org.jetbrains.kotlin.js.translate.intrinsic.operation
 
-import com.google.common.collect.ImmutableSet
 import org.jetbrains.kotlin.js.backend.ast.JsExpression
 import org.jetbrains.kotlin.descriptors.FunctionDescriptor
 import org.jetbrains.kotlin.js.translate.context.TranslationContext
@@ -37,7 +36,7 @@ class BinaryOperationIntrinsics {
 
     private val intrinsicCache = mutableMapOf<IntrinsicKey, BinaryOperationIntrinsic>()
 
-    private val factories = listOf(LongCompareToBOIF, EqualsBOIF, CompareToBOIF, StringPlusCharBOIF, AssignmentBOIF)
+    private val factories = listOf(LongCompareToBOIF, EqualsBOIF, CompareToBOIF, AssignmentBOIF)
 
     fun getIntrinsic(expression: KtBinaryExpression, context: TranslationContext): BinaryOperationIntrinsic {
         val token = getOperationToken(expression)

js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/AssignmentTranslator.java

@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.js.translate.operation;
 
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.kotlin.descriptors.CallableDescriptor;
 import org.jetbrains.kotlin.descriptors.ClassDescriptor;
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor;
 import org.jetbrains.kotlin.descriptors.PropertyDescriptor;
@@ -31,8 +32,7 @@ import org.jetbrains.kotlin.lexer.KtToken;
 import org.jetbrains.kotlin.psi.*;
 import org.jetbrains.kotlin.types.expressions.OperatorConventions;
 
-import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.getDescriptorForReferenceExpression;
+import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.*;
-import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.isVariableReassignment;
 import static org.jetbrains.kotlin.js.translate.utils.PsiUtils.getSimpleName;
 import static org.jetbrains.kotlin.js.translate.utils.PsiUtils.isAssignment;
 import static org.jetbrains.kotlin.js.translate.utils.TranslationUtils.hasCorrespondingFunctionIntrinsic;
@@ -46,7 +46,14 @@ public abstract class AssignmentTranslator extends AbstractTranslator {
     @NotNull
     public static JsExpression translate(@NotNull KtBinaryExpression expression, @NotNull TranslationContext context) {
         if (hasCorrespondingFunctionIntrinsic(context, expression)) {
-            return IntrinsicAssignmentTranslator.doTranslate(expression, context);
+            // This is a hack. This code is conceptually wrong, refactoring required here.
+            // The right implementation is: get assignment via OverloadedAssignmentTranslator and see at the result.
+            // If it's an expression like `a = a + b` then represent it as `a += b`
+            // Another right implementation is: enumerate explicitly which descriptors are OK to intrinsify like this.
+            CallableDescriptor operationDescriptor = getCallableDescriptorForOperationExpression(context.bindingContext(), expression);
+            if (operationDescriptor == null || operationDescriptor.getExtensionReceiverParameter() == null) {
+                return IntrinsicAssignmentTranslator.doTranslate(expression, context);
+            }
         }
         return OverloadedAssignmentTranslator.doTranslate(expression, context);
     }

js/js.translator/testData/box/standardClasses/stringPlus.kt

@@ -0,0 +1,51 @@
+// EXPECTED_REACHABLE_NODES: 489
+
+fun box(): String {
+    var x: String? = foo()
+    var r = x + bar()
+    if (r != "foobar") return "fail1: $r"
+
+    x = null
+    r = x + bar()
+    if (r != "nullbar") return "fail2: $r"
+
+    x = foo()
+    r = x + null
+    if (r != "foonull") return "fail3: $r"
+
+    x = foo()
+    r = x + nullString()
+    if (r != "foonull") return "fail4: $r"
+
+    r = foo()
+    r += bar()
+    if (r != "foobar") return "fail5: $r"
+
+    x = null
+    r = x + null
+    if (r != "nullnull") return "fail6: $r"
+
+    x = foo()
+    x += nullString()
+    if (x != "foonull") return "fail7: $r"
+
+    x = nullString()
+    x += bar()
+    if (x != "nullbar") return "fail8: $r"
+
+    x = nullString()
+    r = x + nullString()
+    if (r != "nullnull") return "fail9: $r"
+
+    x = nullString()
+    x += nullString()
+    if (x != "nullnull") return "fail10: $x"
+
+    return "OK"
+}
+
+fun foo() = "foo"
+
+fun bar() = "bar"
+
+fun nullString(): String? = null