JS: fixes after code review

build.xml

@@ -376,6 +376,14 @@
             </externs>
         </closure-compiler>
 
+        <!--
+         Few hacks to compose one kotlin.js from several smaller files:
+           1. Wrapper function takes Kotlin and _, whereas they are already defined above (see closure-wrapper.txt), so delete these
+              parameters.
+           2. Since we deleted parameters, we can get rid or arguments passed to wrapper function.
+              They are module.exports and require("Kotlin"), accordingly to CommonJS spec.
+           3. We can omit return _, since it's already exported by the code in UMD wrapper.
+        -->
         <replaceregexp file="${js.stdlib.output.dir}/kotlin.js"
                        match="module.exports,\s*require\([^)]+\)"
                        replace=""
@@ -394,6 +402,7 @@
         <property environment="env"/>
         <kotlin-pp src="libraries/stdlib/src" output="${intermediate-sources}/stdlib/js" profile="JS" />
 
+        <!-- We don't want descriptors for built-ins to be serialized, so we compile these files separately. -->
         <new-kotlin2js output="${js.stdlib.output.dir}/tmp-builtins/kotlin.js">
             <src>
                 <union>
@@ -401,6 +410,7 @@
                 </union>
             </src>
         </new-kotlin2js>
+
         <new-kotlin2js output="${js.stdlib.output.dir}/tmp/kotlin.js">
             <src>
                 <union>

js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/jsScopes.kt

@@ -96,15 +96,15 @@ open class JsFunctionScope(parent: JsScope, description: String) : JsScope(paren
                 // disallowed as variable names in strict mode
                 "eval", "arguments",
 
-                // non-reserved words that act like reserved words
+                // global identifiers usually declared in a typical JS interpreter
                 "NaN", "Infinity", "undefined",
-
-                // the special Kotlin object
-                "Kotlin",
-
                 "Error", "Object", "Math", "String", "Number", "Boolean", "Date", "Array", "RegExp", "JSON",
 
-                "require", "define", "module", "window", "self"
+                // global identifiers usually declared in know environments (node.js, browser, require.js, WebWorkers, etc)
+                "require", "define", "module", "window", "self",
+
+                // the special Kotlin object
+                "Kotlin"
         )
     }
 }

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt

@@ -18,10 +18,8 @@ package org.jetbrains.kotlin.js.inline.util
 
 import com.google.dart.compiler.backend.js.ast.*
 import com.google.dart.compiler.backend.js.ast.metadata.staticRef
-
 import org.jetbrains.kotlin.js.inline.util.collectors.InstanceCollector
-import org.jetbrains.kotlin.js.inline.util.collectors.PropertyCollector
+import org.jetbrains.kotlin.js.translate.expression.InlineMetadata
-import org.jetbrains.kotlin.js.translate.expression.*
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 import java.util.*
 
@@ -128,12 +126,6 @@ fun JsFunction.collectFreeVariables() = collectUsedNames(body) - collectDefinedN
 
 fun JsFunction.collectLocalVariables() = collectDefinedNames(body) + parameters.map { it.name }
 
-fun collectJsProperties(scope: JsNode): IdentityHashMap<JsName, JsExpression> {
-    val collector = PropertyCollector()
-    collector.accept(scope)
-    return collector.properties
-}
-
 fun collectNamedFunctions(scope: JsNode) = collectNamedFunctionsAndMetadata(scope).mapValues { it.value.first }
 
 fun collectNamedFunctionsOrMetadata(scope: JsNode) = collectNamedFunctionsAndMetadata(scope).mapValues { it.value.second }

js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectors/PropertyCollector.kt

@@ -1,38 +0,0 @@
-/*
- * Copyright 2010-2015 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.inline.util.collectors
-
-import com.google.dart.compiler.backend.js.ast.*
-import org.jetbrains.kotlin.js.translate.expression.InlineMetadata
-import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
-
-import java.util.IdentityHashMap
-
-class PropertyCollector : RecursiveJsVisitor() {
-    val properties: IdentityHashMap<JsName, JsExpression> = IdentityHashMap()
-
-    override fun visitPropertyInitializer(x: JsPropertyInitializer) {
-        super.visitPropertyInitializer(x)
-
-        val label = x.labelExpr as? JsNameRef
-        val name = label?.name
-        if (name == null) return
-
-        val value = x.valueExpr
-        properties[name] = value
-    }
-}

js/js.libraries/src/builtins/kotlin.kt

@@ -33,6 +33,6 @@ class Enum<T : Enum<T>> : Comparable<Enum<T>> {
     override fun hashCode(): Int = js("Kotlin.identityHashCode(this)")
 
     override fun toString() = name
-}
 
-interface Annotation
+    companion object
+}

js/js.libraries/src/builtins/primitiveCompanionObjects.kt

@@ -16,51 +16,98 @@
 
 package kotlin.js.internal
 
-@JsName("DoubleCompanionObject") private object DoubleCompanionObject {
+@JsName("DoubleCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Double = js("Number.MIN_VALUE")
+private object DoubleCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Double = js("Number.MAX_VALUE")
+    @JsName("MIN_VALUE")
-    @JsName("POSITIVE_INFINITY") val POSITIVE_INFINITY: Double = js("Number.POSITIVE_INFINITY")
+    val MIN_VALUE: Double = js("Number.MIN_VALUE")
-    @JsName("NEGATIVE_INFINITY") val NEGATIVE_INFINITY: Double = js("Number.NEGATIVE_INFINITY")
+
-    @JsName("NaN") val NaN: Double = js("Number.NaN")
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Double = js("Number.MAX_VALUE")
+
+    @JsName("POSITIVE_INFINITY")
+    val POSITIVE_INFINITY: Double = js("Number.POSITIVE_INFINITY")
+
+    @JsName("NEGATIVE_INFINITY")
+    val NEGATIVE_INFINITY: Double = js("Number.NEGATIVE_INFINITY")
+
+    @JsName("NaN")
+    val NaN: Double = js("Number.NaN")
 }
 
-@JsName("FloatCompanionObject") private object FloatCompanionObject {
+@JsName("FloatCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Float = js("Number.MIN_VALUE")
+private object FloatCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Float = js("Number.MAX_VALUE")
+    @JsName("MIN_VALUE")
-    @JsName("POSITIVE_INFINITY") val POSITIVE_INFINITY: Float = js("Number.POSITIVE_INFINITY")
+    val MIN_VALUE: Float = js("Number.MIN_VALUE")
-    @JsName("NEGATIVE_INFINITY") val NEGATIVE_INFINITY: Float = js("Number.NEGATIVE_INFINITY")
+
-    @JsName("NaN") val NaN: Float = js("Number.NaN")
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Float = js("Number.MAX_VALUE")
+
+    @JsName("POSITIVE_INFINITY")
+    val POSITIVE_INFINITY: Float = js("Number.POSITIVE_INFINITY")
+
+    @JsName("NEGATIVE_INFINITY")
+    val NEGATIVE_INFINITY: Float = js("Number.NEGATIVE_INFINITY")
+
+    @JsName("NaN")
+    val NaN: Float = js("Number.NaN")
 }
 
-@JsName("IntCompanionObject") private object IntCompanionObject {
+@JsName("IntCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Int = -2147483647 - 1
+private object IntCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Int = 2147483647
+    @JsName("MIN_VALUE")
+    val MIN_VALUE: Int = -2147483647 - 1
+
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Int = 2147483647
 }
 
-@JsName("LongCompanionObject") private object LongCompanionObject {
+@JsName("LongCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Long = js("Kotlin.Long.MIN_VALUE")
+private object LongCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Long = js("Kotlin.Long.MAX_VALUE")
+    @JsName("MIN_VALUE")
+    val MIN_VALUE: Long = js("Kotlin.Long.MIN_VALUE")
+
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Long = js("Kotlin.Long.MAX_VALUE")
 }
 
-@JsName("ShortCompanionObject") private object ShortCompanionObject {
+@JsName("ShortCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Short = -32768
+private object ShortCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Short = 32767
+    @JsName("MIN_VALUE")
+    val MIN_VALUE: Short = -32768
+
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Short = 32767
 }
 
-@JsName("ByteCompanionObject") private object ByteCompanionObject {
+@JsName("ByteCompanionObject")
-    @JsName("MIN_VALUE") val MIN_VALUE: Byte = -128
+private object ByteCompanionObject {
-    @JsName("MAX_VALUE") val MAX_VALUE: Byte = 127
+    @JsName("MIN_VALUE")
+    val MIN_VALUE: Byte = -128
+
+    @JsName("MAX_VALUE")
+    val MAX_VALUE: Byte = 127
 }
 
-@JsName("CharCompanionObject") private object CharCompanionObject {
+@JsName("CharCompanionObject")
-    @JsName("MIN_HIGH_SURROGATE") public const val MIN_HIGH_SURROGATE: Char = '\uD800'
+private object CharCompanionObject {
-    @JsName("MAX_HIGH_SURROGATE") public const val MAX_HIGH_SURROGATE: Char = '\uDBFF'
+    @JsName("MIN_HIGH_SURROGATE")
-    @JsName("MIN_LOW_SURROGATE") public const val MIN_LOW_SURROGATE: Char = '\uDC00'
+    public const val MIN_HIGH_SURROGATE: Char = '\uD800'
-    @JsName("MAX_LOW_SURROGATE") public const val MAX_LOW_SURROGATE: Char = '\uDFFF'
+
-    @JsName("MIN_SURROGATE") public const val MIN_SURROGATE: Char = MIN_HIGH_SURROGATE
+    @JsName("MAX_HIGH_SURROGATE")
-    @JsName("MAX_SURROGATE") public const val MAX_SURROGATE: Char = MAX_LOW_SURROGATE
+    public const val MAX_HIGH_SURROGATE: Char = '\uDBFF'
+
+    @JsName("MIN_LOW_SURROGATE")
+    public const val MIN_LOW_SURROGATE: Char = '\uDC00'
+
+    @JsName("MAX_LOW_SURROGATE")
+    public const val MAX_LOW_SURROGATE: Char = '\uDFFF'
+
+    @JsName("MIN_SURROGATE")
+    public const val MIN_SURROGATE: Char = MIN_HIGH_SURROGATE
+
+    @JsName("MAX_SURROGATE")
+    public const val MAX_SURROGATE: Char = MAX_LOW_SURROGATE
 }
 
 private object StringCompanionObject {}
 
-private object EnumCompanionObject {}

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

@@ -142,7 +142,7 @@ public final class StaticContext {
     private final DeclarationExporter exporter = new DeclarationExporter(this);
 
     @NotNull
-    private final Set<ClassDescriptor> classes = new HashSet<ClassDescriptor>();
+    private final Set<ClassDescriptor> classes = new LinkedHashSet<ClassDescriptor>();
 
     //TODO: too many parameters in constructor
     private StaticContext(

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

@@ -576,12 +576,12 @@ public class TranslationContext {
     }
 
     @NotNull
-    public JsFunction createTopLevelFunction(@NotNull DeclarationDescriptor descriptor) {
+    public JsFunction createRootScopedFunction(@NotNull DeclarationDescriptor descriptor) {
-        return createTopLevelFunction(descriptor.toString());
+        return createRootScopedFunction(descriptor.toString());
     }
 
     @NotNull
-    public JsFunction createTopLevelFunction(@NotNull String description) {
+    public JsFunction createRootScopedFunction(@NotNull String description) {
         return new JsFunction(staticContext.getRootFunction().getScope(), new JsBlock(), description);
     }
 

js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt

@@ -74,7 +74,7 @@ class ClassTranslator private constructor(
         val scope = context().getScopeForDescriptor(descriptor)
         val context = context().newDeclaration(descriptor)
 
-        val constructorFunction = context.createTopLevelFunction(descriptor)
+        val constructorFunction = context.createRootScopedFunction(descriptor)
         constructorFunction.name = context.getInnerNameForDescriptor(descriptor)
         context.addDeclarationStatement(constructorFunction.makeStmt())
         val enumInitFunction = if (descriptor.kind == ClassKind.ENUM_CLASS) createEnumInitFunction() else null
@@ -141,9 +141,9 @@ class ClassTranslator private constructor(
     }
 
     private fun createEnumInitFunction(): JsFunction {
-        val function = context().createTopLevelFunction(descriptor)
+        val function = context().createRootScopedFunction(descriptor)
         function.name = context().createGlobalName(StaticContext.getSuggestedName(descriptor) + "_initFields")
-        val emptyFunction = context().createTopLevelFunction(descriptor)
+        val emptyFunction = context().createRootScopedFunction(descriptor)
         function.body.statements += JsAstUtils.assignment(JsAstUtils.pureFqn(function.name, null), emptyFunction).makeStmt()
         context().addDeclarationStatement(function.makeStmt())
         return function
@@ -419,7 +419,7 @@ class ClassTranslator private constructor(
     private fun addObjectMethods() {
         context().addDeclarationStatement(JsAstUtils.newVar(cachedInstanceName, JsLiteral.NULL))
 
-        val instanceFun = context().createTopLevelFunction("Instance function: " + descriptor)
+        val instanceFun = context().createRootScopedFunction("Instance function: " + descriptor)
         instanceFun.name = context().getNameForObjectInstance(descriptor)
 
         if (enumInitializerName == null) {

js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DeclarationBodyVisitor.kt

@@ -69,7 +69,7 @@ class DeclarationBodyVisitor(
             context.addDeclarationStatement(JsAstUtils.newVar(enumInstanceName, null))
             enumInitializer.body.statements += JsAstUtils.assignment(pureFqn(enumInstanceName, null), jsEnumEntryCreation).makeStmt()
 
-            val enumInstanceFunction = context.createTopLevelFunction(descriptor)
+            val enumInstanceFunction = context.createRootScopedFunction(descriptor)
             enumInstanceFunction.name = context.getNameForObjectInstance(descriptor)
             context.addDeclarationStatement(enumInstanceFunction.makeStmt())
 

js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/JsDataClassGenerator.java

@@ -189,7 +189,7 @@ class JsDataClassGenerator extends DataClassMethodGenerator {
     }
 
     private JsFunction generateJsMethod(@NotNull FunctionDescriptor functionDescriptor) {
-        JsFunction functionObject = context.createTopLevelFunction(functionDescriptor);
+        JsFunction functionObject = context.createRootScopedFunction(functionDescriptor);
         ClassDescriptor containingClass = (ClassDescriptor) functionDescriptor.getContainingDeclaration();
         UtilsKt.addFunctionToPrototype(context, containingClass, functionDescriptor, functionObject);
         return functionObject;

js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Translation.java

@@ -253,7 +253,7 @@ public final class Translation {
     ) {
         StaticContext staticContext = StaticContext.generateStaticContext(bindingTrace, config, moduleDescriptor);
         JsProgram program = staticContext.getProgram();
-        program.getRootScope().declareName("_");
+        JsName rootPackageName = program.getRootScope().declareName(Namer.getRootPackageName());
 
         JsFunction rootFunction = staticContext.getRootFunction();
         JsBlock rootBlock = rootFunction.getBody();
@@ -270,8 +270,6 @@ public final class Translation {
         }
 
         mayBeGenerateTests(files, config, rootBlock, context);
-
-        JsName rootPackageName = program.getRootScope().declareName(Namer.getRootPackageName());
         rootFunction.getParameters().add(new JsParameter((rootPackageName)));
 
         // Invoke function passing modules as arguments

js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/objects/objectsIntrinsics.kt

@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.translate.intrinsic.objects
 import com.google.dart.compiler.backend.js.ast.JsExpression
 import com.google.dart.compiler.backend.js.ast.JsName
 import org.jetbrains.kotlin.builtins.CompanionObjectMapping
+import org.jetbrains.kotlin.builtins.KotlinBuiltIns
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
 import org.jetbrains.kotlin.js.translate.context.Namer
 import org.jetbrains.kotlin.js.translate.context.StaticContext
@@ -26,6 +27,7 @@ import org.jetbrains.kotlin.js.translate.context.TranslationContext
 import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.name.Name
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe
 
 class DefaultClassObjectIntrinsic(val staticContext: StaticContext, val fqName: FqName): ObjectIntrinsic {
     private val innerName: JsName by lazy {
@@ -42,7 +44,11 @@ class ObjectIntrinsics(private val staticContext: StaticContext) {
     fun getIntrinsic(classDescriptor: ClassDescriptor) = cache.getOrPut(classDescriptor) { createIntrinsic(classDescriptor) }
 
     private fun createIntrinsic(classDescriptor: ClassDescriptor): ObjectIntrinsic {
-        if (!CompanionObjectMapping.isMappedIntrinsicCompanionObject(classDescriptor)) return NO_OBJECT_INTRINSIC
+        if (classDescriptor.fqNameUnsafe == KotlinBuiltIns.FQ_NAMES._enum ||
+            !CompanionObjectMapping.isMappedIntrinsicCompanionObject(classDescriptor)
+        ) {
+            return NO_OBJECT_INTRINSIC
+        }
 
         val containingDeclaration = classDescriptor.containingDeclaration
         val name = Name.identifier(containingDeclaration.name.asString() + "CompanionObject")

js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/CallableReferenceTranslator.kt

@@ -109,7 +109,7 @@ object CallableReferenceTranslator {
             ReferenceTranslator.translateAsValueReference(descriptor, context)
         }
 
-        val function = context.createTopLevelFunction(getter)
+        val function = context.createRootScopedFunction(getter)
         function.body.statements += JsReturn(expression)
 
         return function
@@ -121,7 +121,7 @@ object CallableReferenceTranslator {
             return context.getInnerReference(setter)
         }
 
-        val function = context.createTopLevelFunction(setter)
+        val function = context.createRootScopedFunction(setter)
         val valueParam = function.scope.declareFreshName("value")
         function.parameters += JsParameter(valueParam)
 

js/js.translator/src/org/jetbrains/kotlin/js/translate/reference/ReferenceTranslator.java

@@ -68,10 +68,7 @@ public final class ReferenceTranslator {
         }
 
         if (DescriptorUtils.isObject(descriptor) || DescriptorUtils.isEnumEntry(descriptor)) {
-            if (AnnotationsUtils.isNativeObject(descriptor)) {
+            if (!context.isFromCurrentModule(descriptor)) {
-                return context.getQualifiedReference(descriptor);
-            }
-            else if (!context.isFromCurrentModule(descriptor)) {
                 return getLazyReferenceToObject((ClassDescriptor) descriptor, context);
             }
             else {

js/js.translator/testData/box/closure/closureThisByUsingMethodFromParentClass.kt

@@ -2,8 +2,6 @@ package foo
 
 // HACKS
 
-@native
-const val ROOT = "Kotlin.modules.JS_TESTS"
 @native
 const val PATH_TO_F_CREATOR = "B\$far\$lambda"
 @native

js/js.translator/testData/box/native/nativePropertyWithCustomName.kt

@@ -1,12 +1,13 @@
 package foo
 
-internal val PACKAGE = "kotlin.modules.JS_TESTS.foo"
+// TODO: this feature is deprecated, remove this test when either @native annotation or its parameter get eliminated.
 
-internal @native @JsName("\"O\"") val foo: String = noImpl
+internal @native("\"O\"") val foo: String = noImpl
-internal @native @JsName("boo") val bar: String = noImpl
+internal @native("boo") val bar: String = noImpl
 
 internal class A
-internal fun proto(o: Any?): String = js("o.__proto__")
+internal @native("__proto__") val Any.proto: String get() = noImpl
+internal @native("__proto__") val A.proto: String get() = noImpl
 
 internal fun actual(foo: String, @native("boo") bar: String) = foo + bar
 internal fun expected(foo: String, boo: String) = foo + boo
@@ -23,9 +24,9 @@ fun box(): String {
 
     val a = A()
     val any: Any = a
-    val protoA = js("A.prototype")
+    val protoA = A::class.js.asDynamic().prototype
-    if (proto(a) != proto(any) || proto(a) != protoA)
+    if (a.proto != any.proto || a.proto != protoA)
-        return "a.proto != any.proto /*${proto(a) != proto(any)}*/ || a.proto != A.prototype /*${proto(a) != protoA}*/"
+        return "a.proto != any.proto /*${a.proto != any.proto}*/ || a.proto != A.prototype /*${a.proto != protoA}*/"
 
     return OK
 }

js/js.translator/testData/kotlin_lib_ecma5.js

@@ -363,7 +363,8 @@
                 propertyDescriptor.set.call(thisObject, value);
             }
             else if ("value" in propertyDescriptor) {
-                propertyDescriptor.value = value;
+                throw new Error("Assertion failed: Kotlin compiler should not generate simple JavaScript properties for overridable " +
+                                "Kotlin properties.");
             }
         }
         else {

jslib_files.xml

@@ -3,6 +3,7 @@
         <include name="native/kotlin/Iterator.kt"/>
         <include name="native/kotlin/Collections.kt"/>
         <include name="native/kotlin/CharSequence.kt"/>
+        <include name="native/kotlin/Annotation.kt"/>
         <include name="src/kotlin/annotation/Annotations.kt"/>
         <include name="src/kotlin/Function.kt"/>
         <include name="src/kotlin/Iterators.kt"/>