JS: prohibit stable (public) names with non-identifier chars. Rewrite unstable (private) names with non-identifier chars. See KT-4160

compiler/testData/diagnostics/testsWithJsStdLib/name/illegalName.kt

@@ -0,0 +1,20 @@
+private fun ` .private `(): String = TODO("")
+
+<!NAME_CONTAINS_ILLEGAL_CHARS!>fun ` .public `(): String<!> = TODO("")
+
+<!NAME_CONTAINS_ILLEGAL_CHARS!>@JsName("  __  ")
+fun foo(): String<!> = TODO("")
+
+<!NAME_CONTAINS_ILLEGAL_CHARS!>@JsName("  ___  ")
+private fun bar(): String<!> = TODO("")
+
+@JsName("validName")
+private fun ` .private with @JsName `(): String = TODO("")
+
+private class <!NAME_CONTAINS_ILLEGAL_CHARS!>` .private class `<!> {
+    <!NAME_CONTAINS_ILLEGAL_CHARS!>val ` .field. `<!> = ""
+}
+
+val x: Int
+    <!NAME_CONTAINS_ILLEGAL_CHARS!>@JsName(".")
+    get()<!> = TODO("")

compiler/testData/diagnostics/testsWithJsStdLib/name/illegalName.txt

@@ -0,0 +1,16 @@
+package
+
+public val x: kotlin.Int
+private fun ` .private `(): kotlin.String
+@kotlin.js.JsName(name = "validName") private fun ` .private with @JsName `(): kotlin.String
+public fun ` .public `(): kotlin.String
+@kotlin.js.JsName(name = "  ___  ") private fun bar(): kotlin.String
+@kotlin.js.JsName(name = "  __  ") public fun foo(): kotlin.String
+
+private final class ` .private class ` {
+    public constructor ` .private class `()
+    public final val ` .field. `: kotlin.String = ""
+    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
+}

compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestWithJsStdLibGenerated.java

@@ -440,6 +440,12 @@ public class DiagnosticsTestWithJsStdLibGenerated extends AbstractDiagnosticsTes
             doTest(fileName);
         }
 
+        @TestMetadata("illegalName.kt")
+        public void testIllegalName() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/name/illegalName.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("jsNameAndNamedNative.kt")
         public void testJsNameAndNamedNative() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithJsStdLib/name/jsNameAndNamedNative.kt");

js/js.frontend/src/org/jetbrains/kotlin/js/naming/NameSuggestion.kt

@@ -330,5 +330,12 @@ class NameSuggestion {
 
         private val DeclarationDescriptorWithVisibility.ownEffectiveVisibility
             get() = visibility.effectiveVisibility(this, checkPublishedApi = true).toVisibility()
+
+        @JvmStatic fun sanitizeName(name: String): String {
+            if (name.isEmpty()) return "_"
+
+            val first = name.first().let { if (java.lang.Character.isJavaIdentifierStart(it)) it else '_' }
+            return first.toString() + name.drop(1).map { if (java.lang.Character.isJavaIdentifierPart(it)) it else '_' }.joinToString("")
+        }
     }
 }

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/JsPlatformConfigurator.kt

@@ -57,6 +57,7 @@ object JsPlatformConfigurator : PlatformConfigurator(
         container.useInstance(SyntheticConstructorsProvider.Empty)
         container.useInstance(JsTypeSpecificityComparator)
         container.useInstance(JsNameClashChecker())
+        container.useInstance(JsNameCharsChecker())
         container.useImpl<JsReflectionAPICallChecker>()
         container.useImpl<JsNativeRttiChecker>()
         container.useImpl<JsReifiedNativeChecker>()

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/DefaultErrorMessagesJs.kt

@@ -39,6 +39,7 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy {
         put(ErrorsJs.JSCODE_NO_JAVASCRIPT_PRODUCED, "Argument must be non-empty JavaScript code")
         put(ErrorsJs.NESTED_EXTERNAL_DECLARATION, "Non top-level `external` declaration")
         put(ErrorsJs.WRONG_EXTERNAL_DECLARATION, "Declaration of such kind ({0}) can't be external", Renderers.STRING)
+
         put(ErrorsJs.JS_NAME_CLASH, "JavaScript name ({0}) generated for this declaration clashes with another declaration: {1}",
             Renderers.STRING, Renderers.COMPACT)
         put(ErrorsJs.JS_FAKE_NAME_CLASH, "JavaScript name {0} is generated for different inherited members: {1} and {2}",
@@ -49,6 +50,9 @@ private val DIAGNOSTIC_FACTORY_TO_RENDERER by lazy {
         put(ErrorsJs.JS_NAME_PROHIBITED_FOR_OVERRIDE, "@JsName is prohibited for overridden members")
         put(ErrorsJs.JS_NAME_PROHIBITED_FOR_EXTENSION_PROPERTY, "@JsName is prohibited for extension properties")
         put(ErrorsJs.JS_NAME_PROHIBITED_FOR_NAMED_NATIVE, "@JsName is prohibited for external declaration with explicit name")
+
+        put(ErrorsJs.NAME_CONTAINS_ILLEGAL_CHARS, "Name contains illegal chars that can't appear in JavaScript identifier")
+
         put(ErrorsJs.JS_MODULE_PROHIBITED_ON_VAR, "@JsModule and @JsNonModule annotations prohibited for 'var' declarations. " +
                                                   "Use 'val' instead.")
         put(ErrorsJs.JS_MODULE_PROHIBITED_ON_NON_NATIVE, "@JsModule and @JsNonModule annotations prohibited for non-external declarations.")

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/ErrorsJs.java

@@ -42,6 +42,7 @@ public interface ErrorsJs {
     DiagnosticFactory0<KtExpression> JSCODE_NO_JAVASCRIPT_PRODUCED = DiagnosticFactory0.create(ERROR, DEFAULT);
     DiagnosticFactory1<KtExpression, String> WRONG_EXTERNAL_DECLARATION = DiagnosticFactory1.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory0<KtExpression> NESTED_EXTERNAL_DECLARATION = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
+
     DiagnosticFactory2<KtElement, String, DeclarationDescriptor> JS_NAME_CLASH = DiagnosticFactory2.create(
             ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory3<KtElement, String, DeclarationDescriptor, DeclarationDescriptor> JS_FAKE_NAME_CLASH =
@@ -52,6 +53,10 @@ public interface ErrorsJs {
     DiagnosticFactory0<PsiElement> JS_NAME_PROHIBITED_FOR_OVERRIDE = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<PsiElement> JS_NAME_PROHIBITED_FOR_EXTENSION_PROPERTY = DiagnosticFactory0.create(ERROR);
     DiagnosticFactory0<PsiElement> JS_NAME_PROHIBITED_FOR_NAMED_NATIVE = DiagnosticFactory0.create(ERROR);
+
+    DiagnosticFactory0<PsiElement> NAME_CONTAINS_ILLEGAL_CHARS = DiagnosticFactory0.create(
+            ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
+
     DiagnosticFactory0<KtElement> JS_MODULE_PROHIBITED_ON_VAR = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory0<KtElement> JS_MODULE_PROHIBITED_ON_NON_NATIVE = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);
     DiagnosticFactory0<KtElement> NESTED_JS_MODULE_PROHIBITED = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE_OR_DEFAULT);

js/js.frontend/src/org/jetbrains/kotlin/js/resolve/diagnostics/JsNameCharsChecker.kt

@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+
+/*
+ * 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.resolve.diagnostics
+
+import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
+import org.jetbrains.kotlin.descriptors.PropertyAccessorDescriptor
+import org.jetbrains.kotlin.diagnostics.DiagnosticSink
+import org.jetbrains.kotlin.js.naming.NameSuggestion
+import org.jetbrains.kotlin.js.translate.utils.AnnotationsUtils
+import org.jetbrains.kotlin.psi.KtDeclaration
+import org.jetbrains.kotlin.resolve.BindingContext
+import org.jetbrains.kotlin.resolve.checkers.SimpleDeclarationChecker
+
+class JsNameCharsChecker() : SimpleDeclarationChecker {
+    val suggestion = NameSuggestion()
+
+    override fun check(
+            declaration: KtDeclaration, descriptor: DeclarationDescriptor,
+            diagnosticHolder: DiagnosticSink, bindingContext: BindingContext
+    ) {
+        if (descriptor is PropertyAccessorDescriptor && AnnotationsUtils.getJsName(descriptor) == null) return
+
+        val suggestedName = suggestion.suggest(descriptor) ?: return
+        if (suggestedName.stable && suggestedName.names.any { NameSuggestion.sanitizeName(it) != it }) {
+            diagnosticHolder.report(ErrorsJs.NAME_CONTAINS_ILLEGAL_CHARS.on(declaration))
+        }
+    }
+}

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

@@ -5165,6 +5165,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             doTest(fileName);
         }
 
+        @TestMetadata("peculiarIdentifiers.kt")
+        public void testPeculiarIdentifiers() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/jsName/peculiarIdentifiers.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("privateMethod.kt")
         public void testPrivateMethod() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/jsName/privateMethod.kt");

js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java

@@ -368,7 +368,7 @@ public final class StaticContext {
             assert suggested.getNames().size() == 1 : "Private names must always consist of exactly one name";
             JsName name = nameCache.get(suggested.getDescriptor());
             if (name == null) {
-                String baseName = suggested.getNames().get(0);
+                String baseName = NameSuggestion.sanitizeName(suggested.getNames().get(0));
                 if (suggested.getDescriptor() instanceof LocalVariableDescriptor ||
                     suggested.getDescriptor() instanceof ValueParameterDescriptor
                 ) {
@@ -530,14 +530,14 @@ public final class StaticContext {
                 }
             }
             else {
-                suggestedName = descriptor.getName().asString();
+                suggestedName = NameSuggestion.sanitizeName(descriptor.getName().asString());
             }
         }
 
         if (!(descriptor instanceof PackageFragmentDescriptor) && !DescriptorUtils.isTopLevelDeclaration(descriptor)) {
             DeclarationDescriptor container = descriptor.getContainingDeclaration();
             assert container != null : "We just figured out that descriptor is not for a top-level declaration: " + descriptor;
-            suggestedName = getSuggestedName(container) + "$" + suggestedName;
+            suggestedName = getSuggestedName(container) + "$" + NameSuggestion.sanitizeName(suggestedName);
         }
 
         return suggestedName;

js/js.translator/src/org/jetbrains/kotlin/js/translate/context/UsageTracker.kt

@@ -166,12 +166,12 @@ class UsageTracker(
 
             // Append 'closure$' prefix to avoid name clash between closure and member fields in case of local classes
             else -> {
-                val mangled = NameSuggestion().suggest(this)!!.names.last()
+                val mangled = NameSuggestion.sanitizeName(NameSuggestion().suggest(this)!!.names.last())
                 "closure\$$mangled"
             }
         }
 
-        return scope.declareFreshName(suggestedName)
+        return scope.declareTemporaryName(suggestedName)
     }
 }
 

js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/FunctionBodyTranslator.java

@@ -24,6 +24,7 @@ import org.jetbrains.kotlin.descriptors.ConstructorDescriptor;
 import org.jetbrains.kotlin.descriptors.DeclarationDescriptor;
 import org.jetbrains.kotlin.descriptors.FunctionDescriptor;
 import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor;
+import org.jetbrains.kotlin.js.naming.NameSuggestion;
 import org.jetbrains.kotlin.js.translate.context.Namer;
 import org.jetbrains.kotlin.js.translate.context.TranslationContext;
 import org.jetbrains.kotlin.js.translate.expression.LocalFunctionCollector;
@@ -58,7 +59,7 @@ public final class FunctionBodyTranslator extends AbstractTranslator {
 
         for (FunctionDescriptor localFunction : functionCollector.getFunctions()) {
             String localIdent = localFunction.getName().isSpecial() ? "lambda" : localFunction.getName().asString();
-            JsName localName = functionBodyContext.scope().getParent().declareTemporaryName(localIdent);
+            JsName localName = functionBodyContext.scope().getParent().declareTemporaryName(NameSuggestion.sanitizeName(localIdent));
             JsExpression alias = JsAstUtils.pureFqn(localName, null);
             aliases.put(localFunction, alias);
         }

js/js.translator/testData/box/jsName/peculiarIdentifiers.kt

@@ -0,0 +1,34 @@
+private fun `+`(a: Int, b: Int) = a + b
+
+@JsName("minus")
+fun `-`(a: Int, b: Int) = a - b
+
+fun test1(): Int {
+    val ` x ` = 23
+    val `*` = 3
+    return `-`(` x `, `*`)
+}
+
+fun test2(`p 1`: Int, `.p 2`: Int) = `+`(`p 1`, `.p 2`)
+
+fun test3(): String {
+    val `#` = "K"
+    class ` `(private val `::`: String) {
+        internal fun `@`() = `::` + `#`
+    }
+    return ` `("O").`@`()
+}
+
+fun test4(): String {
+    val `()` = "OK"
+    fun `[]`() = `()`
+    return `[]`()
+}
+
+fun box(): String {
+    if (test1() != 20) return "fail1"
+    if (test2(10, 13) != 23) return "fail2"
+    if (test3() != "OK") return "fail3"
+    if (test4() != "OK") return "fail4"
+    return "OK"
+}

js/js.translator/testData/jsTester/jsTester.kt

@@ -8,8 +8,6 @@ public class JsTestsAsserter() : Asserter {
     public override fun fail(message: String?): Nothing = failWithMessage(message)
 }
 
-@JsName("JsTests.assert")
+public inline fun assert(value: Boolean, message: String?): Unit = js("JsTests").assert(value, message)
-public external fun assert(value: Boolean, message: String?): Unit = noImpl
 
-@JsName("JsTests.fail")
+private inline fun failWithMessage(message: String?): Nothing = js("JsTests").fail(message)
-private external fun failWithMessage(message: String?): Nothing = noImpl