JS: several bugfixes in incremental compilation

1. Fix exported packages sometimes being mixed up
2. Fix metadata losing package fragments sometimes
3. Don't serialize empty packages to .meta.js
4. Preserve order of package fragments in .meta.js
5. In IC tests, compare textual representation of metadata rather
   than binary representation

js/js.serializer/src/org/jetbrains/kotlin/serialization/js/KotlinJavascriptSerializationUtil.kt

@@ -39,7 +39,6 @@ import org.jetbrains.kotlin.utils.KotlinJavascriptMetadataUtils
 import java.io.ByteArrayInputStream
 import java.io.ByteArrayOutputStream
 import java.io.DataOutputStream
-import java.util.*
 import java.util.zip.GZIPInputStream
 import java.util.zip.GZIPOutputStream
 
@@ -109,7 +108,7 @@ object KotlinJavascriptSerializationUtil {
 
         for (fqName in getPackagesFqNames(module)) {
             val fragment = serializePackageFragment(bindingContext, module, fqName)
-            if (fragment.hasPackage() || fragment.class_Count > 0) {
+            if (!fragment.isEmpty()) {
                 builder.addPackageFragment(fragment)
             }
         }
@@ -212,13 +211,11 @@ object KotlinJavascriptSerializationUtil {
             module: ModuleDescriptor,
             languageVersionSettings: LanguageVersionSettings
     ): Map<String, ByteArray> {
-        val contentMap = hashMapOf<String, ByteArray>()
+        val contentMap = mutableMapOf<String, ByteArray>()
 
         for (fqName in getPackagesFqNames(module)) {
             val part = serializePackageFragment(bindingContext, module, fqName)
-            if (part.class_Count == 0 && part.`package`.let { packageProto ->
-                packageProto.functionCount == 0 && packageProto.propertyCount == 0 && packageProto.typeAliasCount == 0
-            }) continue
+            if (part.isEmpty()) continue
 
             val stream = ByteArrayOutputStream()
             with(DataOutputStream(stream)) {
@@ -236,6 +233,9 @@ object KotlinJavascriptSerializationUtil {
         return contentMap
     }
 
+    private fun ProtoBuf.PackageFragment.isEmpty(): Boolean =
+            class_Count == 0 && `package`.let { it.functionCount == 0 && it.propertyCount == 0 && it.typeAliasCount == 0 }
+
     fun serializeHeader(packageFqName: FqName?, languageVersionSettings: LanguageVersionSettings): JsProtoBuf.Header {
         val header = JsProtoBuf.Header.newBuilder()
 
@@ -253,7 +253,7 @@ object KotlinJavascriptSerializationUtil {
     }
 
     private fun getPackagesFqNames(module: ModuleDescriptor): Set<FqName> {
-        return HashSet<FqName>().apply {
+        return mutableSetOf<FqName>().apply {
             getSubPackagesFqNames(module.getPackage(FqName.ROOT), this)
             add(FqName.ROOT)
         }

js/js.serializer/src/org/jetbrains/kotlin/serialization/js/kotlinJavascriptPackageFragmentProvider.kt

@@ -17,13 +17,12 @@
 package org.jetbrains.kotlin.serialization.js
 
 import org.jetbrains.kotlin.contracts.ContractDeserializerImpl
-import org.jetbrains.kotlin.descriptors.ModuleDescriptor
-import org.jetbrains.kotlin.descriptors.NotFoundClasses
-import org.jetbrains.kotlin.descriptors.PackageFragmentProvider
-import org.jetbrains.kotlin.descriptors.PackageFragmentProviderImpl
+import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.descriptors.deserialization.PlatformDependentDeclarationFilter
+import org.jetbrains.kotlin.descriptors.impl.EmptyPackageFragmentDescriptor
 import org.jetbrains.kotlin.incremental.components.LookupTracker
 import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.parentOrNull
 import org.jetbrains.kotlin.serialization.ProtoBuf
 import org.jetbrains.kotlin.serialization.deserialization.*
 import org.jetbrains.kotlin.storage.StorageManager
@@ -36,12 +35,26 @@ fun createKotlinJavascriptPackageFragmentProvider(
         configuration: DeserializationConfiguration,
         lookupTracker: LookupTracker
 ): PackageFragmentProvider {
-    val packageFragments = packageFragmentProtos.mapNotNull { proto ->
+    val packageFragments: MutableList<PackageFragmentDescriptor> = packageFragmentProtos.mapNotNullTo(mutableListOf()) { proto ->
         proto.fqName?.let { fqName ->
             KotlinJavascriptPackageFragment(fqName, storageManager, module, proto, header, configuration)
         }
     }
 
+    // Generate empty PackageFragmentDescriptor instances for packages that aren't mentioned in compilation units directly.
+    // For example, if there's `package foo.bar` directive, we'll get only PackageFragmentDescriptor for `foo.bar`, but
+    // none for `foo`. Various descriptor/scope code relies on presence of such package fragments, and currently we
+    // don't know if it's possible to fix this.
+    // TODO: think about fixing issues in descriptors/scopes
+    val packageFqNames = packageFragmentProtos.mapNotNullTo(mutableSetOf()) { it.fqName }
+    for (packageFqName in packageFqNames.mapNotNull { it.parentOrNull() }) {
+        var ancestorFqName = packageFqName
+        while (!ancestorFqName.isRoot && packageFqNames.add(ancestorFqName)) {
+            packageFragments += EmptyPackageFragmentDescriptor(module, ancestorFqName)
+            ancestorFqName = ancestorFqName.parent()
+        }
+    }
+
     val provider = PackageFragmentProviderImpl(packageFragments)
 
     val notFoundClasses = NotFoundClasses(storageManager, module)
@@ -63,7 +76,7 @@ fun createKotlinJavascriptPackageFragmentProvider(
             platformDependentDeclarationFilter = PlatformDependentDeclarationFilter.NoPlatformDependent
     )
 
-    for (packageFragment in packageFragments) {
+    for (packageFragment in packageFragments.filterIsInstance<KotlinJavascriptPackageFragment>()) {
         packageFragment.components = components
     }
 

js/js.tests/build.gradle.kts

@@ -2,6 +2,7 @@
 apply { plugin("kotlin") }
 
 dependencies {
+    testCompile(protobufFull())
     testCompile(project(":compiler.tests-common"))
     testCompileOnly(project(":compiler:frontend"))
     testCompileOnly(project(":compiler:cli"))
@@ -19,6 +20,7 @@ dependencies {
     testRuntime(project(":compiler:backend-common"))
 //    testRuntime(ideaSdkDeps("*.jar"))
     testRuntime(commonDep("org.fusesource.jansi", "jansi"))
+    testCompile(projectTests(":kotlin-build-common"))
 }
 
 sourceSets {

js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt

@@ -56,6 +56,8 @@ import org.jetbrains.kotlin.js.util.TextOutputImpl
 import org.jetbrains.kotlin.psi.KtFile
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.KtPsiFactory
+import org.jetbrains.kotlin.serialization.DebugProtoBuf
+import org.jetbrains.kotlin.serialization.js.JsSerializerProtocol
 import org.jetbrains.kotlin.serialization.js.KotlinJavascriptSerializationUtil
 import org.jetbrains.kotlin.serialization.js.ModuleKind
 import org.jetbrains.kotlin.test.InTextDirectivesUtils
@@ -64,6 +66,7 @@ import org.jetbrains.kotlin.test.KotlinTestUtils.TestFileFactory
 import org.jetbrains.kotlin.test.KotlinTestWithEnvironment
 import org.jetbrains.kotlin.test.TargetBackend
 import org.jetbrains.kotlin.utils.DFS
+import org.jetbrains.kotlin.utils.KotlinJavascriptMetadata
 import org.jetbrains.kotlin.utils.KotlinJavascriptMetadataUtils
 import java.io.*
 import java.nio.charset.Charset
@@ -343,18 +346,30 @@ abstract class BasicBoxTest(
 
         val originalOutput = FileUtil.loadFile(outputFile)
         val recompiledOutput = removeRecompiledSuffix(FileUtil.loadFile(recompiledOutputFile))
-        TestCase.assertEquals("Output file changed after recompilation", originalOutput, recompiledOutput)
+        assertEquals("Output file changed after recompilation", originalOutput, recompiledOutput)
 
         val originalSourceMap = FileUtil.loadFile(File(outputFile.parentFile, outputFile.name + ".map"))
         val recompiledSourceMap = removeRecompiledSuffix(
                 FileUtil.loadFile(File(recompiledOutputFile.parentFile, recompiledOutputFile.name + ".map")))
-        TestCase.assertEquals("Source map file changed after recompilation", originalSourceMap, recompiledSourceMap)
+        assertEquals("Source map file changed after recompilation", originalSourceMap, recompiledSourceMap)
 
         if (multiModule) {
             val originalMetadata = FileUtil.loadFile(File(outputFile.parentFile, outputFile.nameWithoutExtension + ".meta.js"))
             val recompiledMetadata = removeRecompiledSuffix(
                     FileUtil.loadFile(File(recompiledOutputFile.parentFile, recompiledOutputFile.nameWithoutExtension + ".meta.js")))
-            TestCase.assertEquals("Metadata file changed after recompilation", originalMetadata, recompiledMetadata)
+            assertEquals("Metadata file changed after recompilation",
+                                  metadataAsString(originalMetadata, module.name),
+                                  metadataAsString(recompiledMetadata, module.name))
+        }
+    }
+
+    private fun metadataAsString(metadata: String, moduleName: String): String {
+        val containers = mutableListOf<KotlinJavascriptMetadata>()
+        KotlinJavascriptMetadataUtils.parseMetadata(metadata, containers)
+        val metadataParts = KotlinJavascriptSerializationUtil.readModuleAsProto(containers.single().body, moduleName).data.body
+        return metadataParts.joinToString("-----\n") {
+            val binary = it.toByteArray()
+            DebugProtoBuf.PackageFragment.parseFrom(binary, JsSerializerProtocol.extensionRegistry).toString()
         }
     }
 

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

@@ -3761,6 +3761,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest {
             doTest(fileName);
         }
 
+        @TestMetadata("packagesWithSameName.kt")
+        public void testPackagesWithSameName() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/incremental/packagesWithSameName.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("simple.kt")
         public void testSimple() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/incremental/simple.kt");

js/js.translator/src/org/jetbrains/kotlin/js/translate/general/Merger.kt

@@ -109,7 +109,7 @@ class Merger(private val rootFunction: JsFunction, val internalModuleName: JsNam
                 val exportedTag = statement.exportedTag
                 if (exportedTag != null && !exportedTags.add(exportedTag)) continue
             }
-            exportBlock.statements += nameMap.rename(statement)
+            exportBlock.statements += nameMap.rename(statement.deepCopy())
         }
     }
 

js/js.translator/testData/box/incremental/packagesWithSameName.kt

@@ -0,0 +1,31 @@
+// EXPECTED_REACHABLE_NODES: 1126
+// MODULE: lib
+// FILE: a.kt
+package a.p
+
+fun foo() = "foo"
+
+// FILE: b1.kt
+// RECOMPILE
+package b.p
+
+fun bar() = "bar"
+
+// FILE: b2.kt
+package b.p
+
+fun baz() = "baz"
+
+// MODULE: main(lib)
+// FILE: main.kt
+import a.p.*
+import b.p.*
+
+fun box(): String {
+    val r = foo() + bar() + baz()
+
+    if (r != "foobarbaz") return "fail: $r"
+
+    return "OK"
+}
+