From 05c5ee0534a3d7d9f3919d8737343875b3a47e05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= <radoslaw.wasko@enso.org>
Date: Sat, 9 Dec 2023 15:01:37 +0100
Subject: [PATCH] implement a PoC simply by wrapping body of opted-in functions
 in a type ascription

---
 .../java/org/enso/compiler/core/TreeToIr.java | 42 +++++++-------
 .../ir/module/scope/definition/Method.scala   |  3 -
 .../org/enso/compiler/ExecCompilerTest.java   | 42 ++++++++++++++
 .../enso/interpreter/test/SignatureTest.java  | 55 +++----------------
 4 files changed, 71 insertions(+), 71 deletions(-)

diff --git a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java
index 442a0c40abfdf..97f9678f038c3 100644
--- a/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java
+++ b/engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java
@@ -243,17 +243,19 @@ private List<Definition> translateModuleSymbolImpl(Tree inputAst, List<Definitio
         var methodRef = translateMethodReference(fn.getName(), false);
         var args = translateArgumentsDefinition(fn.getArgs());
         var body = translateExpression(fn.getBody());
+        var loc = getIdentifiedLocation(inputAst, 0, 0, null);
         var returnSignature = resolveReturnTypeSignature(fn);
         if (body == null) {
             var error = translateSyntaxError(inputAst, new Syntax.UnsupportedSyntax("Block without body"));
             yield join(error, appendTo);
         }
+
+        var ascribedBody = addTypeAscription(body, returnSignature, loc);
         var binding = new Method.Binding(
           methodRef,
           args,
-          Option.apply(returnSignature),
-          body,
-          getIdentifiedLocation(inputAst, 0, 0, null),
+          ascribedBody,
+          loc,
           meta(), diag()
         );
         yield join(binding, appendTo);
@@ -274,7 +276,7 @@ private List<Definition> translateModuleSymbolImpl(Tree inputAst, List<Definitio
         var text = buildTextConstant(body, body.getElements());
         var def = new Foreign.Definition(language, text, getIdentifiedLocation(fn.getBody()), meta(), diag());
         var binding = new Method.Binding(
-                methodRef, args, Option.empty(), def, getIdentifiedLocation(inputAst), meta(), diag()
+                methodRef, args, def, getIdentifiedLocation(inputAst), meta(), diag()
         );
         yield join(binding, appendTo);
       }
@@ -305,7 +307,6 @@ private List<Definition> translateModuleSymbolImpl(Tree inputAst, List<Definitio
         var binding = new Method.Binding(
           reference,
           nil(),
-          Option.empty(),
           body.setLocation(aLoc),
           expandToContain(getIdentifiedLocation(a), aLoc),
           meta(), diag()
@@ -503,6 +504,8 @@ private Expression translateFunction(Tree fun, Name name, java.util.List<Argumen
       } catch (SyntaxException ex) {
         return ex.toError();
       }
+
+      var loc = getIdentifiedLocation(fun);
       var body = translateExpression(treeBody);
       if (args.isEmpty()) {
         if (body instanceof Expression.Block block) {
@@ -521,23 +524,15 @@ private Expression translateFunction(Tree fun, Name name, java.util.List<Argumen
           body = translateSyntaxError(fun, Syntax.UnexpectedExpression$.MODULE$);
         }
 
-        if (returnType != null) {
-          // TODO
-          System.out.println("TODO: type ascription on 0-argument blocks");
-        }
-
-        return new Expression.Binding(name, body,
-          getIdentifiedLocation(fun), meta(), diag()
-        );
+        var ascribedBody = addTypeAscription(body, returnType, loc);
+        return new Expression.Binding(name, ascribedBody, loc, meta(), diag());
       } else {
         if (body == null) {
           return translateSyntaxError(fun, Syntax.UnexpectedDeclarationInType$.MODULE$);
         }
 
-        // TODO add ascription here too
-        return new Function.Binding(name, args, body,
-          getIdentifiedLocation(fun), true, meta(), diag()
-        );
+        var ascribedBody = addTypeAscription(body, returnType, loc);
+        return new Function.Binding(name, args, ascribedBody, loc, true, meta(), diag());
       }
    }
 
@@ -548,12 +543,17 @@ private Expression resolveReturnTypeSignature(Tree.Function fun) {
        return null;
      }
 
-     Expression returnType = translateType(returnSignature.getType());
-     // TODO later we ofc want to remove this debug:
-     System.out.println("Method " + fun.getName().codeRepr() + " has returnType: " + returnType.showCode());
-     return returnType;
+     return translateType(returnSignature.getType());
    }
 
+  private Expression addTypeAscription(Expression body, Expression type, Option<IdentifiedLocation> loc) {
+     if (type == null) {
+       return body;
+     }
+
+     return new Type.Ascription(body, type, loc, meta(), diag());
+  }
+
   private Type.Ascription translateTypeSignature(Tree sig, Tree type, Expression typeName) {
     var fn = translateType(type);
     return new Type.Ascription(typeName, fn, getIdentifiedLocation(sig), meta(), diag());
diff --git a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/module/scope/definition/Method.scala b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/module/scope/definition/Method.scala
index 4b5ba77688090..5b7c515835089 100644
--- a/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/module/scope/definition/Method.scala
+++ b/engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/module/scope/definition/Method.scala
@@ -229,8 +229,6 @@ object Method {
     *
     * @param methodReference a reference to the method being defined
     * @param arguments       the arguments to the method
-    * @param returnType      the return type of the method (if the method
-    *                        opted-in to the return type check)
     * @param body            the body of the method
     * @param location        the source location that the node corresponds to
     * @param passData        the pass metadata associated with this node
@@ -239,7 +237,6 @@ object Method {
   sealed case class Binding(
     override val methodReference: Name.MethodReference,
     arguments: List[DefinitionArgument],
-    returnType: Option[Expression],
     override val body: Expression,
     override val location: Option[IdentifiedLocation],
     override val passData: MetadataStorage      = new MetadataStorage(),
diff --git a/engine/runtime/src/test/java/org/enso/compiler/ExecCompilerTest.java b/engine/runtime/src/test/java/org/enso/compiler/ExecCompilerTest.java
index ca20baa682aef..bd2391ed33279 100644
--- a/engine/runtime/src/test/java/org/enso/compiler/ExecCompilerTest.java
+++ b/engine/runtime/src/test/java/org/enso/compiler/ExecCompilerTest.java
@@ -1,11 +1,14 @@
 package org.enso.compiler;
 
+import java.net.URI;
 import java.nio.file.Paths;
 import java.util.logging.Level;
 
+import org.enso.polyglot.MethodNames;
 import org.enso.polyglot.RuntimeOptions;
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.PolyglotException;
+import org.graalvm.polyglot.Source;
 import org.graalvm.polyglot.io.IOAccess;
 import org.junit.AfterClass;
 import static org.junit.Assert.assertEquals;
@@ -245,6 +248,45 @@ public void inlineReturnSignatureWithoutArguments() throws Exception {
     assertEquals("Function-return syntax can be used with 0 arguments", 23, result.asInt());
   }
 
+  /**
+   * This test demonstrates a slightly un-intuitive, but apparently needed by our rules, behaviour of `->` with ascriptions:
+   * 1. for `foo a:Integer -> Integer` it is interpreted as foo (a:Integer) -> Integer - i.e. a function taking an Integer and returning an Integer.
+   * 2. for `foo a : Integer -> Integer`, this results in a compile error currently.
+   */
+  @Test
+  public void weirdReturnTypeSignature1() throws Exception {
+    final URI uri = new URI("memory://rts.enso");
+    final Source src = Source.newBuilder("enso", """
+    from Standard.Base import Integer
+    foo a:Integer -> Integer = a+10
+    """,uri.getAuthority())
+        .uri(uri)
+        .buildLiteral();
+
+    var module = ctx.eval(src);
+    var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
+    assertEquals(11, foo.execute(1).asInt());
+  }
+
+  @Test
+  public void weirdReturnTypeSignature2() throws Exception {
+    final URI uri = new URI("memory://rts.enso");
+    final Source src = Source.newBuilder("enso", """
+    from Standard.Base import Integer
+    foo a : Integer -> Integer = a+10
+    """,uri.getAuthority())
+        .uri(uri)
+        .buildLiteral();
+
+    try {
+      var module = ctx.eval(src);
+      var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
+      fail("Compiler error was expected, but foo evaluated successfully as: " + foo);
+    } catch (PolyglotException ex) {
+      assertEquals("Compile error: The name `foo` could not be found.", ex.getMessage());
+    }
+  }
+
   @Test
   public void testInvalidEnsoProjectRef() throws Exception {
     var module =
diff --git a/engine/runtime/src/test/java/org/enso/interpreter/test/SignatureTest.java b/engine/runtime/src/test/java/org/enso/interpreter/test/SignatureTest.java
index 027aee2ee9eb4..582270917c63f 100644
--- a/engine/runtime/src/test/java/org/enso/interpreter/test/SignatureTest.java
+++ b/engine/runtime/src/test/java/org/enso/interpreter/test/SignatureTest.java
@@ -765,32 +765,6 @@ public void validReturnTypeSignature() throws Exception {
     assertEquals(3, add2.execute(1, 2).asInt());
   }
 
-
-  /**
-   * This test demonstrates a slightly un-intuitive, but apparently needed by our rules, behaviour of `->` with ascriptions:
-   * for `foo a:Integer -> Integer` what happens? TODO
-   * for `foo a : Integer -> Integer` what happens? TODO
-   */
-  @Test
-  public void weirdReturnTypeSignature() throws Exception {
-    final URI uri = new URI("memory://rts.enso");
-    final Source src = Source.newBuilder("enso", """
-    from Standard.Base import Integer
-    foo a:Integer -> Integer = a+10
-    bar a:Integer -> Integer = a+100
-    """,uri.getAuthority())
-        .uri(uri)
-        .buildLiteral();
-
-    // TODO figure out these
-    var module = ctx.eval(src);
-    var add1 = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
-    assertEquals(11, add1.execute(1).asInt());
-
-    var add2 = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "bar");
-    assertEquals(101, add2.execute(1).asInt());
-  }
-
   @Test
   public void returnTypeCheckOptInError() throws Exception {
     final URI uri = new URI("memory://rts.enso");
@@ -808,9 +782,7 @@ public void returnTypeCheckOptInError() throws Exception {
       var res = plusChecked.execute("a", "b");
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("PLus checked: " + e);
-      // TODO
-      assertTrue(true);
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
   }
 
@@ -850,9 +822,7 @@ public void returnTypeCheckOptInErrorZeroArguments() throws Exception {
       var res = plusChecked.execute(2, 3);
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("Constant checked: " + e);
-      // TODO
-      assertTrue(true);
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
   }
 
@@ -876,9 +846,7 @@ public void returnTypeCheckOptInErrorZeroArgumentsExpression() throws Exception
       var res = plusChecked.execute(".");
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("Constant checked: " + e);
-      // TODO
-      assertTrue(true);
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
   }
 
@@ -902,9 +870,7 @@ public void returnTypeCheckOptInErrorZeroArgumentsBlock() throws Exception {
       var res = plusChecked.execute(".");
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("Constant checked: " + e);
-      // TODO
-      assertTrue(true);
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
   }
 
@@ -930,16 +896,12 @@ public void returnTypeCheckOptInAllowDataflowErrors() throws Exception {
       var res = foo.execute(2);
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("foo TWO: " + e);
-      // TODO
-      assertTrue(true);
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
 
-    // TODO
     var res = foo.execute(3);
-    System.out.println(res);
     assertTrue(res.isException());
-    assertContains(res.toString(), "My error");
+    assertContains("My error", res.toString());
   }
 
   @Test
@@ -965,9 +927,8 @@ public void returnTypeCheckOptInTailRec() throws Exception {
       var res = factorial.execute(20);
       fail("Expecting an exception, not: " + res);
     } catch (PolyglotException e) {
-      System.out.println("go checked: " + e);
-      // TODO "TEN :)" is not a valid Integer, so it should explode
-      assertTrue(true);
+      // TODO we may want to change `expression` to 'the return type' or something
+      assertContains("expected `expression` to be Integer, but got Text", e.getMessage());
     }
   }