From 4db61210c06c9e3c2fa3ab045215ab23e26d288d Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Mon, 9 Oct 2023 11:48:43 +0200 Subject: [PATCH] Exporting non-existing symbols fails with compiler error (#7960) Adds a new compiler analysis pass that ensures that all the symbols exported via `export ...` or `from ... export ...` statements exist. If not, generates an IR Error. # Important Notes We already have such a compiler pass for the version with imports in [ImportSymbolAnalysis.scala](https://github.com/enso-org/enso/blob/develop/engine/runtime/src/main/scala/org/enso/compiler/pass/analyse/ImportSymbolAnalysis.scala) --- CHANGELOG.md | 4 +- .../lib/Standard/Base/0.0.0-dev/src/Main.enso | 1 - .../ir/expression/errors/ImportExport.scala | 4 +- .../pass/analyse/ExportSymbolAnalysis.java | 146 ++++++++++++++++ .../pass/analyse/PrivateModuleAnalysis.java | 6 +- .../main/scala/org/enso/compiler/Passes.scala | 3 +- .../org/enso/compiler/test/PassesTest.scala | 4 +- .../test/semantic/ImportExportTest.scala | 158 +++++++++++++++++- 8 files changed, 309 insertions(+), 17 deletions(-) create mode 100644 engine/runtime/src/main/java/org/enso/compiler/pass/analyse/ExportSymbolAnalysis.java diff --git a/CHANGELOG.md b/CHANGELOG.md index fc9a1a17c30..ed8b48e3c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -973,8 +973,9 @@ - [Always persist `TRACE` level logs to a file][7825] - [Downloadable VSCode extension][7861] - [New `project/status` route for reporting LS state][7801] -- [Add Enso-specific assertions][7883]) +- [Add Enso-specific assertions][7883] - [Modules can be `private`][7840] +- [Export of non-existing symbols results in error][7960] [3227]: https://github.com/enso-org/enso/pull/3227 [3248]: https://github.com/enso-org/enso/pull/3248 @@ -1121,6 +1122,7 @@ [7861]: https://github.com/enso-org/enso/pull/7861 [7883]: https://github.com/enso-org/enso/pull/7883 [7840]: https://github.com/enso-org/enso/pull/7840 +[7960]: https://github.com/enso-org/enso/pull/7960 # Enso 2.0.0-alpha.18 (2021-10-12) diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso index 20a90f6b285..0913d77afa9 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso @@ -181,7 +181,6 @@ from project.Data.Numbers export Float, Integer, Number from project.Data.Range.Extensions export all from project.Data.Statistics export all hiding to_moment_statistic, wrap_java_call, calculate_correlation_statistics, calculate_spearman_rank, calculate_correlation_statistics_matrix, compute_fold, empty_value, is_valid from project.Data.Text.Extensions export all -from project.Data.Time.Conversions export all from project.Errors.Problem_Behavior.Problem_Behavior export all from project.Function export all from project.Meta.Enso_Project export enso_project diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/ImportExport.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/ImportExport.scala index 9dadd4e27ed..9039ae7883c 100644 --- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/ImportExport.scala +++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/ImportExport.scala @@ -157,10 +157,10 @@ object ImportExport { case class SymbolDoesNotExist( symbolName: String, - moduleName: String + moduleOrTypeName: String ) extends Reason { override def message: String = - s"The symbol $symbolName (module or type) does not exist in module $moduleName." + s"The symbol $symbolName (module, type, or constructor) does not exist in $moduleOrTypeName." } case class NoSuchConstructor( diff --git a/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/ExportSymbolAnalysis.java b/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/ExportSymbolAnalysis.java new file mode 100644 index 00000000000..b99998d5663 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/ExportSymbolAnalysis.java @@ -0,0 +1,146 @@ +package org.enso.compiler.pass.analyse; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import org.enso.compiler.context.InlineContext; +import org.enso.compiler.context.ModuleContext; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.expression.errors.ImportExport; +import org.enso.compiler.core.ir.module.scope.Export; +import org.enso.compiler.data.BindingsMap; +import org.enso.compiler.pass.IRPass; +import org.enso.interpreter.util.ScalaConversions; +import scala.collection.immutable.Seq; +import scala.jdk.javaapi.CollectionConverters; + +/** + * This pass ensures that all the symbols that are exported exist. If not, an IR error is generated. + */ +public final class ExportSymbolAnalysis implements IRPass { + public static final ExportSymbolAnalysis INSTANCE = new ExportSymbolAnalysis(); + private static scala.collection.immutable.List precursorPasses; + private UUID uuid; + + private ExportSymbolAnalysis() {} + + @Override + public UUID key() { + return null; + } + + @Override + public void org$enso$compiler$pass$IRPass$_setter_$key_$eq(UUID v) { + this.uuid = v; + } + + @Override + public Seq precursorPasses() { + if (precursorPasses == null) { + List passes = List.of( + BindingAnalysis$.MODULE$, + ImportSymbolAnalysis$.MODULE$ + ); + precursorPasses = CollectionConverters.asScala(passes).toList(); + } + return precursorPasses; + } + + @Override + public Seq invalidatedPasses() { + return ScalaConversions.nil(); + } + + @Override + public Module runModule(Module moduleIr, ModuleContext moduleContext) { + List exportErrors = new ArrayList<>(); + var bindingsMap = (BindingsMap) moduleIr.passData().get(BindingAnalysis$.MODULE$).get(); + + moduleIr.exports().foreach(export -> switch (export) { + case Export.Module exportMod -> { + var exportNameParts = exportMod.name().parts(); + var symbolName = exportMod.name().parts().last(); + assert exportNameParts.size() > 1; + var moduleOrTypeName = exportNameParts.apply(exportNameParts.size() - 2); + var foundResolvedExp = findResolvedExportForIr(export, bindingsMap); + if (foundResolvedExp == null) { + exportErrors.add( + ImportExport.apply( + symbolName, + new ImportExport.SymbolDoesNotExist(symbolName.name(), moduleOrTypeName.name()), + ImportExport.apply$default$3(), + ImportExport.apply$default$4() + ) + ); + } else { + if (exportMod.onlyNames().isDefined()) { + assert exportMod.onlyNames().isDefined(); + var exportedSymbols = exportMod.onlyNames().get(); + exportedSymbols.foreach(exportedSymbol -> { + var foundSymbols = foundResolvedExp.target().findExportedSymbolsFor(exportedSymbol.name()); + if (foundSymbols.isEmpty()) { + exportErrors.add( + ImportExport.apply( + exportedSymbol, + new ImportExport.SymbolDoesNotExist(exportedSymbol.name(), moduleOrTypeName.name()), + ImportExport.apply$default$3(), + ImportExport.apply$default$4() + ) + ); + } + return null; + }); + } + } + yield null; + } + default -> export; + }); + + if (exportErrors.isEmpty()) { + return moduleIr; + } else { + return moduleIr.copy( + moduleIr.imports(), + CollectionConverters.asScala(exportErrors).toList(), + moduleIr.bindings(), + moduleIr.location(), + moduleIr.passData(), + moduleIr.diagnostics(), + moduleIr.id() + ); + } + } + + @Override + public Expression runExpression(Expression ir, InlineContext inlineContext) { + return ir; + } + + /** + * Finds a resolved export that corresponds to the export IR. + * @param exportIr Export IR that is being resolved + * @param bindingsMap Bindings map of the module that contains the export IR + * @return null if no resolved export was found, otherwise the resolved export + */ + private BindingsMap.ExportedModule findResolvedExportForIr(Export exportIr, BindingsMap bindingsMap) { + switch (exportIr) { + case Export.Module exportedModIr -> { + var exportedModName = exportedModIr.name().name(); + var foundResolvedExp = bindingsMap.resolvedExports().find(resolvedExport -> { + var resolvedExportName = resolvedExport.target().qualifiedName(); + return resolvedExportName.toString().equals(exportedModName); + }); + return foundResolvedExp.isEmpty() ? null : foundResolvedExp.get(); + } + default -> throw new IllegalStateException("Unexpected value: " + exportIr); + } + } + + @Override + public T updateMetadataInDuplicate(T sourceIr, T copyOfIr) { + return IRPass.super.updateMetadataInDuplicate(sourceIr, copyOfIr); + } +} diff --git a/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java b/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java index a29fdf7a3a9..98c27baaab2 100644 --- a/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java +++ b/engine/runtime/src/main/java/org/enso/compiler/pass/analyse/PrivateModuleAnalysis.java @@ -30,15 +30,11 @@ import scala.jdk.javaapi.CollectionConverters; * Inserts errors into imports/exports IRs if the above conditions are violated. */ public final class PrivateModuleAnalysis implements IRPass { - private static final PrivateModuleAnalysis singleton = new PrivateModuleAnalysis(); + public static final PrivateModuleAnalysis INSTANCE = new PrivateModuleAnalysis(); private UUID uuid; private PrivateModuleAnalysis() {} - public static PrivateModuleAnalysis getInstance() { - return singleton; - } - @Override public void org$enso$compiler$pass$IRPass$_setter_$key_$eq(UUID v) { this.uuid = v; diff --git a/engine/runtime/src/main/scala/org/enso/compiler/Passes.scala b/engine/runtime/src/main/scala/org/enso/compiler/Passes.scala index 4bb232b754d..fd6ff0e0dc3 100644 --- a/engine/runtime/src/main/scala/org/enso/compiler/Passes.scala +++ b/engine/runtime/src/main/scala/org/enso/compiler/Passes.scala @@ -49,7 +49,8 @@ class Passes( LambdaShorthandToLambda, ImportSymbolAnalysis, AmbiguousImportsAnalysis, - PrivateModuleAnalysis.getInstance(), + PrivateModuleAnalysis.INSTANCE, + ExportSymbolAnalysis.INSTANCE, ShadowedPatternFields, UnreachableMatchBranches, NestedPatternMatch, diff --git a/engine/runtime/src/test/scala/org/enso/compiler/test/PassesTest.scala b/engine/runtime/src/test/scala/org/enso/compiler/test/PassesTest.scala index 5ff2d5ffe34..658fa4d8173 100644 --- a/engine/runtime/src/test/scala/org/enso/compiler/test/PassesTest.scala +++ b/engine/runtime/src/test/scala/org/enso/compiler/test/PassesTest.scala @@ -9,6 +9,7 @@ import org.enso.compiler.pass.analyse.{ AliasAnalysis, AmbiguousImportsAnalysis, BindingAnalysis, + ExportSymbolAnalysis, ImportSymbolAnalysis, PrivateModuleAnalysis } @@ -61,7 +62,8 @@ class PassesTest extends CompilerTest { LambdaShorthandToLambda, ImportSymbolAnalysis, AmbiguousImportsAnalysis, - PrivateModuleAnalysis.getInstance(), + PrivateModuleAnalysis.INSTANCE, + ExportSymbolAnalysis.INSTANCE, ShadowedPatternFields, UnreachableMatchBranches, NestedPatternMatch, diff --git a/engine/runtime/src/test/scala/org/enso/compiler/test/semantic/ImportExportTest.scala b/engine/runtime/src/test/scala/org/enso/compiler/test/semantic/ImportExportTest.scala index c2b9ec8a706..e965729dff8 100644 --- a/engine/runtime/src/test/scala/org/enso/compiler/test/semantic/ImportExportTest.scala +++ b/engine/runtime/src/test/scala/org/enso/compiler/test/semantic/ImportExportTest.scala @@ -418,6 +418,147 @@ class ImportExportTest } } + "Exporting non-existing symbols" should { + "fail when exporting from current module" in { + val mainIr = + s""" + | + |from $namespace.$packageName.Main.Main_Type import Main_Constructor + |from $namespace.$packageName.Main.Main_Type export Main_Constructor, Non_Existing_Ctor + | + |type Main_Type + | Main_Constructor + |""".stripMargin + .createModule(packageQualifiedName.createChild("Main")) + .getIr + + mainIr.exports.size shouldEqual 1 + mainIr.exports.head.isInstanceOf[errors.ImportExport] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "Non_Existing_Ctor" + } + + "fail when exporting from other module" in { + """ + |# Empty + |""".stripMargin + .createModule(packageQualifiedName.createChild("A_Module")) + + val mainIr = + s""" + |import $namespace.$packageName.A_Module + |from $namespace.$packageName.A_Module export baz + |""".stripMargin + .createModule(packageQualifiedName.createChild("Main")) + .getIr + + val bindingsMap = mainIr.unwrapBindingMap + bindingsMap shouldNot be(null) + mainIr.exports.size shouldEqual 1 + mainIr.exports.head.isInstanceOf[errors.ImportExport] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "baz" + } + + "fail when exporting from type with `from`" in { + """ + |type A_Type + | A_Constructor + |""".stripMargin + .createModule(packageQualifiedName.createChild("A_Module")) + + val mainIr = + s""" + |import $namespace.$packageName.A_Module.A_Type + |from $namespace.$packageName.A_Module.A_Type export Non_Existing_Ctor + |""".stripMargin + .createModule(packageQualifiedName.createChild("Main")) + .getIr + + mainIr.exports.size shouldEqual 1 + mainIr.exports.head.isInstanceOf[errors.ImportExport] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "Non_Existing_Ctor" + } + + "fail when exporting from type" in { + """ + |type A_Type + | A_Constructor + |""".stripMargin + .createModule(packageQualifiedName.createChild("A_Module")) + + val mainIr = + s""" + |import $namespace.$packageName.A_Module.A_Type + |export $namespace.$packageName.A_Module.A_Type.FOO + |""".stripMargin + .createModule(packageQualifiedName.createChild("Main")) + .getIr + + mainIr.exports.size shouldEqual 1 + mainIr.exports.head.isInstanceOf[errors.ImportExport] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "FOO" + } + + "fail when exporting from module with `from`" in { + """ + |foo = 42 + |bar = 23 + |""".stripMargin + .createModule(packageQualifiedName.createChild("A_Module")) + + val mainIr = + s""" + |import $namespace.$packageName.A_Module + |from $namespace.$packageName.A_Module export foo, bar, baz + |""".stripMargin + .createModule(packageQualifiedName.createChild("Main")) + .getIr + + mainIr.exports.size shouldEqual 1 + mainIr.exports.head.isInstanceOf[errors.ImportExport] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.exports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "baz" + } + } + "Import resolution from another library honor Main" should { "resolve Api from Main" in { val mainIr = """ @@ -434,14 +575,14 @@ class ImportExportTest val in = mainIr.imports.head .asInstanceOf[Import.Module] - in.name.name.toString() should include("Test.Logical_Export.Main") - in.onlyNames.get.map(_.name.toString()) shouldEqual List("Api") + in.name.name should include("Test.Logical_Export.Main") + in.onlyNames.get.map(_.name) shouldEqual List("Api") val errors = mainIr.preorder.filter(x => x.isInstanceOf[Error]) errors.size shouldEqual 0 } - "don't expose Impl from Main" in { + "not expose Impl from Main" in { val mainIr = """ |from Test.Logical_Export import Impl | @@ -450,12 +591,17 @@ class ImportExportTest .createModule(packageQualifiedName.createChild("Main")) .getIr + mainIr.imports.size shouldEqual 1 + mainIr.imports.head.isInstanceOf[errors.ImportExport] shouldBe true mainIr.imports.head .asInstanceOf[errors.ImportExport] .reason - .message should include( - "The symbol Impl (module or type) does not exist in module Test.Logical_Export.Main." - ) + .isInstanceOf[errors.ImportExport.SymbolDoesNotExist] shouldBe true + mainIr.imports.head + .asInstanceOf[errors.ImportExport] + .reason + .asInstanceOf[errors.ImportExport.SymbolDoesNotExist] + .symbolName shouldEqual "Impl" } }