From 11074d312888f250cc3fc746ec3ad0bd06a00039 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Wed, 10 Jan 2024 12:36:57 +0530 Subject: [PATCH 01/22] Handle hash collision with put method --- .../internal/values/TableValueImpl.java | 35 ++++++++++++++-- .../langlib/test/LangLibTableTest.java | 14 ++++++- .../test/resources/test-src/tablelib_test.bal | 42 ++++++++++++++++++- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index badb37a26cf7..6040f1672c1c 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -571,12 +571,12 @@ public void addData(V data) { maxIntKey = ((Long) TypeChecker.anyToInt(key)).intValue(); } + Map.Entry entry = new AbstractMap.SimpleEntry(key, data); Long hash = TableUtils.hash(key, null); if (entries.containsKey(hash)) { updateIndexKeyMappings(hash, key, data); List> extEntries = entries.get(hash); - Map.Entry entry = new AbstractMap.SimpleEntry(key, data); extEntries.add(entry); List extValues = values.get(hash); extValues.add(data); @@ -585,7 +585,6 @@ public void addData(V data) { ArrayList newData = new ArrayList<>(); newData.add(data); - Map.Entry entry = new AbstractMap.SimpleEntry(key, data); putData(key, data, newData, entry, hash); } @@ -616,6 +615,10 @@ public V putData(K key, V data) { ErrorHelper.getErrorDetails(ErrorCodes.KEY_NOT_FOUND_IN_VALUE, key, data)); } + if (entries.containsKey(hash)) { + return handleHashCollisionInPut(key, data, entry, hash); + } + return putData(key, data, newData, entry, hash); } @@ -632,15 +635,39 @@ public V putData(V data) { MapValue dataMap = (MapValue) data; checkInherentTypeViolation(dataMap, tableType); K key = this.keyWrapper.wrapKey(dataMap); + Map.Entry entry = new AbstractMap.SimpleEntry<>(key, data); + Long hash = TableUtils.hash(key, null); + + if (entries.containsKey(hash)) { + return handleHashCollisionInPut(key, data, entry, hash); + } ArrayList newData = new ArrayList<>(); newData.add(data); - Map.Entry entry = new AbstractMap.SimpleEntry<>(key, data); - Long hash = TableUtils.hash(key, null); return putData((K) key, data, newData, entry, hash); } + private V handleHashCollisionInPut(K key, V data, Map.Entry entry, Long hash) { + updateIndexKeyMappings(hash, key, data); + List> entryList = entries.get(hash); + for (Map.Entry extEntry: entryList) { + if (TypeChecker.isEqual(key, extEntry.getKey())) { + entryList.remove(extEntry); + entryList.add(entry); + List valueList = values.get(hash); + valueList.remove(extEntry.getValue()); + valueList.add(data); + return data; + } + } + List> extEntries = entries.get(hash); + extEntries.add(entry); + List extValues = values.get(hash); + extValues.add(data); + return data; + } + public V remove(K key) { keyValues.remove(key); Long hash = TableUtils.hash(key, null); diff --git a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java index c5451fc43175..3e02526bf782 100644 --- a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java +++ b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java @@ -123,8 +123,18 @@ public void testHashCollisionHandlingScenarios() { } @Test - public void testHashCollisionInQuery() { - BRunUtil.invoke(compileResult, "testHashCollisionInQuery"); + public void testHashCollisionInQueryWithAdd() { + BRunUtil.invoke(compileResult, "testHashCollisionInQueryWithAdd"); + } + + @Test + public void testHashCollisionInQueryWithPut() { + BRunUtil.invoke(compileResult, "testHashCollisionInQueryWithPut"); + } + + @Test + public void testHashCollisionInFilter() { + BRunUtil.invoke(compileResult, "testHashCollisionInFilter"); } @Test diff --git a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal index 40394f1b6a5e..aaef8445f433 100644 --- a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal +++ b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal @@ -355,7 +355,7 @@ function testHashCollisionHandlingScenarios() { } -function testHashCollisionInQuery() { +function testHashCollisionInQueryWithAdd() { table key(k) tbl1 = table [{k: "10"}]; table tbl2 = table [{k: 0}]; @@ -379,6 +379,46 @@ function testHashCollisionInQuery() { assertEquals(tbl2, tbl4); } +function testHashCollisionInQueryWithPut() { + table key(k) tbl1 = table [{k: "10"}]; + table tbl2 = table [{k: 0}]; + + tbl1.put({k: 5}); + tbl1.put({k: ()}); + tbl1.put({k: -31}); + tbl1.put({k: 0}); + tbl1.put({k: 100.05}); + tbl1.put({k: 30}); + table tbl3 = + from var tid in tbl1 + where tid["k"] == 0 + select tid; + assertEquals(tbl2, tbl3); + + _ = tbl1.remove(()); + table tbl4 = + from var tid in tbl1 + where tid["k"] == 0 + select tid; + assertEquals(tbl2, tbl4); +} + +function testHashCollisionInFilter() { + table key(k) tbl1 = table [{k: "10"}]; + table key(k) tbl2 = table [{k: ()}, {k: 0}]; + + tbl1.put({k: 5}); + tbl1.put({k: ()}); + tbl1.put({k: -31}); + tbl1.put({k: 0}); + tbl1.put({k: 100.05}); + tbl1.put({k: 30}); + + table tbl3 = tbl1.filter( + function(record {readonly int|string|float? k;} tid) returns boolean + => tid["k"] == 0 || tid["k"] == ()); + assertEquals(tbl2, tbl3); +} public function testGetKeysOfHashCollidedKeys() { table key(k) tbl1 = table [ {k: 5}, {k: 0}, {k: ()}, {k: 2} From 1ebdc03aa6561e22da7f83713938bd75616f10be Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Wed, 17 Jan 2024 11:06:44 +0530 Subject: [PATCH 02/22] Address review suggestions --- .../internal/values/TableValueImpl.java | 6 ++-- .../test/resources/test-src/tablelib_test.bal | 28 ++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index 6040f1672c1c..9c40db1ed944 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -661,10 +661,8 @@ private V handleHashCollisionInPut(K key, V data, Map.Entry entry, Long ha return data; } } - List> extEntries = entries.get(hash); - extEntries.add(entry); - List extValues = values.get(hash); - extValues.add(data); + entryList.add(entry); + values.get(hash).add(data); return data; } diff --git a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal index aaef8445f433..a2bf5745942c 100644 --- a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal +++ b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal @@ -358,6 +358,8 @@ function testHashCollisionHandlingScenarios() { function testHashCollisionInQueryWithAdd() { table key(k) tbl1 = table [{k: "10"}]; table tbl2 = table [{k: 0}]; + table key(k) tbl3 = table [ + {k: "10"}, {k: 5}, {k: ()}, {k: -31}, {k: 0}, {k: 100.05}, {k: 30}]; tbl1.add({k: 5}); tbl1.add({k: ()}); @@ -365,23 +367,27 @@ function testHashCollisionInQueryWithAdd() { tbl1.add({k: 0}); tbl1.add({k: 100.05}); tbl1.add({k: 30}); - table tbl3 = + assertEquals(tbl3, tbl1); + + table tbl4 = from var tid in tbl1 where tid["k"] == 0 select tid; - assertEquals(tbl2, tbl3); + assertEquals(tbl2, tbl4); _ = tbl1.remove(()); - table tbl4 = + table tbl5 = from var tid in tbl1 where tid["k"] == 0 select tid; - assertEquals(tbl2, tbl4); + assertEquals(tbl2, tbl5); } function testHashCollisionInQueryWithPut() { table key(k) tbl1 = table [{k: "10"}]; table tbl2 = table [{k: 0}]; + table key(k) tbl3 = table [ + {k: "10"}, {k: 5}, {k: ()}, {k: -31}, {k: 0}, {k: 100.05}, {k: 30}]; tbl1.put({k: 5}); tbl1.put({k: ()}); @@ -389,18 +395,20 @@ function testHashCollisionInQueryWithPut() { tbl1.put({k: 0}); tbl1.put({k: 100.05}); tbl1.put({k: 30}); - table tbl3 = + assertEquals(tbl3, tbl1); + + table tbl4 = from var tid in tbl1 where tid["k"] == 0 select tid; - assertEquals(tbl2, tbl3); + assertEquals(tbl2, tbl4); _ = tbl1.remove(()); - table tbl4 = + table tbl5 = from var tid in tbl1 where tid["k"] == 0 select tid; - assertEquals(tbl2, tbl4); + assertEquals(tbl2, tbl5); } function testHashCollisionInFilter() { @@ -414,10 +422,10 @@ function testHashCollisionInFilter() { tbl1.put({k: 100.05}); tbl1.put({k: 30}); - table tbl3 = tbl1.filter( + table tbl4 = tbl1.filter( function(record {readonly int|string|float? k;} tid) returns boolean => tid["k"] == 0 || tid["k"] == ()); - assertEquals(tbl2, tbl3); + assertEquals(tbl2, tbl4); } public function testGetKeysOfHashCollidedKeys() { table key(k) tbl1 = table [ From 0a746ff5eb2b5b175a8f2855889eddb31749518e Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Wed, 31 Jan 2024 11:55:59 +0530 Subject: [PATCH 03/22] Address review suggestions --- .../internal/values/TableValueImpl.java | 6 +-- .../langlib/test/LangLibTableTest.java | 41 ++++++++----------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index 9c40db1ed944..e07028ab6992 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -616,7 +616,7 @@ public V putData(K key, V data) { } if (entries.containsKey(hash)) { - return handleHashCollisionInPut(key, data, entry, hash); + return handleHashCollisionForPut(key, data, entry, hash); } return putData(key, data, newData, entry, hash); @@ -639,7 +639,7 @@ public V putData(V data) { Long hash = TableUtils.hash(key, null); if (entries.containsKey(hash)) { - return handleHashCollisionInPut(key, data, entry, hash); + return handleHashCollisionForPut(key, data, entry, hash); } ArrayList newData = new ArrayList<>(); @@ -648,7 +648,7 @@ public V putData(V data) { return putData((K) key, data, newData, entry, hash); } - private V handleHashCollisionInPut(K key, V data, Map.Entry entry, Long hash) { + private V handleHashCollisionForPut(K key, V data, Map.Entry entry, Long hash) { updateIndexKeyMappings(hash, key, data); List> entryList = entries.get(hash); for (Map.Entry extEntry: entryList) { diff --git a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java index 3e02526bf782..89a71714b5d5 100644 --- a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java +++ b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java @@ -117,31 +117,6 @@ public void testGetValue() { BRunUtil.invoke(compileResult, "testGetValue"); } - @Test - public void testHashCollisionHandlingScenarios() { - BRunUtil.invoke(compileResult, "testHashCollisionHandlingScenarios"); - } - - @Test - public void testHashCollisionInQueryWithAdd() { - BRunUtil.invoke(compileResult, "testHashCollisionInQueryWithAdd"); - } - - @Test - public void testHashCollisionInQueryWithPut() { - BRunUtil.invoke(compileResult, "testHashCollisionInQueryWithPut"); - } - - @Test - public void testHashCollisionInFilter() { - BRunUtil.invoke(compileResult, "testHashCollisionInFilter"); - } - - @Test - public void testGetKeysOfHashCollidedKeys() { - BRunUtil.invoke(compileResult, "testGetKeysOfHashCollidedKeys"); - } - @Test public void testGetKeyList() { Object result = BRunUtil.invoke(compileResult, "testGetKeyList"); @@ -612,4 +587,20 @@ public void testTableIterationAfterPut() { BRunUtil.invoke(compileResult, "testTableIterationAfterPut3"); BRunUtil.invoke(compileResult, "testTableIterationAfterPut4"); } + + @Test(dataProvider = "functionsToTestHashCollisionInTable") + public void testHashCollisionInTable(String function) { + BRunUtil.invoke(compileResult, function); + } + + @DataProvider + public Object[] functionsToTestHashCollisionInTable() { + return new String[]{ + "testHashCollisionHandlingScenarios", + "testHashCollisionInQueryWithAdd", + "testHashCollisionInQueryWithPut", + "testHashCollisionInFilter", + "testGetKeysOfHashCollidedKeys" + }; + } } From feba581dec2c25610c56c8a006379287a8a0ab8a Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Thu, 7 Mar 2024 15:13:06 +0530 Subject: [PATCH 04/22] Refactor the code for better readability --- .../internal/values/TableValueImpl.java | 7 ++-- .../test/resources/test-src/tablelib_test.bal | 36 +++++++++++++------ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index e07028ab6992..30e680cd8d77 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -616,7 +616,7 @@ public V putData(K key, V data) { } if (entries.containsKey(hash)) { - return handleHashCollisionForPut(key, data, entry, hash); + return updateExistingEntry(key, data, entry, hash); } return putData(key, data, newData, entry, hash); @@ -639,7 +639,7 @@ public V putData(V data) { Long hash = TableUtils.hash(key, null); if (entries.containsKey(hash)) { - return handleHashCollisionForPut(key, data, entry, hash); + return updateExistingEntry(key, data, entry, hash); } ArrayList newData = new ArrayList<>(); @@ -648,10 +648,11 @@ public V putData(V data) { return putData((K) key, data, newData, entry, hash); } - private V handleHashCollisionForPut(K key, V data, Map.Entry entry, Long hash) { + private V updateExistingEntry(K key, V data, Map.Entry entry, Long hash) { updateIndexKeyMappings(hash, key, data); List> entryList = entries.get(hash); for (Map.Entry extEntry: entryList) { + // Handle hash-collided entries if (TypeChecker.isEqual(key, extEntry.getKey())) { entryList.remove(extEntry); entryList.add(entry); diff --git a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal index a2bf5745942c..3ee983658b63 100644 --- a/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal +++ b/langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal @@ -384,17 +384,19 @@ function testHashCollisionInQueryWithAdd() { } function testHashCollisionInQueryWithPut() { - table key(k) tbl1 = table [{k: "10"}]; - table tbl2 = table [{k: 0}]; - table key(k) tbl3 = table [ - {k: "10"}, {k: 5}, {k: ()}, {k: -31}, {k: 0}, {k: 100.05}, {k: 30}]; - - tbl1.put({k: 5}); - tbl1.put({k: ()}); - tbl1.put({k: -31}); - tbl1.put({k: 0}); - tbl1.put({k: 100.05}); - tbl1.put({k: 30}); + table key(k) tbl1 = table [{k: "10", v: 1}]; + table tbl2 = table [{k: 0, v: 2}]; + table key(k) tbl3 = table [ + {k: "10", v: 1}, {k: 5, v: 2}, {k: 0, v: 2}, {k: (), v: 3}, {k: -31, v: 4}, {k: 100.05, v: 1}, {k: 30, v: 2}]; + + tbl1.put({k: 5, v: 1}); + tbl1.put({k: (), v: 1}); + tbl1.put({k: -31, v: 4}); + tbl1.put({k: 0, v: 2}); + tbl1.put({k: 5, v: 2}); + tbl1.put({k: 100.05, v: 1}); + tbl1.put({k: 30, v: 2}); + tbl1.put({k: (), v: 3}); assertEquals(tbl3, tbl1); table tbl4 = @@ -433,6 +435,18 @@ public function testGetKeysOfHashCollidedKeys() { ]; assertEquals(tbl1.keys(), [5, 0, (), 2]); + + table key(k) tbl2 = table [ + {k: 5}, {k: 0}, {k: 2} + ]; + tbl2.add({k: ()}); + assertEquals(tbl2.keys(), [5, 0, 2, ()]); + + table key(k) tbl3 = table [ + {k: 5}, {k: 0}, {k: 2} + ]; + tbl3.put({k: ()}); + assertEquals(tbl3.keys(), [5, 0, 2, ()]); } function testGetKeyList() returns any[] { From 0feb7207bbd306bc98f3b3432b2d6913f820d465 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Thu, 7 Mar 2024 15:25:08 +0530 Subject: [PATCH 05/22] Extract adding a new value to putNewData --- .../internal/values/TableValueImpl.java | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index 30e680cd8d77..522f17d39153 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -583,9 +583,7 @@ public void addData(V data) { return; } - ArrayList newData = new ArrayList<>(); - newData.add(data); - putData(key, data, newData, entry, hash); + putNewData(key, data, entry, hash); } public V getData(K key) { @@ -602,9 +600,6 @@ public V getData(K key) { } public V putData(K key, V data) { - ArrayList newData = new ArrayList<>(); - newData.add(data); - Map.Entry entry = new AbstractMap.SimpleEntry(key, data); Object actualKey = this.keyWrapper.wrapKey((MapValue) data); Long actualHash = TableUtils.hash(actualKey, null); @@ -619,10 +614,12 @@ public V putData(K key, V data) { return updateExistingEntry(key, data, entry, hash); } - return putData(key, data, newData, entry, hash); + return putNewData(key, data, entry, hash); } - private V putData(K key, V value, List data, Map.Entry entry, Long hash) { + private V putNewData(K key, V value, Map.Entry entry, Long hash) { + List data = new ArrayList<>(); + data.add(value); updateIndexKeyMappings(hash, key, value); List> entryList = new ArrayList<>(); entryList.add(entry); @@ -642,10 +639,7 @@ public V putData(V data) { return updateExistingEntry(key, data, entry, hash); } - ArrayList newData = new ArrayList<>(); - newData.add(data); - - return putData((K) key, data, newData, entry, hash); + return putNewData((K) key, data, entry, hash); } private V updateExistingEntry(K key, V data, Map.Entry entry, Long hash) { From 3672912e3413cdca23be02f0e2f8e93728fed21a Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Fri, 8 Mar 2024 16:42:14 +0530 Subject: [PATCH 06/22] Address review suggestions --- .../ballerina/runtime/internal/values/TableValueImpl.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java index 522f17d39153..796554b95492 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TableValueImpl.java @@ -613,7 +613,6 @@ public V putData(K key, V data) { if (entries.containsKey(hash)) { return updateExistingEntry(key, data, entry, hash); } - return putNewData(key, data, entry, hash); } @@ -638,22 +637,19 @@ public V putData(V data) { if (entries.containsKey(hash)) { return updateExistingEntry(key, data, entry, hash); } - return putNewData((K) key, data, entry, hash); } private V updateExistingEntry(K key, V data, Map.Entry entry, Long hash) { updateIndexKeyMappings(hash, key, data); + List valueList = values.get(hash); List> entryList = entries.get(hash); for (Map.Entry extEntry: entryList) { // Handle hash-collided entries if (TypeChecker.isEqual(key, extEntry.getKey())) { entryList.remove(extEntry); - entryList.add(entry); - List valueList = values.get(hash); valueList.remove(extEntry.getValue()); - valueList.add(data); - return data; + break; } } entryList.add(entry); From f4a4e2cb85ce8505f418fb9c217ccac2b072415d Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Fri, 27 Oct 2023 12:03:21 +0530 Subject: [PATCH 07/22] Support actions in Ballerina shell --- .../ballerina/shell/DiagnosticReporter.java | 2 +- .../snippet/factory/BasicSnippetFactory.java | 24 ++++++--- .../test/evaluator/AbstractEvaluatorTest.java | 8 +++ .../test/evaluator/ActionEvaluatorTest.java | 52 +++++++++++++++++++ .../shell/test/evaluator/base/TestCase.java | 12 +++++ .../testcases/evaluator/actions.remote.json | 24 +++++++++ .../testcases/evaluator/actions.resource.json | 24 +++++++++ .../testcases/evaluator/actions.start.json | 20 +++++++ .../testcases/evaluator/actions.var.json | 14 +++++ 9 files changed, 172 insertions(+), 8 deletions(-) create mode 100644 ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java create mode 100644 ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.remote.json create mode 100644 ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.resource.json create mode 100644 ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.start.json create mode 100644 ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/DiagnosticReporter.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/DiagnosticReporter.java index 824e4b3d9c2d..89327f65e3a9 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/DiagnosticReporter.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/DiagnosticReporter.java @@ -86,7 +86,7 @@ public void addAllDiagnostics(Collection diagnostics) { * * @return All the collected diagnostics. */ - public Collection diagnostics() { + public List diagnostics() { return this.diagnostics; } diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java index 230ebce7eccc..d5c761a34337 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java @@ -153,19 +153,29 @@ public List createVariableDeclarationSnippets(Node n qualifiers = NodeFactory.createNodeList(varNode.finalKeyword().get()); } - if (((VariableDeclarationNode) node).initializer().get().kind() == SyntaxKind.QUERY_ACTION) { + if (((VariableDeclarationNode) node).initializer().get().kind().toString().endsWith("_ACTION")) { TypedBindingPatternNode typedBindingPatternNode = varNode.typedBindingPattern(); TypeDescriptorNode typeDescriptorNode = typedBindingPatternNode.typeDescriptor(); BindingPatternNode bindingPatternNode = typedBindingPatternNode.bindingPattern(); - String functionPart = "function() returns "; + + // Check if the type descriptor of the variable is 'var' as it is not yet supported. + if (typeDescriptorNode.kind() == SyntaxKind.VAR_TYPE_DESC) { + addErrorDiagnostic("Actions not allowed with a 'var' TypeDescriptor."); + return null; + } + + boolean hasCheck = ((VariableDeclarationNode) node).initializer().get().kind() == SyntaxKind.CHECK_ACTION; String functionBody = varNode.initializer().get().toString(); - String functionString = "var " + "f_" + varFunctionCount + " = " + functionPart - + typeDescriptorNode.toString() + "{" + typeDescriptorNode + bindingPatternNode.toString() - + " = " + functionBody + ";" + "return " + bindingPatternNode + ";};"; + String functionDefinition = (hasCheck ? "function() returns error|" : + "function() returns ") + typeDescriptorNode; + String functionString = + functionDefinition + "f_" + varFunctionCount + " = " + functionDefinition + "{" + + typeDescriptorNode + bindingPatternNode.toString() + " = " + functionBody + ";" + + "return " + bindingPatternNode + ";};"; varNode = (VariableDeclarationNode) NodeParser.parseStatement(functionString); newNode = (VariableDeclarationNode) NodeParser - .parseStatement(typeDescriptorNode + " " + bindingPatternNode - + " = f_" + varFunctionCount + "();"); + .parseStatement(typeDescriptorNode + " " + bindingPatternNode + " = " + + (hasCheck ? "check " : "") + " f_" + varFunctionCount + "();"); } varFunctionCount += 1; diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java index b7a4e78101bc..323658b54504 100644 --- a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java @@ -83,6 +83,14 @@ protected void testEvaluate(String fileName) throws BallerinaShellException { ShellCompilation shellCompilation = evaluator.getCompilation(testCase.getCode()); Optional compilation = shellCompilation.getPackageCompilation(); + + if (compilation.isEmpty() && testCase.getStderr().size() > 0) { + for (int i = 0; i < testCase.getStderr().size(); i++) { + Assert.assertEquals(evaluator.diagnostics().get(i).toString(), testCase.getStderr().get(i)); + } + continue; + } + String expr = evaluator.getValue(compilation).get().getResult(); Assert.assertEquals(invoker.getStdOut(), testCase.getStdout(), testCase.getDescription()); Assert.assertEquals(expr, testCase.getExpr(), testCase.getDescription()); diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java new file mode 100644 index 000000000000..72c548399352 --- /dev/null +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2023, WSO2 Inc. (http://wso2.com) All Rights Reserved. + * + * Licensed 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. + */ + +package io.ballerina.shell.test.evaluator; + +import io.ballerina.shell.exceptions.BallerinaShellException; +import org.testng.annotations.Test; + +/** + * Test simple snippets with action invocations. + * + * @since 2201.9.0 + */ +public class ActionEvaluatorTest extends AbstractEvaluatorTest { + private static final String START_EVALUATOR_TESTCASE = "testcases/evaluator/actions.start.json"; + private static final String REMOTE_EVALUATOR_TESTCASE = "testcases/evaluator/actions.remote.json"; + private static final String RESOURCE_EVALUATOR_TESTCASE = "testcases/evaluator/actions.resource.json"; + private static final String VAR_EVALUATOR_TESTCASE = "testcases/evaluator/actions.var.json"; + + @Test + public void testEvaluateStart() throws BallerinaShellException { + testEvaluate(START_EVALUATOR_TESTCASE); + } + + @Test + public void testEvaluateRemote() throws BallerinaShellException { + testEvaluate(REMOTE_EVALUATOR_TESTCASE); + } + + @Test + public void testEvaluateResource() throws BallerinaShellException { + testEvaluate(RESOURCE_EVALUATOR_TESTCASE); + } + + @Test + public void testEvaluateVar() throws BallerinaShellException { + testEvaluate(VAR_EVALUATOR_TESTCASE); + } +} diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/base/TestCase.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/base/TestCase.java index 85c717283da8..503b2733321f 100644 --- a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/base/TestCase.java +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/base/TestCase.java @@ -18,6 +18,9 @@ package io.ballerina.shell.test.evaluator.base; +import java.util.ArrayList; +import java.util.List; + /** * One statement in a evaluator test case. * @@ -29,6 +32,7 @@ public class TestCase { private String expr; private String stdout = ""; private String error; + private List stderr = new ArrayList<>(); public String getDescription() { return description; @@ -69,4 +73,12 @@ public String getError() { public void setError(String error) { this.error = error; } + + public List getStderr() { + return stderr; + } + + public void setStderr(List stderr) { + this.stderr = stderr; + } } diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.remote.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.remote.json new file mode 100644 index 000000000000..8a7139f74c36 --- /dev/null +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.remote.json @@ -0,0 +1,24 @@ +[ + { + "description": "Define a client class", + "code": "client class MyClient { remote function invoke(string param) returns string { return string `remote method invoked with ${param}`; } }" + }, + { + "description": "Initialize a client class", + "code": "MyClient myClient = new;" + }, + { + "description": "Invoke a client remote method as an expression", + "code": "myClient->invoke(\"input\");", + "expr": "\"remote method invoked with input\"" + }, + { + "description": "Invoke a client remote method with a variable", + "code": "string returnValue = myClient->invoke(\"input\");" + }, + { + "description": "Check the return value", + "code": "returnValue", + "expr": "\"remote method invoked with input\"" + } +] diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.resource.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.resource.json new file mode 100644 index 000000000000..ee8924d7575e --- /dev/null +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.resource.json @@ -0,0 +1,24 @@ +[ + { + "description": "Define a client class", + "code": "client class MyClient { resource function get root/[string path](string param) returns string { return string `${path} -> ${param}`; } }" + }, + { + "description": "Initialize a client class", + "code": "MyClient myClient = new;" + }, + { + "description": "Invoke a client resource method as an expression", + "code": "myClient ->/root/name(\"input\");", + "expr": "\"name -> input\"" + }, + { + "description": "Invoke a client resource method with a variable", + "code": "string returnValue = myClient ->/root/name(\"input\");" + }, + { + "description": "Check the return value", + "code": "returnValue", + "expr": "\"name -> input\"" + } +] diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.start.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.start.json new file mode 100644 index 000000000000..16c1b82692d7 --- /dev/null +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.start.json @@ -0,0 +1,20 @@ +[ + { + "description": "Initialize the function", + "code": "function name() returns int { int x = 3; x += 3 * 2; return x; }" + }, + { + "description": "Execute the function with start as an expression", + "code": "start name()", + "expr": "future {isDone:true,result:9}" + }, + { + "description": "Execute the function with start with a variable assignment", + "code": "future futureResult = start name()" + }, + { + "description": "Check the return value", + "code": "futureResult", + "expr": "future {isDone:true,result:9}" + } +] diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json new file mode 100644 index 000000000000..3abc3fea91b4 --- /dev/null +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json @@ -0,0 +1,14 @@ +[ + { + "description": "Initialize the function", + "code": "function name() returns int { int x = 3; x += 3 * 2; return x; }" + }, + { + "description": "Execute the function with start with a variable assignment", + "code": "var futureResult = start name()", + "stderr": [ + "Actions not allowed with a 'var' TypeDescriptor.", + "Compilation aborted due to invalid input." + ] + } +] From bf8b64c49d0021da52f19a34cbf58d6b5bb71d0d Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Fri, 27 Oct 2023 14:13:56 +0530 Subject: [PATCH 08/22] Restrict the user from using the wrapper name --- .../shell/parser/ParserConstants.java | 1 + .../shell/parser/SerialTreeParser.java | 3 ++- .../shell/parser/trials/ModuleMemberTrial.java | 3 ++- .../snippet/factory/BasicSnippetFactory.java | 18 ++++++++++-------- .../test/evaluator/ActionEvaluatorTest.java | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java index e42d123d780f..5e5f1187f7a4 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java @@ -26,6 +26,7 @@ * @since 2.0.0 */ public class ParserConstants { + public static final String WRAPPER_PREFIX = "__shell_wrapper__"; public static final Set RESTRICTED_FUNCTION_NAMES = Set.of("main", "init", "__java_recall", "__java_memorize", "__recall_any", "__recall_any_error", "__memorize", "__stmts", "__run"); diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java index 79b1faa27ac6..4076f176a6ab 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java @@ -123,7 +123,8 @@ public Collection parseDeclarations(String source) throws TreeParserExcept private boolean isModuleDeclarationAllowed(ModuleMemberDeclarationNode declarationNode) { if (declarationNode instanceof FunctionDefinitionNode) { String functionName = ((FunctionDefinitionNode) declarationNode).functionName().text(); - if (RESTRICTED_FUNCTION_NAMES.contains(functionName)) { + if (RESTRICTED_FUNCTION_NAMES.contains(functionName) || + functionName.startsWith(ParserConstants.WRAPPER_PREFIX)) { addWarnDiagnostic("Found '" + functionName + "' function in the declarations.\n" + "Discarded '" + functionName + "' function without loading."); return false; diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java index a4f2f0995a73..188938842688 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java @@ -80,7 +80,8 @@ public Collection parse(String source) throws ParserTrialFailedException { private void validateModuleDeclaration(ModuleMemberDeclarationNode declarationNode) { if (declarationNode instanceof FunctionDefinitionNode) { String functionName = ((FunctionDefinitionNode) declarationNode).functionName().text(); - if (RESTRICTED_FUNCTION_NAMES.contains(functionName)) { + if (RESTRICTED_FUNCTION_NAMES.contains(functionName) || + functionName.startsWith(ParserConstants.WRAPPER_PREFIX)) { String message = "Function name " + "'" + functionName + "'" + " not allowed in Ballerina Shell.\n"; throw new InvalidMethodException(message); } diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java index d5c761a34337..e216db74412a 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java @@ -65,6 +65,7 @@ import io.ballerina.compiler.syntax.tree.WhileStatementNode; import io.ballerina.compiler.syntax.tree.XMLNamespaceDeclarationNode; import io.ballerina.shell.exceptions.SnippetException; +import io.ballerina.shell.parser.ParserConstants; import io.ballerina.shell.snippet.SnippetSubKind; import io.ballerina.shell.snippet.types.ExpressionSnippet; import io.ballerina.shell.snippet.types.ImportDeclarationSnippet; @@ -164,18 +165,19 @@ public List createVariableDeclarationSnippets(Node n return null; } - boolean hasCheck = ((VariableDeclarationNode) node).initializer().get().kind() == SyntaxKind.CHECK_ACTION; + boolean hasCheck = + ((VariableDeclarationNode) node).initializer().get().kind() == SyntaxKind.CHECK_ACTION; String functionBody = varNode.initializer().get().toString(); String functionDefinition = (hasCheck ? "function() returns error|" : "function() returns ") + typeDescriptorNode; - String functionString = - functionDefinition + "f_" + varFunctionCount + " = " + functionDefinition + "{" + - typeDescriptorNode + bindingPatternNode.toString() + " = " + functionBody + ";" + - "return " + bindingPatternNode + ";};"; + String functionName = ParserConstants.WRAPPER_PREFIX + varFunctionCount; + String functionString = String.format("%s %s = %s {%s %s = %s; return %s;};", functionDefinition, + functionName, functionDefinition, typeDescriptorNode, bindingPatternNode, functionBody, + bindingPatternNode); varNode = (VariableDeclarationNode) NodeParser.parseStatement(functionString); - newNode = (VariableDeclarationNode) NodeParser - .parseStatement(typeDescriptorNode + " " + bindingPatternNode + " = " + - (hasCheck ? "check " : "") + " f_" + varFunctionCount + "();"); + newNode = (VariableDeclarationNode) NodeParser.parseStatement( + String.format("%s %s = %s %s();", typeDescriptorNode, bindingPatternNode, + (hasCheck ? "check " : ""), functionName)); } varFunctionCount += 1; diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java index 72c548399352..bb9d111a5a57 100644 --- a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, WSO2 Inc. (http://wso2.com) All Rights Reserved. + * Copyright (c) 2023, WSO2 LLC. (http://wso2.com) All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 2621dc8adf1feb0c8d7fe84f70de6bef83253613 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Tue, 2 Jan 2024 21:31:55 +0530 Subject: [PATCH 09/22] Refactor the checking of action syntax --- .../snippet/factory/BasicSnippetFactory.java | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java index e216db74412a..00a4a471b6b6 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java @@ -76,6 +76,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Optional; /** * A factory that will create snippets from given nodes. @@ -154,7 +155,13 @@ public List createVariableDeclarationSnippets(Node n qualifiers = NodeFactory.createNodeList(varNode.finalKeyword().get()); } - if (((VariableDeclarationNode) node).initializer().get().kind().toString().endsWith("_ACTION")) { + Optional varInitNode = varNode.initializer(); + if (varInitNode.isEmpty()) { + return null; + } + + SyntaxKind nodeKind = varInitNode.get().kind(); + if (isSupportedAction(nodeKind)) { TypedBindingPatternNode typedBindingPatternNode = varNode.typedBindingPattern(); TypeDescriptorNode typeDescriptorNode = typedBindingPatternNode.typeDescriptor(); BindingPatternNode bindingPatternNode = typedBindingPatternNode.bindingPattern(); @@ -165,9 +172,8 @@ public List createVariableDeclarationSnippets(Node n return null; } - boolean hasCheck = - ((VariableDeclarationNode) node).initializer().get().kind() == SyntaxKind.CHECK_ACTION; - String functionBody = varNode.initializer().get().toString(); + boolean hasCheck = nodeKind == SyntaxKind.CHECK_ACTION; + String functionBody = varInitNode.get().toString(); String functionDefinition = (hasCheck ? "function() returns error|" : "function() returns ") + typeDescriptorNode; String functionName = ParserConstants.WRAPPER_PREFIX + varFunctionCount; @@ -278,4 +284,25 @@ private boolean containsIsolated(Node node) { return false; } + + private boolean isSupportedAction(SyntaxKind nodeKind) { + switch (nodeKind) { + case REMOTE_METHOD_CALL_ACTION: + case BRACED_ACTION: + case CHECK_ACTION: + case START_ACTION: + case TRAP_ACTION: + case FLUSH_ACTION: + case ASYNC_SEND_ACTION: + case SYNC_SEND_ACTION: + case RECEIVE_ACTION: + case WAIT_ACTION: + case QUERY_ACTION: + case COMMIT_ACTION: + case CLIENT_RESOURCE_ACCESS_ACTION: + return true; + default: + return false; + } + } } From 5d4c39a6958d33660185d9fb87ada419f561d46b Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Sun, 17 Mar 2024 21:28:12 +0530 Subject: [PATCH 10/22] Refactor the checking of restricted functions --- .../io/ballerina/shell/parser/ParserConstants.java | 11 +++++++++++ .../io/ballerina/shell/parser/SerialTreeParser.java | 3 +-- .../shell/parser/trials/ModuleMemberTrial.java | 3 +-- .../shell/snippet/factory/BasicSnippetFactory.java | 13 ++++++++----- .../resources/testcases/evaluator/actions.var.json | 2 +- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java index 5e5f1187f7a4..f22c0524150c 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java @@ -26,10 +26,21 @@ * @since 2.0.0 */ public class ParserConstants { + public static final String WRAPPER_PREFIX = "__shell_wrapper__"; public static final Set RESTRICTED_FUNCTION_NAMES = Set.of("main", "init", "__java_recall", "__java_memorize", "__recall_any", "__recall_any_error", "__memorize", "__stmts", "__run"); + /** + * Checks if the given function name is restricted. + * + * @param functionName Function name to check. + * @return True if the function name is restricted. + */ + public static boolean isFunctionNameRestricted(String functionName) { + return RESTRICTED_FUNCTION_NAMES.contains(functionName) || functionName.startsWith(WRAPPER_PREFIX); + } + private ParserConstants() {} } diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java index 4076f176a6ab..4f0e322c3c3b 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java @@ -123,8 +123,7 @@ public Collection parseDeclarations(String source) throws TreeParserExcept private boolean isModuleDeclarationAllowed(ModuleMemberDeclarationNode declarationNode) { if (declarationNode instanceof FunctionDefinitionNode) { String functionName = ((FunctionDefinitionNode) declarationNode).functionName().text(); - if (RESTRICTED_FUNCTION_NAMES.contains(functionName) || - functionName.startsWith(ParserConstants.WRAPPER_PREFIX)) { + if (ParserConstants.isFunctionNameRestricted(functionName)) { addWarnDiagnostic("Found '" + functionName + "' function in the declarations.\n" + "Discarded '" + functionName + "' function without loading."); return false; diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java index 188938842688..10924c2f477f 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java @@ -80,8 +80,7 @@ public Collection parse(String source) throws ParserTrialFailedException { private void validateModuleDeclaration(ModuleMemberDeclarationNode declarationNode) { if (declarationNode instanceof FunctionDefinitionNode) { String functionName = ((FunctionDefinitionNode) declarationNode).functionName().text(); - if (RESTRICTED_FUNCTION_NAMES.contains(functionName) || - functionName.startsWith(ParserConstants.WRAPPER_PREFIX)) { + if (ParserConstants.isFunctionNameRestricted(functionName)) { String message = "Function name " + "'" + functionName + "'" + " not allowed in Ballerina Shell.\n"; throw new InvalidMethodException(message); } diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java index 00a4a471b6b6..65032adeb4c7 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java @@ -155,12 +155,15 @@ public List createVariableDeclarationSnippets(Node n qualifiers = NodeFactory.createNodeList(varNode.finalKeyword().get()); } - Optional varInitNode = varNode.initializer(); - if (varInitNode.isEmpty()) { + Optional optVarInitNode = varNode.initializer(); + if (optVarInitNode.isEmpty()) { + assert false : "This line is unreachable as the error is captured before."; + addErrorDiagnostic("Variable declaration without an initializer is not allowed"); return null; } - SyntaxKind nodeKind = varInitNode.get().kind(); + ExpressionNode varInitNode = optVarInitNode.get(); + SyntaxKind nodeKind = varInitNode.kind(); if (isSupportedAction(nodeKind)) { TypedBindingPatternNode typedBindingPatternNode = varNode.typedBindingPattern(); TypeDescriptorNode typeDescriptorNode = typedBindingPatternNode.typeDescriptor(); @@ -168,12 +171,12 @@ public List createVariableDeclarationSnippets(Node n // Check if the type descriptor of the variable is 'var' as it is not yet supported. if (typeDescriptorNode.kind() == SyntaxKind.VAR_TYPE_DESC) { - addErrorDiagnostic("Actions not allowed with a 'var' TypeDescriptor."); + addErrorDiagnostic("Actions not allowed with a 'var' TypeDescriptor"); return null; } boolean hasCheck = nodeKind == SyntaxKind.CHECK_ACTION; - String functionBody = varInitNode.get().toString(); + String functionBody = varInitNode.toSourceCode(); String functionDefinition = (hasCheck ? "function() returns error|" : "function() returns ") + typeDescriptorNode; String functionName = ParserConstants.WRAPPER_PREFIX + varFunctionCount; diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json index 3abc3fea91b4..9c0d3849a015 100644 --- a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json @@ -7,7 +7,7 @@ "description": "Execute the function with start with a variable assignment", "code": "var futureResult = start name()", "stderr": [ - "Actions not allowed with a 'var' TypeDescriptor.", + "Actions not allowed with a 'var' TypeDescriptor", "Compilation aborted due to invalid input." ] } From 8ee57fe39621394c2dabf423201062b836d4ff58 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Tue, 19 Mar 2024 13:59:38 +0530 Subject: [PATCH 11/22] Address review suggestions --- .../ballerina/shell/parser/ParserConstants.java | 4 ++-- .../snippet/factory/BasicSnippetFactory.java | 16 ++++++++-------- .../test/evaluator/AbstractEvaluatorTest.java | 2 +- .../test/evaluator/ActionEvaluatorTest.java | 2 +- .../testcases/evaluator/actions.var.json | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java index f22c0524150c..a56297d31a91 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/ParserConstants.java @@ -35,8 +35,8 @@ public class ParserConstants { /** * Checks if the given function name is restricted. * - * @param functionName Function name to check. - * @return True if the function name is restricted. + * @param functionName function name to check + * @return true if the function name is restricted. false otherwise */ public static boolean isFunctionNameRestricted(String functionName) { return RESTRICTED_FUNCTION_NAMES.contains(functionName) || functionName.startsWith(WRAPPER_PREFIX); diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java index 65032adeb4c7..371d7b7b51e8 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/snippet/factory/BasicSnippetFactory.java @@ -171,22 +171,22 @@ public List createVariableDeclarationSnippets(Node n // Check if the type descriptor of the variable is 'var' as it is not yet supported. if (typeDescriptorNode.kind() == SyntaxKind.VAR_TYPE_DESC) { - addErrorDiagnostic("Actions not allowed with a 'var' TypeDescriptor"); + addErrorDiagnostic("'var' type is not yet supported for actions. Please specify the exact type."); return null; } - boolean hasCheck = nodeKind == SyntaxKind.CHECK_ACTION; - String functionBody = varInitNode.toSourceCode(); - String functionDefinition = (hasCheck ? "function() returns error|" : + boolean isCheckAction = nodeKind == SyntaxKind.CHECK_ACTION; + String initAction = varInitNode.toSourceCode(); + String functionTypeDesc = (isCheckAction ? "function() returns error|" : "function() returns ") + typeDescriptorNode; String functionName = ParserConstants.WRAPPER_PREFIX + varFunctionCount; - String functionString = String.format("%s %s = %s {%s %s = %s; return %s;};", functionDefinition, - functionName, functionDefinition, typeDescriptorNode, bindingPatternNode, functionBody, + String functionVarDecl = String.format("%s %s = %s {%s %s = %s; return %s;};", functionTypeDesc, + functionName, functionTypeDesc, typeDescriptorNode, bindingPatternNode, initAction, bindingPatternNode); - varNode = (VariableDeclarationNode) NodeParser.parseStatement(functionString); + varNode = (VariableDeclarationNode) NodeParser.parseStatement(functionVarDecl); newNode = (VariableDeclarationNode) NodeParser.parseStatement( String.format("%s %s = %s %s();", typeDescriptorNode, bindingPatternNode, - (hasCheck ? "check " : ""), functionName)); + (isCheckAction ? "check " : ""), functionName)); } varFunctionCount += 1; diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java index 323658b54504..895d17cd73e8 100644 --- a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/AbstractEvaluatorTest.java @@ -84,7 +84,7 @@ protected void testEvaluate(String fileName) throws BallerinaShellException { ShellCompilation shellCompilation = evaluator.getCompilation(testCase.getCode()); Optional compilation = shellCompilation.getPackageCompilation(); - if (compilation.isEmpty() && testCase.getStderr().size() > 0) { + if (compilation.isEmpty() && !testCase.getStderr().isEmpty()) { for (int i = 0; i < testCase.getStderr().size(); i++) { Assert.assertEquals(evaluator.diagnostics().get(i).toString(), testCase.getStderr().get(i)); } diff --git a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java index bb9d111a5a57..1bbc48cb470c 100644 --- a/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java +++ b/ballerina-shell/modules/shell-core/src/test/java/io/ballerina/shell/test/evaluator/ActionEvaluatorTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, WSO2 LLC. (http://wso2.com) All Rights Reserved. + * Copyright (c) 2024, WSO2 LLC. (http://wso2.com) * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json index 9c0d3849a015..a6c7ba0c3dcd 100644 --- a/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json +++ b/ballerina-shell/modules/shell-core/src/test/resources/testcases/evaluator/actions.var.json @@ -7,7 +7,7 @@ "description": "Execute the function with start with a variable assignment", "code": "var futureResult = start name()", "stderr": [ - "Actions not allowed with a 'var' TypeDescriptor", + "'var' type is not yet supported for actions. Please specify the exact type.", "Compilation aborted due to invalid input." ] } From 959fa73904b403a485ac2ed403b4f8da1da741f0 Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Fri, 5 Jan 2024 16:35:50 +0530 Subject: [PATCH 12/22] Fix const module list access --- .../analyzer/ConstantTypeChecker.java | 5 +- .../bala/constant/ListConstantInBalaTest.java | 68 +++++++++++++++++++ .../constant/list-literal-constant.bal | 37 ++++++++++ .../test_project/list-constant.bal | 13 ++++ 4 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java create mode 100644 tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal create mode 100644 tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java index be0f462815e6..9019839b0fdd 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java @@ -1355,8 +1355,9 @@ private BType checkTupleType(BTupleType tupleType, BLangListConstructorExpr list } private BTupleType createNewTupleType(Location pos, List memberTypes, AnalyzerData data) { - BTypeSymbol tupleTypeSymbol = Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, 0, Names.EMPTY, - data.env.enclPkg.symbol.pkgID, null, data.env.scope.owner, pos, SOURCE); + BTypeSymbol tupleTypeSymbol = + Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, data.constantSymbol.flags, Names.EMPTY, + data.env.enclPkg.symbol.pkgID, null, data.env.scope.owner, pos, SOURCE); List members = new ArrayList<>(); memberTypes.forEach(m -> members.add(new BTupleMember(m, Symbols.createVarSymbolForTupleMember(m)))); diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java new file mode 100644 index 000000000000..6479f3b45c3b --- /dev/null +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (https://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. + */ +package org.ballerinalang.test.bala.constant; + +import org.ballerinalang.test.BCompileUtil; +import org.ballerinalang.test.BRunUtil; +import org.ballerinalang.test.CompileResult; +import org.testng.Assert; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Test cases for reading list constants. + */ +public class ListConstantInBalaTest { + + private CompileResult compileResult; + + @BeforeClass + public void setup() { + BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project"); + compileResult = BCompileUtil.compile("test-src/bala/test_bala/constant/list-literal-constant.bal"); + } + + @Test(dataProvider = "listAccessTestDataProvider") + public void testConstantListAccess(String testCase) { + try { + BRunUtil.invoke(compileResult, testCase); + } catch (Exception e) { + Assert.fail(testCase + " test case failed."); + } + } + + @DataProvider(name = "listAccessTestDataProvider") + public Object[] listAccessTestDataProvider() { + return new Object[]{ + "testSimpleArrayAccess", + "testSimpleTupleAccess", + "test2DTupleAccess", + "test2DArrayAccess", + "testFixedLengthArrayAccess", + "testArrayWithRestAccess", + "test2DUnionArrayAccess" + }; + } + + @AfterClass + public void tearDown() { + compileResult = null; + } +} diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal new file mode 100644 index 000000000000..97ae33a3d534 --- /dev/null +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal @@ -0,0 +1,37 @@ +import testorg/foo; + +function testSimpleArrayAccess() { + assertEqual(foo:l1, ["a", "b", "c"]); +} + +function testSimpleTupleAccess() { + assertEqual(foo:l2, [1, "d"]); +} + +function test2DTupleAccess() { + assertEqual(foo:l3, [1, ["e", 2]]); +} + +function test2DArrayAccess() { + assertEqual(foo:l4, [[1, 2, 3], [4, 5, 6]]); +} + +function testFixedLengthArrayAccess() { + assertEqual(foo:l5, [true, false, true]); +} + +function testArrayWithRestAccess() { + assertEqual(foo:l7, [1, "f", "g"]); +} + +function test2DUnionArrayAccess() { + assertEqual(foo:l8, [[1, "2", 3], [4, 5, 6]]); + assertEqual(foo:l9, [[1, 2, 3], ["4", "5", "6"]]); +} + +function assertEqual(anydata actual, anydata expected) { + if expected == actual { + return; + } + panic error(string `expected '${expected.toBalString()}', found '${actual.toBalString()}'`); +} diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal new file mode 100644 index 000000000000..5dadfa94ae7f --- /dev/null +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal @@ -0,0 +1,13 @@ +type IntArr int[]; + +const int length = 3; + +public const string[] l1 = ["a", "b", "c"]; +public const [int, string] l2 = [1, "d"]; +public const [int, [string, int]] l3 = [1, ["e", 2]]; +public const int[][] l4 = [[1, 2, 3], [4, 5, 6]]; +public const boolean[length] l5 = [true, false, true]; +public const IntArr l6 = [1, 2, 3]; +public const [int, string...] l7 = [1, "f", "g"]; +public const (string|int)[][] l8 = [[1, "2", 3], [4, 5, 6]]; +public const [(string[]|int[])...] l9 = [[1, 2, 3], ["4", "5", "6"]]; From accbd9f8f288deed4d05a3cf90aadb75ed10dc4d Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Mon, 8 Jan 2024 06:52:36 +0530 Subject: [PATCH 13/22] Add missing since in test --- .../test/bala/constant/ListConstantInBalaTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java index 6479f3b45c3b..187eec952e81 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -28,6 +28,8 @@ /** * Test cases for reading list constants. + * + * @since 2201.9.0 */ public class ListConstantInBalaTest { From 75a6bfcaecff212571a9b7159e6fca587f44ddc7 Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Mon, 22 Jan 2024 06:29:23 +0530 Subject: [PATCH 14/22] Add negative tests --- .../ListConstantInBalaNegativeTest.java | 66 +++++++++++++++++++ .../constant/list_constant_negative.bal | 20 ++++++ 2 files changed, 86 insertions(+) create mode 100644 tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java create mode 100644 tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java new file mode 100644 index 000000000000..b5ddbe398a08 --- /dev/null +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (https://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. + */ +package org.ballerinalang.test.bala.constant; + +import org.ballerinalang.test.BAssertUtil; +import org.ballerinalang.test.BCompileUtil; +import org.ballerinalang.test.CompileResult; +import org.testng.Assert; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +/** + * Negative Bala test cases for list constants. + * + * @since 2201.9.0 + */ + +public class ListConstantInBalaNegativeTest { + + @BeforeClass + public void setup() { + BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project"); + } + + @Test + public void testNegative() { + CompileResult compileResult = BCompileUtil.compile( + "test-src/bala/test_bala/constant/list_constant_negative.bal"); + int i = 0; + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected '[string,string,int...]', found '[\"a\",\"b\",\"c\"] & readonly'", 4, 34); + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected 'int[]', found '[true,false,true] & readonly'", 5, 15); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 10, 5); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 11, 5); + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected '(\"a\"|\"b\"|\"c\")', found 'string:Char'", 11, 12); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 14, 5); + BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", + 14, 12); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 18, 17); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 19, 18); + Assert.assertEquals(compileResult.getErrorCount(), i); + } + +} + + diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal new file mode 100644 index 000000000000..eda058124916 --- /dev/null +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -0,0 +1,20 @@ +import testorg/foo; + +function testIncompatibleAssignment() { + [string, string, int...] _ = foo:l1; + int[] _ = foo:l5; +} + +function testInvalidUpdates() { + var a = foo:l1; + a[0] = "1"; + a.push("2"); + + var b = foo:l7; + b.push("l7"); +} + +function testInvalidAccess() { + float[] _ = foo:l10; + string[] _ = foo:l11; +} From c5e07c275e3fa50e3810337d02568858e5c8679b Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Thu, 8 Feb 2024 10:17:40 +0530 Subject: [PATCH 15/22] Address review comments --- .../compiler/semantics/analyzer/ConstantTypeChecker.java | 2 +- .../test/bala/constant/ListConstantInBalaNegativeTest.java | 2 -- .../test/bala/constant/ListConstantInBalaTest.java | 7 +------ .../bala/test_bala/constant/list_constant_negative.bal | 2 +- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java index 9019839b0fdd..1bc584fc9500 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java @@ -1356,7 +1356,7 @@ private BType checkTupleType(BTupleType tupleType, BLangListConstructorExpr list private BTupleType createNewTupleType(Location pos, List memberTypes, AnalyzerData data) { BTypeSymbol tupleTypeSymbol = - Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, data.constantSymbol.flags, Names.EMPTY, + Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, Flags.PUBLIC, Names.EMPTY, data.env.enclPkg.symbol.pkgID, null, data.env.scope.owner, pos, SOURCE); List members = new ArrayList<>(); memberTypes.forEach(m -> diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java index b5ddbe398a08..ece1d1d0c13a 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java @@ -62,5 +62,3 @@ public void testNegative() { } } - - diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java index 187eec952e81..3e416a033cb6 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -20,7 +20,6 @@ import org.ballerinalang.test.BCompileUtil; import org.ballerinalang.test.BRunUtil; import org.ballerinalang.test.CompileResult; -import org.testng.Assert; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; @@ -43,11 +42,7 @@ public void setup() { @Test(dataProvider = "listAccessTestDataProvider") public void testConstantListAccess(String testCase) { - try { - BRunUtil.invoke(compileResult, testCase); - } catch (Exception e) { - Assert.fail(testCase + " test case failed."); - } + BRunUtil.invoke(compileResult, testCase); } @DataProvider(name = "listAccessTestDataProvider") diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index eda058124916..701a66129838 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -14,7 +14,7 @@ function testInvalidUpdates() { b.push("l7"); } -function testInvalidAccess() { +function testUndefinedMemberAccess() { float[] _ = foo:l10; string[] _ = foo:l11; } From 3cc329e8da3170d2d014e54ff17497841cf0afdb Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Sun, 25 Feb 2024 09:35:09 +0530 Subject: [PATCH 16/22] Add missing license headers --- .../ListConstantInBalaNegativeTest.java | 20 ++++++------- .../bala/constant/ListConstantInBalaTest.java | 2 +- .../constant/list_constant_negative.bal | 16 ++++++++++ ...constant.bal => list_literal_constant.bal} | 16 ++++++++++ .../test_project/list-constant.bal | 13 --------- .../test_project/list_constant.bal | 29 +++++++++++++++++++ 6 files changed, 72 insertions(+), 24 deletions(-) rename tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/{list-literal-constant.bal => list_literal_constant.bal} (56%) delete mode 100644 tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal create mode 100644 tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java index ece1d1d0c13a..4a863cab5f34 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java @@ -29,7 +29,6 @@ * * @since 2201.9.0 */ - public class ListConstantInBalaNegativeTest { @BeforeClass @@ -43,21 +42,22 @@ public void testNegative() { "test-src/bala/test_bala/constant/list_constant_negative.bal"); int i = 0; BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected '[string,string,int...]', found '[\"a\",\"b\",\"c\"] & readonly'", 4, 34); + "incompatible types: expected '[string,string,int...]', found '[\"a\",\"b\",\"c\"] & readonly'", 20, + 34); BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected 'int[]', found '[true,false,true] & readonly'", 5, 15); + "incompatible types: expected 'int[]', found '[true,false,true] & readonly'", 21, 15); BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 10, 5); + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 26, 5); BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 11, 5); + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 27, 5); BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected '(\"a\"|\"b\"|\"c\")', found 'string:Char'", 11, 12); + "incompatible types: expected '(\"a\"|\"b\"|\"c\")', found 'string:Char'", 27, 12); BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 14, 5); + "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 30, 5); BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", - 14, 12); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 18, 17); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 19, 18); + 30, 12); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 34, 17); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 35, 18); Assert.assertEquals(compileResult.getErrorCount(), i); } diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java index 3e416a033cb6..0700b4c2a6e7 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -37,7 +37,7 @@ public class ListConstantInBalaTest { @BeforeClass public void setup() { BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project"); - compileResult = BCompileUtil.compile("test-src/bala/test_bala/constant/list-literal-constant.bal"); + compileResult = BCompileUtil.compile("test-src/bala/test_bala/constant/list_literal_constant.bal"); } @Test(dataProvider = "listAccessTestDataProvider") diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index 701a66129838..acf82c3afed3 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -1,3 +1,19 @@ +// 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. + import testorg/foo; function testIncompatibleAssignment() { diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal similarity index 56% rename from tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal rename to tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal index 97ae33a3d534..1c499cf54e45 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list-literal-constant.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal @@ -1,3 +1,19 @@ +// Copyright (c) 2023, WSO2 LLC. (https://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 + import testorg/foo; function testSimpleArrayAccess() { diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal deleted file mode 100644 index 5dadfa94ae7f..000000000000 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list-constant.bal +++ /dev/null @@ -1,13 +0,0 @@ -type IntArr int[]; - -const int length = 3; - -public const string[] l1 = ["a", "b", "c"]; -public const [int, string] l2 = [1, "d"]; -public const [int, [string, int]] l3 = [1, ["e", 2]]; -public const int[][] l4 = [[1, 2, 3], [4, 5, 6]]; -public const boolean[length] l5 = [true, false, true]; -public const IntArr l6 = [1, 2, 3]; -public const [int, string...] l7 = [1, "f", "g"]; -public const (string|int)[][] l8 = [[1, "2", 3], [4, 5, 6]]; -public const [(string[]|int[])...] l9 = [[1, 2, 3], ["4", "5", "6"]]; diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal new file mode 100644 index 000000000000..8ccd04b1b3e3 --- /dev/null +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal @@ -0,0 +1,29 @@ +// Copyright (c) 2023, WSO2 LLC. (https://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 IntArr int[]; + +const int length = 3; + +public const string[] l1 = ["a", "b", "c"]; +public const [int, string] l2 = [1, "d"]; +public const [int, [string, int]] l3 = [1, ["e", 2]]; +public const int[][] l4 = [[1, 2, 3], [4, 5, 6]]; +public const boolean[length] l5 = [true, false, true]; +public const IntArr l6 = [1, 2, 3]; +public const [int, string...] l7 = [1, "f", "g"]; +public const (string|int)[][] l8 = [[1, "2", 3], [4, 5, 6]]; +public const [(string[]|int[])...] l9 = [[1, 2, 3], ["4", "5", "6"]]; From 4682e8fe00c6cdd34b7d8c2b1eb9815c712c511f Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Tue, 12 Mar 2024 10:24:19 +0530 Subject: [PATCH 17/22] Update recordTypeSymbol flags --- .../compiler/semantics/analyzer/ConstantTypeChecker.java | 2 +- .../test-src/bala/test_bala/constant/list_constant_negative.bal | 2 +- .../test-src/bala/test_bala/constant/list_literal_constant.bal | 2 +- .../test-src/bala/test_projects/test_project/list_constant.bal | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java index 1bc584fc9500..ac525596f543 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java @@ -2058,7 +2058,7 @@ private BRecordTypeSymbol createRecordTypeSymbol(PackageID pkgID, Location locat SymbolEnv env = data.env; Name genName = Names.fromString(anonymousModelHelper.getNextAnonymousTypeKey(pkgID, data.anonTypeNameSuffixes)); - BRecordTypeSymbol recordSymbol = Symbols.createRecordSymbol(data.constantSymbol.flags, genName, + BRecordTypeSymbol recordSymbol = Symbols.createRecordSymbol(Flags.PUBLIC, genName, pkgID, null, env.scope.owner, location, origin); recordSymbol.scope = new Scope(recordSymbol); return recordSymbol; diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index acf82c3afed3..6c9b79746037 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -1,4 +1,4 @@ -// Copyright (c) 2023 WSO2 LLC. (http://www.wso2.com). +// Copyright (c) 2024 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 diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal index 1c499cf54e45..dd56e80b1f53 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_literal_constant.bal @@ -1,4 +1,4 @@ -// Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com). +// Copyright (c) 2024, WSO2 LLC. (https://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 diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal index 8ccd04b1b3e3..9899182ce6cc 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal @@ -1,4 +1,4 @@ -// Copyright (c) 2023, WSO2 LLC. (https://www.wso2.com). +// Copyright (c) 2024, WSO2 LLC. (https://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 From 1482d611073f301368b3f24e3f1f8c2c20011d4a Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Wed, 13 Mar 2024 16:50:20 +0530 Subject: [PATCH 18/22] Add non public access tests --- .../semantics/analyzer/ConstantTypeChecker.java | 5 +++-- .../bala/constant/ListConstantInBalaNegativeTest.java | 10 ++++++++-- .../bala/test_bala/constant/list_constant_negative.bal | 10 ++++++++-- .../bala/test_projects/test_project/list_constant.bal | 3 +++ 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java index ac525596f543..a630e356ad70 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java @@ -1355,9 +1355,10 @@ private BType checkTupleType(BTupleType tupleType, BLangListConstructorExpr list } private BTupleType createNewTupleType(Location pos, List memberTypes, AnalyzerData data) { + SymbolEnv symbolEnv = data.env; BTypeSymbol tupleTypeSymbol = - Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, Flags.PUBLIC, Names.EMPTY, - data.env.enclPkg.symbol.pkgID, null, data.env.scope.owner, pos, SOURCE); + Symbols.createTypeSymbol(SymTag.TUPLE_TYPE, Flags.PUBLIC, Names.EMPTY, symbolEnv.enclPkg.symbol.pkgID, + null, symbolEnv.scope.owner, pos, SOURCE); List members = new ArrayList<>(); memberTypes.forEach(m -> members.add(new BTupleMember(m, Symbols.createVarSymbolForTupleMember(m)))); diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java index 4a863cab5f34..8d6e6f40c79d 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java @@ -56,8 +56,14 @@ public void testNegative() { "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 30, 5); BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", 30, 12); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 34, 17); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 35, 18); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l13'", 34, 17); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l14'", 35, 18); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l10'", 39, 15); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 39, 15); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l11'", 40, 29); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 40, 29); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l12'", 41, 18); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l12'", 41, 18); Assert.assertEquals(compileResult.getErrorCount(), i); } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index 6c9b79746037..bfb7617eac5f 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -31,6 +31,12 @@ function testInvalidUpdates() { } function testUndefinedMemberAccess() { - float[] _ = foo:l10; - string[] _ = foo:l11; + float[] _ = foo:l13; + string[] _ = foo:l14; +} + +function testNonPublicConstAccess() { + int[] _ = foo:l10; + [int, int, boolean] _ = foo:l11; + string[] _ = foo:l12; } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal index 9899182ce6cc..cae666c03b71 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_projects/test_project/list_constant.bal @@ -27,3 +27,6 @@ public const IntArr l6 = [1, 2, 3]; public const [int, string...] l7 = [1, "f", "g"]; public const (string|int)[][] l8 = [[1, "2", 3], [4, 5, 6]]; public const [(string[]|int[])...] l9 = [[1, 2, 3], ["4", "5", "6"]]; +const int[] l10 = [1, 2, 3]; +const [int, int, boolean] l11 = [1, 2, true]; +const l12 = l1; From 4c135eb43701cd3e04d91723eca2bc253ae87c01 Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Fri, 15 Mar 2024 06:22:29 +0530 Subject: [PATCH 19/22] Remove unnecessary types --- .../constant/ListConstantInBalaNegativeTest.java | 16 ++++++++-------- .../constant/list_constant_negative.bal | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java index 8d6e6f40c79d..4d586a0bf563 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java @@ -56,14 +56,14 @@ public void testNegative() { "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 30, 5); BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", 30, 12); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l13'", 34, 17); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l14'", 35, 18); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l10'", 39, 15); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 39, 15); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l11'", 40, 29); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 40, 29); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l12'", 41, 18); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l12'", 41, 18); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l13'", 34, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l14'", 35, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l10'", 39, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 39, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l11'", 40, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 40, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l12'", 41, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l12'", 41, 9); Assert.assertEquals(compileResult.getErrorCount(), i); } diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index bfb7617eac5f..1e7aef464bb5 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -31,12 +31,12 @@ function testInvalidUpdates() { } function testUndefinedMemberAccess() { - float[] _ = foo:l13; - string[] _ = foo:l14; + _ = foo:l13; + _ = foo:l14; } function testNonPublicConstAccess() { - int[] _ = foo:l10; - [int, int, boolean] _ = foo:l11; - string[] _ = foo:l12; + _ = foo:l10; + _ = foo:l11; + _ = foo:l12; } From bad3d812c4628eb289c9381e1e8cde87893cc093 Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Mon, 18 Mar 2024 16:35:26 +0530 Subject: [PATCH 20/22] Address review comments --- .../analyzer/ConstantTypeChecker.java | 2 +- .../ListConstantInBalaNegativeTest.java | 70 ------------------- .../bala/constant/ListConstantInBalaTest.java | 33 +++++++++ .../constant/list_constant_negative.bal | 2 +- 4 files changed, 35 insertions(+), 72 deletions(-) delete mode 100644 tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java index a630e356ad70..2c7c9b2b13a1 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/ConstantTypeChecker.java @@ -2059,7 +2059,7 @@ private BRecordTypeSymbol createRecordTypeSymbol(PackageID pkgID, Location locat SymbolEnv env = data.env; Name genName = Names.fromString(anonymousModelHelper.getNextAnonymousTypeKey(pkgID, data.anonTypeNameSuffixes)); - BRecordTypeSymbol recordSymbol = Symbols.createRecordSymbol(Flags.PUBLIC, genName, + BRecordTypeSymbol recordSymbol = Symbols.createRecordSymbol(data.constantSymbol.flags, genName, pkgID, null, env.scope.owner, location, origin); recordSymbol.scope = new Scope(recordSymbol); return recordSymbol; diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java deleted file mode 100644 index 4d586a0bf563..000000000000 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaNegativeTest.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (c) 2024, WSO2 LLC. (https://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. - */ -package org.ballerinalang.test.bala.constant; - -import org.ballerinalang.test.BAssertUtil; -import org.ballerinalang.test.BCompileUtil; -import org.ballerinalang.test.CompileResult; -import org.testng.Assert; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.Test; - -/** - * Negative Bala test cases for list constants. - * - * @since 2201.9.0 - */ -public class ListConstantInBalaNegativeTest { - - @BeforeClass - public void setup() { - BCompileUtil.compileAndCacheBala("test-src/bala/test_projects/test_project"); - } - - @Test - public void testNegative() { - CompileResult compileResult = BCompileUtil.compile( - "test-src/bala/test_bala/constant/list_constant_negative.bal"); - int i = 0; - BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected '[string,string,int...]', found '[\"a\",\"b\",\"c\"] & readonly'", 20, - 34); - BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected 'int[]', found '[true,false,true] & readonly'", 21, 15); - BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 26, 5); - BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 27, 5); - BAssertUtil.validateError(compileResult, i++, - "incompatible types: expected '(\"a\"|\"b\"|\"c\")', found 'string:Char'", 27, 12); - BAssertUtil.validateError(compileResult, i++, - "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 30, 5); - BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", - 30, 12); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l13'", 34, 9); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l14'", 35, 9); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l10'", 39, 9); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 39, 9); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l11'", 40, 9); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 40, 9); - BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l12'", 41, 9); - BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l12'", 41, 9); - Assert.assertEquals(compileResult.getErrorCount(), i); - } - -} diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java index 0700b4c2a6e7..a601b65b5457 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -17,9 +17,11 @@ */ package org.ballerinalang.test.bala.constant; +import org.ballerinalang.test.BAssertUtil; import org.ballerinalang.test.BCompileUtil; import org.ballerinalang.test.BRunUtil; import org.ballerinalang.test.CompileResult; +import org.testng.Assert; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; @@ -58,6 +60,37 @@ public Object[] listAccessTestDataProvider() { }; } + @Test + public void testNegativeConstantListAccess() { + CompileResult compileResult = BCompileUtil.compile( + "test-src/bala/test_bala/constant/list_constant_negative.bal"); + int i = 0; + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected '[string,string,int...]', found '[\"a\",\"b\",\"c\"] & readonly'", 20, + 34); + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected 'int[]', found '[true,false,true] & readonly'", 21, 15); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 26, 5); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[\"a\",\"b\",\"c\"] & readonly'", 27, 5); + BAssertUtil.validateError(compileResult, i++, + "incompatible types: expected '(\"a\"|\"b\"|\"c\")', found 'string:Char'", 27, 12); + BAssertUtil.validateError(compileResult, i++, + "cannot update 'readonly' value of type '[1,\"f\",\"g\"] & readonly'", 30, 5); + BAssertUtil.validateError(compileResult, i++, "incompatible types: expected '(1|\"f\"|\"g\")', found 'string'", + 30, 12); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l13'", 34, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l14'", 35, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l10'", 39, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l10'", 39, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l11'", 40, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l11'", 40, 9); + BAssertUtil.validateError(compileResult, i++, "attempt to refer to non-accessible symbol 'l12'", 41, 9); + BAssertUtil.validateError(compileResult, i++, "undefined symbol 'l12'", 41, 9); + Assert.assertEquals(compileResult.getErrorCount(), i); + } + @AfterClass public void tearDown() { compileResult = null; diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal index 1e7aef464bb5..afbb3ba33f80 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/bala/test_bala/constant/list_constant_negative.bal @@ -30,7 +30,7 @@ function testInvalidUpdates() { b.push("l7"); } -function testUndefinedMemberAccess() { +function testUndefinedSymbolAccess() { _ = foo:l13; _ = foo:l14; } From 0016c1cc8aeb18d7db4523a22cb375aa5c6d05e5 Mon Sep 17 00:00:00 2001 From: poorna2152 Date: Tue, 19 Mar 2024 11:05:04 +0530 Subject: [PATCH 21/22] Update function and dataprovider names --- .../test/bala/constant/ListConstantInBalaTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java index a601b65b5457..e211fc4331b3 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/bala/constant/ListConstantInBalaTest.java @@ -42,13 +42,13 @@ public void setup() { compileResult = BCompileUtil.compile("test-src/bala/test_bala/constant/list_literal_constant.bal"); } - @Test(dataProvider = "listAccessTestDataProvider") + @Test(dataProvider = "constantListAccessTestDataProvider") public void testConstantListAccess(String testCase) { BRunUtil.invoke(compileResult, testCase); } - @DataProvider(name = "listAccessTestDataProvider") - public Object[] listAccessTestDataProvider() { + @DataProvider(name = "constantListAccessTestDataProvider") + public Object[] constantListAccessTestDataProvider() { return new Object[]{ "testSimpleArrayAccess", "testSimpleTupleAccess", @@ -61,7 +61,7 @@ public Object[] listAccessTestDataProvider() { } @Test - public void testNegativeConstantListAccess() { + public void testConstantListAccessNegative() { CompileResult compileResult = BCompileUtil.compile( "test-src/bala/test_bala/constant/list_constant_negative.bal"); int i = 0; From 71349f5a6373d0de21f12005aa5aec7046f12b25 Mon Sep 17 00:00:00 2001 From: Nipuna Fernando Date: Tue, 19 Mar 2024 19:30:41 +0530 Subject: [PATCH 22/22] Remove unused restricted function names --- .../java/io/ballerina/shell/parser/SerialTreeParser.java | 5 ++--- .../io/ballerina/shell/parser/trials/ModuleMemberTrial.java | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java index 4f0e322c3c3b..ce933d37e90a 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/SerialTreeParser.java @@ -40,7 +40,6 @@ import java.util.Collection; import java.util.List; import java.util.Objects; -import java.util.Set; /** * Parses the source code line using a trial based method. @@ -50,7 +49,7 @@ * @since 2.0.0 */ public class SerialTreeParser extends TrialTreeParser { - private static final Set RESTRICTED_FUNCTION_NAMES = ParserConstants.RESTRICTED_FUNCTION_NAMES; + private static final String COMMAND_PREFIX = "/"; private final List nodeParserTrials; @@ -103,7 +102,7 @@ public Collection parseDeclarations(String source) throws TreeParserExcept ModulePartTrial modulePartTrial = new ModulePartTrial(this); Collection nodes = modulePartTrial.parse(source); List declarationNodes = new ArrayList<>(); - for (Node node:nodes) { + for (Node node : nodes) { ModulePartNode modulePartNode = (ModulePartNode) node; modulePartNode.imports().forEach(declarationNodes::add); modulePartNode.members().stream().filter(this::isModuleDeclarationAllowed) diff --git a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java index 10924c2f477f..6ce131e0bbe1 100644 --- a/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java +++ b/ballerina-shell/modules/shell-core/src/main/java/io/ballerina/shell/parser/trials/ModuleMemberTrial.java @@ -34,7 +34,6 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; -import java.util.Set; /** * Attempts to capture a module member declaration. @@ -45,8 +44,6 @@ */ public class ModuleMemberTrial extends TreeParserTrial { - private static final Set RESTRICTED_FUNCTION_NAMES = ParserConstants.RESTRICTED_FUNCTION_NAMES; - public ModuleMemberTrial(TrialTreeParser parentParser) { super(parentParser); }