Fix scala-js#4499: Shorten internal module names in FewestModules mode

linker/shared/src/main/scala/org/scalajs/linker/frontend/modulesplitter/MaxModuleAnalyzer.scala

@@ -57,34 +57,45 @@ private[modulesplitter] final class MaxModuleAnalyzer extends ModuleAnalyzer {
      *
      * We sort the ModuleIDs to not depend on map iteration order (or the
      * order of the input files).
-     *
-     * All of this is to avoid module ID collisions, for example with the
-     * following set of public modules: {`a`, `b`, `a-b`}.
      */
     val publicIDs = sortedSet(publicIDsUnordered)(Ordering.by[ModuleID, String](_.id))
     val dynamicIDs = sortedSet(dynamicIDsUnordered)
 
-    val seenIDs = mutable.Set.empty[ModuleID]
-
-    val tups = for {
-      dynamicModules <- dynamicIDs.subsets()
-      publicModules <- publicIDs.subsets()
-      if publicModules.nonEmpty || dynamicModules.nonEmpty
+    /* Classes tagged with exactly a single public module, get assigned to that
+     * public module.
+     */
+    val publicTups = for {
+      publicID <- publicIDs
     } yield {
-      var candidate = ModuleID((
-          publicModules.toList.map(_.id) ++
-          dynamicModules.toList.map(_.nameString)
-      ).mkString("-"))
+      (Set(publicID), Set.empty[ClassName]) -> publicID
+    }
 
-      while (seenIDs.contains(candidate))
-        candidate = ModuleID(candidate.id + "$")
+    /* For all other tag combinations, we generate a new internal module with a
+     * number (earlier versions concatenated all the tags; which lead to too
+     * long names, see #4499).
+     */
+    var nextID = 0
 
-      seenIDs.add(candidate)
+    @tailrec
+    def newInternalID(): ModuleID = {
+      val candidate = ModuleID(s"internal-$nextID")
+      nextID += 1
+      // Avoid module ID collisions by checking against public module IDs.
+      if (publicIDs.contains(candidate))
+        newInternalID()
+      else
+        candidate
+    }
 
-      (publicModules, dynamicModules) -> candidate
+    val internalTups = for {
+      dynamicModules <- dynamicIDs.subsets()
+      publicModules <- publicIDs.subsets()
+      if publicModules.size > 1 || dynamicModules.nonEmpty
+    } yield {
+      (publicModules, dynamicModules) -> newInternalID()
     }
 
-    tups.toMap
+    (publicTups ++ internalTups).toMap
   }
 
   private def sortedSet[T: Ordering](s: Iterable[T]): immutable.SortedSet[T] = {

linker/shared/src/test/scala/org/scalajs/linker/MaxModuleSplittingTest.scala

@@ -0,0 +1,69 @@
+/*
+ * Scala.js (https://www.scala-js.org/)
+ *
+ * Copyright EPFL.
+ *
+ * Licensed under Apache License 2.0
+ * (https://www.apache.org/licenses/LICENSE-2.0).
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+package org.scalajs.linker
+
+import scala.concurrent._
+
+import org.junit.Test
+import org.junit.Assert._
+
+import org.scalajs.ir.Names._
+import org.scalajs.ir.Trees._
+import org.scalajs.ir.Types._
+
+import org.scalajs.junit.async._
+
+import org.scalajs.linker.interface._
+import org.scalajs.linker.testutils.LinkingUtils._
+import org.scalajs.linker.testutils.TestIRBuilder._
+
+class MaxModuleSplittingTest {
+  import scala.concurrent.ExecutionContext.Implicits.global
+
+  /** Smoke test to ensure modules do not get merged too much. */
+  @Test
+  def avoidsCollisions(): AsyncResult = await {
+    val classDefs = Seq(
+        mainTestClassDef({
+            consoleLog(str("Hello World!"))
+        })
+    )
+
+    val expectedFiles = Set(
+        "internal-0.js", // public module
+        "internal-1.js", // public module
+        "internal-2.js"  // internal module, avoiding internal-0 and internal-1.
+    )
+
+    val linkerConfig = StandardConfig()
+      .withModuleKind(ModuleKind.ESModule)
+      .withSourceMap(false)
+
+    val mainInitializer =
+      ModuleInitializer.mainMethodWithArgs("Test", "main")
+
+    val moduleInitializers = List(
+      mainInitializer.withModuleID("internal-0"),
+      mainInitializer.withModuleID("internal-1"),
+    )
+
+    val outputDirectory = MemOutputDirectory()
+
+    for {
+      _ <- testLink(classDefs, moduleInitializers,
+          config = linkerConfig, output = outputDirectory)
+    } yield {
+      assertEquals(expectedFiles, outputDirectory.fileNames().toSet)
+    }
+  }
+}