Fix issue with global variable as default value for class field and function parameter #40774

chiranSachintha · 2023-06-20T04:42:38Z

Purpose

$title.

Fixes #40769

Check List

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/klass/ClassTest.java

tests/jballerina-unit-test/src/test/resources/test-src/klass/simple_class.bal

codecov · 2023-06-20T06:15:30Z

Codecov Report

Patch coverage: 83.33% and project coverage change: +0.05% 🎉

Comparison is base (7f9a532) 76.39% compared to head (b58a604) 76.45%.
Report is 256 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #40774      +/-   ##
============================================
+ Coverage     76.39%   76.45%   +0.05%     
- Complexity    52054    52173     +119     
============================================
  Files          2857     2857              
  Lines        195858   196066     +208     
  Branches      25398    25430      +32     
============================================
+ Hits         149623   149893     +270     
+ Misses        37942    37864      -78     
- Partials       8293     8309      +16

Files Changed	Coverage Δ
...llerinalang/compiler/desugar/ClosureGenerator.java	`79.19% <0.00%> (ø)`
...va/io/ballerina/jsonmapper/JsonToRecordMapper.java	`89.29% <100.00%> (-0.47%)`	⬇️
...erina/jsonmapper/diagnostic/DiagnosticMessage.java	`77.77% <100.00%> (ø)`

... and 55 files with indirect coverage changes

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

github-actions · 2023-07-05T19:33:27Z

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-21T19:33:21Z

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.

…ina-lang into issue-40769

tests/jballerina-unit-test/src/test/resources/test-src/klass/simple_class.bal

MaryamZi · 2023-08-07T07:04:22Z

tests/jballerina-unit-test/src/test/resources/test-src/klass/simple_class.bal

+function testModuleVariableReferencingClass() {
+    ModuleVariableReferencingClass c = new;
+    assertEquality(c.i, 111222);
+    assertEquality(c1.i, 111222);
+}
+
+function testLocalObjectConstructorReferencingModuleVariable() {


Can we make the names uniform? They are both doing the same thing, just different constructs, right?

Updated as testClassReferencingModuleVariable and testLocalObjectConstructorReferencingModuleVariable

tests/jballerina-unit-test/src/test/resources/test-src/klass/simple_class.bal

MaryamZi · 2023-08-07T07:05:58Z

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/klass/ClassTest.java

+        BRunUtil.invoke(compileResult, "testModuleVariableReferencingClass");
+    }
+
+    @Test


Is this the correct file for this test? Maybe both can go in org.ballerinalang.test.object.ObjectTest?

Moved to ObjectTest

...allerina-unit-test/src/test/resources/test-src/object/object-referencing-module-variable.bal

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java

MaryamZi · 2023-08-14T06:52:14Z

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java

@@ -47,11 +47,14 @@ public class ObjectTest {

    private CompileResult checkInInitializerResult;
    private CompileResult checkFunctionReferencesResult;
+    private CompileResult checkObjectWithDefaultableFieldsResult;


Field with default value. Or just object with default values.

change as checkObjectWithDefaultValuesResult

MaryamZi · 2023-08-14T06:52:34Z

tests/jballerina-unit-test/src/test/resources/test-src/object/object-with-defaultable-field.bal

+
+const ASSERTION_ERROR_REASON = "AssertionError";
+
+function assertEquality(any expected, any actual) {


Why any and not anydata?

MaryamZi · 2023-08-14T10:35:38Z

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java

@@ -941,6 +942,16 @@ public void testNonPublicSymbolsWarningInServiceClass() {
        Assert.assertEquals(result.getDiagnostics().length, 0);
    }

+    @Test
+    public void testClassWithModuleDefaultValue() {


What does module default value mean?

Suggested change

public void testClassWithModuleDefaultValue() {

public void testClassWithModuleLevelVarAsDefaultValue() {

Updated. I am trying to specify this test case using a module variable as a default value.

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java

tests/jballerina-unit-test/src/test/resources/test-src/object/object-with-defaultable-field.bal

MaryamZi · 2023-08-15T03:22:41Z

tests/jballerina-unit-test/src/test/resources/test-src/object/object-with-defaultable-field.bal

+const ASSERTION_ERROR_REASON = "AssertionError";
+
+function assertEquality(anydata expected, anydata actual) {
+    if expected is anydata && actual is anydata && expected == actual {


Don't you get warnings for the checks now? Always true?

Please update the logic, removing unnecessary checks.

didn't get the warning.

Hint rather. Didn't you get unnecessary condition: expression will always evaluate to 'true'?

chiranSachintha added 2 commits June 20, 2023 09:43

Update closure variable logic

f6c3695

Add unit tests

c7764ca

chiranSachintha requested review from sameerajayasoma and hasithaa as code owners June 20, 2023 04:42

chiranSachintha added the Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. label Jun 20, 2023

chiranSachintha requested review from MaryamZi and dulajdilshan June 20, 2023 04:46

MaryamZi reviewed Jun 20, 2023

View reviewed changes

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

chiranSachintha removed the Stale label Jul 6, 2023

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

chiranSachintha removed the Stale label Jul 24, 2023

chiranSachintha added 2 commits August 1, 2023 22:05

Fix review suggestions

116537a

Merge branch 'master' of https://github.com/ballerina-platform/baller…

9cbd31d

…ina-lang into issue-40769

chiranSachintha requested a review from MaryamZi August 1, 2023 16:38

MaryamZi reviewed Aug 7, 2023

View reviewed changes

MaryamZi added this to the 2201.8.0 milestone Aug 7, 2023

Fix review suggestions

95ed36b

MaryamZi reviewed Aug 14, 2023

View reviewed changes

Fix review suggestions

86d73de

chiranSachintha requested a review from MaryamZi August 14, 2023 04:24

MaryamZi reviewed Aug 14, 2023

View reviewed changes

Fix review suggestions

78ef046

chiranSachintha requested a review from MaryamZi August 14, 2023 09:06

MaryamZi reviewed Aug 14, 2023

View reviewed changes

Fix review suggestions

0c23902

MaryamZi reviewed Aug 15, 2023

View reviewed changes

Refactor tests

b58a604

MaryamZi approved these changes Aug 15, 2023

View reviewed changes

chiranSachintha merged commit 8c04b7b into ballerina-platform:master Aug 15, 2023
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue with global variable as default value for class field and function parameter #40774

Fix issue with global variable as default value for class field and function parameter #40774

chiranSachintha commented Jun 20, 2023

codecov bot commented Jun 20, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023

github-actions bot commented Jul 21, 2023

MaryamZi Aug 7, 2023

chiranSachintha Aug 14, 2023

MaryamZi Aug 7, 2023

chiranSachintha Aug 14, 2023

MaryamZi Aug 14, 2023

chiranSachintha Aug 14, 2023

MaryamZi Aug 14, 2023

MaryamZi Aug 14, 2023

chiranSachintha Aug 15, 2023

MaryamZi Aug 15, 2023

MaryamZi Aug 15, 2023

chiranSachintha Aug 15, 2023

MaryamZi Aug 15, 2023


		const ASSERTION_ERROR_REASON = "AssertionError";

		function assertEquality(any expected, any actual) {

	public void testClassWithModuleDefaultValue() {
	public void testClassWithModuleLevelVarAsDefaultValue() {

Fix issue with global variable as default value for class field and function parameter #40774

Fix issue with global variable as default value for class field and function parameter #40774

Conversation

chiranSachintha commented Jun 20, 2023

Purpose

Check List

codecov bot commented Jun 20, 2023 • edited Loading

Codecov Report

github-actions bot commented Jul 5, 2023

github-actions bot commented Jul 21, 2023

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

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

codecov bot commented Jun 20, 2023 •

edited

Loading