Submodules can be private (#8581)

enso-org · Dec 19, 2023 · 4cb2439 · 4cb2439
1 parent 277dfb1
commit 4cb2439
Show file tree

Hide file tree

Showing 12 changed files with 68 additions and 53 deletions.
diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Internal/Array_Like_Helpers.enso
@@ -1,3 +1,5 @@
+private
+
 import project.Any.Any
 import project.Data.Array.Array
 import project.Data.Maybe.Maybe

diff --git a/docs/semantics/encapsulation.md b/docs/semantics/encapsulation.md
@@ -58,11 +58,11 @@ methods and constructors.
 Modules can be specified as private. Private modules cannot be imported from
 other projects. Private modules can be imported from the same project.
 
-A hierarchy of submodules cannot mix public and private modules. In other words,
-if a module is public, its whole subtree must be public as well. For example,
-having a public module `A` and private submodule `A.B` is forbidden and shall be
-reported as an error during compilation. But having a private module `A` as well
-as private module `A.B` is OK.
+A hierarchy of submodules can mix public and private modules. By _hierarchy_, we
+mean a parent-child relationship between modules. It does not make sense to
+create a public submodule of a private module and export it, but it is allowed.
+Note that this is because of current limitations of the implementation, this
+might be more strict in the future.
 
 ### Types
 

diff --git a/.../runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/.../runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java
@@ -65,6 +65,7 @@ public Module runModule(Module moduleIr, ModuleContext moduleContext) {
     List<Import> importErrors = new ArrayList<>();
     List<Export> exportErrors = new ArrayList<>();
     var isCurrentModulePrivate = moduleIr.isPrivate();
+    var isCurrentModuleSynthetic = moduleContext.isSynthetic();
 
     // Ensure that imported modules from a different project are not private.
     bindingsMap
@@ -97,38 +98,21 @@ public Module runModule(Module moduleIr, ModuleContext moduleContext) {
               ImportExport.apply$default$4()));
     }
 
-    // Ensure that private modules are not exported and that the hierarchy of submodules
-    // does not mix public and private modules.
+    // Ensure that private modules are not exported
     bindingsMap
         .getDirectlyExportedModules()
         .foreach(
             expModule -> {
               var expModuleRef = expModule.target().module().unsafeAsModule("should succeed");
-              if (expModuleRef.isPrivate()) {
+              if (expModuleRef.isPrivate() && !isCurrentModuleSynthetic) {
                 var associatedExportIR = findExportIRByName(moduleIr, expModuleRef.getName());
                 assert associatedExportIR.isDefined();
-                if (isSubmoduleName(moduleContext.getName(), expModuleRef.getName())) {
-                  var haveSameVisibility = isCurrentModulePrivate == expModuleRef.isPrivate();
-                  if (!haveSameVisibility) {
-                    exportErrors.add(
-                        ImportExport.apply(
-                            associatedExportIR.get(),
-                            new ImportExport.SubmoduleVisibilityMismatch(
-                                moduleContext.getName().toString(),
-                                expModuleRef.getName().toString(),
-                                isCurrentModulePrivate ? "private" : "public",
-                                expModuleRef.isPrivate() ? "private" : "public"),
-                            ImportExport.apply$default$3(),
-                            ImportExport.apply$default$4()));
-                  }
-                } else {
-                  exportErrors.add(
-                      ImportExport.apply(
-                          associatedExportIR.get(),
-                          new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()),
-                          ImportExport.apply$default$3(),
-                          ImportExport.apply$default$4()));
-                }
+                exportErrors.add(
+                    ImportExport.apply(
+                        associatedExportIR.get(),
+                        new ImportExport.ExportPrivateModule(expModuleRef.getName().toString()),
+                        ImportExport.apply$default$3(),
+                        ImportExport.apply$default$4()));
               }
               return null;
             });
@@ -152,14 +136,6 @@ public Module runModule(Module moduleIr, ModuleContext moduleContext) {
         moduleIr.id());
   }
 
-  private boolean isSubmoduleName(QualifiedName parentModName, QualifiedName subModName) {
-    if (subModName.getParent().isDefined()) {
-      return parentModName.item().equals(subModName.getParent().get().item());
-    } else {
-      return false;
-    }
-  }
-
   @Override
   public Expression runExpression(Expression ir, InlineContext inlineContext) {
     return ir;

diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_4/src/Main.enso b/engine/runtime/src/test/resources/Test_Private_Modules_4/src/Main.enso
@@ -1,7 +1,5 @@
 import project.Sub
 export project.Sub
 
-# Fails at compile time - cannot mix private and public modules in a module subtree.
-
 main =
     42
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_6/package.yaml b/engine/runtime/src/test/resources/Test_Private_Modules_6/package.yaml
@@ -0,0 +1,7 @@
+name: Test_Private_Modules_6
+license: APLv2
+enso-version: default
+version: "0.0.1"
+author: "Enso Team <[email protected]>"
+maintainer: "Enso Team <[email protected]>"
+prefer-local-libraries: true
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_6/src/Main.enso b/engine/runtime/src/test/resources/Test_Private_Modules_6/src/Main.enso
@@ -0,0 +1,5 @@
+import project.Sub.Priv
+
+main =
+    Priv.foo
+
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_6/src/Sub/Priv.enso b/engine/runtime/src/test/resources/Test_Private_Modules_6/src/Sub/Priv.enso
@@ -0,0 +1,6 @@
+# This is a private module, that is under a synthetic "Sub" module.
+# By default, synthetic modules are public.
+
+private
+
+foo = 42
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_7/package.yaml b/engine/runtime/src/test/resources/Test_Private_Modules_7/package.yaml
@@ -0,0 +1,7 @@
+name: Test_Private_Modules_7
+license: APLv2
+enso-version: default
+version: "0.0.1"
+author: "Enso Team <[email protected]>"
+maintainer: "Enso Team <[email protected]>"
+prefer-local-libraries: true
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Main.enso b/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Main.enso
@@ -0,0 +1,6 @@
+import project.Sub.Pub
+export project.Sub.Pub
+
+main =
+    Pub.foo
+
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Sub.enso b/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Sub.enso
@@ -0,0 +1,4 @@
+# This is a private parent module
+
+private
+
diff --git a/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Sub/Pub.enso b/engine/runtime/src/test/resources/Test_Private_Modules_7/src/Sub/Pub.enso
@@ -0,0 +1,2 @@
+
+foo = 42
diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/ImportsTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/ImportsTest.scala
@@ -226,7 +226,7 @@ class ImportsTest extends PackageTest {
     ).toString shouldEqual "42"
   }
 
-  "Private modules" should "be able to import non-private stuff" in {
+  "Private modules" should "be able to import non-private stuff from different project" in {
     evalTestProject(
       "Test_Private_Modules_2"
     ).toString shouldEqual "(Pub_Mod_Type.Value 42)"
@@ -243,13 +243,10 @@ class ImportsTest extends PackageTest {
     )
   }
 
-  "Private modules" should "not be able to mix private and public submodules" in {
-    val e = the[InterpreterException] thrownBy evalTestProject(
+  "Private modules" should "be able to mix private and public submodules" in {
+    evalTestProject(
       "Test_Private_Modules_4"
-    )
-    e.getMessage() should include(
-      "Cannot export submodule 'local.Test_Private_Modules_4.Sub.Priv_SubMod' of module 'local.Test_Private_Modules_4.Sub'"
-    )
+    ) shouldEqual 42
   }
 
   "Private module" should "be able to have only private submodules" in {
@@ -258,17 +255,22 @@ class ImportsTest extends PackageTest {
     ) shouldEqual 42
   }
 
-  "Private modules" should "be able to mix private and public submodules when private checks are disabled" in {
+  "Private modules" should "be able to import private modules from different project when private checks are disabled" in {
     evalTestProject(
-      "Test_Private_Modules_4",
+      "Test_Private_Modules_3",
       Map(RuntimeOptions.DISABLE_PRIVATE_CHECK -> "true")
+    ) shouldEqual "Success"
+  }
+
+  "Private modules" should "be able to have a private submodule under a public synthetic module" in {
+    evalTestProject(
+      "Test_Private_Modules_6"
     ) shouldEqual 42
   }
 
-  "Private modules" should "be able to import private modules from different project when private checks are disabled" in {
+  "Private modules" should "be able to have a public submodule under a private module" ignore {
     evalTestProject(
-      "Test_Private_Modules_3",
-      Map(RuntimeOptions.DISABLE_PRIVATE_CHECK -> "true")
-    ) shouldEqual "Success"
+      "Test_Private_Modules_7"
+    ) shouldEqual 42
   }
 }