Fix typing with optional fields and open records

bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/TypeChecker.java

@@ -1152,6 +1152,12 @@ private static boolean checkIsRecordType(BRecordType sourceRecordType, BRecordTy
                 if (!SymbolFlags.isFlagOn(targetField.getFlags(), SymbolFlags.OPTIONAL)) {
                     return false;
                 }
+
+                if (!sourceRecordType.sealed && !checkIsType(sourceRecordType.restFieldType, targetField.getFieldType(),
+                                                             unresolvedTypes)) {
+                    return false;
+                }
+
                 continue;
             }
 
@@ -1313,6 +1319,12 @@ private static boolean checkIsRecordType(MapValue sourceRecordValue, BRecordType
                 if (!SymbolFlags.isFlagOn(targetField.getFlags(), SymbolFlags.OPTIONAL)) {
                     return false;
                 }
+
+                if (!sourceRecordType.sealed && !checkIsType(sourceRecordType.restFieldType, targetField.getFieldType(),
+                                                             unresolvedTypes)) {
+                    return false;
+                }
+
                 continue;
             }
 

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java

@@ -3504,6 +3504,11 @@ private boolean checkFieldEquivalency(BRecordType lhsType, BRecordType rhsType,
                 if (!Symbols.isOptional(lhsField.symbol) || isInvalidNeverField(lhsField, rhsType)) {
                     return false;
                 }
+
+                if (!rhsType.sealed && !isAssignable(rhsType.restFieldType, lhsField.type, unresolvedTypes)) {
+                    return false;
+                }
+
                 continue;
             }
             if (hasIncompatibleReadOnlyFlags(lhsField.symbol.flags, rhsField.symbol.flags)) {

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/record/RecordAssignabilityTest.java

@@ -17,58 +17,82 @@
  */
 package org.ballerinalang.test.record;
 
-import org.ballerinalang.test.BAssertUtil;
 import org.ballerinalang.test.BCompileUtil;
+import org.ballerinalang.test.BRunUtil;
 import org.ballerinalang.test.CompileResult;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import static org.ballerinalang.test.BAssertUtil.validateError;
 import static org.testng.Assert.assertEquals;
 
 /**
  * Test record assignability.
  */
 public class RecordAssignabilityTest {
 
+    private final CompileResult result = BCompileUtil.compile("test-src/record/record_assignability.bal");
+
+    @DataProvider
+    public static Object[] recordAssignabilityTestFunctions() {
+        return new String[]{
+                "testOpenRecordToRecordWithOptionalFieldTypingRuntimeNegative",
+                "testRecordToRecordWithOptionalFieldTypingRuntimePositive"
+        };
+    }
+
+    @Test(dataProvider = "recordAssignabilityTestFunctions")
+    public void testRecordAssignability(String testFunction) {
+        BRunUtil.invoke(result, testFunction);
+    }
+
     @Test
     public void testRecordAssignabilityNegative() {
         CompileResult negativeResult = BCompileUtil.compile("test-src/record/record_assignability_negative.bal");
         int i = 0;
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; anydata...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; anydata...; |}'", 19, 55);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; anydata...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; anydata...; |}'", 20, 54);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'(boolean|string|int|record {| (boolean|int) b; anydata...; |})'," +
                 " found '(boolean|string|int|record {| (boolean|string) b; anydata...; |})'", 23, 53);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'(boolean|string|int|record {| (boolean|int) b; anydata...; |})'," +
                 " found '(boolean|string|int|record {| (boolean|string) b; anydata...; |})'", 24, 52);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; |}'", 29, 57);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; |}'", 30, 56);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string)...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean)...; |}'", 33, 58);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string)...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean)...; |}'", 34, 57);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; (int|string)...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; (int|boolean)...; |}'", 37, 72);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'record {| (int|string|boolean) a; (int|string) b; (int|string)...; |}'," +
                 " found 'record {| (int|string|boolean) a; (int|boolean) b; (int|boolean)...; |}'", 38, 71);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'(boolean|string|int|record {| (boolean|int) b; |})'," +
                 " found '(boolean|string|int|record {| (boolean|string) b; |})'", 41, 55);
-        BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
+        validateError(negativeResult, i++, "incompatible types: expected " +
                 "'(boolean|string|int|record {| (boolean|int) b; |})'," +
                 " found '(boolean|string|int|record {| (boolean|string) b; |})'", 42, 54);
+        validateError(negativeResult, i++, "incompatible types: expected 'R1', found 'R2'", 60, 12);
+        validateError(negativeResult, i++, "incompatible types: expected 'R1', found 'record {| (int|string)...; |}'",
+                      63, 12);
+        validateError(negativeResult, i++, "incompatible types: expected 'R1', found 'record {| int...; |}'", 66, 12);
+        validateError(negativeResult, i++, "incompatible types: expected 'R3', found 'record {| int...; |}'", 67, 12);
+        validateError(negativeResult, i++, "incompatible types: expected 'record {| int a?; int b; anydata...; |}', " +
+                "found 'record {| readonly int? b; int...; |}'", 70, 33);
         assertEquals(negativeResult.getErrorCount(), i);
     }
 }

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/types/never/NeverTypeTest.java

@@ -208,15 +208,17 @@ public void testNeverTypeNegative() {
         BAssertUtil.validateError(negativeCompileResult, i++, "incompatible types: expected " +
                 "'record {| |} & readonly', found 'record {| int x; never?...; |}'", 258, 25);
         BAssertUtil.validateError(negativeCompileResult, i++, "incompatible types: expected " +
-                "'record {| int x; never...; |}', found 'record {| |} & readonly'", 261, 41);
+                "'record {| int x; never...; |}', found 'record {| |} & readonly'", 261, 40);
+        BAssertUtil.validateError(negativeCompileResult, i++, "incompatible types: expected 'record {| never i?; " +
+                "anydata...; |}', found 'record {| never?...; |}'", 264, 28);
         BAssertUtil.validateError(negativeCompileResult, i++, "cannot define a variable of type 'never' or " +
-                "equivalent to type 'never'", 264, 1);
+                "equivalent to type 'never'", 267, 1);
         BAssertUtil.validateError(negativeCompileResult, i++, "cannot define a variable of type 'never' or " +
-                "equivalent to type 'never'", 267, 5);
+                "equivalent to type 'never'", 270, 5);
         BAssertUtil.validateError(negativeCompileResult, i++, "cannot define a variable of type 'never' or " +
-                "equivalent to type 'never'", 268, 5);
+                "equivalent to type 'never'", 271, 5);
         BAssertUtil.validateError(negativeCompileResult, i++, "cannot define a variable of type 'never' or " +
-                "equivalent to type 'never'", 272, 1);
+                "equivalent to type 'never'", 275, 1);
         Assert.assertEquals(negativeCompileResult.getErrorCount(), i);
     }
 

tests/jballerina-unit-test/src/test/resources/test-src/record/record_assignability.bal

@@ -0,0 +1,116 @@
+// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.com).
+//
+// WSO2 LLC. licenses this file to you under the Apache License,
+// Version 2.0 (the "License"); you may not use this file except
+// in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+type R1 record {
+    string y?;
+};
+
+type R2 record {
+    int x?;
+};
+
+type R3 record {|
+    string y?;
+    int...;
+|};
+
+type R4 record {
+    int a?;
+    string b;
+};
+
+type R5 record {
+    string b;
+};
+
+type R6 record {
+    int a?;
+    int b;
+};
+
+type R7 record {
+    readonly int? b;
+};
+
+function testOpenRecordToRecordWithOptionalFieldTypingRuntimeNegative() {
+    R2 a = {x: 1, "y": 10};
+    assertFalse(a is R1);
+
+    record {| int|string...; |} b = {};
+    assertFalse(b is R1);
+
+    record {| int...; |} c = {};
+    assertFalse(c is R1);
+    assertFalse(c is R3);
+
+    R5[] d = [];
+    assertFalse(d is R4[]);
+
+    R7 e = {b: 1};
+    assertFalse(e is R6);
+}
+
+type R8 record {
+    string y?;
+};
+
+type R9 record {|
+    int x?;
+    string...;
+|};
+
+type R10 record {
+    int a?;
+    int b;
+};
+
+type R11 record {|
+    readonly int? b;
+    int...;
+|};
+
+function testRecordToRecordWithOptionalFieldTypingRuntimePositive() {
+    R9 a = {};
+    R8 _ = a; // OK
+    assertTrue(<any>a is R8);
+
+    record {| int x?; |} b = {};
+    assertTrue(<any>b is R8);
+
+    record {| int x?; never...; |} c = {};
+    assertTrue(<any>c is R8);
+
+    R9[] d = [];
+    R8[] _ = d; // OK
+    assertTrue(<any>d is R8[]);
+
+    record {| readonly int? b; int...; |} e = {b: 1};
+    assertTrue(e is record { int a?; int b; });
+}
+
+function assertTrue(anydata actual) {
+    assertEquality(true, actual);
+}
+
+function assertFalse(anydata actual) {
+    assertEquality(false, actual);
+}
+
+function assertEquality(anydata expected, anydata actual) {
+    if expected != actual {
+        panic error(string `expected ${expected.toBalString()}, found ${actual.toBalString()}`);
+    }
+}

tests/jballerina-unit-test/src/test/resources/test-src/record/record_assignability_negative.bal

@@ -41,3 +41,31 @@ function testClosedRecordAssignabilityNegative() {
     boolean|string|int|record {|boolean|int b;|} r8 = r7;
     boolean|string|int|record {|boolean|int b;|} _ = r7;
 }
+
+type R1 record {
+    string y?;
+};
+
+type R2 record {
+    int x?;
+};
+
+type R3 record {|
+    string y?;
+    int...;
+|};
+
+function testOpenRecordToRecordWithIncompatibleOptionalFieldTyping() {
+    R2 r1 = {x: 1, "y": 10};
+    R1 _ = r1;
+
+    record {|int|string...;|} r2 = {};
+    R1 _ = r2;
+
+    record {|int...;|} r3 = {};
+    R1 _ = r3;
+    R3 _ = r3;
+
+    record {|readonly int? b; int...;|} r4 = {b: 1};
+    record {int a?; int b;} _ = r4;
+}

tests/jballerina-unit-test/src/test/resources/test-src/types/never/never-type-negative.bal

@@ -248,17 +248,20 @@ function testNeverFieldTypeBinding() {
 }
 
 function testNeverRestFieldType() {
-    record {|never...; |} a = {};
+    record {|never...;|} a = {};
     record {|int x;|} copy = a;
 
     record {|int x?;|} a1 = {};
     record {|never...; |} copy2 = a1;
 
-    record {|int x;never?...; |} a2 = {x: 12};
+    record {|int x;never?...;|} a2 = {x: 12};
     record {||} copy3 = a2;
 
     record {||} a3 = {};
-    record {|int x;never...; |} copy4 = a3;
+    record {|int x;never...;|} copy4 = a3;
+
+    record {|never?...;|} a4 = {};
+    record {never i?;} _ = a4;
 }
 
 never N = check error("Error"); // error

tests/jballerina-unit-test/src/test/resources/test-src/types/never/never_type_runtime.bal

@@ -160,7 +160,8 @@ function testNeverFieldTypeCheck() {
     assertEquality(true, r7 is record {never x?;});
 
     record {|never?...; |} r8 = {};
-    assertEquality(true, r8 is record {never x?;});
+    assertEquality(false, r8 is record {never x?;});
+    assertEquality(true, r8 is record {never? x?;});
 
     record {|int?...; |} r9 = {};
     assertEquality(false, r9 is record {never x?;});
@@ -191,10 +192,6 @@ function testNeverFieldTypeCheck() {
     record {never i?;} y1 = x1;
     assertEquality(true, y1 is record {|never...; |});
 
-    record {|never?...; |} x2 = {};
-    record {never i?;} y2 = x2;
-    assertEquality(true, y2 is record {|never?...; |});
-
     record {||} x3 = {};
     record {never i?;} y3 = x3;
     assertEquality(true, y3 is record {||});