Fix #4499: Shorten internal module IDs in FewestModules mode

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

@@ -57,31 +57,36 @@ 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]
+    /* Internal ModuleIDs are simply generated with consecutive numbers.
+     * (earlier versions concatenated all the tags; which lead to too
+     * long names, see #4499).
+     */
+    val internalIDs = Iterator.from(0)
+      .map(i => ModuleID(s"internal-$i"))
+      .filterNot(publicIDs.contains(_)) // avoid module ID collisions.
 
     val tups = for {
       dynamicModules <- dynamicIDs.subsets()
       publicModules <- publicIDs.subsets()
       if publicModules.nonEmpty || dynamicModules.nonEmpty
     } yield {
-      var candidate = ModuleID((
-          publicModules.toList.map(_.id) ++
-          dynamicModules.toList.map(_.nameString)
-      ).mkString("-"))
-
-      while (seenIDs.contains(candidate))
-        candidate = ModuleID(candidate.id + "$")
-
-      seenIDs.add(candidate)
+      val moduleID = {
+        if (dynamicModules.isEmpty && publicModules.size == 1) {
+          /* Classes tagged with exactly a single public module, get assigned to
+           * that public module.
+           */
+          publicModules.head
+        } else {
+          // for all other tag combinations, we generate a new internal module
+          internalIDs.next()
+        }
+      }
 
-      (publicModules, dynamicModules) -> candidate
+      (publicModules, dynamicModules) -> moduleID
     }
 
     tups.toMap

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)
+    }
+  }
+}