Method return type checks avoid conversion when the type fits

CHANGELOG.md

@@ -39,8 +39,10 @@
 #### Enso Language & Runtime
 
 - [Print out warnings associated with local variables][10842]
+- [Method return type checks avoid conversion when the type fits][10882]
 
 [10842]: https://github.com/enso-org/enso/pull/10842
+[10882]: https://github.com/enso-org/enso/pull/10882
 
 # Enso 2024.3
 

engine/runtime/src/main/java/org/enso/interpreter/node/callable/argument/ReadArgumentCheckNode.java

@@ -55,9 +55,26 @@ public abstract class ReadArgumentCheckNode extends Node {
     this.comment = comment;
   }
 
-  /** */
-  public static ExpressionNode wrap(ExpressionNode original, ReadArgumentCheckNode check) {
-    return new TypeCheckExpressionNode(original, check);
+  /**
+   * Wraps {@code original} node with a check for specified type. One can specify whether it is OK
+   * to accept {@link EnsoMultiValue} without conversion or not. Currently the explicit conversions:
+   *
+   * <pre>
+   * x:Integer
+   * </pre>
+   *
+   * extract the Integer component from a multi value (if it is there). However the implicit checks
+   * like <em>method return type</em> allow multi value to propagage as a whole (if it has the right
+   * component requested by the type).
+   *
+   * @param original node computing the original expression value
+   * @param check node that performs the type check
+   * @param acceptWholeMultiValue is it OK to accept multi value?
+   * @return
+   */
+  public static ExpressionNode wrapWithTypeCheck(
+      ExpressionNode original, ReadArgumentCheckNode check, boolean acceptWholeMultiValue) {
+    return new TypeCheckExpressionNode(original, check, acceptWholeMultiValue);
   }
 
   /**
@@ -553,16 +570,22 @@ public Object execute(VirtualFrame frame) {
   private static final class TypeCheckExpressionNode extends ExpressionNode {
     @Child private ExpressionNode original;
     @Child private ReadArgumentCheckNode check;
+    private final boolean acceptWholeMultiValue;
 
-    TypeCheckExpressionNode(ExpressionNode original, ReadArgumentCheckNode check) {
+    private TypeCheckExpressionNode(
+        ExpressionNode original, ReadArgumentCheckNode check, boolean acceptWholeMultiValue) {
       this.check = check;
       this.original = original;
+      this.acceptWholeMultiValue = acceptWholeMultiValue;
     }
 
     @Override
     public Object executeGeneric(VirtualFrame frame) {
       var value = original.executeGeneric(frame);
       var result = check.handleCheckOrConversion(frame, value);
+      if (acceptWholeMultiValue && value instanceof EnsoMultiValue) {
+        return value;
+      }
       return result;
     }
 

engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala

@@ -1303,19 +1303,25 @@ class IrToTruffle(
       }
       runtimeExpression.setTailStatus(getTailStatus(ir))
 
+      def attachTypes(allowWholeMultiValue: Boolean) = {
+        val types: Option[TypeSignatures.Signature] =
+          ir.getMetadata(TypeSignatures)
+        types.foreach { tpe =>
+          val checkNode =
+            extractAscribedType(tpe.comment.orNull, tpe.signature);
+          if (checkNode != null) {
+            runtimeExpression = ReadArgumentCheckNode.wrapWithTypeCheck(
+              runtimeExpression,
+              checkNode,
+              allowWholeMultiValue
+            )
+          }
+        }
+      }
       ir match {
         case _: Expression.Binding =>
-        case _ =>
-          val types: Option[TypeSignatures.Signature] =
-            ir.getMetadata(TypeSignatures)
-          types.foreach { tpe =>
-            val checkNode =
-              extractAscribedType(tpe.comment.orNull, tpe.signature);
-            if (checkNode != null) {
-              runtimeExpression =
-                ReadArgumentCheckNode.wrap(runtimeExpression, checkNode)
-            }
-          }
+        case _: Expression.Block   => attachTypes(true)
+        case _                     => attachTypes(false)
       }
       runtimeExpression
     }
@@ -2204,7 +2210,8 @@ class IrToTruffle(
         if (typeCheck == null) {
           (argExpressions.toArray, bodyExpr)
         } else {
-          val bodyWithCheck = ReadArgumentCheckNode.wrap(bodyExpr, typeCheck)
+          val bodyWithCheck =
+            ReadArgumentCheckNode.wrapWithTypeCheck(bodyExpr, typeCheck, true)
           (argExpressions.toArray, bodyWithCheck)
         }
       }

test/Base_Tests/src/Main.enso

@@ -5,6 +5,7 @@ from Standard.Test import all
 import project.Semantic.Any_Spec
 import project.Semantic.Case_Spec
 import project.Semantic.Conversion_Spec
+import project.Semantic.Conversion_Database_Spec
 import project.Semantic.Default_Args_Spec
 import project.Semantic.Error_Spec
 import project.Semantic.Import_Loop.Spec as Import_Loop_Spec
@@ -103,6 +104,7 @@ main filter=Nothing =
         Function_Spec.add_specs suite_builder
         Case_Spec.add_specs suite_builder
         Conversion_Spec.add_specs suite_builder
+        Conversion_Database_Spec.add_specs suite_builder
         Default_Args_Spec.add_specs suite_builder
         Error_Spec.add_specs suite_builder
         Environment_Spec.add_specs suite_builder

test/Base_Tests/src/Semantic/Conversion_Database_Spec.enso

@@ -0,0 +1,82 @@
+from Standard.Base import all
+from Standard.Test import all
+
+
+#class Connection a where
+#    get_tables :: a -> [String]
+
+type Connection
+    private By value dict
+
+    get_tables self = self.dict.get_tables self.value
+
+    new value dict =
+        Connection.By value dict
+
+type Snowflake_Connection
+    private Details details:Snowflake_Connection_Details
+
+    get_warehouses self  -> Vector Text = ["SNOWFLAKE_WAREHOUSE"]
+
+# database_connect :: Connection c => Connection_Details d c => d -> c
+database_connect details:Any -> Connection =
+    Connection.from details
+
+# data Snowflake_Connection_Details = Snowflake_Connection_Details String String String -- username, password etc.
+type Snowflake_Connection_Details
+    Value username:Text password:Text etc:Text
+
+two_roles_of_snowflake_connection -> Pair Text =
+    # let snowflake_connection = database_connect (Snowflake_Connection_Details "foo" "bar" "baz")
+    details = Snowflake_Connection_Details.Value "foo" "bar" "baz"
+    snowflake_connection = database_connect details
+
+    # get_tables is methods from Connection type
+    # it is there because `database_connect` returns Connection
+    r1 = snowflake_connection.get_tables
+    # get_warehouses is a method from Snowflake_Connection type
+    # it has been "mixed in" in Connection.from (that:Snowflake_Connection)
+    r2 = snowflake_connection.get_warehouses
+
+    Pair.new r1.to_text r2.to_text
+
+# data Snowflake_Connection = Snowflake_Connection_Impl -- here we'd have internals of the connection
+#instance Connection Snowflake_Connection where
+#    get_tables c = ["some Snowflake table"] -- we'd call JDBC here ofc.
+
+type Snowflake_Connection_Impl
+    get_tables self c:Snowflake_Connection =
+        ["some Snowflake table for "+c.to_text]
+
+Connection.from (that:Snowflake_Connection_Details) =
+    c = Snowflake_Connection.Details that
+    mixin = c:(Snowflake_Connection & Connection)
+    mixin
+
+Connection.from (that:Snowflake_Connection) =
+    Connection.new that Snowflake_Connection_Impl
+
+#
+# running
+#
+
+main =
+    pair = two_roles_of_snowflake_connection
+    IO.println pair.first
+    IO.println pair.second
+
+#
+# testing
+#
+
+add_specs suite_builder =
+    suite_builder.group "Conversion Database Example" group_builder->
+        group_builder.specify "snowflake_connection plays two roles" <|
+            pair = two_roles_of_snowflake_connection
+
+            pair.first . should_contain "some Snowflake table for"
+            pair.first . should_contain "Snowflake_Connection_Details"
+            pair.first . should_contain "foo"
+            pair.first . should_contain "bar"
+            pair.first . should_contain "baz"
+            pair.second . should_equal "[SNOWFLAKE_WAREHOUSE]"

test/Base_Tests/src/Semantic/Conversion_Spec.enso

@@ -98,6 +98,20 @@ type Fool
 
 Fool.from (that : Any) = Fool.Value that
 
+type Mixin
+    Value mix:Text
+
+    text_and_mixin t:Text =
+        mixin = t:(Text & Mixin)
+        mixin
+
+    text_and_mixin_with_check t:Text -> Mixin =
+        v = Mixin.text_and_mixin t
+        v
+
+Mixin.from (that:Text) = Mixin.Value that
+Mixin.from (that:Any) = Mixin.text_and_mixin that.to_text
+
 type Blob
     Text c:Text
     Binary b:File
@@ -450,6 +464,38 @@ add_specs suite_builder =
 
             do_duration now
 
+        group_builder.specify "Returning Text & Mixin from method" <|
+            m = Mixin.text_and_mixin "hi there"
+
+            m:Text . should_equal "hi there"
+            (m:Mixin).mix . should_equal "hi there"
+
+            Meta.type_of (m:Text) . should_equal Text
+            Meta.type_of (m:Mixin) . should_equal Mixin
+            Meta.type_of m . should_equal Text
+
+        group_builder.specify "Returning Text & Mixin from method with check" <|
+            m = Mixin.text_and_mixin_with_check "hi there"
+
+            m:Text . should_equal "hi there"
+            (m:Mixin).mix . should_equal "hi there"
+
+            Meta.type_of (m:Text) . should_equal Text
+            Meta.type_of (m:Mixin) . should_equal Mixin
+            Meta.type_of m . should_equal Text
+
+        group_builder.specify "Returning Text & Mixin from Conversion method" <|
+            v = 42
+            m = v:Mixin
+
+            m:Text . should_equal "42"
+            (m:Mixin).mix . should_equal "42"
+
+            Meta.type_of (m:Text) . should_equal Text
+            Meta.type_of (m:Mixin) . should_equal Mixin
+            Meta.type_of m . should_equal Text
+
+
     suite_builder.group "Autoscoped Constructors" group_builder->
 
         group_builder.specify "Foo.Value as autoscoped" <|