Merge pull request #43185 from MaryamZi/fix-43117

Disallow `check` in parameter defaults
ballerina-platform · Jul 31, 2024 · 87df251 · 87df251
2 parents b17ee20 + 0d75609
commit 87df251
Show file tree

Hide file tree

Showing 10 changed files with 206 additions and 33 deletions.
diff --git a/...r/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticErrorCode.java b/...r/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticErrorCode.java
@@ -819,6 +819,7 @@ public enum DiagnosticErrorCode implements DiagnosticCode {
     EXPRESSION_OF_FUTURE_TYPE_EXPECTED("BCE4057", "future.expression.expected"),
     INSTANTIATION_ERROR("BCE4058", "instantiation.error"),
     INVALID_BINDING_PATTERN_IN_ON_FAIL("BCE4059", "invalid.binding.pattern.in.on.fail"),
+    INVALID_USAGE_OF_CHECK_IN_PARAMETER_DEFAULT("BCE4060", "invalid.usage.of.check.in.parameter.default")
     ;
 
     private final String diagnosticId;

diff --git a/...a-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/CodeAnalyzer.java b/...a-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/CodeAnalyzer.java
@@ -514,8 +514,7 @@ private void visitFunction(BLangFunction funcNode, AnalyzerData data) {
         if (funcNode.body != null) {
 
             DefaultValueState prevDefaultValueState = data.defaultValueState;
-            if (prevDefaultValueState == DefaultValueState.RECORD_FIELD_DEFAULT ||
-                    prevDefaultValueState == DefaultValueState.OBJECT_FIELD_INITIALIZER) {
+            if (inDefaultValue(prevDefaultValueState)) {
                 data.defaultValueState = DefaultValueState.FUNCTION_IN_DEFAULT_VALUE;
             }
             analyzeNode(funcNode.body, data);
@@ -1707,7 +1706,12 @@ public void visit(BLangSimpleVariable varNode, AnalyzerData data) {
 
         analyzeTypeNode(varNode.typeNode, data);
 
+        DefaultValueState prevDefaultValueState = data.defaultValueState;
+        if (varNode.flagSet.contains(Flag.DEFAULTABLE_PARAM)) {
+            data.defaultValueState = DefaultValueState.PARAMETER_DEFAULT;
+        }
         analyzeExpr(varNode.expr, data);
+        data.defaultValueState = prevDefaultValueState;
 
         if (Objects.isNull(varNode.symbol)) {
             return;
@@ -3113,8 +3117,7 @@ public void visit(BLangLambdaFunction bLangLambdaFunction, AnalyzerData data) {
     public void visit(BLangArrowFunction bLangArrowFunction, AnalyzerData data) {
 
         DefaultValueState prevDefaultValueState = data.defaultValueState;
-        if (prevDefaultValueState == DefaultValueState.RECORD_FIELD_DEFAULT ||
-                prevDefaultValueState == DefaultValueState.OBJECT_FIELD_INITIALIZER) {
+        if (inDefaultValue(prevDefaultValueState)) {
             data.defaultValueState = DefaultValueState.FUNCTION_IN_DEFAULT_VALUE;
         }
         analyzeExpr(bLangArrowFunction.body.expr, data);
@@ -3298,43 +3301,49 @@ public void visit(BLangCheckedExpr checkedExpr, AnalyzerData data) {
         data.failVisited = true;
         analyzeExpr(checkedExpr.expr, data);
 
-        if (data.env.scope.owner.getKind() == SymbolKind.PACKAGE) {
-            // Check at module level.
+        DefaultValueState defaultValueState = data.defaultValueState;
+        if (defaultValueState == DefaultValueState.PARAMETER_DEFAULT) {
+            dlog.error(checkedExpr.pos, DiagnosticErrorCode.INVALID_USAGE_OF_CHECK_IN_PARAMETER_DEFAULT);
             return;
         }
 
-        BLangInvokableNode enclInvokable = data.env.enclInvokable;
+        if (defaultValueState == DefaultValueState.RECORD_FIELD_DEFAULT) {
+            dlog.error(checkedExpr.pos,
+                       DiagnosticErrorCode.INVALID_USAGE_OF_CHECK_IN_RECORD_FIELD_DEFAULT_EXPRESSION);
+            return;
+        }
+
+        if (defaultValueState == DefaultValueState.OBJECT_FIELD_INITIALIZER) {
+            BAttachedFunction initializerFunc =
+                    ((BObjectTypeSymbol) getEnclosingClass(data.env).getBType().tsymbol).initializerFunc;
 
-        List<BType> equivalentErrorTypeList = checkedExpr.equivalentErrorTypeList;
-        if (equivalentErrorTypeList != null && !equivalentErrorTypeList.isEmpty()) {
-            if (data.defaultValueState == DefaultValueState.RECORD_FIELD_DEFAULT) {
+            if (initializerFunc == null) {
                 dlog.error(checkedExpr.pos,
-                           DiagnosticErrorCode.INVALID_USAGE_OF_CHECK_IN_RECORD_FIELD_DEFAULT_EXPRESSION);
+                        DiagnosticErrorCode
+                                .INVALID_USAGE_OF_CHECK_IN_OBJECT_FIELD_INITIALIZER_IN_OBJECT_WITH_NO_INIT_METHOD);
                 return;
             }
 
-            if (data.defaultValueState == DefaultValueState.OBJECT_FIELD_INITIALIZER) {
-                BAttachedFunction initializerFunc =
-                        ((BObjectTypeSymbol) getEnclosingClass(data.env).getBType().tsymbol).initializerFunc;
-
-                if (initializerFunc == null) {
-                    dlog.error(checkedExpr.pos,
-                            DiagnosticErrorCode
-                                    .INVALID_USAGE_OF_CHECK_IN_OBJECT_FIELD_INITIALIZER_IN_OBJECT_WITH_NO_INIT_METHOD);
-                    return;
-                }
-
-                BType exprErrorTypes = types.getErrorTypes(checkedExpr.expr.getBType());
-                BType initMethodReturnType = initializerFunc.type.retType;
-                if (!types.isAssignable(exprErrorTypes, initMethodReturnType)) {
-                    dlog.error(checkedExpr.pos, DiagnosticErrorCode
-                            .INVALID_USAGE_OF_CHECK_IN_OBJECT_FIELD_INITIALIZER_WITH_INIT_METHOD_RETURN_TYPE_MISMATCH,
-                            initMethodReturnType, exprErrorTypes);
-                }
+            BType exprErrorTypes = types.getErrorTypes(checkedExpr.expr.getBType());
+            if (exprErrorTypes == symTable.semanticError) {
                 return;
             }
+
+            BType initMethodReturnType = initializerFunc.type.retType;
+            if (!types.isAssignable(exprErrorTypes, initMethodReturnType)) {
+                dlog.error(checkedExpr.pos, DiagnosticErrorCode
+                        .INVALID_USAGE_OF_CHECK_IN_OBJECT_FIELD_INITIALIZER_WITH_INIT_METHOD_RETURN_TYPE_MISMATCH,
+                        initMethodReturnType, exprErrorTypes);
+            }
+            return;
         }
 
+        if (data.env.scope.owner.getKind() == SymbolKind.PACKAGE) {
+            // Check at module level.
+            return;
+        }
+
+        BLangInvokableNode enclInvokable = data.env.enclInvokable;
         if (enclInvokable == null) {
             return;
         }
@@ -4391,8 +4400,15 @@ private BLangExpression getMatchedExprIfCalledInMatchGuard(BLangInvocation invoc
         return null;
     }
 
+    private boolean inDefaultValue(DefaultValueState prevDefaultValueState) {
+        return prevDefaultValueState == DefaultValueState.RECORD_FIELD_DEFAULT ||
+                prevDefaultValueState == DefaultValueState.OBJECT_FIELD_INITIALIZER ||
+                prevDefaultValueState == DefaultValueState.PARAMETER_DEFAULT;
+    }
+
     private enum DefaultValueState {
         NOT_IN_DEFAULT_VALUE,
+        PARAMETER_DEFAULT,
         RECORD_FIELD_DEFAULT,
         OBJECT_FIELD_INITIALIZER,
         FUNCTION_IN_DEFAULT_VALUE

diff --git a/compiler/ballerina-lang/src/main/resources/compiler.properties b/compiler/ballerina-lang/src/main/resources/compiler.properties
@@ -2031,3 +2031,6 @@ error.inferred.query.construct.type.as.stream=\
 
 error.invalid.binding.pattern.in.on.fail=\
   invalid binding pattern in ''on fail'' clause: only a capture binding pattern or an error binding pattern is allowed
+
+error.invalid.usage.of.check.in.parameter.default=\
+  cannot use ''check'' in the default value of a parameter
diff --git a/...ns/FunctionsWithDefaultableArguments.java → ...nctionsWithDefaultableParametersTest.java b/...ns/FunctionsWithDefaultableArguments.java → ...nctionsWithDefaultableParametersTest.java
@@ -22,6 +22,7 @@
 import io.ballerina.runtime.api.values.BArray;
 import io.ballerina.runtime.api.values.BMap;
 import io.ballerina.runtime.api.values.BString;
+import org.ballerinalang.test.BAssertUtil;
 import org.ballerinalang.test.BCompileUtil;
 import org.ballerinalang.test.BRunUtil;
 import org.ballerinalang.test.CompileResult;
@@ -37,8 +38,10 @@
  *
  * @since 0.995.0
  */
-public class FunctionsWithDefaultableArguments {
+public class FunctionsWithDefaultableParametersTest {
 
+    private static final String CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER =
+            "cannot use 'check' in the default value of a parameter";
     private CompileResult result;
 
     @BeforeClass
@@ -270,6 +273,37 @@ public void testUsingParamInAnonFuncDefaultValueOfSubsequentParam() {
         BRunUtil.invoke(result, "testUsingParamInAnonFuncDefaultValueOfSubsequentParam");
     }
 
+    @Test
+    public void testValidCheckUsageViaDefaults() {
+        BRunUtil.invoke(result, "testValidCheckUsageViaDefaults");
+    }
+
+    @Test
+    public void testCheckInParameterDefaultNegative() {
+        int i = 0;
+        CompileResult result = BCompileUtil.compile(
+                "test-src/functions/check_in_parameter_default_negative.bal");
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 19, 21);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 21, 47);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 25, 19);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 28, 31);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 31, 37);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 31, 69);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 35, 41);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 38, 23);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 42, 37);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 43, 37);
+        BAssertUtil.validateError(result, i++, "cannot use 'check' in the default expression of a record field",
+                                  43, 75);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 47, 37);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 47, 72);
+        BAssertUtil.validateError(result, i++, CANNOT_USE_CHECK_IN_THE_DEFAULT_VALUE_OF_A_PARAMETER, 52, 21);
+        BAssertUtil.validateWarning(result, i++, "invalid usage of the 'check' expression operator: no expression " +
+                "type is equivalent to error type", 52, 27);
+        Assert.assertEquals(i - 1, result.getErrorCount());
+        Assert.assertEquals(1, result.getWarnCount());
+    }
+
     @AfterClass
     public void tearDown() {
         result = null;

diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java
@@ -907,8 +907,18 @@ public void testInvalidUsageOfCheckInObjectFieldInitializer() {
                 " with the return type of the 'init' method: expected 'MyError?', found 'error'", 226, 51);
         BAssertUtil.validateWarning(result, i++, "unused variable 'a'", 233, 5);
         BAssertUtil.validateWarning(result, i++, "unused variable 'b'", 234, 5);
-        Assert.assertEquals(result.getErrorCount(), i - 3);
-        Assert.assertEquals(result.getWarnCount(), 3);
+        BAssertUtil.validateError(result, i++, "cannot use 'check' in an object field initializer of an object with " +
+                "no 'init' method", 242, 14);
+        BAssertUtil.validateWarning(result, i++, "invalid usage of the 'check' expression operator: no expression " +
+                "type is equivalent to error type", 242, 20);
+        BAssertUtil.validateError(result, i++, "cannot use 'check' in an object field initializer of an object with " +
+                "no 'init' method", 243, 14);
+        BAssertUtil.validateWarning(result, i++, "invalid usage of the 'check' expression operator: no expression " +
+                "type is equivalent to error type", 247, 20);
+        BAssertUtil.validateError(result, i++, "usage of 'check' in field initializer is allowed only when compatible" +
+                " with the return type of the 'init' method: expected 'MyError?', found 'error'", 248, 14);
+        Assert.assertEquals(result.getErrorCount(), i - 5);
+        Assert.assertEquals(result.getWarnCount(), 5);
     }
 
     @Test(dataProvider = "checkInObjectFieldInitializerTests")

diff --git a/...allerina-unit-test/src/test/java/org/ballerinalang/test/record/RecordDefNegativeTest.java b/...allerina-unit-test/src/test/java/org/ballerinalang/test/record/RecordDefNegativeTest.java
@@ -134,6 +134,13 @@ public void testInvalidUsageOfCheckInRecordFieldDefaultValue() {
                 80, 13);
         BAssertUtil.validateError(compileResult, i++, INVALID_USAGE_OF_CHECK_IN_RECORD_FIELD_DEFAULT_EXPRESSION,
                 85, 13);
-        Assert.assertEquals(compileResult.getErrorCount(), i);
+        BAssertUtil.validateError(compileResult, i++, INVALID_USAGE_OF_CHECK_IN_RECORD_FIELD_DEFAULT_EXPRESSION,
+                97, 14);
+        BAssertUtil.validateWarning(compileResult, i++, "invalid usage of the 'check' expression operator: no " +
+                "expression type is equivalent to error type", 97, 20);
+        BAssertUtil.validateError(compileResult, i++, INVALID_USAGE_OF_CHECK_IN_RECORD_FIELD_DEFAULT_EXPRESSION,
+                98, 16);
+        Assert.assertEquals(compileResult.getErrorCount(), i - 1);
+        Assert.assertEquals(compileResult.getWarnCount(), 1);
     }
 }
diff --git a/...a-unit-test/src/test/resources/test-src/functions/check_in_parameter_default_negative.bal b/...a-unit-test/src/test/resources/test-src/functions/check_in_parameter_default_negative.bal
@@ -0,0 +1,52 @@
+// 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.
+
+isolated function intOrError() returns int|error => error("err!");
+
+function f1(int a = check intOrError()) returns int => a;
+
+function f2(any a = function (int intResult = check intOrError()) returns int|error {
+    return intResult + 1;
+}) returns any => a;
+
+function (int a = check intOrError()) returns int f3 = a => a;
+
+function f4() returns error? {
+    var _ = function (int a = check intOrError()) returns int => a;
+
+    function (string, int, int) _ =
+        function (string s, int i = check intOrError(), int j = i + check intOrError()) {
+        };
+
+    record {|
+        function fn = function (int v = check intOrError()) {};
+    |} _ = {};
+
+    function (int a = check intOrError()) returns int _ = a => a;
+}
+
+type Foo record {|
+    function fm = function (int v = check intOrError()) {};
+    function fn = function (int v = check intOrError(), record {| int x = check intOrError(); |} u = {}) {};
+|};
+
+class Bar {
+    function fn = function (int v = check intOrError(), int[] w = [12, check intOrError()]) {};
+}
+
+isolated function intFunc() returns int => 1;
+
+function f5(int a = check intFunc()) returns int => a;
diff --git a/...ina-unit-test/src/test/resources/test-src/functions/functions_with_default_parameters.bal b/...ina-unit-test/src/test/resources/test-src/functions/functions_with_default_parameters.bal
@@ -348,6 +348,33 @@ function testUsingParamInAnonFuncDefaultValueOfSubsequentParam() {
     assertEquality(baz4(10), 31);
 }
 
+isolated function intOrErrorError() returns int|error => error("err!");
+isolated function intOrErrorInt() returns int|error => 4;
+
+function functionWithIndirectUsageOfCheckInParamDefault(
+    function () returns int|error fn = function () returns int|error {
+        int intResult = check intOrErrorError();
+        return intResult + 1;
+    }) returns int|error => fn();
+
+function testValidCheckUsageViaDefaults() {
+    int|error res = functionWithIndirectUsageOfCheckInParamDefault();
+    assertEquality(true, res is error);
+    error err = <error> res;
+    assertEquality("err!", err.message());
+
+    record {|
+        function (int intP = checkpanic intOrErrorError()) returns int|error x = function (int intP) returns int|error {
+            int val = check intOrErrorInt();
+            return val + intP;
+        };
+        string y;
+    |} rec = {y: "test"};
+    function (int intP = 1 + checkpanic intOrErrorInt()) returns int|error x = rec.x;
+    assertEquality(9, x());
+    assertEquality("test", rec.y);
+}
+
 const ASSERTION_ERROR_REASON = "AssertionError";
 
 function assertEquality(any|error expected, any|error actual) {

diff --git a/...-test/src/test/resources/test-src/object/object_field_initializer_with_check_negative.bal b/...-test/src/test/resources/test-src/object/object_field_initializer_with_check_negative.bal
@@ -235,3 +235,19 @@ function testObjectConstructor() {
 }
 
 isolated function fn() returns int|error => 0;
+
+isolated function intFunc() returns int => 1;
+
+class Corge {
+    int i1 = check intFunc();
+    int i2 = check fn();
+};
+
+class Grault {
+    int i1 = check intFunc();
+    int i2 = check fn();
+
+    function init() returns MyError? {
+
+    }
+}
diff --git a/...-test/src/test/resources/test-src/record/negative/record_field_default_value_negative.bal b/...-test/src/test/resources/test-src/record/negative/record_field_default_value_negative.bal
@@ -90,3 +90,10 @@ class Qux {
         return;
     }
 }
+
+isolated function intFunc() returns int => 1;
+
+type Quux record {|
+    int i1 = check intFunc();
+    int[] i2 = check f1();
+|};