Address review comments

ballerina-platform · Jun 26, 2024 · a5047f5 · a5047f5
1 parent 872d3aa
commit a5047f5
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 54 deletions.
diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/query/GroupByClauseTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/query/GroupByClauseTest.java
@@ -249,63 +249,63 @@ public void testNegativeCases() {
         BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected '[int,int]', found '[int...]'",
                 70, 36);
         BAssertUtil.validateError(negativeResult, i++, "invalid operation: type " +
-                        "'seq record {| string name; int price1; |}' does not support field access", 87, 29);
+                        "'seq record {| string name; int price1; |}' does not support field access", 78, 29);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 87, 29);
+                "list constructor or function invocation", 78, 29);
         BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected 'int[]', found 'seq int?'",
-                108, 32);
-        BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 116, 40);
+                99, 32);
+        BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 107, 40);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element" +
-                " list constructor or function invocation", 116, 40);
-        BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 120, 33);
+                " list constructor or function invocation", 107, 40);
+        BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 111, 33);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element" +
-                " list constructor or function invocation", 120, 33);
+                " list constructor or function invocation", 111, 33);
         BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
-                133, 36);
+                124, 36);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 133, 36);
+                "list constructor or function invocation", 124, 36);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 133, 45);
+                "list constructor or function invocation", 124, 45);
         BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
-                136, 28);
+                127, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 136, 28);
+                "list constructor or function invocation", 127, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 136, 37);
+                "list constructor or function invocation", 127, 37);
         BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
-                139, 28);
+                130, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 139, 28);
+                "list constructor or function invocation", 130, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 139, 37);
+                "list constructor or function invocation", 130, 37);
         BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
-                142, 28);
+                133, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 142, 28);
+                "list constructor or function invocation", 133, 28);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 142, 37);
+                "list constructor or function invocation", 133, 37);
         BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
-                145, 26);
+                136, 26);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 145, 26);
+                "list constructor or function invocation", 136, 26);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 145, 35);
-        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 148, 36);
-        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after rest argument", 151, 39);
-        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 154, 37);
-        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 154, 40);
+                "list constructor or function invocation", 136, 35);
+        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 139, 36);
+        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after rest argument", 142, 39);
+        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 145, 37);
+        BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 145, 40);
         BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected '(any|error)[]', found 'int'",
-                157, 37);
+                148, 37);
         BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
-                "'([int,int...]|record {| int n; |})', found '[int...]'", 167, 43);
+                "'([int,int...]|record {| int n; |})', found '[int...]'", 158, 43);
         BAssertUtil.validateError(negativeResult, i++, "invalid grouping key type 'error', expected a subtype of " +
-                "'anydata'", 175, 26);
+                "'anydata'", 166, 26);
         BAssertUtil.validateError(negativeResult, i++, "invalid grouping key type 'error', expected a subtype of " +
-                "'anydata'", 178, 26);
+                "'anydata'", 169, 26);
         BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected 'string', found 'seq string'",
-                200, 24);
+                191, 24);
         BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
-                "list constructor or function invocation", 200, 24);
+                "list constructor or function invocation", 191, 24);
         Assert.assertEquals(negativeResult.getErrorCount(), i);
     }
 

diff --git a/...na-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal b/...na-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal
@@ -264,30 +264,28 @@ function testVarAssignmentOfRecordLiteral3() {
     };
     assertEquality("Peter", fName);
     assertEquality(true, married);
-    assertEquality(12, age.get("age"));
-    assertEquality("Y", age.get("format"));
+    assertEquality({age: 12, format: "Y"}, age);
+    assertEquality({}, theMap);
 }
 
-function testVarAssignmentOfRecordLiteral4() returns error? {
+function testVarAssignmentOfRecordLiteral4() {
     string fName;
     boolean married;
     json age;
-    map<anydata> theMap;
+    map<any|error> theMap;
 
     {
         name: fName,
         age,
-        married,
         ...theMap
     } = {
         name: "Peter",
         married: true,
         age: {age: 12, format: "Y"}
     };
     assertEquality("Peter", fName);
-    assertEquality(true, married);
-    assertEquality(12, check age.age);
-    assertEquality("Y", check age.format);
+    assertEquality({age: 12, format: "Y"}, age);
+    assertEquality({married: true}, theMap);
 }
 
 function testVarAssignmentOfRecordLiteral5() {
@@ -305,7 +303,12 @@ function testVarAssignmentOfRecordLiteral5() {
         state: "New"
     };
     assertEquality("Peter", fName);
-    assertEquality("New York", theMap.get("city"));
+    assertEquality({
+                       married: true,
+                       age: {age: 12, format: "Y"},
+                       city: "New York",
+                       state: "New"
+                   }, theMap);
 }
 
 function testVarAssignmentOfRecordLiteral6() {
@@ -348,8 +351,15 @@ function testVarAssignmentOfRecordLiteral7() {
         }
     };
     assertEquality(1, empId);
-    assertEquality(30, otherEmpDetails.get("age"));
-    assertEquality(2, (<map<anydata>>otherDepDetails.get("location")).get("floor"));
+    assertEquality({
+                       id: 123,
+                       name: "Marketing",
+                       location: {floor: 2, area: "B"}
+                   }, otherDepDetails);
+    assertEquality({
+                       name: "John Doe",
+                       age: 30
+                   }, otherEmpDetails);
 }
 
 function testVarAssignmentOfRecordLiteral8() {
@@ -371,7 +381,14 @@ function testVarAssignmentOfRecordLiteral8() {
     };
     assertEquality(1, id);
     assertEquality("John Doe", name);
-    assertEquality(2, (<map<anydata>>(<map<anydata>>otherDetails.get("department")).get("location")).get("floor"));
+    assertEquality({
+                       age: 30,
+                       department: {
+                           id: 123,
+                           name: "Marketing",
+                           location: {floor: 2, area: "B"}
+                       }
+                   }, otherDetails);
 }
 
 type EmployeeDetails record {|
@@ -521,6 +538,13 @@ function testMappingBindingPatternAgainstOpenRecordInTupleDestructuring() {
     assertEquality(3, a);
     assertEquality(4, b);
 
+    record {|int b; string c;|} rec1 = {b: 2, c: "C"};
+    string c;
+    [{a, b, c}] = [{a: 1, ...rec1}];
+    assertEquality(1, a);
+    assertEquality(2, b);
+    assertEquality("C", c);
+
     [record {int a; int b; stream<int> c;}] r4 = [{a: 5, b: 6, c: new}];
     [{a, b}] = r4;
     assertEquality(5, a);
@@ -579,7 +603,7 @@ function testMappingBindingPatternAgainstOpenRecordInTupleDestructuring() {
     // error(x = {d}) = err;
 }
 
-function assertEquality(anydata expected, anydata actual) {
+function assertEquality(anydata expected, any actual) {
     if expected == actual {
         return;
     }

diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/query/group_by_clause_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/query/group_by_clause_negative.bal
@@ -71,15 +71,6 @@ function testTupleDestructure() {
                 };
 }
 
-function testRecordDestructure() {
-    _ = from var {name, price} in [{name: "Saman", price: 11}]
-                group by name
-                do {
-                    int[] prices;
-                    {prices} = {prices: [price]}; // error
-                };
-}
-
 function testSeqVarInInvalidPositions2() {
     var input = [{name: "Saman", price1: 11}];
     var xx = from var item in input