Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

dulajdilshan · 2023-04-24T04:05:57Z

Purpose

$title

Fixes #39324
Fixes #19804

Original PR: #40113 (invalidated)

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Encountered the following issue

[Bug]: NoSuchMethodError exception when running with tuple destructure #41174

Check List

MaryamZi · 2023-04-24T04:14:51Z

Please update the title to reflect what's actually being fixed.

codecov · 2023-04-24T05:35:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.33%. Comparing base (1922024) to head (3de0d3e).
Report is 94 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #40283      +/-   ##
============================================
- Coverage     77.35%   77.33%   -0.03%     
- Complexity    58436    58545     +109     
============================================
  Files          3454     3460       +6     
  Lines        219598   219990     +392     
  Branches      28870    28914      +44     
============================================
+ Hits         169880   170136     +256     
- Misses        40330    40449     +119     
- Partials       9388     9405      +17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...erina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal

dulajdilshan · 2023-06-08T11:14:38Z

Experiencing an NPE in ballerina runtime for the following

public function main() {
  int a;
  int b;
  [{a, b}] = [{a: 10, b: 11}];
}

Working on fixing it

github-actions · 2023-06-23T19:33:18Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

dulajdilshan · 2023-06-28T03:06:14Z

Experiencing an NPE in ballerina runtime for the following
public function main() {
  int a;
  int b;
  [{a, b}] = [{a: 10, b: 11}];
}
Working on fixing it

This scenario seems to be fixe via #37184

dulajdilshan · 2023-06-28T04:13:26Z

This scenario seems to be fixed via #37184

Had a chat with @MaryamZi and @chiranSachintha. It seems this scenario needs to be disallowed

github-actions · 2023-07-13T19:33:29Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2023-07-17T19:33:17Z

Closed PR due to inactivity for more than 18 days.

MaryamZi · 2024-06-29T11:06:32Z

Please fix the build/test failures.

MaryamZi · 2024-07-02T11:59:50Z

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal

+        name: fName,
+        age,
+        married,
+        ...theMap


theMap isn't asserted

That's intentional. We don't need to assert it. We have asserted it in the other tests.

we can remove this test. It seems this test is not necessary.

That's intentional. We don't need to assert it. We have asserted it in the other tests.

Why can't we just assert it? We either need to assert it or remove that binding.

Fixed via 6568a25

MaryamZi · 2024-07-03T14:05:31Z

Still not supported with typed binding patterns so I would update the title and add a comment on #19804 saying this is only enabled for destructuring assignments in this PR. Ideally, it should be straightforward to support the same with typed binding patterns too.

MaryamZi · 2024-07-18T09:01:16Z

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

@@ -2199,11 +2199,6 @@ public void visit(BLangSimpleVariableDef varDefNode, AnalyzerData data) {

    @Override
    public void visit(BLangRecordVariableDef varDefNode, AnalyzerData data) {
-        // TODO: 10/18/18 Need to support record literals as well
-        if (varDefNode.var.expr != null && varDefNode.var.expr.getKind() == RECORD_LITERAL_EXPR) {
-            dlog.error(varDefNode.pos, DiagnosticErrorCode.INVALID_LITERAL_FOR_TYPE, "record binding pattern");


Why isn't this DiagnosticErrorCode removed?

Removed via 08f5e57c

MaryamZi · 2024-07-18T09:02:13Z

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

@@ -2199,11 +2199,6 @@ public void visit(BLangSimpleVariableDef varDefNode, AnalyzerData data) {

    @Override
    public void visit(BLangRecordVariableDef varDefNode, AnalyzerData data) {
-        // TODO: 10/18/18 Need to support record literals as well
-        if (varDefNode.var.expr != null && varDefNode.var.expr.getKind() == RECORD_LITERAL_EXPR) {
-            dlog.error(varDefNode.pos, DiagnosticErrorCode.INVALID_LITERAL_FOR_TYPE, "record binding pattern");


Where have the tests been added for this?

Noticed that they've been added in RecordVariableReferenceTest.java. That's not the correct class. Should go in RecordVariableDefinitionTest.

MaryamZi · 2024-07-18T09:07:21Z

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal

+
+    var {country, ...rest} = {country: "USA", zipCode: 80001};
+    assertEquality("USA", country);
+    assertEquality({zipCode: 80001}, rest);


Add more tests including where the member binding patterns are in turn structured binding patterns.

Also add negative tests for unspecified fields, optional field binding, etc.

@dulajdilshan can you please point to the negative tests?

Added in edebb78

MaryamZi

Title needs to be changed too.

MaryamZi · 2024-07-23T10:31:07Z

...-test/src/test/resources/test-src/statements/variabledef/record-variable-definition-stmt.bal

+    assertEquality("Alabama", state);
+    assertEquality(35004, postalCode);
+
+    record {|string street; float...;|} {street, ...coordinates} = {street: "Highway 61", "lat": 37.30, "lon": -90.40};


Also add for when some of the required fields are bound as rest.

Added in edebb78

MaryamZi · 2024-08-02T09:00:51Z

...esources/test-src/expressions/varref/record-variable-reference-in-destructuring-negative.bal

+    int b;
+    int c;
+    [{a, b}]= l1;
+    [int, [record {int b?;}]] l2= [1, [{b: 1}]];


Suggested change

[int, [record {int b?;}]] l2= [1, [{b: 1}]];

[int, [record {int b?;}]] l2 = [1, [{b: 1}]];

Fix everywhere.

Addressed via 3de0d3e

MaryamZi · 2024-08-02T09:03:07Z

...esources/test-src/expressions/varref/record-variable-reference-in-destructuring-negative.bal

Use underscores for file names.

Also proper terms for new files/code - e.g., record_bp_in_list_bp_destructuring_assignment_negative.bal

dulajdilshan requested review from sameerajayasoma and hasithaa as code owners April 24, 2023 04:05

dulajdilshan mentioned this pull request Apr 24, 2023

Fix using mapping-binding pattern within a list binding pattern against an open record #40113

Closed

13 tasks

dulajdilshan requested review from MaryamZi, suleka96 and chiranSachintha April 24, 2023 04:07

dulajdilshan changed the title ~~Fix/39324~~ Fix using mapping-binding pattern within a list binding pattern against an open record Apr 24, 2023

MaryamZi reviewed Apr 24, 2023

View reviewed changes

dulajdilshan requested a review from MaryamZi April 25, 2023 07:45

MaryamZi reviewed Apr 27, 2023

View reviewed changes

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal Outdated Show resolved Hide resolved

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal Outdated Show resolved Hide resolved

dulajdilshan force-pushed the fix/39324 branch from fd03c00 to 7c72d3c Compare May 11, 2023 16:05

dulajdilshan requested a review from MaryamZi May 12, 2023 14:09

MaryamZi added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label May 15, 2023

MaryamZi reviewed May 15, 2023

View reviewed changes

dulajdilshan added the Area/Compiler label May 17, 2023

dulajdilshan requested a review from MaryamZi May 19, 2023 03:03

MaryamZi reviewed Jun 2, 2023

View reviewed changes

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal Outdated Show resolved Hide resolved

...erina-unit-test/src/test/resources/test-src/expressions/varref/record-variable-reference.bal Outdated Show resolved Hide resolved

github-actions bot added the Stale label Jun 23, 2023

dulajdilshan removed the Stale label Jun 26, 2023

github-actions bot added the Stale label Jul 13, 2023

github-actions bot closed this Jul 17, 2023

dulajdilshan removed the Stale label Jul 18, 2023

dulajdilshan force-pushed the fix/39324 branch from 9333cec to a5047f5 Compare June 26, 2024 06:28

Address review comments

3519ace

dulajdilshan force-pushed the fix/39324 branch from a5047f5 to 3519ace Compare June 26, 2024 11:54

Fix tests

6b1feaa

Fix test failure

03024d3

dulajdilshan requested a review from MaryamZi July 1, 2024 05:58

Update tests

82e3439

MaryamZi reviewed Jul 2, 2024

View reviewed changes

dulajdilshan mentioned this pull request Jul 15, 2024

Re-enable support for using mapping constructors with record binding patterns #19804

Closed

dulajdilshan added 2 commits July 18, 2024 05:41

Enable type BP against record-literal

6568a25

Merge branch 'master' into fix/39324

cb7e540

dulajdilshan requested a review from MaryamZi July 18, 2024 02:33

MaryamZi reviewed Jul 18, 2024

View reviewed changes

dulajdilshan added 2 commits July 23, 2024 01:02

Remove diagnostic error code: BCE2506

08f5e57

Move and add more tests

82ff635

dulajdilshan requested a review from MaryamZi July 23, 2024 06:51

MaryamZi reviewed Jul 23, 2024

View reviewed changes

dulajdilshan added 2 commits August 1, 2024 21:01

Add negative tests

edebb78

Merge branch 'master' into fix/39324

94bf9c0

dulajdilshan requested a review from MaryamZi August 1, 2024 15:32

MaryamZi reviewed Aug 2, 2024

View reviewed changes

Fix tests

3de0d3e

dulajdilshan requested a review from MaryamZi August 2, 2024 09:44

MaryamZi approved these changes Aug 2, 2024

View reviewed changes

dulajdilshan merged commit 1caf979 into ballerina-platform:master Aug 2, 2024
17 of 18 checks passed

dulajdilshan deleted the fix/39324 branch August 2, 2024 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

dulajdilshan commented Apr 24, 2023 •

edited

Loading

MaryamZi commented Apr 24, 2023

codecov bot commented Apr 24, 2023 •

edited

Loading

dulajdilshan commented Jun 8, 2023 •

edited

Loading

github-actions bot commented Jun 23, 2023

dulajdilshan commented Jun 28, 2023 •

edited

Loading

dulajdilshan commented Jun 28, 2023

github-actions bot commented Jul 13, 2023

github-actions bot commented Jul 17, 2023

MaryamZi commented Jun 29, 2024

MaryamZi Jul 2, 2024

dulajdilshan Jul 2, 2024

dulajdilshan Jul 2, 2024

MaryamZi Jul 3, 2024

dulajdilshan Jul 18, 2024

MaryamZi commented Jul 3, 2024

MaryamZi Jul 18, 2024

dulajdilshan Jul 22, 2024

MaryamZi Jul 18, 2024

MaryamZi Jul 18, 2024

dulajdilshan Jul 22, 2024

MaryamZi Jul 18, 2024

MaryamZi Jul 23, 2024

dulajdilshan Aug 1, 2024

MaryamZi left a comment

MaryamZi Jul 23, 2024

dulajdilshan Aug 1, 2024

MaryamZi Aug 2, 2024

dulajdilshan Aug 2, 2024

MaryamZi Aug 2, 2024

	[int, [record {int b?;}]] l2= [1, [{b: 1}]];
	[int, [record {int b?;}]] l2 = [1, [{b: 1}]];

Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

Conversation

dulajdilshan commented Apr 24, 2023 • edited Loading

Purpose

Approach

Samples

Remarks

Encountered the following issue

Check List

MaryamZi commented Apr 24, 2023

codecov bot commented Apr 24, 2023 • edited Loading

Codecov Report

dulajdilshan commented Jun 8, 2023 • edited Loading

github-actions bot commented Jun 23, 2023

dulajdilshan commented Jun 28, 2023 • edited Loading

dulajdilshan commented Jun 28, 2023

github-actions bot commented Jul 13, 2023

github-actions bot commented Jul 17, 2023

MaryamZi commented Jun 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryamZi commented Jul 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaryamZi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dulajdilshan commented Apr 24, 2023 •

edited

Loading

codecov bot commented Apr 24, 2023 •

edited

Loading

dulajdilshan commented Jun 8, 2023 •

edited

Loading

dulajdilshan commented Jun 28, 2023 •

edited

Loading