Allow multiple exports of the same module (#3897)

Previously, when exporting the same module multiple times only the first statement would count and the rest would be discarded by the compiler.

This change allows for multiple exports of the same module e.g.,
```
export project.F1
from project.F1 export foo
```
Multiple exports may however lead to conflicts when combined with hiding names. Added logic in `ImportResolver` to detect such scenarios.

This fixes https://www.pivotaltracker.com/n/projects/2539304/stories/183092447

# Important Notes
Added a bunch of scenarios to simulate pos and neg results.

CHANGELOG.md

@@ -438,14 +438,15 @@
 - [Fix performance of method calls on polyglot arrays][3781]
 - [Improved support for static and non-static builtins][3791]
 - [Missing foreign language generates proper Enso error][3798]
+- [Connecting IGV 4 Enso with Engine sources][3810]
 - [Made Vector performance to be on par with Array][3811]
 - [Introduced IO Permission Contexts][3828]
 - [Accept Array-like object seamlessly in builtins][3817]
 - [Initialize Builtins at Native Image build time][3821]
-- [Add the `Self` keyword referring to current type][3844]
 - [Split Atom suggestion entry to Type and Constructor][3835]
-- [Connecting IGV 4 Enso with Engine sources][3810]
+- [Add the `Self` keyword referring to current type][3844]
 - [Support VCS for projects in Language Server][3851]
+- [Support multiple exports of the same module][3897]
 
 [3227]: https://github.com/enso-org/enso/pull/3227
 [3248]: https://github.com/enso-org/enso/pull/3248
@@ -502,14 +503,15 @@
 [3781]: https://github.com/enso-org/enso/pull/3781
 [3791]: https://github.com/enso-org/enso/pull/3791
 [3798]: https://github.com/enso-org/enso/pull/3798
+[3810]: https://github.com/enso-org/enso/pull/3810
 [3811]: https://github.com/enso-org/enso/pull/3811
-[3828]: https://github.com/enso-org/enso/pull/3828
 [3817]: https://github.com/enso-org/enso/pull/3817
 [3821]: https://github.com/enso-org/enso/pull/3821
-[3844]: https://github.com/enso-org/enso/pull/3844
+[3828]: https://github.com/enso-org/enso/pull/3828
 [3835]: https://github.com/enso-org/enso/pull/3835
-[3810]: https://github.com/enso-org/enso/pull/3810
+[3844]: https://github.com/enso-org/enso/pull/3844
 [3851]: https://github.com/enso-org/enso/pull/3851
+[3897]: https://github.com/enso-org/enso/pull/3897
 
 # Enso 2.0.0-alpha.18 (2021-10-12)
 

build.sbt

@@ -2100,8 +2100,8 @@ buildStdLib := Def.inputTaskDyn {
   val cmd: String = allStdBits.parsed
   val root: File  = engineDistributionRoot.value
   // Ensure that a complete distribution was built at least once.
-  // Becasuse of `if` in the sbt task definition and usage of `streams.value` one has to
+  // Because of `if` in the sbt task definition and usage of `streams.value` one has to
-  // delegate to another task defintion (sbt restriction).
+  // delegate to another task definition (sbt restriction).
   if ((root / "manifest.yaml").exists) {
     pkgStdLibInternal.toTask(cmd)
   } else buildEngineDistribution

engine/runtime/src/main/scala/org/enso/compiler/Compiler.scala

@@ -105,7 +105,11 @@ class Compiler(
     */
   def runImportsResolution(module: Module): List[Module] = {
     initialize()
-    importResolver.mapImports(module)
+    try {
+      importResolver.mapImports(module)
+    } catch {
+      case e: ImportResolver.HiddenNamesConflict => reportExportConflicts(e)
+    }
   }
 
   /** Processes the provided language sources, registering any bindings in the
@@ -368,7 +372,12 @@ class Compiler(
   }
 
   private def runImportsAndExportsResolution(module: Module): List[Module] = {
-    val importedModules = importResolver.mapImports(module)
+    val importedModules =
+      try {
+        importResolver.mapImports(module)
+      } catch {
+        case e: ImportResolver.HiddenNamesConflict => reportExportConflicts(e)
+      }
 
     val requiredModules =
       try { new ExportsResolution().run(importedModules) }
@@ -818,6 +827,16 @@ class Compiler(
     }
   }
 
+  private def reportExportConflicts(exception: Throwable): Nothing = {
+    if (context.isStrictErrors) {
+      context.getOut.println("Compiler encountered errors:")
+      context.getOut.println(exception.getMessage)
+      throw new CompilationAbortedException
+    } else {
+      throw exception
+    }
+  }
+
   /** Report the errors encountered when initializing the package repository.
     *
     * @param err the package repository error

engine/runtime/src/main/scala/org/enso/compiler/data/BindingsMap.scala

@@ -255,46 +255,48 @@ case class BindingsMap(
     *         as and any further symbol restrictions.
     */
   def getDirectlyExportedModules: List[ExportedModule] =
-    resolvedImports.collect { case ResolvedImport(_, Some(exp), mod) =>
+    resolvedImports.collect { case ResolvedImport(_, exports, mod) =>
-      val hidingEnsoProject =
+      exports.map { exp =>
-        SymbolRestriction.Hiding(Set(Generated.ensoProjectMethodName))
+        val hidingEnsoProject =
-      val restriction = if (exp.isAll) {
+          SymbolRestriction.Hiding(Set(Generated.ensoProjectMethodName))
-        val definedRestriction = if (exp.onlyNames.isDefined) {
+        val restriction = if (exp.isAll) {
-          SymbolRestriction.Only(
+          val definedRestriction = if (exp.onlyNames.isDefined) {
-            exp.onlyNames.get
+            SymbolRestriction.Only(
-              .map(name =>
+              exp.onlyNames.get
-                SymbolRestriction
+                .map(name =>
-                  .AllowedResolution(name.name.toLowerCase, None)
+                  SymbolRestriction
-              )
+                    .AllowedResolution(name.name.toLowerCase, None)
-              .toSet
+                )
-          )
+                .toSet
-        } else if (exp.hiddenNames.isDefined) {
+            )
-          SymbolRestriction.Hiding(
+          } else if (exp.hiddenNames.isDefined) {
-            exp.hiddenNames.get.map(_.name.toLowerCase).toSet
+            SymbolRestriction.Hiding(
+              exp.hiddenNames.get.map(_.name.toLowerCase).toSet
+            )
+          } else {
+            SymbolRestriction.All
+          }
+          SymbolRestriction.Intersect(
+            List(hidingEnsoProject, definedRestriction)
           )
         } else {
-          SymbolRestriction.All
+          SymbolRestriction.Only(
-        }
+            Set(
-        SymbolRestriction.Intersect(
+              SymbolRestriction.AllowedResolution(
-          List(hidingEnsoProject, definedRestriction)
+                exp.getSimpleName.name.toLowerCase,
-        )
+                Some(mod)
-      } else {
+              )
-        SymbolRestriction.Only(
-          Set(
-            SymbolRestriction.AllowedResolution(
-              exp.getSimpleName.name.toLowerCase,
-              Some(mod)
             )
           )
-        )
+        }
+        val rename = if (!exp.isAll) {
+          Some(exp.getSimpleName.name)
+        } else {
+          None
+        }
+        ExportedModule(mod, rename, restriction)
       }
-      val rename = if (!exp.isAll) {
+    }.flatten
-        Some(exp.getSimpleName.name)
-      } else {
-        None
-      }
-      ExportedModule(mod, rename, restriction)
-    }
 }
 
 object BindingsMap {
@@ -675,7 +677,7 @@ object BindingsMap {
     */
   case class ResolvedImport(
     importDef: IR.Module.Scope.Import.Module,
-    exports: Option[IR.Module.Scope.Export.Module],
+    exports: List[IR.Module.Scope.Export.Module],
     target: ImportTarget
   ) {
 

engine/runtime/src/main/scala/org/enso/compiler/phase/ImportResolver.scala

@@ -28,6 +28,7 @@ import scala.collection.mutable
   * @param compiler the compiler instance for the compiling context.
   */
 class ImportResolver(compiler: Compiler) {
+  import ImportResolver._
 
   /** Runs the import mapping logic.
     *
@@ -121,8 +122,65 @@ class ImportResolver(compiler: Compiler) {
   ): (IR.Module.Scope.Import, Option[BindingsMap.ResolvedImport]) = {
     val impName = imp.name.name
     val exp = module.exports
-      .collect { case ex: Export.Module => ex }
+      .collect { case ex: Export.Module if ex.name.name == impName => ex }
-      .find(_.name.name == impName)
+    val fromAllExports = exp.filter(_.isAll)
+    fromAllExports match {
+      case _ :: _ :: _ =>
+        // Detect potential conflicts when importing all and hiding names for the exports of the same module
+        val unqualifiedImports = fromAllExports.collect {
+          case e if e.onlyNames.isEmpty => e
+        }
+        val qualifiedImports = fromAllExports.collect {
+          case IR.Module.Scope.Export.Module(
+                _,
+                _,
+                _,
+                Some(onlyNames),
+                _,
+                _,
+                _,
+                _,
+                _
+              ) =>
+            onlyNames.map(_.name)
+        }
+        val importsWithHiddenNames = fromAllExports.collect {
+          case e @ IR.Module.Scope.Export.Module(
+                _,
+                _,
+                _,
+                _,
+                Some(hiddenNames),
+                _,
+                _,
+                _,
+                _
+              ) =>
+            (e, hiddenNames)
+        }
+        importsWithHiddenNames.foreach { case (e, hidden) =>
+          val unqualifiedConflicts = unqualifiedImports.filter(_ != e)
+          if (unqualifiedConflicts.nonEmpty) {
+            throw HiddenNamesShadowUnqualifiedExport(
+              e.name.name,
+              hidden.map(_.name)
+            )
+          }
+
+          val qualifiedConflicts =
+            qualifiedImports
+              .filter(_ != e)
+              .flatten
+              .intersect(hidden.map(_.name))
+          if (qualifiedConflicts.nonEmpty) {
+            throw HiddenNamesShadowQualifiedExport(
+              e.name.name,
+              qualifiedConflicts
+            )
+          }
+        }
+      case _ =>
+    }
     val libraryName = imp.name.parts match {
       case namespace :: name :: _ =>
         LibraryName(namespace.name, name.name)
@@ -175,3 +233,37 @@ class ImportResolver(compiler: Compiler) {
     }
   }
 }
+
+object ImportResolver {
+  trait HiddenNamesConflict {
+    def getMessage(): String
+  }
+
+  private case class HiddenNamesShadowUnqualifiedExport(
+    name: String,
+    hiddenNames: List[String]
+  ) extends Exception(
+        s"""Hidden '${hiddenNames.mkString(",")}' name${if (
+          hiddenNames.size == 1
+        ) ""
+        else
+          "s"} of the export module ${name} conflict${if (hiddenNames.size == 1)
+          "s"
+        else
+          ""} with the unqualified export"""
+      )
+      with HiddenNamesConflict
+
+  private case class HiddenNamesShadowQualifiedExport(
+    name: String,
+    conflict: List[String]
+  ) extends Exception(
+        s"""Hidden '${conflict.mkString(",")}' name${if (conflict.size == 1) ""
+        else
+          "s"} of the exported module ${name} conflict${if (conflict.size == 1)
+          "s"
+        else
+          ""} with the qualified export"""
+      )
+      with HiddenNamesConflict
+}

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_1/package.yaml

@@ -0,0 +1,6 @@
+name: Test_Multiple_Conflicting_Exports_1
+license: APLv2
+enso-version: default
+version: "0.0.1"
+author: "Enso Team <contact@enso.org>"
+maintainer: "Enso Team <contact@enso.org>"

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_1/src/F1.enso

@@ -0,0 +1,2 @@
+foo = 42
+bar = "z"

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_1/src/F2.enso

@@ -0,0 +1,5 @@
+import project.F1
+from project.F1 import foo
+export project.F1
+from project.F1 export foo
+from project.F1 export all hiding foo

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_1/src/Main.enso

@@ -0,0 +1,7 @@
+from Standard.Base import IO
+from project.F2 import all
+
+main =
+    IO.println F1.bar
+    IO.println foo
+

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_2/package.yaml

@@ -0,0 +1,6 @@
+name: Test_Multiple_Conflicting_Exports_2
+license: APLv2
+enso-version: default
+version: "0.0.1"
+author: "Enso Team <contact@enso.org>"
+maintainer: "Enso Team <contact@enso.org>"

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_2/src/F1.enso

@@ -0,0 +1,2 @@
+foo = 42
+bar = "z"

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_2/src/F2.enso

@@ -0,0 +1,6 @@
+import project.F1
+from project.F1 import foo
+export project.F1
+
+from project.F1 export all
+from project.F1 export all hiding bar

engine/runtime/src/test/resources/Test_Multiple_Conflicting_Exports_2/src/Main.enso

@@ -0,0 +1,7 @@
+from Standard.Base import IO
+from project.F2 import all
+
+main =
+    IO.println F1.bar
+    IO.println foo
+

engine/runtime/src/test/resources/Test_Multiple_Exports/package.yaml

@@ -0,0 +1,6 @@
+name: Test_Multiple_Exports
+license: APLv2
+enso-version: default
+version: "0.0.1"
+author: "Enso Team <contact@enso.org>"
+maintainer: "Enso Team <contact@enso.org>"

engine/runtime/src/test/resources/Test_Multiple_Exports/src/F1.enso

@@ -0,0 +1,2 @@
+foo = 42
+bar = "z"

engine/runtime/src/test/resources/Test_Multiple_Exports/src/F2.enso

@@ -0,0 +1,4 @@
+import project.F1
+from project.F1 import foo
+export project.F1
+from project.F1 export foo

engine/runtime/src/test/resources/Test_Multiple_Exports/src/Main.enso

@@ -0,0 +1,7 @@
+from Standard.Base import IO
+from project.F2 import all
+
+main =
+    IO.println F1.bar
+    IO.println foo
+    0

engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/ImportsTest.scala

@@ -88,14 +88,41 @@ class ImportsTest extends PackageTest {
   }
 
   "Compiler" should "detect name conflicts preventing users from importing submodules" in {
-    the[InterpreterException] thrownBy (evalTestProject(
+    the[InterpreterException] thrownBy evalTestProject(
       "TestSubmodulesNameConflict"
-    )) should have message "Method `c_mod_method` of C could not be found."
+    ) should have message "Method `c_mod_method` of C could not be found."
     val outLines = consumeOut
     outLines(2) should include
     "Declaration of type C shadows module local.TestSubmodulesNameConflict.A.B.C making it inaccessible via a qualified name."
   }
 
+  "Compiler" should "accept exports of the same module" in {
+    evalTestProject("Test_Multiple_Exports") shouldEqual 0
+    val outLines = consumeOut
+    outLines(0) shouldEqual "z"
+    outLines(1) shouldEqual "42"
+  }
+
+  "Compiler" should "reject qualified exports of the same module with conflicting hidden names" in {
+    the[InterpreterException] thrownBy evalTestProject(
+      "Test_Multiple_Conflicting_Exports_1"
+    ) should have message "Compilation aborted due to errors."
+    val outLines = consumeOut
+    outLines(
+      1
+    ) shouldEqual "Hidden 'foo' name of the exported module local.Test_Multiple_Conflicting_Exports_1.F1 conflicts with the qualified export"
+  }
+
+  "Compiler" should "reject unqualified exports of the same module with conflicting hidden names" in {
+    the[InterpreterException] thrownBy evalTestProject(
+      "Test_Multiple_Conflicting_Exports_2"
+    ) should have message "Compilation aborted due to errors."
+    val outLines = consumeOut
+    outLines(
+      1
+    ) shouldEqual "Hidden 'bar' name of the export module local.Test_Multiple_Conflicting_Exports_2.F1 conflicts with the unqualified export"
+  }
+
   "Constructors" should "be importable" in {
     evalTestProject("Test_Type_Imports").toString shouldEqual "(Some 10)"
   }