Skip to content

Commit

Permalink
Merge pull request #41100 from ballerina-platform/fix-subtyping-w-opt…
Browse files Browse the repository at this point in the history
…ional-fields

Fix typing with optional fields and open records
  • Loading branch information
MaryamZi authored Aug 15, 2023
2 parents f6430ee + 9a9c189 commit 1417c40
Show file tree
Hide file tree
Showing 8 changed files with 213 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3496,6 +3496,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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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()}`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?;});
Expand Down Expand Up @@ -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 {||});
Expand Down

0 comments on commit 1417c40

Please sign in to comment.