From b0d05a367a3d3b86c43694efff93761586c002fb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Thu, 30 Jan 2025 18:03:10 +0100 Subject: [PATCH 1/7] Prepare API generator for automatic check (#12175) * Add ApiModificationTest * Fix docs * Add DocsGenerateOrderTest * No signature is generated for empty modules. Or for a module that contains only imports. * Move some methods to DumpTestUtils * Implement BindingSorter * BindingSorter takes care of extension methods * DocsGenerate respects order of bindings * DocsEmitSignatures emits correct markdown format * No signatures for synthetic modules * Conversion methods are sorted after instance methods * Ensure generated docs dir has same structure as src dir * Update Signatures_Spec * Use ScalaConversions --- .../org/enso/compiler/docs/BindingSorter.java | 170 +++++++++++++ .../compiler/docs/DocsEmitSignatures.java | 16 +- .../org/enso/compiler/docs/DocsGenerate.java | 48 +++- .../dump/test/ApiModificationTest.java | 199 +++++++++++++++ .../compiler/dump/test/BindingSorterTest.java | 229 ++++++++++++++++++ .../dump/test/DocsGenerateOrderTest.java | 131 ++++++++++ .../compiler/dump/test/DocsGenerateTest.java | 122 +++++++--- .../compiler/dump/test/DumpTestUtils.java | 74 ++++++ .../enso/scala/wrapper/ScalaConversions.java | 4 + .../org/enso/test/utils/ProjectUtils.java | 2 +- test/Examples_Tests/data/signatures/Meta.md | 68 +++--- test/Examples_Tests/src/Signatures_Spec.enso | 2 +- 12 files changed, 981 insertions(+), 84 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/docs/BindingSorter.java create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/ApiModificationTest.java create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/BindingSorterTest.java create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateOrderTest.java create mode 100644 engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DumpTestUtils.java diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/BindingSorter.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/BindingSorter.java new file mode 100644 index 000000000000..d8ec947d0cf5 --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/BindingSorter.java @@ -0,0 +1,170 @@ +package org.enso.compiler.docs; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.module.scope.Definition; +import org.enso.compiler.core.ir.module.scope.Definition.Data; +import org.enso.compiler.core.ir.module.scope.Definition.Type; +import org.enso.compiler.core.ir.module.scope.definition.Method; +import scala.jdk.javaapi.CollectionConverters; + +/** + * Bindings are sorted to categories. Every category is sorted alphabetically. + * Categories are roughly: + * + */ +public final class BindingSorter { + private BindingSorter() {} + + /** + * Returns sorted list of bindings defined on the given {@code moduleIr}. + */ + public static List sortedBindings(Module moduleIr) { + var bindings = CollectionConverters.asJava(moduleIr.bindings()); + var comparator = new BindingComparator(moduleIr); + return bindings.stream().sorted(comparator).toList(); + } + + public static List sortConstructors(List constructors) { + var comparator = new ConstructorComparator(); + return constructors.stream().sorted(comparator).toList(); + } + + + private static int compareTypes(Type type1, Type type2) { + return type1.name().name().compareTo(type2.name().name()); + } + + + private static final class BindingComparator implements java.util.Comparator { + private final Module moduleIr; + private Set typeNames; + + private BindingComparator(Module moduleIr) { + this.moduleIr = moduleIr; + } + + @Override + public int compare(Definition def1, Definition def2) { + return switch (def1) { + case Method method1 when def2 instanceof Method methods -> + compareMethods(method1, methods); + case Type type1 when def2 instanceof Type type2 -> + compareTypes(type1, type2); + case Type type1 when def2 instanceof Method method2 -> compareTypeAndMethod(type1, method2); + case Method method1 when def2 instanceof Type type2 -> + -compareTypeAndMethod(type2, method1); + default -> throw new AssertionError("unexpected type " + def1.getClass()); + }; + } + + private int compareTypeAndMethod(Type type, Method method) { + if (method.typeName().isDefined()) { + if (isExtensionMethod(method)) { + return -1; + } + var typeName = type.name().name(); + var methodTypeName = method.typeName().get().name(); + if (typeName.equals(methodTypeName)) { + return -1; + } else { + return typeName.compareTo(methodTypeName); + } + } + return -1; + } + + + private int compareMethods(Method method1, Method method2) { + return switch (method1) { + case + Method.Explicit explicitMethod1 when method2 instanceof Method.Explicit explicitMethod2 -> { + if (explicitMethod1.isPrivate() != explicitMethod2.isPrivate()) { + if (explicitMethod1.isPrivate()) { + yield 1; + } else { + yield -1; + } + } + if (isExtensionMethod(explicitMethod1) != isExtensionMethod(explicitMethod2)) { + if (isExtensionMethod(explicitMethod1)) { + yield 1; + } else { + yield -1; + } + } + var type1 = explicitMethod1.methodReference().typePointer(); + var type2 = explicitMethod2.methodReference().typePointer(); + if (type1.isDefined() && type2.isDefined()) { + // Both methods are instance or static methods - compare by type name + var typeName1 = type1.get().name(); + var typeName2 = type2.get().name(); + if (typeName1.equals(typeName2)) { + // Methods are defined on the same type + yield explicitMethod1.methodName().name() + .compareTo(explicitMethod2.methodName().name()); + } else { + yield type1.get().name().compareTo(type2.get().name()); + } + } else if (type1.isDefined() && !type2.isDefined()) { + // Instance or static methods on types have precedence over module methods + yield -1; + } else if (!type1.isDefined() && type2.isDefined()) { + yield 1; + } + assert !type1.isDefined() && !type2.isDefined(); + yield explicitMethod1.methodName().name() + .compareTo(explicitMethod2.methodName().name()); + } + // Comparison of conversion methods is not supported. + case Method.Conversion conversion1 when method2 instanceof Method.Conversion conversion2 -> + 0; + case Method.Explicit explicit when method2 instanceof Method.Conversion -> -1; + case Method.Conversion conversion when method2 instanceof Method.Explicit -> 1; + default -> throw new AssertionError( + "Unexpected type: method1=%s, method2=%s".formatted(method1.getClass(), + method2.getClass())); + }; + } + + /** + * An extension method is a method that is defined on a type that is defined outside the + * current module. + */ + private boolean isExtensionMethod(Method method) { + if (method.typeName().isDefined()) { + var typeName = method.typeName().get().name(); + return !typeNamesInModule().contains(typeName); + } + return false; + } + + private Set typeNamesInModule() { + if (typeNames == null) { + typeNames = new HashSet<>(); + moduleIr.bindings().foreach(binding -> { + if (binding instanceof Definition.Type type) { + typeNames.add(type.name().name()); + } + return null; + }); + } + return typeNames; + } + } + + private static final class ConstructorComparator implements java.util.Comparator { + + @Override + public int compare(Data cons1, Data cons2) { + return cons1.name().name().compareTo(cons2.name().name()); + } + } +} diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsEmitSignatures.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsEmitSignatures.java index 2d8184b52787..80785d180d6b 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsEmitSignatures.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsEmitSignatures.java @@ -21,9 +21,13 @@ public boolean visitUnknown(IR ir, PrintWriter w) throws IOException { @Override public boolean visitModule(QualifiedName name, Module module, PrintWriter w) throws IOException { - w.println("## Enso Signatures 1.0"); - w.println("## module " + name); - return true; + if (isEmpty(module)) { + return false; + } else { + w.println("## Enso Signatures 1.0"); + w.println("## module " + name); + return true; + } } @Override @@ -31,6 +35,7 @@ public void visitMethod(Definition.Type t, Method.Explicit m, PrintWriter w) thr if (t != null) { w.append(" - "); } else { + w.append("- "); if (m.typeName().isDefined()) { var fqn = DocsUtils.toFqnOrSimpleName(m.typeName().get()); w.append(fqn + "."); @@ -43,6 +48,7 @@ public void visitMethod(Definition.Type t, Method.Explicit m, PrintWriter w) thr public void visitConversion(Method.Conversion c, PrintWriter w) throws IOException { assert c.typeName().isDefined() : "Conversions need type name: " + c; var fqn = DocsUtils.toFqnOrSimpleName(c.typeName().get()); + w.append("- "); w.append(fqn + "."); w.append(DocsVisit.toSignature(c)); w.append(" -> ").println(fqn); @@ -64,4 +70,8 @@ public void visitConstructor(Definition.Type t, Definition.Data d, PrintWriter w throws IOException { w.println(" - " + DocsVisit.toSignature(d)); } + + private static boolean isEmpty(Module mod) { + return mod.bindings().isEmpty() && mod.exports().isEmpty(); + } } diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsGenerate.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsGenerate.java index b1348e8e08c1..365343b66001 100644 --- a/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsGenerate.java +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/docs/DocsGenerate.java @@ -1,5 +1,8 @@ package org.enso.compiler.docs; +import static org.enso.scala.wrapper.ScalaConversions.asJava; +import static org.enso.scala.wrapper.ScalaConversions.asScala; + import java.io.IOException; import java.io.PrintWriter; import java.util.IdentityHashMap; @@ -10,8 +13,6 @@ import org.enso.compiler.core.ir.module.scope.definition.Method; import org.enso.filesystem.FileSystem; import org.enso.pkg.QualifiedName; -import scala.collection.immutable.Seq; -import scala.jdk.CollectionConverters; /** Generator of documentation for an Enso project. */ public final class DocsGenerate { @@ -32,28 +33,53 @@ public static File write( DocsVisit visitor, org.enso.pkg.Package pkg, Iterable modules) throws IOException { var fs = pkg.fileSystem(); - var docs = fs.getChild(pkg.root(), "docs"); - var api = fs.getChild(docs, "api"); - fs.createDirectories(api); + var apiDir = defaultOutputDir(pkg); + fs.createDirectories(apiDir); for (var module : modules) { + if (module.isSynthetic()) { + continue; + } var ir = module.getIr(); assert ir != null : "need IR for " + module; if (ir.isPrivate()) { continue; } var moduleName = module.getName(); - var dir = createPkg(fs, api, moduleName); + var dir = createDirs(fs, apiDir, stripNamespace(moduleName)); var md = fs.getChild(dir, moduleName.item() + ".md"); try (var mdWriter = fs.newBufferedWriter(md); var pw = new PrintWriter(mdWriter)) { visitModule(visitor, moduleName, ir, pw); } } + return apiDir; + } + + public static File defaultOutputDir(org.enso.pkg.Package pkg) { + var fs = pkg.fileSystem(); + var docs = fs.getChild(pkg.root(), "docs"); + var api = fs.getChild(docs, "api"); return api; } - private static File createPkg(FileSystem fs, File root, QualifiedName pkg) + /** + * Strips namespace part from the given qualified {@code name}. + * + * @param name + */ + private static QualifiedName stripNamespace(QualifiedName name) { + if (!name.isSimple()) { + var path = name.pathAsJava(); + assert path.size() >= 2; + var dropped = path.subList(2, path.size()); + return new QualifiedName(asScala(dropped), name.item()); + } else { + return name; + } + } + + private static File createDirs(FileSystem fs, File root, QualifiedName pkg) throws IOException { var dir = root; for (var item : pkg.pathAsJava()) { @@ -68,7 +94,7 @@ public static void visitModule( var dispatch = DocsDispatch.create(visitor, w); if (dispatch.dispatchModule(moduleName, ir)) { - var moduleBindings = asJava(ir.bindings()); + var moduleBindings = BindingSorter.sortedBindings(ir); var alreadyDispatched = new IdentityHashMap(); for (var b : moduleBindings) { if (alreadyDispatched.containsKey(b)) { @@ -77,7 +103,7 @@ public static void visitModule( switch (b) { case Definition.Type t -> { if (dispatch.dispatchType(t)) { - for (var d : asJava(t.members())) { + for (var d : BindingSorter.sortConstructors(asJava(t.members()))) { if (!d.isPrivate()) { dispatch.dispatchConstructor(t, d); } @@ -111,8 +137,4 @@ public static void visitModule( } } } - - private static Iterable asJava(Seq seq) { - return CollectionConverters.IterableHasAsJava(seq).asJava(); - } } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/ApiModificationTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/ApiModificationTest.java new file mode 100644 index 000000000000..04591dbfc9f2 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/ApiModificationTest.java @@ -0,0 +1,199 @@ +package org.enso.compiler.dump.test; + +import static org.enso.test.utils.ContextUtils.defaultContextBuilder; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +import java.io.IOException; +import java.util.function.BiConsumer; +import org.graalvm.polyglot.Context; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +/** Tests recognitions of API changes in Enso code. */ +public final class ApiModificationTest { + private static Context ctx; + + @BeforeClass + public static void initCtx() { + ctx = defaultContextBuilder().build(); + } + + @AfterClass + public static void disposeCtx() { + ctx.close(); + ctx = null; + } + + @Test + public void reorderingMethods_DoesNotModifyApi() throws IOException { + var prevSrc = """ + method_1 = 1 + method_2 = 2 + """; + var newSrc = """ + method_2 = 2 + method_1 = 1 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat( + "Signature not changed when reordering methods", prevSignature, is(newSignature)); + }); + } + + @Test + public void reorderingTypes_DoesNotModifyApi() throws IOException { + var prevSrc = """ + type Type_1 + type Type_2 + """; + var newSrc = """ + type Type_2 + type Type_1 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat( + "Signature not changed when reordering types", prevSignature, is(newSignature)); + }); + } + + @Test + public void reorderingTypeAndMethod_DoesNotModifyApi() throws IOException { + var prevSrc = """ + type Type + method = 1 + """; + var newSrc = """ + method = 1 + type Type + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat( + "Signature not changed when reordering type and method", + prevSignature, + is(newSignature)); + }); + } + + @Test + public void reorderingMethodsInType_DoesNotModifyApi() throws IOException { + var prevSrc = + """ + type Type + method_1 = 1 + method_2 = 2 + """; + var newSrc = + """ + type Type + method_2 = 2 + method_1 = 1 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat("Signature not changed when methods in type", prevSignature, is(newSignature)); + }); + } + + @Test + public void reorderingConstructorsInType_DoesNotModifyApi() throws IOException { + var prevSrc = """ + type Type + Cons_2 + Cons_1 + """; + var newSrc = """ + type Type + Cons_1 + Cons_2 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat( + "Signature not changed when reordering constructors in type", + prevSignature, + is(newSignature)); + }); + } + + @Test + public void addingMethod_ModifiesApi() throws IOException { + var prevSrc = """ + method_1 = 1 + """; + var newSrc = """ + method_1 = 1 + method_2 = 2 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat("Different signatures", prevSignature, is(not(newSignature))); + assertThat( + "No method_2 in prev signature", prevSignature, not(containsString("method_2"))); + assertThat("method_2 in new signature", newSignature, containsString("method_2")); + }); + } + + @Test + public void renamingMethod_ModifiesApi() throws IOException { + var prevSrc = """ + method = 1 + """; + var newSrc = """ + renamed_method = 2 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat("Different signatures", prevSignature, is(not(newSignature))); + }); + } + + @Test + public void removingMethod_ModifiesApi() throws IOException { + var prevSrc = """ + method_1 = 1 + method_2 = 2 + """; + var newSrc = """ + method_2 = 2 + """; + compareSignatures( + prevSrc, + newSrc, + (prevSignature, newSignature) -> { + assertThat("Different signatures", prevSignature, is(not(newSignature))); + assertThat(newSignature, not(containsString("method_1"))); + }); + } + + private static void compareSignatures( + String prevSource, String newSource, BiConsumer signatureComparator) + throws IOException { + var modName = "local.Proj.Main"; + var prevSignature = DumpTestUtils.generateSignatures(ctx, prevSource, modName); + var newSignature = DumpTestUtils.generateSignatures(ctx, newSource, modName); + assertThat("Signature was generated", prevSignature.isEmpty(), is(false)); + assertThat("Signature was generated", newSignature.isEmpty(), is(false)); + signatureComparator.accept(prevSignature, newSignature); + } +} diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/BindingSorterTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/BindingSorterTest.java new file mode 100644 index 000000000000..f04df23e55c6 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/BindingSorterTest.java @@ -0,0 +1,229 @@ +package org.enso.compiler.dump.test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.is; + +import java.util.Arrays; +import java.util.List; +import org.enso.compiler.core.ir.Empty; +import org.enso.compiler.core.ir.Expression; +import org.enso.compiler.core.ir.MetadataStorage; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.Name; +import org.enso.compiler.core.ir.Name.MethodReference; +import org.enso.compiler.core.ir.module.scope.Definition; +import org.enso.compiler.core.ir.module.scope.definition.Method; +import org.enso.compiler.docs.BindingSorter; +import org.enso.persist.Persistance.Reference; +import org.junit.Test; +import scala.Option; +import scala.jdk.javaapi.CollectionConverters; + +public final class BindingSorterTest { + @Test + public void compareTwoModuleMethods() { + var method1 = method(null, "method_a"); + var method2 = method(null, "method_b"); + var sorted = sortBindings(method2, method1); + assertThat("Nothing is dropped", sorted.size(), is(2)); + assertThat("method1 is first", sorted.get(0), is(method1)); + assertThat("method2 is second", sorted.get(1), is(method2)); + } + + @Test + public void comparePrivateAndPublicMethod() { + var publicMethod = method(null, "method_XXX", false, false); + var privMethod = method(null, "AAAA", false, true); + var sorted = sortBindings(privMethod, publicMethod); + assertThat(sorted.get(0), is(publicMethod)); + assertThat(sorted.get(1), is(privMethod)); + } + + @Test + public void compareTypeAndModuleMethod() { + var type = type("My_Type"); + var method = method(null, "AAA"); + var sorted = sortBindings(method, type); + assertThat(sorted.get(0), is(type)); + assertThat(sorted.get(1), is(method)); + } + + @Test + public void compareInstanceMethodAndType() { + var aType = type("A_Type"); + var aTypeMethod = method("A_Type", "method"); + var zType = type("Z_Type"); + var sorted = sortBindings(zType, aTypeMethod, aType); + var expected = List.of(aType, aTypeMethod, zType); + assertSameItems(expected, sorted); + } + + @Test + public void compareInstanceMethods_OnDifferentTypes() { + var method1 = method("A_Type", "method"); + var method2 = method("Z_Type", "method"); + var sorted = sortBindings(method2, method1); + assertThat(sorted.get(0), is(method1)); + assertThat(sorted.get(1), is(method2)); + } + + @Test + public void compareInstanceMethods_InSameType() { + var method1 = method("Type", "method_A"); + var method2 = method("Type", "method_B"); + var sorted = sortBindings(method2, method1); + assertThat(sorted.get(0), is(method1)); + assertThat(sorted.get(1), is(method2)); + } + + @Test + public void compareTypes() { + var type1 = type("AA_Type"); + var type2 = type("XX_Type"); + var sorted = sortBindings(type2, type1); + assertThat(sorted.get(0), is(type1)); + assertThat(sorted.get(1), is(type2)); + } + + @Test + public void compareConstructors_InSameType() { + var constructor1 = constructor("A_Cons"); + var constructor2 = constructor("B_Cons"); + var sorted = sortConstructors(constructor2, constructor1); + assertThat(sorted.get(0), is(constructor1)); + assertThat(sorted.get(1), is(constructor2)); + } + + @Test + public void compareInstanceMethodAndModuleMethod() { + var moduleMethod = method(null, "AA", false, false); + var type = type("My_Type"); + var instanceMethod = method("My_Type", "XX", false, false); + var sorted = sortBindings(instanceMethod, moduleMethod, type); + assertThat(sorted.get(0), is(type)); + assertThat(sorted.get(1), is(instanceMethod)); + assertThat(sorted.get(2), is(moduleMethod)); + } + + @Test + public void compareInstanceMethodAndExtensionMethod() { + var type = type("My_Type"); + var instanceMethod = method("My_Type", "XX"); + var extensionMethod = method("Any", "AA"); + var sorted = sortBindings(extensionMethod, instanceMethod, type); + assertThat(sorted.get(0), is(type)); + assertThat(sorted.get(1), is(instanceMethod)); + assertThat(sorted.get(2), is(extensionMethod)); + } + + @Test + public void compareInstanceMethodAndConversionMethod() { + var type = type("My_Type"); + var instanceMethod = method("My_Type", "AA"); + var conversionMethod = conversionMethod("My_Type", "Any"); + var sorted = sortBindings(conversionMethod, instanceMethod, type); + assertThat(sorted.get(0), is(type)); + assertThat(sorted.get(1), is(instanceMethod)); + assertThat(sorted.get(2), is(conversionMethod)); + } + + @Test + public void compareModuleMethodAndConversionMethod() { + var moduleMethod = method(null, "AA"); + var conversionMethod = conversionMethod("My_Type", "Any"); + var sorted = sortBindings(conversionMethod, moduleMethod); + assertThat(sorted.get(0), is(moduleMethod)); + assertThat(sorted.get(1), is(conversionMethod)); + } + + private static void assertSameItems(List expected, List actual) { + var expectedArr = expected.toArray(); + assertThat(actual, contains(expectedArr)); + } + + private static List sortBindings(Definition... items) { + var modIr = module(items); + return BindingSorter.sortedBindings(modIr); + } + + private static List sortConstructors(Definition.Data... items) { + return BindingSorter.sortConstructors(Arrays.stream(items).toList()); + } + + private static Module module(Definition... bindings) { + var bindingsList = Arrays.asList(bindings); + return new Module( + emptyScalaList(), + emptyScalaList(), + CollectionConverters.asScala(bindingsList).toList(), + false, + null, + new MetadataStorage()); + } + + private static Method.Explicit method(String typeName, String methodName) { + return method(typeName, methodName, false, false); + } + + private static Method.Explicit method( + String typeName, String methodName, boolean isStatic, boolean isPrivate) { + MethodReference methodRef; + if (typeName != null) { + methodRef = + new Name.MethodReference( + Option.apply(name(typeName, false)), + name(methodName, true), + null, + new MetadataStorage()); + } else { + methodRef = + new Name.MethodReference( + Option.empty(), name(methodName, true), null, new MetadataStorage()); + } + Reference bodyRef = Reference.of(empty()); + return new Method.Explicit( + methodRef, bodyRef, isStatic, isPrivate, false, null, new MetadataStorage()); + } + + private static Method.Conversion conversionMethod(String targetTypeName, String sourceTypeName) { + var methodRef = + new Name.MethodReference( + Option.apply(name(targetTypeName, false)), + name("from", true), + null, + new MetadataStorage()); + return new Method.Conversion( + methodRef, name(sourceTypeName, false), empty(), null, new MetadataStorage()); + } + + private static Name name(String nm, boolean isMethod) { + return new Name.Literal(nm, isMethod, null, Option.empty(), new MetadataStorage()); + } + + private static Definition.Data constructor(String name) { + return new Definition.Data( + name(name, false), emptyScalaList(), emptyScalaList(), false, null, new MetadataStorage()); + } + + private static Definition.Type type(String name, List constructors) { + return new Definition.Type( + name(name, false), + emptyScalaList(), + CollectionConverters.asScala(constructors).toList(), + null, + new MetadataStorage()); + } + + private static Definition.Type type(String name) { + return type(name, List.of()); + } + + private static Empty empty() { + return new Empty(null); + } + + private static scala.collection.immutable.List emptyScalaList() { + return scala.collection.immutable.List$.MODULE$.empty(); + } +} diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateOrderTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateOrderTest.java new file mode 100644 index 000000000000..7136345faf1b --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateOrderTest.java @@ -0,0 +1,131 @@ +package org.enso.compiler.dump.test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.List; +import org.enso.compiler.core.IR; +import org.enso.compiler.core.ir.Module; +import org.enso.compiler.core.ir.Name; +import org.enso.compiler.core.ir.module.scope.Definition.Data; +import org.enso.compiler.core.ir.module.scope.Definition.Type; +import org.enso.compiler.core.ir.module.scope.definition.Method.Conversion; +import org.enso.compiler.core.ir.module.scope.definition.Method.Explicit; +import org.enso.compiler.docs.DocsVisit; +import org.enso.pkg.QualifiedName; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** + * IR elements in a module should be visited in a specific order. This test checks that the order is + * correct. + */ +public class DocsGenerateOrderTest { + @ClassRule public static final TemporaryFolder TMP_DIR = new TemporaryFolder(); + + @Test + public void moduleElementsAreVisitedInCorrectOrder() throws IOException { + var projName = "Proj"; + var moduleName = "local." + projName + ".Main"; + var aTypeName = "A_Type"; + var bTypeName = "B_Type"; + var src = + """ + import Standard.Base.Any.Any + + b_module_method = 1 # 7 + + type B_Type # 5 + + Any.extension_method self = 3 # 8 + + type A_Type # 0 + B_Cons # 2 + A_Cons # 1 + b_method self = 1 # 4 + a_method self = 2 # 3 + + B_Type.from _:A_Type = 4 # 9 + a_module_method = 2 # 6 + """; + var projDir = TMP_DIR.newFolder(); + var timestampVisitor = new TimestampVisitor(); + DumpTestUtils.generateDocumentation(projDir.toPath(), "Proj", src, timestampVisitor); + var events = timestampVisitor.events; + List expectedEvents = + List.of( + new VisitedModule(moduleName), + new VisitedType(aTypeName), + new VisitedConstructor(aTypeName, "A_Cons"), + new VisitedConstructor(aTypeName, "B_Cons"), + new VisitedMethod(aTypeName, "a_method"), + new VisitedMethod(aTypeName, "b_method"), + new VisitedType(bTypeName), + new VisitedMethod(null, "a_module_method"), + new VisitedMethod(null, "b_module_method"), + new VisitedMethod(null, "extension_method"), + new VisitedConversion(bTypeName, aTypeName)); + var expectedEventsArr = expectedEvents.toArray(Event[]::new); + assertThat(events, contains(expectedEventsArr)); + } + + private static final class TimestampVisitor implements DocsVisit { + private final List events = new ArrayList<>(); + + @Override + public boolean visitModule(QualifiedName name, Module ir, PrintWriter writer) { + events.add(new VisitedModule(name.toString())); + return true; + } + + @Override + public boolean visitUnknown(IR ir, PrintWriter w) { + return true; + } + + @Override + public void visitMethod(Type t, Explicit m, PrintWriter writer) { + var typeName = t == null ? null : t.name().name(); + events.add(new VisitedMethod(typeName, m.methodName().name())); + } + + @Override + public void visitConversion(Conversion c, PrintWriter w) { + var targetTypeName = c.typeName().get().name(); + var sourceTypeName = ((Name.Literal) c.sourceTypeName()).name(); + events.add(new VisitedConversion(targetTypeName, sourceTypeName)); + } + + @Override + public boolean visitType(Type t, PrintWriter w) { + events.add(new VisitedType(t.name().name())); + return true; + } + + @Override + public void visitConstructor(Type t, Data d, PrintWriter w) { + events.add(new VisitedConstructor(t.name().name(), d.name().name())); + } + } + + /** All names are unqualified */ + private interface Event {} + + private record VisitedModule(String moduleName) implements Event {} + + /** + * @param typeName Can be null if this is a module method + * @param methodName + */ + private record VisitedMethod(String typeName, String methodName) implements Event {} + + private record VisitedConversion(String targetTypeName, String sourceTypeName) implements Event {} + + private record VisitedType(String typeName) implements Event {} + + private record VisitedConstructor(String typeName, String consName) implements Event {} +} diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateTest.java index ac20a576897c..e51c541a18dc 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DocsGenerateTest.java @@ -1,7 +1,10 @@ package org.enso.compiler.dump.test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -9,6 +12,7 @@ import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; +import java.util.Set; import org.enso.compiler.Compiler; import org.enso.compiler.core.IR; import org.enso.compiler.core.ir.Module; @@ -16,10 +20,12 @@ import org.enso.compiler.core.ir.module.scope.definition.Method; import org.enso.compiler.docs.DocsGenerate; import org.enso.compiler.docs.DocsVisit; +import org.enso.editions.LibraryName; import org.enso.interpreter.runtime.EnsoContext; import org.enso.pkg.QualifiedName; import org.enso.test.utils.ContextUtils; import org.enso.test.utils.ProjectUtils; +import org.enso.test.utils.SourceModule; import org.graalvm.polyglot.Context; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -225,6 +231,86 @@ public void privateAreHidden() throws Exception { assertEquals("No constructors", 0, v.visitConstructor.size()); } + @Test + public void noSignatureIsGenerated_ForEmptyModule() throws IOException { + var emptyCode = ""; + var modName = "local.Empty.Main"; + var sig = DumpTestUtils.generateSignatures(ctx, emptyCode, modName); + assertTrue("Empty signature for empty module", sig.isEmpty()); + } + + @Test + public void noSignatureIsGenerated_ForModuleContainingOnlyImports() throws IOException { + var codeWithImports = + """ + import Standard.Base.Any.Any + import Standard.Base.Data.Vector.Vector + """; + var modName = "local.Empty.Main"; + var sig = DumpTestUtils.generateSignatures(ctx, codeWithImports, modName); + assertTrue("Empty signature for module with only imports", sig.isEmpty()); + } + + @Test + public void generatedSignature_HasCorrectMarkdownFormat() throws IOException { + var code = + """ + from Standard.Base import all + + module_method = 42 + + type My_Type + Cons x + instance_method self = 42 + + My_Type.static_method = 42 + Any.extension_method = 42 + My_Type.from (that: Integer) = My_Type.Cons that + """; + var modName = "local.Proj.Main"; + var sig = DumpTestUtils.generateSignatures(ctx, code, modName); + sig.lines() + .forEach( + line -> { + assertThat( + "Is heading or a list item", + line, + anyOf(startsWith("#"), startsWith("-"), startsWith(" -"))); + }); + } + + @Test + public void generatedSignaturesForProject_HasSameDirectoryHierarchyAsSources() + throws IOException { + var projName = "Proj"; + var modules = + Set.of( + new SourceModule(QualifiedName.fromString("Main"), "main = 42"), + new SourceModule(QualifiedName.fromString("Subdir.Submodule"), "submodule = 42")); + var projDir = TEMP.newFolder(projName); + ProjectUtils.createProject(projName, modules, projDir.toPath()); + ProjectUtils.generateProjectDocs( + "api", + ContextUtils.defaultContextBuilder(), + projDir.toPath(), + ctx -> { + var ensoCtx = ContextUtils.leakContext(ctx); + var pkg = + ensoCtx + .getPackageRepository() + .getPackageForLibrary(LibraryName.apply("local", projName)); + assertThat(pkg.isDefined(), is(true)); + var signatureOutDir = DocsGenerate.defaultOutputDir(pkg.get()); + assertThat( + "Default output dir for signatures was created", signatureOutDir.exists(), is(true)); + var srcDir = pkg.get().sourceDir(); + assertThat(srcDir.resolve("Main.enso").exists(), is(true)); + assertThat(signatureOutDir.resolve("Main.md").exists(), is(true)); + assertThat(srcDir.resolve("Subdir").resolve("Submodule.enso").exists(), is(true)); + assertThat(signatureOutDir.resolve("Subdir").resolve("Submodule.md").exists(), is(true)); + }); + } + @Test public void vectorWithElements() throws Exception { var code = @@ -309,33 +395,10 @@ public void intersectionTypes() throws Exception { sig); } - private static void generateDocumentation(String name, String code, DocsVisit v) + private static void generateDocumentation(String projectName, String code, DocsVisit v) throws IOException { - var pathCalc = TEMP.newFolder(name); - ProjectUtils.createProject(name, code, pathCalc.toPath()); - ProjectUtils.generateProjectDocs( - "api", - ContextUtils.defaultContextBuilder(), - pathCalc.toPath(), - (context) -> { - var enso = ContextUtils.leakContext(context); - var modules = enso.getTopScope().getModules(); - var optMod = - modules.stream().filter(m -> m.getName().toString().contains(name)).findFirst(); - assertTrue( - "Found " + name + " in " + modules.stream().map(m -> m.getName()).toList(), - optMod.isPresent()); - var mod = optMod.get(); - assertEquals("local." + name + ".Main", mod.getName().toString()); - var ir = mod.getIr(); - assertNotNull("Ir for " + mod + " found", ir); - - try { - DocsGenerate.visitModule(v, mod.getName(), ir, null); - } catch (IOException e) { - throw raise(RuntimeException.class, e); - } - }); + var pathCalc = TEMP.newFolder(projectName); + DumpTestUtils.generateDocumentation(pathCalc.toPath(), projectName, code, v); } private static final class MockVisitor implements DocsVisit { @@ -383,10 +446,5 @@ public void visitConstructor(Definition.Type t, Definition.Data d, PrintWriter w } } - @SuppressWarnings("unchecked") - private static E raise(Class type, Exception t) throws E { - throw (E) t; - } - record TypeAnd(Definition.Type t, IRElement ir) {} } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DumpTestUtils.java b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DumpTestUtils.java new file mode 100644 index 000000000000..7af9812852a8 --- /dev/null +++ b/engine/runtime-integration-tests/src/test/java/org/enso/compiler/dump/test/DumpTestUtils.java @@ -0,0 +1,74 @@ +package org.enso.compiler.dump.test; + +import static org.enso.test.utils.ContextUtils.compileModule; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintWriter; +import java.nio.file.Path; +import org.enso.compiler.docs.DocsGenerate; +import org.enso.compiler.docs.DocsVisit; +import org.enso.pkg.QualifiedName; +import org.enso.test.utils.ContextUtils; +import org.enso.test.utils.ProjectUtils; +import org.graalvm.polyglot.Context; + +final class DumpTestUtils { + private DumpTestUtils() {} + + static void generateDocumentation(Path projDir, String projName, String code, DocsVisit v) + throws IOException { + ProjectUtils.createProject(projName, code, projDir); + ProjectUtils.generateProjectDocs( + "api", + ContextUtils.defaultContextBuilder(), + projDir, + (context) -> { + var enso = ContextUtils.leakContext(context); + var modules = enso.getTopScope().getModules(); + var optMod = + modules.stream().filter(m -> m.getName().toString().contains(projName)).findFirst(); + assertTrue( + "Found " + projName + " in " + modules.stream().map(m -> m.getName()).toList(), + optMod.isPresent()); + var mod = optMod.get(); + assertEquals("local." + projName + ".Main", mod.getName().toString()); + var ir = mod.getIr(); + assertNotNull("Ir for " + mod + " found", ir); + + try { + DocsGenerate.visitModule(v, mod.getName(), ir, null); + } catch (IOException e) { + throw raise(RuntimeException.class, e); + } + }); + } + + /** + * Returns generated signatures as string. Uses {@link org.enso.compiler.docs.DocsEmitSignatures} + * visitor. + * + * @param moduleSrc Source code of the module. + * @param modName FQN of the module. + * @return Signature string for the module. + */ + static String generateSignatures(Context context, String moduleSrc, String modName) + throws IOException { + var modIr = compileModule(context, moduleSrc, modName); + var sigGenerator = DocsVisit.createSignatures(); + var out = new ByteArrayOutputStream(); + var writer = new PrintWriter(out); + var modFqn = QualifiedName.fromString(modName); + DocsGenerate.visitModule(sigGenerator, modFqn, modIr, writer); + writer.flush(); + return out.toString(); + } + + @SuppressWarnings("unchecked") + private static E raise(Class type, Exception t) throws E { + throw (E) t; + } +} diff --git a/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java b/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java index a2020178888f..0abb5f19d513 100644 --- a/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java +++ b/lib/java/scala-libs-wrapper/src/main/java/org/enso/scala/wrapper/ScalaConversions.java @@ -32,6 +32,10 @@ public static List asJava(Seq list) { return CollectionConverters.asJava(list); } + public static scala.collection.immutable.List asScala(List list) { + return CollectionConverters.asScala(list).toList(); + } + @SuppressWarnings("unchecked") public static scala.collection.immutable.List nil() { return (scala.collection.immutable.List) scala.collection.immutable.Nil$.MODULE$; diff --git a/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java b/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java index 40704d9f6b30..fabf6eccc025 100644 --- a/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java +++ b/lib/java/test-utils/src/main/java/org/enso/test/utils/ProjectUtils.java @@ -111,7 +111,7 @@ public static void testProjectRun( /** * Tests running the project located in the given {@code projDir}. Is equal to running {@code enso - * --run }. + * --docs --in-project }. * * @param docsFormat format of the documentation to generate * @param ctxBuilder A context builder that might be initialized with some specific options. diff --git a/test/Examples_Tests/data/signatures/Meta.md b/test/Examples_Tests/data/signatures/Meta.md index c4f8ac6fe8f7..53e39d010ead 100644 --- a/test/Examples_Tests/data/signatures/Meta.md +++ b/test/Examples_Tests/data/signatures/Meta.md @@ -1,47 +1,47 @@ ## Enso Signatures 1.0 ## module Standard.Base.Meta -- type Type - - constructors self -> (Standard.Base.Data.Vector.Vector Standard.Base.Meta.Constructor) - - methods self -> Standard.Base.Data.Vector.Vector - - qualified_name self -> Standard.Base.Data.Text.Text - - name self -> Standard.Base.Data.Text.Text - - find qualified_name:Standard.Base.Data.Text.Text -> Standard.Base.Any.Any - type Atom - - value self -> Standard.Base.Any.Any - - fields self -> (Standard.Base.Data.Vector.Vector Standard.Base.Any.Any) - constructor self -> Standard.Base.Meta.Constructor + - fields self -> (Standard.Base.Data.Vector.Vector Standard.Base.Any.Any) + - value self -> Standard.Base.Any.Any - type Constructor - - value self -> Standard.Base.Function.Function + - declaring_type self -> Standard.Base.Meta.Type - fields self -> (Standard.Base.Data.Vector.Vector Standard.Base.Data.Text.Text) - name self -> Standard.Base.Data.Text.Text - new self fields:(Standard.Base.Data.Vector.Vector|Standard.Base.Data.Array.Array) -> Standard.Base.Any.Any - - declaring_type self -> Standard.Base.Meta.Type -- type Primitive - - value self -> Standard.Base.Any.Any -- type Unresolved_Symbol - - value self -> Standard.Base.Any.Any - - rename self new_name:Standard.Base.Data.Text.Text -> Standard.Base.Meta.Unresolved_Symbol - - name self -> Standard.Base.Data.Text.Text + - value self -> Standard.Base.Function.Function - type Error - value self -> Standard.Base.Any.Any -- type Polyglot - - value self -> Standard.Base.Any.Any - - get_language self -> Standard.Base.Meta.Language -Standard.Base.Any.Any.is_same_object_as self value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -Standard.Base.Any.Any.is_a self typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -Standard.Base.Error.Error.is_a self typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -atom_with_hole factory:Standard.Base.Any.Any -> Standard.Base.Any.Any -meta ~value:Standard.Base.Any.Any -> (Standard.Base.Meta.Atom|Standard.Base.Meta.Constructor|Standard.Base.Meta.Primitive|Standard.Base.Meta.Polyglot|Standard.Base.Meta.Unresolved_Symbol|Standard.Base.Meta.Error|Standard.Base.Meta.Type) -is_same_object value_1:Standard.Base.Any.Any value_2:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -is_a value:Standard.Base.Any.Any typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -type_of value:Standard.Base.Any.Any -> Standard.Base.Any.Any -get_annotation target:Standard.Base.Any.Any method:Standard.Base.Any.Any parameter_name:Standard.Base.Any.Any -> Standard.Base.Any.Any - type Language - Java - Unknown -is_atom_constructor ~value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -is_atom value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -is_error value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -is_type value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -is_polyglot value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean -Standard.Base.Meta.Type.from that:Standard.Base.Any.Any -> Standard.Base.Meta.Type +- type Polyglot + - get_language self -> Standard.Base.Meta.Language + - value self -> Standard.Base.Any.Any +- type Primitive + - value self -> Standard.Base.Any.Any +- type Type + - constructors self -> (Standard.Base.Data.Vector.Vector Standard.Base.Meta.Constructor) + - find qualified_name:Standard.Base.Data.Text.Text -> Standard.Base.Any.Any + - methods self -> Standard.Base.Data.Vector.Vector + - name self -> Standard.Base.Data.Text.Text + - qualified_name self -> Standard.Base.Data.Text.Text +- type Unresolved_Symbol + - name self -> Standard.Base.Data.Text.Text + - rename self new_name:Standard.Base.Data.Text.Text -> Standard.Base.Meta.Unresolved_Symbol + - value self -> Standard.Base.Any.Any +- atom_with_hole factory:Standard.Base.Any.Any -> Standard.Base.Any.Any +- get_annotation target:Standard.Base.Any.Any method:Standard.Base.Any.Any parameter_name:Standard.Base.Any.Any -> Standard.Base.Any.Any +- is_a value:Standard.Base.Any.Any typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_atom value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_atom_constructor ~value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_error value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_polyglot value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_same_object value_1:Standard.Base.Any.Any value_2:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- is_type value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- meta ~value:Standard.Base.Any.Any -> (Standard.Base.Meta.Atom|Standard.Base.Meta.Constructor|Standard.Base.Meta.Primitive|Standard.Base.Meta.Polyglot|Standard.Base.Meta.Unresolved_Symbol|Standard.Base.Meta.Error|Standard.Base.Meta.Type) +- type_of value:Standard.Base.Any.Any -> Standard.Base.Any.Any +- Standard.Base.Any.Any.is_a self typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- Standard.Base.Any.Any.is_same_object_as self value:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- Standard.Base.Error.Error.is_a self typ:Standard.Base.Any.Any -> Standard.Base.Data.Boolean.Boolean +- Standard.Base.Meta.Type.from that:Standard.Base.Any.Any -> Standard.Base.Meta.Type diff --git a/test/Examples_Tests/src/Signatures_Spec.enso b/test/Examples_Tests/src/Signatures_Spec.enso index 7218b58b511c..d78c83d855d7 100644 --- a/test/Examples_Tests/src/Signatures_Spec.enso +++ b/test/Examples_Tests/src/Signatures_Spec.enso @@ -8,7 +8,7 @@ add_specs suite_builder = suite_builder.group "Standard.Base Signature Checks" g setup = Api_Setup.create 'Standard' 'Base' [ "--docs=api" ] group_builder.specify "Standard.Base.Meta" <| - snapshot = setup.api_dir / "Standard" / "Base" / "Meta.md" + snapshot = setup.api_dir / "Meta.md" # Instructions to regenerate the expected snapshot can be found at # https://github.com/enso-org/enso/pull/12031#discussion_r1923133269 From 9daf16d7f3632c8a0b488b0c7724ca7378529ef0 Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Mon, 3 Feb 2025 12:28:45 +0300 Subject: [PATCH 2/7] GitHub Artifacts API update (#12210) * fix: Engine workflow remove uploading of native-image-args * feat: upload test results * misc: cleanup tests artifacts * feat: upload fbs-schema * misc: format * feat: upload ide artifacts * feat: ide cleanup * feat: ide download project-manager * feat: ide preserve file permissions * debug: attempt to fix yaml * debug: attempt 2 * debug: attempt 3 * debug: attempt 4 * fix: mkdir on Windows * update: wasm changed files * misc: cleanup package ide * remove: upload examples * fix: wasm lint * misc: cleanup * misc: cleanup test artifacts * update: release build * Revert "update: release build" This reverts commit 19dba26312f3d48e76b606544f8a35c88a64db5f. --- .github/workflows/ide-packaging-optional.yml | 24 ++- .github/workflows/ide-packaging.yml | 63 +++++- .github/workflows/wasm-changed-files.yml | 2 +- build_tools/build/examples/artifact.rs | 70 ------- build_tools/build/src/ci_gen/job.rs | 190 +++++++++++++----- build_tools/build/src/ci_gen/step.rs | 16 ++ build_tools/build/src/engine/context.rs | 79 +------- build_tools/build/src/engine/env.rs | 1 + build_tools/build/src/paths.rs | 8 - build_tools/build/src/project.rs | 26 --- build_tools/build/src/project/gui.rs | 1 + build_tools/build/src/project/ide.rs | 18 -- build_tools/build/src/project/wasm/test.rs | 2 +- build_tools/build/src/source.rs | 4 +- build_tools/ci_utils/src/actions/artifacts.rs | 2 +- .../src/actions/workflow/definition.rs | 3 +- build_tools/ci_utils/src/os/target.rs | 8 + build_tools/cli/src/lib.rs | 26 +-- 18 files changed, 264 insertions(+), 279 deletions(-) delete mode 100644 build_tools/build/examples/artifact.rs diff --git a/.github/workflows/ide-packaging-optional.yml b/.github/workflows/ide-packaging-optional.yml index 485bc4c0995d..8a8f1a1834b9 100644 --- a/.github/workflows/ide-packaging-optional.yml +++ b/.github/workflows/ide-packaging-optional.yml @@ -51,6 +51,15 @@ jobs: - run: ./run backend get env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Archive project-manager + run: tar -cvf project-manager.tar -C dist/backend . + - name: Upload project-manager + uses: actions/upload-artifact@v4 + with: + name: project-manager-macos + path: project-manager.tar + - name: Cleanup + run: rm project-manager.tar - if: "(always()) && (contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)" name: Clean after run: ./run git-clean @@ -145,7 +154,15 @@ jobs: run: ./run git-clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: ./run ide build --backend-source current-ci-run --gui-upload-artifact false + - name: Download project-manager + uses: actions/download-artifact@v4 + with: + name: project-manager-macos + path: dist/backend + - run: |- + tar -xvf dist/backend/project-manager.tar -C dist/backend + rm dist/backend/project-manager.tar + - run: ./run ide build --backend-source local --gui-upload-artifact false env: APPLEID: ${{ secrets.APPLE_NOTARIZATION_USERNAME }} APPLEIDPASS: ${{ secrets.APPLE_NOTARIZATION_PASSWORD }} @@ -181,6 +198,11 @@ jobs: compression-level: 0 name: test-traces-macos-amd64 path: app/ide-desktop/client/test-traces + - name: Upload ide + uses: actions/upload-artifact@v4 + with: + name: ide-macos + path: dist/ide/enso-*.dmg - run: rm $HOME/.enso/credentials env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ide-packaging.yml b/.github/workflows/ide-packaging.yml index 2f1b00dae956..4088c148e93a 100644 --- a/.github/workflows/ide-packaging.yml +++ b/.github/workflows/ide-packaging.yml @@ -51,6 +51,25 @@ jobs: - run: ./run backend get env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Upload Edition File + uses: actions/upload-artifact@v4 + with: + name: Edition File + path: distribution/editions/*.yaml + - name: Upload fbs-schema + uses: actions/upload-artifact@v4 + with: + name: fbs-schema + path: engine/language-server/src/main/schema/ + - name: Archive project-manager + run: tar -cvf project-manager.tar -C dist/backend . + - name: Upload project-manager + uses: actions/upload-artifact@v4 + with: + name: project-manager-linux + path: project-manager.tar + - name: Cleanup + run: rm project-manager.tar - if: "(always()) && (contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)" name: Clean after run: ./run git-clean @@ -92,6 +111,15 @@ jobs: - run: ./run backend get env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Archive project-manager + run: tar -cvf project-manager.tar -C dist/backend . + - name: Upload project-manager + uses: actions/upload-artifact@v4 + with: + name: project-manager-windows + path: project-manager.tar + - name: Cleanup + run: rm project-manager.tar - if: "(always()) && (contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)" name: Clean after run: ./run git-clean @@ -146,6 +174,11 @@ jobs: ENSO_IDE_SENTRY_DSN: ${{ vars.ENSO_CLOUD_SENTRY_DSN }} ENSO_IDE_STRIPE_KEY: ${{ vars.ENSO_CLOUD_STRIPE_KEY }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Upload gui + uses: actions/upload-artifact@v4 + with: + name: gui + path: dist/gui/ - if: "(always()) && (contains(github.event.pull_request.labels.*.name, 'CI: Clean build required') || inputs.clean_build_required)" name: Clean after run: ./run git-clean @@ -240,7 +273,15 @@ jobs: run: ./run git-clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: ./run ide build --backend-source current-ci-run --gui-upload-artifact false + - name: Download project-manager + uses: actions/download-artifact@v4 + with: + name: project-manager-linux + path: dist/backend + - run: |- + tar -xvf dist/backend/project-manager.tar -C dist/backend + rm dist/backend/project-manager.tar + - run: ./run ide build --backend-source local --gui-upload-artifact false env: ENSO_IDE_AG_GRID_LICENSE_KEY: ${{ vars.ENSO_AG_GRID_LICENSE_KEY }} ENSO_IDE_API_URL: ${{ vars.ENSO_CLOUD_API_URL }} @@ -270,6 +311,11 @@ jobs: compression-level: 0 name: test-traces-linux-amd64 path: app/ide-desktop/client/test-traces + - name: Upload ide + uses: actions/upload-artifact@v4 + with: + name: ide-linux + path: dist/ide/enso-*.AppImage - run: rm $HOME/.enso/credentials env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -313,7 +359,15 @@ jobs: run: ./run git-clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - run: ./run ide build --backend-source current-ci-run --gui-upload-artifact false + - name: Download project-manager + uses: actions/download-artifact@v4 + with: + name: project-manager-windows + path: dist/backend + - run: |- + tar -xvf dist/backend/project-manager.tar -C dist/backend + rm dist/backend/project-manager.tar + - run: ./run ide build --backend-source local --gui-upload-artifact false env: ENSO_IDE_AG_GRID_LICENSE_KEY: ${{ vars.ENSO_AG_GRID_LICENSE_KEY }} ENSO_IDE_API_URL: ${{ vars.ENSO_CLOUD_API_URL }} @@ -344,6 +398,11 @@ jobs: compression-level: 0 name: test-traces-windows-amd64 path: app/ide-desktop/client/test-traces + - name: Upload ide + uses: actions/upload-artifact@v4 + with: + name: ide-windows + path: dist/ide/enso-*.exe - run: rm $HOME/.enso/credentials env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/wasm-changed-files.yml b/.github/workflows/wasm-changed-files.yml index fdbf1aa2b345..6998435d8222 100644 --- a/.github/workflows/wasm-changed-files.yml +++ b/.github/workflows/wasm-changed-files.yml @@ -30,7 +30,7 @@ jobs: files: | .cargo/** app/rust-ffi/** - build/** + build_tools/** lib/rust/** tools/language-server/logstat/** tools/language-server/wstest/** diff --git a/build_tools/build/examples/artifact.rs b/build_tools/build/examples/artifact.rs deleted file mode 100644 index 83e295c0b473..000000000000 --- a/build_tools/build/examples/artifact.rs +++ /dev/null @@ -1,70 +0,0 @@ -use enso_build::prelude::*; - -use ide_ci::actions::artifacts; -use ide_ci::actions::artifacts::download::ArtifactDownloader; -use ide_ci::actions::artifacts::run_session::SessionClient; -use tempfile::TempDir; - - - -#[tokio::main] -async fn main() -> Result { - let dir = std::env::current_exe()?.parent().unwrap().to_owned(); - - debug!("Will upload {}", dir.display()); - let provider = artifacts::discover_recursive(dir); - artifacts::upload(provider, "MyPrecious", default()).await?; - - - let file = std::env::current_exe()?; - debug!("Will upload {}", file.display()); - let artifact_name = file.file_name().unwrap().to_str().unwrap(); - let provider = artifacts::single_file_provider(file.clone())?; - artifacts::upload(provider, artifact_name, default()).await?; - debug!("Upload done!"); - // artifacts::upload_single_file(file, ) - - let context = artifacts::context::Context::new_from_env()?; - let session = SessionClient::new(&context)?; - - // debug!("Checking artifacts through official API"); - // let octocrab = setup_octocrab()?; - // let run_id = ide_ci::actions::env::run_id()?; - // debug!("Got run ID {run_id}"); - // let run = octocrab.workflows("enso-org", "ci-build").get(run_id).await?; - // debug!("Got run information {run:?}"); - // let artifacts = octocrab - // .actions() - // .list_workflow_run_artifacts("enso-org", "ci-build", run_id) - // .send() - // .await?; - // debug!("Got artifacts information {artifacts:?}"); - - - debug!("Checking artifacts through runtime API"); - let list = session.list_artifacts().await?; - dbg!(&list); - - let relevant_entry = list - .iter() - .find(|artifact| artifact.name == artifact_name) - .ok_or_else(|| anyhow!("Failed to find artifact by name {artifact_name}."))?; - - dbg!(&relevant_entry); - - let items = artifacts::raw::endpoints::get_container_items( - &context.json_client()?, - relevant_entry.file_container_resource_url.clone(), - &relevant_entry.name, - ) - .await?; - dbg!(&items); - - let temp = TempDir::new()?; - let downloader = ArtifactDownloader::new(session.clone(), artifact_name).await?; - downloader.download_all_to(temp.path()).await?; - - let expected_path = temp.path().join(artifact_name); - assert_eq!(std::fs::read(expected_path)?, std::fs::read(&file)?); - Ok(()) -} diff --git a/build_tools/build/src/ci_gen/job.rs b/build_tools/build/src/ci_gen/job.rs index aae178f0be32..51fa29946fc2 100644 --- a/build_tools/build/src/ci_gen/job.rs +++ b/build_tools/build/src/ci_gen/job.rs @@ -11,6 +11,7 @@ use crate::ci_gen::RunnerType; use crate::ci_gen::RELEASE_CLEANING_POLICY; use crate::engine::env as engine_env; use crate::ide::web::env as ide_env; +use crate::paths; use core::panic; use ide_ci::actions::workflow::definition::cancel_workflow_action; @@ -222,6 +223,14 @@ impl JobArchetype for JvmTests { fn job(&self, target: Target) -> Job { let graal_edition = self.graal_edition; let job_name = format!("JVM Tests ({graal_edition})"); + let test_results_dir = paths::ENSO_TEST_JUNIT_DIR + .get() + .ok() + .and_then(|buf| buf.to_str().map(|s| s.to_owned())) + .unwrap_or_else(|| "target/test-results/".to_owned()); + let _upload_artifact_job = step::upload_artifact("Upload test results") + .with_custom_argument("name", format!("Test_Results_{}", target.0)) + .with_custom_argument("path", test_results_dir); let mut job = RunStepsBuilder::new("backend test jvm") .customize(move |step| vec![step, step::engine_test_reporter(target, graal_edition)]) .build_job(job_name, target) @@ -444,7 +453,18 @@ impl JobArchetype for GuiBuild { fn job(&self, target: Target) -> Job { let command: &str = "gui build"; RunStepsBuilder::new(command) - .customize(|step| vec![expose_gui_vars(step)]) + .customize(move |step| { + let mut steps = vec![expose_gui_vars(step)]; + + if target.0 == OS::Linux { + let upload_gui = step::upload_artifact("Upload gui") + .with_custom_argument("name", "gui") + .with_custom_argument("path", "dist/gui/"); + steps.push(upload_gui); + } + + steps + }) .build_job("GUI build", target) } } @@ -465,7 +485,44 @@ pub struct BuildBackend; impl JobArchetype for BuildBackend { fn job(&self, target: Target) -> Job { - plain_job(target, "Build Backend", "backend get") + RunStepsBuilder::new("backend get") + .customize(move |step| { + let mut steps = vec![step]; + + if target.0 == OS::Linux { + let upload_edition_file = step::upload_artifact("Upload Edition File") + .with_custom_argument("name", paths::EDITION_FILE_ARTIFACT_NAME) + .with_custom_argument("path", "distribution/editions/*.yaml"); + steps.push(upload_edition_file); + + let upload_fbs_schema = step::upload_artifact("Upload fbs-schema") + .with_custom_argument("name", "fbs-schema") + .with_custom_argument("path", "engine/language-server/src/main/schema/"); + steps.push(upload_fbs_schema) + } + + let archive_project_manager = Step { + name: Some("Archive project-manager".into()), + run: Some("tar -cvf project-manager.tar -C dist/backend .".into()), + ..Default::default() + }; + steps.push(archive_project_manager); + + let upload_project_manager = step::upload_artifact("Upload project-manager") + .with_custom_argument("name", format!("project-manager-{}", target.0)) + .with_custom_argument("path", "project-manager.tar"); + steps.push(upload_project_manager); + + let cleanup = Step { + name: Some("Cleanup".into()), + run: Some("rm project-manager.tar".into()), + ..Default::default() + }; + steps.push(cleanup); + + steps + }) + .build_job("Build Backend", target) } } @@ -610,56 +667,85 @@ pub struct PackageIde; impl JobArchetype for PackageIde { fn job(&self, target: Target) -> Job { - RunStepsBuilder::new( - "ide build --backend-source current-ci-run --gui-upload-artifact false", - ) - .customize(move |step| { - let mut steps = prepare_packaging_steps(target.0, step, PackagingTarget::Development); - const TEST_COMMAND: &str = "corepack pnpm -r --filter enso exec playwright test"; - let test_step = match target.0 { - OS::Linux => shell(format!("xvfb-run {TEST_COMMAND}")) - // See https://askubuntu.com/questions/1512287/obsidian-appimage-the-suid-sandbox-helper-binary-was-found-but-is-not-configu - .with_env("ENSO_TEST_APP_ARGS", "--no-sandbox"), - - OS::MacOS => - // MacOS CI runners are very slow - shell(format!("{TEST_COMMAND} --timeout 300000")), - _ => shell(TEST_COMMAND), - }; - let test_step = test_step - .with_env("DEBUG", "pw:browser log:") - .with_secret_exposed_as(secret::ENSO_CLOUD_TEST_ACCOUNT_USERNAME, "ENSO_TEST_USER") - .with_secret_exposed_as( - secret::ENSO_CLOUD_TEST_ACCOUNT_PASSWORD, - "ENSO_TEST_USER_PASSWORD", - ); - steps.push(test_step); - - let upload_test_traces_step = Step { - r#if: Some("failure()".into()), - name: Some("Upload Test Traces".into()), - uses: Some("actions/upload-artifact@v4".into()), - with: Some(Argument::Other(BTreeMap::from_iter([ - ("name".into(), format!("test-traces-{}-{}", target.0, target.1).into()), - ("path".into(), "app/ide-desktop/client/test-traces".into()), - ("compression-level".into(), 0.into()), // The traces are in zip already. - ]))), - ..Default::default() - }; - steps.push(upload_test_traces_step); - - // After the E2E tests run, they create a credentials file in user home directory. - // If that file is not cleaned up, future runs of our tests may randomly get - // authenticated into Enso Cloud. We want to run tests as an authenticated - // user only when we explicitly set that up, not randomly. So we clean the credentials - // file. - let cloud_credentials_path = "$HOME/.enso/credentials"; - let cleanup_credentials_step = shell(format!("rm {cloud_credentials_path}")); - steps.push(cleanup_credentials_step); - - steps - }) - .build_job("Package New IDE", target) + RunStepsBuilder::new("ide build --backend-source local --gui-upload-artifact false") + .customize(move |step| { + let mut steps = vec![]; + + let download_project_manager = step::download_artifact("Download project-manager") + .with_custom_argument("name", format!("project-manager-{}", target.0)) + .with_custom_argument("path", "dist/backend"); + steps.push(download_project_manager); + + let unpack_project_manager = Step { + run: Some( + "tar -xvf dist/backend/project-manager.tar -C dist/backend +rm dist/backend/project-manager.tar" + .into(), + ), + ..Default::default() + }; + steps.push(unpack_project_manager); + + let mut packaging_steps = + prepare_packaging_steps(target.0, step, PackagingTarget::Development); + steps.append(&mut packaging_steps); + + const TEST_COMMAND: &str = "corepack pnpm -r --filter enso exec playwright test"; + let test_step = match target.0 { + OS::Linux => shell(format!("xvfb-run {TEST_COMMAND}")) + // See https://askubuntu.com/questions/1512287/obsidian-appimage-the-suid-sandbox-helper-binary-was-found-but-is-not-configu + .with_env("ENSO_TEST_APP_ARGS", "--no-sandbox"), + + OS::MacOS => + // MacOS CI runners are very slow + shell(format!("{TEST_COMMAND} --timeout 300000")), + _ => shell(TEST_COMMAND), + }; + let test_step = test_step + .with_env("DEBUG", "pw:browser log:") + .with_secret_exposed_as( + secret::ENSO_CLOUD_TEST_ACCOUNT_USERNAME, + "ENSO_TEST_USER", + ) + .with_secret_exposed_as( + secret::ENSO_CLOUD_TEST_ACCOUNT_PASSWORD, + "ENSO_TEST_USER_PASSWORD", + ); + steps.push(test_step); + + let upload_test_traces_step = Step { + r#if: Some("failure()".into()), + name: Some("Upload Test Traces".into()), + uses: Some("actions/upload-artifact@v4".into()), + with: Some(Argument::Other(BTreeMap::from_iter([ + ("name".into(), format!("test-traces-{}-{}", target.0, target.1).into()), + ("path".into(), "app/ide-desktop/client/test-traces".into()), + ("compression-level".into(), 0.into()), // The traces are in zip already. + ]))), + ..Default::default() + }; + steps.push(upload_test_traces_step); + + let upload_ide = step::upload_artifact("Upload ide") + .with_custom_argument("name", format!("ide-{}", target.0)) + .with_custom_argument( + "path", + format!("dist/ide/enso-*.{}", target.0.package_extension()), + ); + steps.push(upload_ide); + + // After the E2E tests run, they create a credentials file in user home directory. + // If that file is not cleaned up, future runs of our tests may randomly get + // authenticated into Enso Cloud. We want to run tests as an authenticated + // user only when we explicitly set that up, not randomly. So we clean the + // credentials file. + let cloud_credentials_path = "$HOME/.enso/credentials"; + let cleanup_credentials_step = shell(format!("rm {cloud_credentials_path}")); + steps.push(cleanup_credentials_step); + + steps + }) + .build_job("Package New IDE", target) } } diff --git a/build_tools/build/src/ci_gen/step.rs b/build_tools/build/src/ci_gen/step.rs index 788731f02e58..48e81ef7b991 100644 --- a/build_tools/build/src/ci_gen/step.rs +++ b/build_tools/build/src/ci_gen/step.rs @@ -49,3 +49,19 @@ pub fn extra_stdlib_test_reporter((os, arch): Target, graal_edition: graalvm::Ed let path = format!("{}/*/*.xml", env_expression(&paths::ENSO_TEST_JUNIT_DIR)); test_reporter(step_name, report_name, path) } + +pub fn upload_artifact(step_name: impl Into) -> Step { + Step { + name: Some(step_name.into()), + uses: Some("actions/upload-artifact@v4".into()), + ..default() + } +} + +pub fn download_artifact(step_name: impl Into) -> Step { + Step { + name: Some(step_name.into()), + uses: Some("actions/download-artifact@v4".into()), + ..default() + } +} diff --git a/build_tools/build/src/engine/context.rs b/build_tools/build/src/engine/context.rs index 059e4d5a13df..96019ba92926 100644 --- a/build_tools/build/src/engine/context.rs +++ b/build_tools/build/src/engine/context.rs @@ -220,35 +220,6 @@ impl RunContext { Ok(()) } - /// During the native-image build, the engine generates arg files. This function uploads them as - /// artifacts on the CI, so we can inspect them later. - /// Note that if something goes wrong, the native image arg files may not be present. - async fn upload_native_image_arg_files(&self) -> Result { - debug!("Uploading Native Image Arg Files"); - let engine_runner_ni_argfile = - &self.repo_root.engine.runner.target.native_image_args_txt.path; - let launcher_ni_argfile = &self.repo_root.engine.launcher.target.native_image_args_txt.path; - let project_manager_ni_argfile = - &self.repo_root.lib.scala.project_manager.target.native_image_args_txt.path; - let native_image_arg_files = [ - (engine_runner_ni_argfile, "Engine Runner native-image-args"), - (launcher_ni_argfile, "Launcher native-image-args"), - (project_manager_ni_argfile, "Project Manager native-image-args"), - ]; - for (argfile, artifact_name) in native_image_arg_files { - if argfile.exists() { - ide_ci::actions::artifacts::upload_single_file(argfile, artifact_name).await?; - } else { - warn!( - "Native Image Arg File for {} not found at {}", - artifact_name, - argfile.display() - ); - } - } - Ok(()) - } - pub async fn build(&self) -> Result { self.prepare_build_env().await?; if ide_ci::ci::run_in_ci() { @@ -267,29 +238,13 @@ impl RunContext { ide_ci::fs::remove_glob(bench_report_xml)?; } - - let _test_results_upload_guard = - if self.config.test_jvm || self.config.test_standard_library.is_some() { - // If we run tests, make sure that old and new results won't end up mixed together. - let test_results_dir = ENSO_TEST_JUNIT_DIR - .get() - .unwrap_or_else(|_| self.paths.repo_root.target.test_results.path.clone()); - ide_ci::fs::reset_dir(&test_results_dir)?; - - // If we are run in CI conditions and we prepared some test results, we want to - // upload them as a separate artifact to ease debugging. And we do want to do that - // even if the tests fail and we are leaving the scope with an error. - is_in_env().then(|| { - scopeguard::guard(test_results_dir, |test_results_dir| { - ide_ci::global::spawn( - "Upload test results", - upload_test_results(test_results_dir), - ); - }) - }) - } else { - None - }; + if self.config.test_jvm || self.config.test_standard_library.is_some() { + // If we run tests, make sure that old and new results won't end up mixed together. + let test_results_dir = ENSO_TEST_JUNIT_DIR + .get() + .unwrap_or_else(|_| self.paths.repo_root.target.test_results.path.clone()); + ide_ci::fs::reset_dir(&test_results_dir)?; + }; // Workaround for incremental compilation issue, as suggested by kustosz. // We target files like @@ -460,8 +415,6 @@ impl RunContext { } if is_in_env() { - self.upload_native_image_arg_files().await?; - // If we were running any benchmarks, they are complete by now. Upload the report. for bench in &self.config.execute_benchmarks { match bench { @@ -553,24 +506,6 @@ impl RunContext { } } - if self.config.build_engine_package { - if TARGET_OS == OS::Linux && ide_ci::ci::run_in_ci() { - self.paths.upload_edition_file_artifact().await?; - } - - let schema_dir = self.paths.repo_root.join_iter([ - "engine", - "language-server", - "src", - "main", - "schema", - ]); - if is_in_env() { - ide_ci::actions::artifacts::upload_compressed_directory(&schema_dir, "fbs-schema") - .await?; - } - } - let graal_version = engine::deduce_graal_bundle(&self.repo_root.build_sbt).await?; for bundle in ret.bundles() { bundle.create(&self.repo_root, &graal_version).await?; diff --git a/build_tools/build/src/engine/env.rs b/build_tools/build/src/engine/env.rs index d7133e1c0590..03e467201656 100644 --- a/build_tools/build/src/engine/env.rs +++ b/build_tools/build/src/engine/env.rs @@ -1,6 +1,7 @@ //! Environment variables used by the engine's SBT-based build system. //use crate::prelude::*; + use crate::engine; use ide_ci::cache::goodie::graalvm; diff --git a/build_tools/build/src/paths.rs b/build_tools/build/src/paths.rs index 2ec8057ce856..849d81a94e5b 100644 --- a/build_tools/build/src/paths.rs +++ b/build_tools/build/src/paths.rs @@ -166,14 +166,6 @@ impl Paths { self.repo_root.distribution.editions.edition_yaml.to_path_buf() } - pub async fn upload_edition_file_artifact(&self) -> Result { - ide_ci::actions::artifacts::upload_single_file( - self.edition_file(), - EDITION_FILE_ARTIFACT_NAME, - ) - .await - } - pub async fn download_edition_file_artifact(&self) -> Result { ide_ci::actions::artifacts::download_single_file_artifact( EDITION_FILE_ARTIFACT_NAME, diff --git a/build_tools/build/src/project.rs b/build_tools/build/src/project.rs index d316b9c1252f..0ad54edb6457 100644 --- a/build_tools/build/src/project.rs +++ b/build_tools/build/src/project.rs @@ -14,7 +14,6 @@ use crate::source::WithDestination; use ide_ci::actions::artifacts; use ide_ci::cache; use ide_ci::cache::Cache; -use ide_ci::ok_ready_boxed; use ide_ci::programs::git; use octocrab::models::repos::Asset; @@ -167,32 +166,16 @@ pub trait IsTarget: Clone + Debug + Sized + Send + Sync + 'static { job: BuildTargetJob, ) -> BoxFuture<'static, Result> { let span = debug_span!("Building.", ?self, ?context, ?job).entered(); - let upload_artifacts = job.should_upload_artifact; let artifact_fut = self.build_internal(context, job.map(|job| job.input)); let this = self.clone(); async move { let artifact = artifact_fut.await.context(format!("Failed to build {this:?}."))?; - // We upload only built artifacts. There would be no point in uploading something that - // we've just downloaded. That's why the uploading code is here. - if upload_artifacts { - this.perhaps_upload_artifact(&artifact).await?; - } Ok(artifact) } .instrument(span.exit()) .boxed() } - fn perhaps_upload_artifact(&self, artifact: &Self::Artifact) -> BoxFuture<'static, Result> { - let should_upload_artifact = ide_ci::actions::workflow::is_in_env(); - trace!("Got target {:?}, should it be uploaded? {}", self, should_upload_artifact); - if should_upload_artifact { - self.upload_artifact(ready(Ok(artifact.clone()))) - } else { - ok_ready_boxed(()) - } - } - /// Produce an artifact from build inputs. fn build_internal( &self, @@ -200,15 +183,6 @@ pub trait IsTarget: Clone + Debug + Sized + Send + Sync + 'static { job: WithDestination, ) -> BoxFuture<'static, Result>; - /// Upload artifact to the current GitHub Actions run. - fn upload_artifact( - &self, - output: impl Future> + Send + 'static, - ) -> BoxFuture<'static, Result> { - let name = self.artifact_name(); - async move { artifacts::upload_compressed_directory(output.await?, name).await }.boxed() - } - fn download_artifact( &self, context: Context, diff --git a/build_tools/build/src/project/gui.rs b/build_tools/build/src/project/gui.rs index ae6a55f72342..0e1734a776bf 100644 --- a/build_tools/build/src/project/gui.rs +++ b/build_tools/build/src/project/gui.rs @@ -12,6 +12,7 @@ use crate::project::Context; use crate::project::IsArtifact; use crate::project::IsTarget; use crate::source::WithDestination; + use ide_ci::ok_ready_boxed; use ide_ci::programs::Pnpm; diff --git a/build_tools/build/src/project/ide.rs b/build_tools/build/src/project/ide.rs index 840f6cdf6695..171316fd5b6c 100644 --- a/build_tools/build/src/project/ide.rs +++ b/build_tools/build/src/project/ide.rs @@ -4,10 +4,6 @@ use crate::prelude::*; use crate::project::gui::ide_desktop_from_context; use crate::project::Context; -use ide_ci::actions::artifacts::upload_compressed_directory; -use ide_ci::actions::artifacts::upload_single_file; -use ide_ci::actions::workflow::is_in_env; - #[derive(Clone, Debug)] @@ -47,20 +43,6 @@ impl Artifact { } } - pub async fn upload_as_ci_artifact(&self, prefix: impl AsRef) -> Result { - if is_in_env() { - let prefix = prefix.as_ref(); - upload_compressed_directory(&self.unpacked, format!("{prefix}-unpacked-{TARGET_OS}")) - .await?; - let packed_artifact_name = format!("{prefix}-{TARGET_OS}"); - upload_single_file(&self.image, &packed_artifact_name).await?; - upload_single_file(&self.image_checksum, &packed_artifact_name).await?; - } else { - info!("Not in the CI environment, will not upload the artifacts.") - } - Ok(()) - } - pub fn start_unpacked( &self, extra_ide_options: impl IntoIterator>, diff --git a/build_tools/build/src/project/wasm/test.rs b/build_tools/build/src/project/wasm/test.rs index 356e9de70688..374879d78130 100644 --- a/build_tools/build/src/project/wasm/test.rs +++ b/build_tools/build/src/project/wasm/test.rs @@ -24,7 +24,7 @@ pub const PACKAGE_BLACKLIST: [&str; 1] = ["integration-test"]; pub const WASM_TEST_ATTRIBUTES: [&str; 2] = ["#[wasm_bindgen_test]", "#[wasm_bindgen_test(async)]"]; /// Subdirectories in the crate directory that contain sources for the crate. -pub const SOURCE_SUBDIRECTORIES: [&str; 4] = ["src", "benches", "examples", "tests"]; +pub const SOURCE_SUBDIRECTORIES: [&str; 3] = ["src", "benches", "tests"]; // =============== // === Browser === diff --git a/build_tools/build/src/source.rs b/build_tools/build/src/source.rs index 9b66c10d2fea..760d477c9bc3 100644 --- a/build_tools/build/src/source.rs +++ b/build_tools/build/src/source.rs @@ -28,9 +28,7 @@ impl ExternalSource { #[derive(Clone, Debug)] pub struct BuildSource { /// Data needed to build the target. - pub input: Target::BuildInput, - /// Whether to upload the resulting artifact as CI artifact. - pub should_upload_artifact: bool, + pub input: Target::BuildInput, } /// Describes how to get a target. diff --git a/build_tools/ci_utils/src/actions/artifacts.rs b/build_tools/ci_utils/src/actions/artifacts.rs index 178d3aca611c..beedddad6a3d 100644 --- a/build_tools/ci_utils/src/actions/artifacts.rs +++ b/build_tools/ci_utils/src/actions/artifacts.rs @@ -66,7 +66,7 @@ pub fn discover_recursive( } -pub fn upload( +fn upload( file_provider: impl Stream + Send + 'static, artifact_name: impl Into, options: UploadOptions, diff --git a/build_tools/ci_utils/src/actions/workflow/definition.rs b/build_tools/ci_utils/src/actions/workflow/definition.rs index 0746363d0fe5..07e62b4013c9 100644 --- a/build_tools/ci_utils/src/actions/workflow/definition.rs +++ b/build_tools/ci_utils/src/actions/workflow/definition.rs @@ -1,12 +1,11 @@ //! Model of a workflow definition and related utilities. -use serde_yaml::Value; - use crate::prelude::*; use crate::convert_case::ToKebabCase; use crate::env::accessor::RawVariable; +use serde_yaml::Value; use std::collections::btree_map::Entry; use std::collections::BTreeMap; use std::sync::atomic::AtomicU64; diff --git a/build_tools/ci_utils/src/os/target.rs b/build_tools/ci_utils/src/os/target.rs index 9467be2232a5..02818f5dd476 100644 --- a/build_tools/ci_utils/src/os/target.rs +++ b/build_tools/ci_utils/src/os/target.rs @@ -80,6 +80,14 @@ impl OS { OS::MacOS => "", } } + + pub const fn package_extension(self) -> &'static str { + match self { + OS::Windows => "exe", + OS::Linux => "AppImage", + OS::MacOS => "dmg", + } + } } #[cfg(test)] diff --git a/build_tools/cli/src/lib.rs b/build_tools/cli/src/lib.rs index 2866d5a6b864..5f3b2aaba2d5 100644 --- a/build_tools/cli/src/lib.rs +++ b/build_tools/cli/src/lib.rs @@ -134,12 +134,9 @@ impl Processor { { let span = info_span!("Resolving.", ?target, ?source).entered(); let destination = source.output_path.output_path; - let should_upload_artifact = source.build_args.upload_artifact; let source = match source.source { arg::SourceKind::Build => T::resolve(self, source.build_args.input) - .map_ok(move |input| { - Source::BuildLocally(BuildSource { input, should_upload_artifact }) - }) + .map_ok(move |input| Source::BuildLocally(BuildSource { input })) .boxed(), arg::SourceKind::Local => ok_ready_boxed(Source::External(ExternalSource::LocalFile(source.path))), @@ -209,16 +206,10 @@ impl Processor { &self, job: BuildJob, ) -> BoxFuture<'static, Result>> { - let BuildJob { input: BuildDescription { input, upload_artifact }, output_path } = job; + let BuildJob { input: BuildDescription { input, .. }, output_path } = job; let input = self.resolve_inputs::(input); async move { - Ok(WithDestination::new( - BuildSource { - input: input.await?, - should_upload_artifact: upload_artifact, - }, - output_path.output_path, - )) + Ok(WithDestination::new(BuildSource { input: input.await? }, output_path.output_path)) } .boxed() } @@ -551,16 +542,7 @@ impl Processor { }; let target = Ide { target_os: self.triple.os, target_arch: self.triple.arch }; - let artifact_name_prefix = input.artifact_name.clone(); - let build_job = target.build(&self.context, input, output_path); - async move { - let artifacts = build_job.await?; - if is_in_env() { - artifacts.upload_as_ci_artifact(artifact_name_prefix).await?; - } - Ok(artifacts) - } - .boxed() + target.build(&self.context, input, output_path) } pub fn target(&self) -> Result { From 7cbe77715b9c0d15c498dc577694c0c37795711c Mon Sep 17 00:00:00 2001 From: James Dunkerley Date: Mon, 3 Feb 2025 11:41:24 +0000 Subject: [PATCH 3/7] Table Widget Fixes (#12196) - Fix `new_text` widget for `Column.text_replace`. ![image](https://github.com/user-attachments/assets/c1e80774-f5a5-4649-ab43-fa78cd1e050a) - Use the default widget for `Join_Kind`. ![image](https://github.com/user-attachments/assets/db1edfeb-4b9d-49d7-8dbb-ad759d19e68b) - Added `Missing_Argument` to `right` for `cross_join`. ![image](https://github.com/user-attachments/assets/945b1006-5136-4548-bba5-48256f1680db) - Add new `Row_Limit` type and use this for `right_row_limit` on `cross_tab`. Added conversion for backward compatibility. ![image](https://github.com/user-attachments/assets/0eb0c2ab-fd0e-481c-b02c-7a5aad3138a4) - Add `Missing_Argument` to `lookup_table` on `Table.merge`. ![image](https://github.com/user-attachments/assets/288ba195-ce7e-4c66-8a2b-11e7ca79aeec) - Add `Missing_Argument` to `key_columns` on `Table.merge` but thrown when no entries in the vector. ![image](https://github.com/user-attachments/assets/a66c97e7-8943-4063-bd1f-54706f90f113) - Default for `add_new_columns` switched to `True`. - `Columns_To_Keep` dropdown shows the `..` to be consistent. ![image](https://github.com/user-attachments/assets/84ec481d-8379-4231-9bfe-fa59803e15b8) - `Missing_Argument` for `Table.zip`. ![image](https://github.com/user-attachments/assets/4a85e827-be32-4299-98af-b359a2e12878) --- .../Base/0.0.0-dev/src/Widget_Helpers.enso | 6 ++--- .../Database/0.0.0-dev/src/DB_Column.enso | 3 ++- .../Database/0.0.0-dev/src/DB_Table.enso | 23 +++++++++------- .../Standard/Table/0.0.0-dev/src/Column.enso | 3 ++- .../Table/0.0.0-dev/src/Columns_To_Keep.enso | 2 +- .../src/Internal/Widget_Helpers.enso | 7 ----- .../Table/0.0.0-dev/src/Row_Limit.enso | 20 ++++++++++++++ .../Standard/Table/0.0.0-dev/src/Table.enso | 27 ++++++++++++------- .../Join/Cross_Join_Spec.enso | 2 ++ .../Join/Lookup_Spec.enso | 11 ++++---- .../src/In_Memory/Table_Conversion_Spec.enso | 6 ++--- 11 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 distribution/lib/Standard/Table/0.0.0-dev/src/Row_Limit.enso diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Widget_Helpers.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Widget_Helpers.enso index 4355902bb2f9..e95d66795f75 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Widget_Helpers.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Widget_Helpers.enso @@ -21,9 +21,9 @@ make_text_secret_selector display:Display=..When_Modified -> Widget = ## PRIVATE Creates a Regex / Text Widget for search and replace. -make_regex_text_widget : Widget -make_regex_text_widget = - make_any_selector add_text=True add_regex=True add_named_pattern=True +make_regex_text_widget : Display -> Widget +make_regex_text_widget display:Display=..Always = + make_any_selector display=display add_text=True add_regex=True add_named_pattern=True ## PRIVATE Creates a Single_Choice Widget for delimiters. diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso index e23ad4f13d9f..6b24578cae4f 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso @@ -5,7 +5,7 @@ import Standard.Base.Errors.Common.Index_Out_Of_Bounds import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Errors.Illegal_State.Illegal_State import Standard.Base.Internal.Rounding_Helpers -from Standard.Base.Metadata.Widget import Numeric_Input +from Standard.Base.Metadata.Widget import Numeric_Input, Text_Input from Standard.Base.Widget_Helpers import make_data_cleanse_vector_selector, make_format_chooser, make_regex_text_widget import Standard.Table.Internal.Column_Naming_Helper.Column_Naming_Helper @@ -1643,6 +1643,7 @@ type DB_Column column.text_replace '"(.*?)"'.to_regex '($1)' @term make_regex_text_widget + @new_text (Text_Input display=..Always) text_replace : Text | Regex | DB_Column -> Text | DB_Column -> Case_Sensitivity -> Boolean -> DB_Column ! Unsupported_Database_Operation text_replace self (term : Text | Regex | DB_Column = "") new_text="" case_sensitivity:Case_Sensitivity=..Default only_first=False = Value_Type.expect_text self <| case_sensitivity.disallow_non_default_locale <| diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Table.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Table.enso index 843c3196d66a..67ed8c188ab7 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Table.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Table.enso @@ -42,6 +42,7 @@ import Standard.Table.Internal.Unique_Name_Strategy.Unique_Name_Strategy import Standard.Table.Internal.Value_Type_Helpers import Standard.Table.Internal.Widget_Helpers import Standard.Table.Row.Row +import Standard.Table.Row_Limit.Row_Limit import Standard.Table.Rows_To_Read.Rows_To_Read import Standard.Table.Value_Type.By_Type from Standard.Table import Aggregate_Column, Auto, Blank_Selector, Column_Ref, Data_Formatter, Fill_With, Join_Condition, Join_Kind, Match_Columns, Position, Previous_Value, Report_Unmatched, Set_Mode, Simple_Expression, Sort_Column, Table, Value_Type @@ -1586,7 +1587,6 @@ type DB_Table allows to join the two tables on equality of corresponding columns with the same name. So `table.join other on=["A", "B"]` is a shorthand for: table.join other on=[Join_Condition.Equals "A" "A", Join_Condition.Equals "B" "B"] - @join_kind Widget_Helpers.make_join_kind_selector @on Widget_Helpers.make_join_condition_selector join : DB_Table -> Join_Kind -> Join_Condition | Text | Vector (Join_Condition | Text) -> Text -> Problem_Behavior -> DB_Table join self right=(Missing_Argument.throw "right") (join_kind : Join_Kind = ..Left_Outer) (on : Join_Condition | Text | Vector (Join_Condition | Text) = (default_join_condition self join_kind)) (right_prefix:Text="Right ") (on_problems:Problem_Behavior=..Report_Warning) = @@ -1694,13 +1694,14 @@ type DB_Table ? Result Ordering For Database Tables The ordering of rows in the resulting table is not specified. - cross_join : DB_Table -> Integer | Nothing -> Text -> Problem_Behavior -> DB_Table - cross_join self right:DB_Table right_row_limit=100 right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = + cross_join : DB_Table -> Row_Limit -> Text -> Problem_Behavior -> DB_Table + cross_join self right:DB_Table=(Missing_Argument.throw "right") right_row_limit:Row_Limit=(..Limit 100) right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = Feature.Cross_Join.if_supported_else_throw self.connection.dialect "cross_join" <| - limit_problems = case right_row_limit.is_nothing.not && (right.row_count > right_row_limit) of - True -> - [Cross_Join_Row_Limit_Exceeded.Error right_row_limit right.row_count] - False -> [] + row_limit = case right_row_limit of + Row_Limit.No_Limit -> Nothing + Row_Limit.Limit limit -> limit + limit_problems = if row_limit.is_nothing || (right.row_count < row_limit) then [] else + [Cross_Join_Row_Limit_Exceeded.Error row_limit right.row_count] on_problems.attach_problems_before limit_problems <| self.join_or_cross_join right join_kind=Join_Kind_Cross on=[] right_prefix on_problems @@ -1761,8 +1762,12 @@ type DB_Table that are not present in this table, an `Unexpected_Extra_Columns`. @key_columns Widget_Helpers.make_column_name_multi_selector merge : DB_Table -> (Vector (Integer | Text | Regex) | Text | Integer | Regex) -> Boolean -> Boolean -> Problem_Behavior -> DB_Table ! Missing_Input_Columns | Non_Unique_Key | Unmatched_Rows_In_Lookup - merge self lookup_table:DB_Table key_columns:(Vector (Integer | Text | Regex) | Text | Integer | Regex) add_new_columns:Boolean=False allow_unmatched_rows:Boolean=True on_problems:Problem_Behavior=..Report_Warning = + merge self lookup_table:DB_Table=(Missing_Argument.throw "lookup_table") key_columns:(Vector (Integer | Text | Regex) | Text | Integer | Regex)=[] add_new_columns:Boolean=True allow_unmatched_rows:Boolean=True on_problems:Problem_Behavior=..Report_Warning = Feature.Merge.if_supported_else_throw self.connection.dialect "merge" <| + if lookup_table.is_error then lookup_table + if key_columns == [] then + Error.throw (Missing_Argument.throw "key_columns") + Helpers.ensure_same_connection "table" [self, lookup_table] <| Lookup_Query_Helper.build_lookup_query self lookup_table key_columns add_new_columns allow_unmatched_rows on_problems @@ -1898,7 +1903,7 @@ type DB_Table `Undefined_Column_Order` problem and returning an empty table. @keep_unmatched (make_single_choice [["True", "Boolean.True"], ["False", "Boolean.False"], ["Report", Report_Unmatched.to Meta.Type . qualified_name]]) zip : DB_Table -> Boolean | Report_Unmatched -> Text -> Problem_Behavior -> DB_Table - zip self right keep_unmatched=Report_Unmatched right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = + zip self right:DB_Table=(Missing_Argument.throw "right") keep_unmatched=Report_Unmatched right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = _ = [right, keep_unmatched, right_prefix, on_problems] Error.throw (Unsupported_Database_Operation.Error "zip") diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso index 6f0e4865e01c..d5088f50b5e0 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso @@ -13,7 +13,7 @@ import Standard.Base.Errors.Illegal_State.Illegal_State import Standard.Base.Internal.Polyglot_Helpers import Standard.Base.Internal.Rounding_Helpers from Standard.Base.Data.Index_Sub_Range import drop_helper, normalize_ranges, take_helper -from Standard.Base.Metadata.Widget import Numeric_Input +from Standard.Base.Metadata.Widget import Numeric_Input, Text_Input from Standard.Base.Widget_Helpers import make_data_cleanse_vector_selector, make_format_chooser, make_regex_text_widget import project.Constants.Previous_Value @@ -1492,6 +1492,7 @@ type Column column.text_replace '"(.*?)"'.to_regex '($1)' @term make_regex_text_widget + @new_text (Text_Input display=..Always) text_replace : Text | Regex | Column -> Text | Column -> Case_Sensitivity -> Boolean -> Column text_replace self (term : Text | Regex | Column = "") new_text="" case_sensitivity:Case_Sensitivity=..Sensitive only_first=False = Value_Type.expect_text self <| diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Columns_To_Keep.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Columns_To_Keep.enso index fe708a05d82d..63194e0223cc 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Columns_To_Keep.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Columns_To_Keep.enso @@ -35,4 +35,4 @@ type Columns_To_Keep that variant is only meant to be used as the default value. default_widget display:Display=..When_Modified -> Widget = make_single_choice display=display <| - ["In_Any", "In_All", "In_List"].map c-> [c, ".."+c] + ["..In_Any", "..In_All", "..In_List"].map c-> [c, c] diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso index 1f83162d9640..fe41d8f8e821 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Widget_Helpers.enso @@ -181,13 +181,6 @@ make_filter_condition_selector table display:Display=..Always cache=Nothing = builder.append (Option "Is Not Empty" "..Not_Empty") Single_Choice options display=display -## PRIVATE - Make a join kind selector - Needed to override display. -make_join_kind_selector : Display -> Widget -make_join_kind_selector display:Display=..Always = - options = ["Inner", "Left_Outer", "Right_Outer", "Full", "Left_Exclusive", "Right_Exclusive"].map n-> Option n ".."+n - Single_Choice display=display values=options - ## PRIVATE Make a join condition selector. make_join_condition_selector : Table -> Display -> Any -> Widget diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Row_Limit.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Row_Limit.enso new file mode 100644 index 000000000000..7beb01214554 --- /dev/null +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Row_Limit.enso @@ -0,0 +1,20 @@ +from Standard.Base import all +from Standard.Base.Metadata import Display, make_single_choice, Widget + +## PRIVATE + Type to specify the maximum number of rows or unlimited. +type Row_Limit + ## Allow unlimited. + No_Limit + + ## Limit to a specific number of rows. + Limit n:Integer=100 + +## PRIVATE +Row_Limit.from (that:Nothing) = + _ = that + Row_Limit.No_Limit + +## PRIVATE +Row_Limit.from (that:Integer) = + Row_Limit.Limit that diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Table.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Table.enso index 2aaaf63716a2..016622f1bf01 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Table.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Table.enso @@ -69,6 +69,7 @@ import project.Match_Columns.Match_Columns import project.Position.Position import project.Prefix_Name.Prefix_Name import project.Row.Row +import project.Row_Limit.Row_Limit import project.Rows_To_Read.Rows_To_Read import project.Set_Mode.Set_Mode import project.Simple_Expression.Simple_Expression @@ -2018,7 +2019,9 @@ type Table @fields (Widget.Vector_Editor item_editor=Widget.Text_Input item_default='""') expand_column : Text | Integer -> (Vector Text) | Nothing -> Prefix_Name -> Table ! Type_Error | No_Such_Column | Index_Out_Of_Bounds expand_column self (column : Text | Integer) (fields : (Vector Text) | Nothing = Nothing) (prefix : Prefix_Name = ..Column_Name) = - Expand_Objects_Helpers.expand_column self column fields prefix + ## If the user enters no fields then treat as not entered. + used_fields = if fields.is_nothing.not && fields == [] then Nothing else fields + Expand_Objects_Helpers.expand_column self column used_fields prefix ## GROUP Standard.Base.Calculations ICON split @@ -2761,7 +2764,6 @@ type Table So `table.join other on=["A", "B"]` is a shorthand for: `table.join other on=[..Equals "A" "A", ..Equals "B" "B"]` - @join_kind Widget_Helpers.make_join_kind_selector @on Widget_Helpers.make_join_condition_selector join : Table -> Join_Kind -> Vector (Join_Condition | Text) | Text -> Text -> Problem_Behavior -> Table join self right:Table=(Missing_Argument.throw "right") (join_kind : Join_Kind = ..Left_Outer) on=[Join_Condition.Equals self.column_names.first] right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = Out_Of_Memory.handle_java_exception "join" <| @@ -2789,7 +2791,7 @@ type Table - right_row_limit: If the number of rows in the right table exceeds this, then a `Cross_Join_Row_Limit_Exceeded` problem is raised. The check exists to avoid exploding the size of the table by accident. This check - can be disabled by setting this parameter to `Nothing`. + can be disabled by setting this parameter to `All_Rows`. - right_prefix: The prefix added to right table column names in case of name conflict. See "Column Renaming" below for more information. - on_problems: Specifies how to handle problems if they occur, reporting @@ -2814,10 +2816,13 @@ type Table table. This applies only if the order of the rows was specified (for example, by sorting the table; in-memory tables will keep the memory layout order while for database tables the order may be unspecified). - cross_join : Table -> Integer | Nothing -> Text -> Problem_Behavior -> Table - cross_join self right:Table right_row_limit=100 right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = Out_Of_Memory.handle_java_exception "cross_join" <| - limit_problems = if right_row_limit.is_nothing || (right.row_count < right_row_limit) then [] else - [Cross_Join_Row_Limit_Exceeded.Error right_row_limit right.row_count] + cross_join : Table -> Row_Limit -> Text -> Problem_Behavior -> Table + cross_join self right:Table=(Missing_Argument.throw "right") right_row_limit:Row_Limit=(..Limit 100) right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = Out_Of_Memory.handle_java_exception "cross_join" <| + row_limit = case right_row_limit of + Row_Limit.No_Limit -> Nothing + Row_Limit.Limit limit -> limit + limit_problems = if row_limit.is_nothing || (right.row_count < row_limit) then [] else + [Cross_Join_Row_Limit_Exceeded.Error row_limit right.row_count] on_problems.attach_problems_before limit_problems <| new_java_table = Java_Problems.with_problem_aggregator on_problems java_aggregator-> self.java_table.crossJoin right.java_table right_prefix java_aggregator @@ -2880,7 +2885,11 @@ type Table that are not present in this table, an `Unexpected_Extra_Columns`. @key_columns Widget_Helpers.make_column_name_multi_selector merge : Table -> (Vector (Integer | Text | Regex) | Text | Integer | Regex) -> Boolean -> Boolean -> Problem_Behavior -> Table ! Missing_Input_Columns | Non_Unique_Key | Unmatched_Rows_In_Lookup - merge self lookup_table:Table key_columns:(Vector (Integer | Text | Regex) | Text | Integer | Regex) add_new_columns:Boolean=False allow_unmatched_rows:Boolean=True on_problems:Problem_Behavior=..Report_Warning = + merge self lookup_table:Table=(Missing_Argument.throw "lookup_table") key_columns:(Vector (Integer | Text | Regex) | Text | Integer | Regex)=[] add_new_columns:Boolean=True allow_unmatched_rows:Boolean=True on_problems:Problem_Behavior=..Report_Warning = + if lookup_table.is_error then lookup_table + if key_columns == [] then + Error.throw (Missing_Argument.throw "key_columns") + lookup_columns = Lookup_Helpers.prepare_columns_for_lookup self lookup_table key_columns add_new_columns allow_unmatched_rows on_problems java_descriptions = lookup_columns.map make_java_lookup_column_description on_problems=No_Wrap.Value @@ -3048,7 +3057,7 @@ type Table The ordering of rows in the resulting table is not specified. @keep_unmatched (make_single_choice [["True", "Boolean.True"], ["False", "Boolean.False"], ["Report", Report_Unmatched.to Meta.Type . qualified_name]]) zip : Table -> Boolean | Report_Unmatched -> Text -> Problem_Behavior -> Table - zip self right:Table keep_unmatched=Report_Unmatched right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = + zip self right:Table=(Missing_Argument.throw "right") keep_unmatched=Report_Unmatched right_prefix:Text="Right " on_problems:Problem_Behavior=..Report_Warning = keep_unmatched_bool = case keep_unmatched of Report_Unmatched -> True b : Boolean -> b diff --git a/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso index e2fde76cedf3..8201e110c39d 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Join/Cross_Join_Spec.enso @@ -90,8 +90,10 @@ add_cross_join_specs suite_builder setup = problems = [Cross_Join_Row_Limit_Exceeded.Error 100 101] Problems.test_problem_handling action problems tester + t2.cross_join t101 right_row_limit=..No_Limit . row_count . should_equal 202 t2.cross_join t101 right_row_limit=Nothing . row_count . should_equal 202 t2.cross_join t3 right_row_limit=2 on_problems=..Report_Error . should_fail_with Cross_Join_Row_Limit_Exceeded + t2.cross_join t3 right_row_limit=(..Limit 2) on_problems=..Report_Error . should_fail_with Cross_Join_Row_Limit_Exceeded group_builder.specify "should ensure 1-1 mapping even with duplicate rows" <| t1 = table_builder [["X", [2, 1, 2, 2]], ["Y", [5, 4, 5, 5]]] diff --git a/test/Table_Tests/src/Common_Table_Operations/Join/Lookup_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Join/Lookup_Spec.enso index 6ac2c7c2c321..966345bbd2e9 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Join/Lookup_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Join/Lookup_Spec.enso @@ -1,5 +1,6 @@ from Standard.Base import all import Standard.Base.Errors.Common.Floating_Point_Equality +import Standard.Base.Errors.Common.Missing_Argument import Standard.Base.Errors.Illegal_Argument.Illegal_Argument from Standard.Table import all @@ -42,11 +43,11 @@ add_merge_specs suite_builder setup = m2.at "X" . to_vector . should_equal [1, 2, 3, 2] m2.at "Y" . to_vector . should_equal ["A", "B", "A", "B"] - group_builder.specify "should allow to add new columns from a lookup table" <| + group_builder.specify "should allow to add new columns from a lookup table (add_new_columns=True (default))" <| lookup = table_builder [["code", ["a", "b", "c"]], ["status", ["new", "old", "changed"]]] my_table = table_builder [["id", [1, 2, 3, 4]], ["code", ["a", "c", "c", "b"]], ["hmm", [10, 20, 30, 40]]] - t2 = my_table.merge lookup key_columns="code" add_new_columns=True + t2 = my_table.merge lookup key_columns="code" t2.column_names . should_equal ["id", "code", "hmm", "status"] m2 = t2 |> materialize |> _.sort "id" m2.at "id" . to_vector . should_equal [1, 2, 3, 4] @@ -63,11 +64,11 @@ add_merge_specs suite_builder setup = m3.at "is_X" . to_vector . should_equal [True, True, False, True] m3.at "X" . to_vector . should_equal ["Yes", "Yes", "No", "Yes"] - group_builder.specify "will warn if extra columns are unexpected (add_new_columns=False) (default)" <| + group_builder.specify "will warn if extra columns are unexpected (add_new_columns=False)" <| lookup = table_builder [["code", ["a", "b", "c"]], ["status", ["new", "old", "changed"]]] my_table = table_builder [["id", [1, 2, 3, 4]], ["code", ["a", "c", "c", "b"]], ["hmm", [10, 20, 30, 40]]] - t2 = my_table.merge lookup key_columns="code" + t2 = my_table.merge lookup key_columns="code" add_new_columns=False t2.column_names . should_equal ["id", "code", "hmm"] m2 = t2 |> materialize |> _.sort "id" m2.at "id" . to_vector . should_equal [1, 2, 3, 4] @@ -329,7 +330,7 @@ add_merge_specs suite_builder setup = my_table = table_builder [["X", [2, 1]], ["Z", [10, 20]]] r2 = my_table.merge lookup key_columns=[] add_new_columns=True - r2.should_fail_with Illegal_Argument + r2.should_fail_with Missing_Argument if setup.is_database.not then group_builder.specify "(in-memory only) will preserve the 279table_builder of rows from the original table" <| lookup = table_builder [["Y", [1, 0]], ["V", ["TRUE", "FALSE"]]] diff --git a/test/Table_Tests/src/In_Memory/Table_Conversion_Spec.enso b/test/Table_Tests/src/In_Memory/Table_Conversion_Spec.enso index 387762768702..a37292f61979 100644 --- a/test/Table_Tests/src/In_Memory/Table_Conversion_Spec.enso +++ b/test/Table_Tests/src/In_Memory/Table_Conversion_Spec.enso @@ -255,11 +255,11 @@ add_specs suite_builder = r.should_fail_with Illegal_Argument r.catch.message.should_contain "as all inputs had no fields" - group_builder.specify "will error when fields=[]" <| + group_builder.specify "will add all fields when fields=[]" <| table = Table.new [["aaa", [1, 2]], ["bbb", data.uniform_json], ["ccc", [5, 6]]] r = table.expand_column "bbb" fields=[] - r.should_fail_with Illegal_Argument - r.catch.message . should_equal "The fields parameter cannot be empty." + expected = Table.new [["aaa", [1, 2]], ["bbb first", ["Mary", "Joe"]], ["bbb last", ["Smith", "Burton"]], ["bbb age", [23, 34]], ["ccc", [5, 6]]] + r . should_equal expected group_builder.specify "Can expand with no prefix" <| table = Table.new [["aaa", [1, 2]], ["bbb", data.non_uniform_json], ["ccc", [5, 6]]] From 36d967ecfd1874a550f7c0f6b5afba16fbda25af Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Mon, 3 Feb 2025 15:24:50 +0300 Subject: [PATCH 4/7] New icon component (#12091) This PR adds support for icons generated from figma and deprecates the imports from `#/assets/` folder Closes: https://github.com/enso-org/enso/issues/12062 --- Note: This PR ***does not*** migrate any icons. It only deprecates the legacy approach. Migrations will be done in future PRs. --- Please refer to Storybook for visual review. Also this PR adds a canvas with all icons. --- .github/workflows/gui-checks.yml | 5 + .github/workflows/storybook.yml | 3 + app/gui/env.d.ts | 11 ++ app/gui/scripts/generateIconMetadata.mjs | 2 +- .../AriaComponents/Button/Button.stories.tsx | 4 +- .../AriaComponents/Button/Button.tsx | 16 ++- .../AriaComponents/Button/CloseButton.tsx | 9 +- .../AriaComponents/Button/CopyButton.tsx | 5 +- .../components/AriaComponents/Button/types.ts | 10 +- .../AriaComponents/Dialog/Close.tsx | 20 ++- .../AriaComponents/Dialog/Dialog.tsx | 1 + .../AriaComponents/Dialog/DialogDismiss.tsx | 17 ++- .../AriaComponents/Form/components/Reset.tsx | 5 +- .../AriaComponents/Form/components/Submit.tsx | 13 +- .../AriaComponents/Menu/MenuItem.tsx | 18 +-- .../components/AriaComponents/types.ts | 43 ++++++- .../components/Breadcrumbs/BreadcrumbItem.tsx | 21 +--- .../Breadcrumbs/Breadcrumbs.stories.tsx | 13 +- .../components/Breadcrumbs/Breadcrumbs.tsx | 9 +- .../components/Icon/Icon.stories.tsx | 74 ++++++----- .../src/dashboard/components/Icon/Icon.tsx | 117 ++++++++++++++---- .../src/dashboard/components/Icon/index.ts | 4 +- .../components/Paywall/PaywallAlert.tsx | 9 +- .../Paywall/PaywallDialogButton.tsx | 13 +- .../components/Paywall/UpgradeButton.tsx | 13 +- .../Paywall/components/PaywallButton.tsx | 6 +- app/gui/src/dashboard/components/SvgIcon.tsx | 33 ----- app/gui/src/dashboard/components/SvgMask.tsx | 8 +- .../components/styled/SidebarTabButton.tsx | 2 +- eslint.config.mjs | 2 +- package.json | 2 +- 31 files changed, 315 insertions(+), 193 deletions(-) delete mode 100644 app/gui/src/dashboard/components/SvgIcon.tsx diff --git a/.github/workflows/gui-checks.yml b/.github/workflows/gui-checks.yml index 1a114b3f4f7b..accd40d7648d 100644 --- a/.github/workflows/gui-checks.yml +++ b/.github/workflows/gui-checks.yml @@ -15,6 +15,11 @@ permissions: statuses: write # Write access to commit statuses checks: write +env: + # Workaround for https://github.com/nodejs/corepack/issues/612 + # See: https://github.com/nodejs/corepack/blob/main/README.md#environment-variables + COREPACK_DEFAULT_TO_LATEST: 0 + jobs: lint: name: đź‘® Lint GUI diff --git a/.github/workflows/storybook.yml b/.github/workflows/storybook.yml index 1a7dec273de7..bb9bfa61ba51 100644 --- a/.github/workflows/storybook.yml +++ b/.github/workflows/storybook.yml @@ -16,6 +16,9 @@ permissions: statuses: write # Write access to commit statuses env: + # Workaround for https://github.com/nodejs/corepack/issues/612 + # See: https://github.com/nodejs/corepack/blob/main/README.md#environment-variables + COREPACK_DEFAULT_TO_LATEST: 0 ENSO_BUILD_SKIP_VERSION_CHECK: "true" PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1 diff --git a/app/gui/env.d.ts b/app/gui/env.d.ts index 9dfeb5ffa61f..0a84bf999156 100644 --- a/app/gui/env.d.ts +++ b/app/gui/env.d.ts @@ -199,3 +199,14 @@ declare global { (message: string, projectId?: string | null, metadata?: object | null): void } } + +// Add additional types for svg imports from `#/assets/*.svg` +declare module 'vite/client' { + declare module '#/assets/*.svg' { + /** + * @deprecated Prefer defined keys over importing from `#/assets/*.svg + */ + const src: string + export default src + } +} diff --git a/app/gui/scripts/generateIconMetadata.mjs b/app/gui/scripts/generateIconMetadata.mjs index 242b2c76f8fd..e93936267f65 100644 --- a/app/gui/scripts/generateIconMetadata.mjs +++ b/app/gui/scripts/generateIconMetadata.mjs @@ -15,7 +15,7 @@ await fs.writeFile( // Please run \`bazel run //:write_all\` to regenerate this file whenever \`icons.svg\` is changed. /** All icon names present in icons.svg. */ -const iconNames = [ +export const iconNames = [ ${iconNames?.map((name) => ` '${name}',`).join('\n')} ] as const diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/Button.stories.tsx b/app/gui/src/dashboard/components/AriaComponents/Button/Button.stories.tsx index 6eec46bd6c40..801eafa992be 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/Button.stories.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Button/Button.stories.tsx @@ -9,7 +9,7 @@ import { expect, userEvent, within } from '@storybook/test' import { Button, type BaseButtonProps } from '.' import { Badge } from '../../Badge' -type Story = StoryObj> +type Story = StoryObj> const variants = [ 'primary', @@ -40,7 +40,7 @@ export default { addonStart: { control: false }, addonEnd: { control: false }, }, -} as Meta> +} satisfies Meta> export const Variants: Story = { render: () => ( diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx b/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx index 327ea8fd4791..cee8ed33c59b 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Button/Button.tsx @@ -10,11 +10,10 @@ import { } from 'react' import * as aria from '#/components/aria' -import { StatelessSpinner } from '#/components/StatelessSpinner' -import SvgMask from '#/components/SvgMask' - import { useVisualTooltip } from '#/components/AriaComponents/Text' import { Tooltip, TooltipTrigger } from '#/components/AriaComponents/Tooltip' +import { Icon as IconComponent } from '#/components/Icon' +import { StatelessSpinner } from '#/components/StatelessSpinner' import { useEventCallback } from '#/hooks/eventCallbackHooks' import { forwardRef } from '#/utilities/react' import { ButtonGroup, ButtonGroupJoin } from './ButtonGroup' @@ -28,7 +27,10 @@ const ICON_LOADER_DELAY = 150 // Manually casting types to make TS infer the final type correctly (e.g. RenderProps in icon) // eslint-disable-next-line no-restricted-syntax export const Button = memo( - forwardRef(function Button(props: ButtonProps, ref: ForwardedRef) { + forwardRef(function Button( + props: ButtonProps, + ref: ForwardedRef, + ) { props = useMergedButtonStyles(props) const { className, @@ -252,7 +254,9 @@ export const Button = memo( ) }), -) as unknown as ((props: ButtonProps & { ref?: ForwardedRef }) => ReactNode) & { +) as unknown as (( + props: ButtonProps & { ref?: ForwardedRef }, +) => ReactNode) & { // eslint-disable-next-line @typescript-eslint/naming-convention Group: typeof ButtonGroup // eslint-disable-next-line @typescript-eslint/naming-convention @@ -382,7 +386,7 @@ const Icon = memo(function Icon(props: IconProps) { const actualIcon = (() => { return typeof icon === 'string' ? - + {icon} : {icon} })() diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/CloseButton.tsx b/app/gui/src/dashboard/components/AriaComponents/Button/CloseButton.tsx index 120d13d2dbe5..265ae955eca5 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/CloseButton.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Button/CloseButton.tsx @@ -8,10 +8,15 @@ import { Button } from './Button' import type { ButtonProps } from './types' /** Props for a {@link CloseButton}. */ -export type CloseButtonProps = Omit +export type CloseButtonProps = Omit< + ButtonProps, + 'children' | 'rounding' | 'size' | 'variant' +> /** A styled button with a close icon that appears on hover. */ -export const CloseButton = memo(function CloseButton(props: CloseButtonProps) { +export const CloseButton = memo(function CloseButton( + props: CloseButtonProps, +) { const { getText } = useText() const { diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/CopyButton.tsx b/app/gui/src/dashboard/components/AriaComponents/Button/CopyButton.tsx index e2567b6837a7..5a5f57801211 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/CopyButton.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Button/CopyButton.tsx @@ -17,7 +17,8 @@ import type { ButtonProps } from './types' // ================== /** Props for a {@link CopyButton}. */ -export interface CopyButtonProps extends Omit { +export interface CopyButtonProps + extends Omit, 'icon' | 'loading' | 'onPress'> { /** The text to copy to the clipboard. */ readonly copyText: string /** @@ -38,7 +39,7 @@ export interface CopyButtonProps extends Omit(props: CopyButtonProps) { const { variant = 'icon', copyIcon = CopyIcon, diff --git a/app/gui/src/dashboard/components/AriaComponents/Button/types.ts b/app/gui/src/dashboard/components/AriaComponents/Button/types.ts index 797a6f5957c0..72a1a7c5761b 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Button/types.ts +++ b/app/gui/src/dashboard/components/AriaComponents/Button/types.ts @@ -42,11 +42,11 @@ export interface LinkRenderProps extends aria.LinkRenderProps { } /** Props for a Button. */ -export type ButtonProps = - | (BaseButtonProps & +export type ButtonProps = + | (BaseButtonProps & Omit & PropsWithoutHref) - | (BaseButtonProps & + | (BaseButtonProps & Omit & PropsWithHref) @@ -61,7 +61,7 @@ interface PropsWithoutHref { } /** Base props for a button. */ -export interface BaseButtonProps +export interface BaseButtonProps extends Omit, TestIdProps { /** If `true`, the loader will not be shown. */ @@ -70,7 +70,7 @@ export interface BaseButtonProps readonly tooltip?: ReactElement | string | false | null readonly tooltipPlacement?: aria.Placement /** The icon to display in the button */ - readonly icon?: IconProp + readonly icon?: IconProp /** When `true`, icon will be shown only when hovered. */ readonly showIconOnHover?: boolean /** diff --git a/app/gui/src/dashboard/components/AriaComponents/Dialog/Close.tsx b/app/gui/src/dashboard/components/AriaComponents/Dialog/Close.tsx index f49ea796f9c8..61eaad4d3c3e 100644 --- a/app/gui/src/dashboard/components/AriaComponents/Dialog/Close.tsx +++ b/app/gui/src/dashboard/components/AriaComponents/Dialog/Close.tsx @@ -4,25 +4,23 @@ * Close button for a dialog. */ -import invariant from 'tiny-invariant' - import { useEventCallback } from '#/hooks/eventCallbackHooks' import { type ButtonProps, Button } from '../Button' import * as dialogProvider from './DialogProvider' /** Props for {@link Close} component. */ -export type CloseProps = ButtonProps +export type CloseProps = ButtonProps /** Close button for a dialog. */ -export function Close(props: CloseProps) { - const dialogContext = dialogProvider.useDialogContext() - - invariant(dialogContext, 'Close must be used inside a DialogProvider') +export function Close(props: CloseProps) { + const dialogContext = dialogProvider.useDialogStrictContext() - const onPressCallback = useEventCallback>((event) => { - dialogContext.close() - return props.onPress?.(event) - }) + const onPressCallback = useEventCallback['onPress']>>( + (event) => { + dialogContext.close() + return props.onPress?.(event) + }, + ) return