Move annotations into fields of Function and ConstructorDefinition (#11374)

Move annotations into fields of Function and ConstructorDefinition.

# Important Notes
New syntax: Constructor argument-definition lines
- Each argument in a type-constructor definition may be specified on its own (indented) line.

Relaxed syntax: Unparenthesized arguments to annotations
- A generic annotation now uses the rest of the line as its argument expression; the expression no longer needs to be parenthesized.

Author: kazcw

CHANGELOG.md

@@ -26,6 +26,12 @@
 [11235]: https://github.com/enso-org/enso/pull/11235
 [11255]: https://github.com/enso-org/enso/pull/11255
 
+#### Enso Language & Runtime
+
+- [Arguments in constructor definitions may now be on their own lines][11374]
+
+[11374]: https://github.com/enso-org/enso/pull/11374
+
 # Enso 2024.4
 
 #### Enso IDE

app/ydoc-shared/src/ast/parse.ts

@@ -1,5 +1,14 @@
 import * as map from 'lib0/map'
-import type { AstId, Module, NodeChild, Owned, OwnedRefs, TextElement, TextToken } from '.'
+import type {
+  AstId,
+  FunctionFields,
+  Module,
+  NodeChild,
+  Owned,
+  OwnedRefs,
+  TextElement,
+  TextToken,
+} from '.'
 import {
   Token,
   asOwned,
@@ -166,12 +175,20 @@ class Abstractor {
         break
       }
       case RawAst.Tree.Type.Function: {
-        const name = this.abstractTree(tree.name)
+        const annotationLines = Array.from(tree.annotationLines, anno => ({
+          annotation: {
+            operator: this.abstractToken(anno.annotation.operator),
+            annotation: this.abstractToken(anno.annotation.annotation),
+            argument: anno.annotation.argument && this.abstractTree(anno.annotation.argument),
+          },
+          newlines: Array.from(anno.newlines, this.abstractToken.bind(this)),
+        }))
         const signatureLine = tree.signatureLine && {
           signature: this.abstractTypeSignature(tree.signatureLine.signature),
           newlines: Array.from(tree.signatureLine.newlines, this.abstractToken.bind(this)),
         }
         const private_ = tree.private && this.abstractToken(tree.private)
+        const name = this.abstractTree(tree.name)
         const argumentDefinitions = Array.from(tree.args, arg => ({
           open: arg.open && this.abstractToken(arg.open),
           open2: arg.open2 && this.abstractToken(arg.open2),
@@ -190,15 +207,15 @@ class Abstractor {
         }))
         const equals = this.abstractToken(tree.equals)
         const body = tree.body !== undefined ? this.abstractTree(tree.body) : undefined
-        node = Function.concrete(
-          this.module,
+        node = Function.concrete(this.module, {
+          annotationLines,
           signatureLine,
           private_,
           name,
           argumentDefinitions,
           equals,
           body,
-        )
+        } satisfies FunctionFields<OwnedRefs>)
         break
       }
       case RawAst.Tree.Type.Ident: {

app/ydoc-shared/src/ast/tree.ts

@@ -451,6 +451,8 @@ type StructuralField<T extends TreeRefs = RawRefs> =
   | VectorElement<T>
   | TypeSignature<T>
   | SignatureLine<T>
+  | FunctionAnnotation<T>
+  | AnnotationLine<T>
 
 /** Type whose fields are all suitable for storage as `Ast` fields. */
 interface FieldObject<T extends TreeRefs> {
@@ -568,6 +570,10 @@ function mapRefs<T extends TreeRefs, U extends TreeRefs>(
   field: VectorElement<T>,
   f: MapRef<T, U>,
 ): VectorElement<U>
+function mapRefs<T extends TreeRefs, U extends TreeRefs>(
+  field: AnnotationLine<T>,
+  f: MapRef<T, U>,
+): AnnotationLine<U>
 function mapRefs<T extends TreeRefs, U extends TreeRefs>(
   field: SignatureLine<T>,
   f: MapRef<T, U>,
@@ -2035,6 +2041,17 @@ interface ArgumentType<T extends TreeRefs = RawRefs> {
   type: T['ast']
 }
 
+interface FunctionAnnotation<T extends TreeRefs = RawRefs> {
+  operator: T['token']
+  annotation: T['token']
+  argument: T['ast'] | undefined
+}
+
+interface AnnotationLine<T extends TreeRefs = RawRefs> {
+  annotation: FunctionAnnotation<T>
+  newlines: T['token'][]
+}
+
 interface TypeSignature<T extends TreeRefs = RawRefs> {
   name: T['ast']
   operator: T['token']
@@ -2046,13 +2063,14 @@ interface SignatureLine<T extends TreeRefs = RawRefs> {
   newlines: T['token'][]
 }
 
-export interface FunctionFields {
-  signatureLine: SignatureLine | undefined
-  private_: NodeChild<SyncTokenId> | undefined
-  name: NodeChild<AstId>
-  argumentDefinitions: ArgumentDefinition[]
-  equals: NodeChild<SyncTokenId>
-  body: NodeChild<AstId> | undefined
+export interface FunctionFields<T extends TreeRefs = RawRefs> {
+  annotationLines: AnnotationLine<T>[]
+  signatureLine: SignatureLine<T> | undefined
+  private_: T['token'] | undefined
+  name: T['ast']
+  argumentDefinitions: ArgumentDefinition<T>[]
+  equals: T['token']
+  body: T['ast'] | undefined
 }
 /** TODO: Add docs */
 export class Function extends Ast {
@@ -2086,24 +2104,24 @@ export class Function extends Ast {
   /** TODO: Add docs */
   static concrete(
     module: MutableModule,
-    signatureLine: SignatureLine<OwnedRefs> | undefined,
-    private_: NodeChild<Token> | undefined,
-    name: NodeChild<Owned>,
-    argumentDefinitions: ArgumentDefinition<OwnedRefs>[],
-    equals: NodeChild<Token>,
-    body: NodeChild<Owned> | undefined,
+    fields: Partial<FunctionFields<OwnedRefs>> & { name: object } & { equals: object },
   ) {
     const base = module.baseObject('Function')
     const id_ = base.get('id')
-    const fields = composeFieldData(base, {
-      signatureLine: signatureLine && mapRefs(signatureLine, ownedToRaw(module, id_)),
-      private_,
-      name: concreteChild(module, name, id_),
-      argumentDefinitions: argumentDefinitions.map(def => mapRefs(def, ownedToRaw(module, id_))),
-      equals,
-      body: concreteChild(module, body, id_),
+    const rawFields = composeFieldData(base, {
+      annotationLines: (fields.annotationLines ?? []).map(anno =>
+        mapRefs(anno, ownedToRaw(module, id_)),
+      ),
+      signatureLine: fields.signatureLine && mapRefs(fields.signatureLine, ownedToRaw(module, id_)),
+      private_: fields.private_,
+      name: concreteChild(module, fields.name, id_),
+      argumentDefinitions: (fields.argumentDefinitions ?? []).map(def =>
+        mapRefs(def, ownedToRaw(module, id_)),
+      ),
+      equals: fields.equals,
+      body: concreteChild(module, fields.body, id_),
     })
-    return asOwned(new MutableFunction(module, fields))
+    return asOwned(new MutableFunction(module, rawFields))
   }
 
   /** TODO: Add docs */
@@ -2116,15 +2134,12 @@ export class Function extends Ast {
     // Note that a function name may not be an operator if the function is not in the body of a type definition, but we
     // can't easily enforce that because we don't currently make a syntactic distinction between top-level functions and
     // type methods.
-    return MutableFunction.concrete(
-      module,
-      undefined,
-      undefined,
-      unspaced(Ident.newAllowingOperators(module, name)),
+    return MutableFunction.concrete(module, {
+      name: unspaced(Ident.newAllowingOperators(module, name)),
       argumentDefinitions,
-      spaced(makeEquals()),
-      autospaced(body),
-    )
+      equals: spaced(makeEquals()),
+      body: autospaced(body),
+    })
   }
 
   /** Construct a function with simple (name-only) arguments and a body block. */
@@ -2161,7 +2176,15 @@ export class Function extends Ast {
 
   /** TODO: Add docs */
   *concreteChildren(_verbatim?: boolean): IterableIterator<RawNodeChild> {
-    const { signatureLine, private_, name, argumentDefinitions, equals, body } = getAll(this.fields)
+    const { annotationLines, signatureLine, private_, name, argumentDefinitions, equals, body } =
+      getAll(this.fields)
+    for (const anno of annotationLines) {
+      const { operator, annotation, argument } = anno.annotation
+      yield operator
+      yield annotation
+      if (argument) yield argument
+      yield* anno.newlines
+    }
     if (signatureLine) {
       const { signature, newlines } = signatureLine
       const { name, operator, type } = signature

distribution/lib/Standard/Database/0.0.0-dev/src/Connection/Postgres.enso

@@ -30,7 +30,14 @@ type Postgres
        - use_ssl: Whether to use SSL (defaults to `SSL_Mode.Prefer`).
        - client_cert: The client certificate to use or `Nothing` if not needed.
     @credentials Credentials.default_widget
-    Server (host:Text=default_postgres_host) (port:Integer=default_postgres_port) (database:Text=default_postgres_database) (schema:Text="") (credentials:(Credentials|Nothing)=Nothing) (use_ssl:SSL_Mode=SSL_Mode.Prefer) (client_cert:(Client_Certificate|Nothing)=Nothing)
+    Server
+        host : Text = default_postgres_host
+        port : Integer = default_postgres_port
+        database : Text = default_postgres_database
+        schema : Text = ""
+        credentials : Credentials|Nothing = Nothing
+        use_ssl : SSL_Mode = SSL_Mode.Prefer
+        client_cert : Client_Certificate|Nothing = Nothing
 
     ## PRIVATE
        Attempt to resolve the constructor.

distribution/lib/Standard/Table/0.0.0-dev/src/Data_Formatter.enso

@@ -59,9 +59,9 @@ type Data_Formatter
        - time_formats: Expected time formats.
        - true_values: Values representing True.
        - false_values: Values representing False.
-    @datetime_formats (make_vector_widget make_date_time_format_selector)
-    @date_formats (make_vector_widget make_date_format_selector)
-    @time_formats (make_vector_widget make_time_format_selector)
+    @datetime_formats make_vector_widget make_date_time_format_selector
+    @date_formats make_vector_widget make_date_format_selector
+    @time_formats make_vector_widget make_time_format_selector
     Value trim_values:Boolean=True allow_leading_zeros:Boolean=False decimal_point:Text|Auto=Auto thousand_separator:Text='' allow_exponential_notation:Boolean=False datetime_formats:(Vector Date_Time_Formatter)=[Date_Time_Formatter.default_enso_zoned_date_time] date_formats:(Vector Date_Time_Formatter)=[Date_Time_Formatter.iso_date] time_formats:(Vector Text)=[Date_Time_Formatter.iso_time] true_values:(Vector Text)=["True","true","TRUE"] false_values:(Vector Text)=["False","false","FALSE"]
 
     ## PRIVATE
@@ -120,9 +120,9 @@ type Data_Formatter
        - datetime_formats: Expected datetime formats.
        - date_formats: Expected date formats.
        - time_formats: Expected time formats.
-    @datetime_formats (make_vector_widget make_date_time_format_selector)
-    @date_formats (make_vector_widget make_date_format_selector)
-    @time_formats (make_vector_widget make_time_format_selector)
+    @datetime_formats make_vector_widget make_date_time_format_selector
+    @date_formats make_vector_widget make_date_format_selector
+    @time_formats make_vector_widget make_time_format_selector
     with_datetime_formats : (Vector Date_Time_Formatter | Date_Time_Formatter) -> (Vector Date_Time_Formatter | Date_Time_Formatter) -> (Vector Date_Time_Formatter | Date_Time_Formatter) -> Data_Formatter
     with_datetime_formats self datetime_formats:(Vector | Date_Time_Formatter)=self.datetime_formats date_formats:(Vector | Date_Time_Formatter)=self.date_formats time_formats:(Vector | Date_Time_Formatter)=self.time_formats =
         convert_formats formats =

docs/syntax/types.md

@@ -297,6 +297,18 @@ below.
   fully support currying - e.g. one can create `fn = Maybe.Just` and later apply
   two to it (`fn 2`) to obtain new atom.
 
+  Constructor arguments may be defined on the same line as above (similarly to
+  the syntax for defining functions); alternatively, each argument may be
+  defined on its own line in an indented block following the constructor name.
+  This can make complex constructor definitions easier to read:
+
+  ```ruby
+  type Maybe
+      Nothing
+      Just
+          value : Integer
+  ```
+
 - **Autoscoped Constructors:** Referencing constructors via their type name may
   lead to long and boilerplate code. To simplify referencing constructors when
   the _context is known_ a special `..` syntax is supported. Should there be a

engine/runtime-integration-tests/src/test/java/org/enso/compiler/test/ErrorCompilerTest.java

@@ -25,14 +25,14 @@ public void unfinishedLiteral1() throws Exception {
   }
 
   @Test
-  public void brokenAnnotation() throws Exception {
+  public void brokenAnnotationMissingArgument() throws Exception {
     var ir = parse("""
     @anno
     fn = 10
     """);
 
     assertSingleSyntaxError(
-        ir, Syntax.UnexpectedExpression$.MODULE$, "Unexpected expression", 0, 13);
+        ir, Syntax.UnexpectedExpression$.MODULE$, "Unexpected expression", 0, 5);
   }
 
   @Test

engine/runtime-integration-tests/src/test/scala/org/enso/compiler/test/pass/resolve/ModuleAnnotationsTest.scala

@@ -7,6 +7,7 @@ import org.enso.compiler.core.ir.Function
 import org.enso.compiler.core.ir.Module
 import org.enso.compiler.core.ir.Name
 import org.enso.compiler.core.ir.expression.Comment
+import org.enso.compiler.core.ir.expression.errors
 import org.enso.compiler.core.ir.module.scope.Definition
 import org.enso.compiler.core.ir.module.scope.definition
 import org.enso.compiler.pass.{PassConfiguration, PassGroup, PassManager}
@@ -184,7 +185,7 @@ class ModuleAnnotationsTest extends CompilerTest {
       ir.bindings.head shouldBe a[Definition.SugaredType]
       val typ = ir.bindings.head.asInstanceOf[Definition.SugaredType]
       typ.body.length shouldEqual 3
-      typ.body(0) shouldBe an[Name.GenericAnnotation]
+      typ.body(0) shouldBe an[errors.Syntax]
       typ.body(1) shouldBe an[Comment]
       typ.body(2) shouldBe a[Definition.Data]
     }

engine/runtime-integration-tests/src/test/scala/org/enso/interpreter/test/semantic/ConstructorsTest.scala

@@ -104,5 +104,30 @@ class ConstructorsTest extends InterpreterTest {
           |""".stripMargin
       eval(testCode) shouldEqual 55
     }
+
+    "support argument-definition lines" in {
+      val testCode =
+        """import Standard.Base.Nothing.Nothing
+          |import Standard.Base.Any.Any
+          |
+          |type C2
+          |    Cons2
+          |        # Each argument may be specified on its own indented line.
+          |        a
+          |        # Blank lines (or plain comments) are allowed between arguments.
+          |        b
+          |
+          |Nothing.genList = i -> if i == 0 then Nil2 else C2.Cons2 i (Nothing.genList (i - 1))
+          |
+          |type Nil2
+          |
+          |Nothing.sumList = list -> case list of
+          |  C2.Cons2 h t -> h + Nothing.sumList t
+          |  Nil2 -> 0
+          |
+          |main = Nothing.sumList (Nothing.genList 10)
+          |""".stripMargin
+      eval(testCode) shouldEqual 55
+    }
   }
 }