Merge pull request #16244 from rdhananjaya/indirect-error-ctor-exprs

Introduce indirect error constructors
ballerina-platform · Jul 11, 2019 · 9a99645 · 9a99645
2 parents b52f9a9 + b7543d9
commit 9a99645
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 14 deletions.
diff --git a/compiler/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticCode.java b/compiler/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticCode.java
@@ -411,6 +411,8 @@ public enum DiagnosticCode {
 
     CANNOT_INFER_ERROR_TYPE("cannot.infer.error.type"),
     INVALID_ERROR_CONSTRUCTOR_DETAIL("invalid.error.detail.rec.does.not.match"),
+    INDIRECT_ERROR_CTOR_REASON_NOT_ALLOWED("invalid.error.reason.argument.to.indirect.error.constructor"),
+    INDIRECT_ERROR_CTOR_NOT_ALLOWED_ON_NON_CONST_REASON("invalid.indirect.error.constructor.invocation"),
 
     // Seal inbuilt function related codes
     INCOMPATIBLE_STAMP_TYPE("incompatible.stamp.type"),

diff --git a/...ng/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java b/...ng/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java
@@ -649,7 +649,6 @@ public void visit(BLangErrorVariable varNode) {
                 varNode.type = errorType;
             } else if (varNode.type.tag == TypeTags.ERROR) {
                 errorType.detailType = ((BErrorType) varNode.type).detailType;
-                varNode.type = errorType;
             }
 
             // Set error reason type.

diff --git a/...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java b/...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
@@ -1284,7 +1284,9 @@ public void visit(BLangErrorVarRef varRefExpr) {
         }
 
         BType errorDetailType = new BMapType(TypeTags.MAP, errorRefRestFieldType, null);
-        resultType = new BErrorType(errorTSymbol, varRefExpr.reason.type, errorDetailType);
+        BErrorType errorType = new BErrorType(errorTSymbol, varRefExpr.reason.type, errorDetailType);
+        errorTSymbol.type = errorType;
+        resultType = errorType;
     }
 
     private boolean checkErrorRestParamVarRef(BLangErrorVarRef varRefExpr, boolean unresolvedReference) {
@@ -2679,12 +2681,20 @@ private void checkFunctionInvocationExpr(BLangInvocation iExpr) {
             }
         }
 
-        // if no such function found, then try resolving in package
-        if (funcSymbol == symTable.notFoundSymbol) {
-            funcSymbol = symResolver.lookupSymbolInPackage(iExpr.pos, env, pkgAlias, funcName, SymTag.VARIABLE);
+        // This if check is to avoid duplicate error message about 'undefined module'
+        if (symResolver.resolvePkgSymbol(iExpr.pos, env, pkgAlias) != symTable.notFoundSymbol) {
+            if (funcSymbol == symTable.notFoundSymbol) {
+                funcSymbol = symResolver.lookupSymbolInPackage(iExpr.pos, env, pkgAlias, funcName, SymTag.VARIABLE);
+            }
+            if (funcSymbol == symTable.notFoundSymbol) {
+                funcSymbol = symResolver.lookupSymbolInPackage(iExpr.pos, env, pkgAlias, funcName, SymTag.CONSTRUCTOR);
+            }
         }
 
-        if ((funcSymbol.tag & SymTag.ERROR) == SymTag.ERROR) {
+        if ((funcSymbol.tag & SymTag.ERROR) == SymTag.ERROR
+            || ((funcSymbol.tag & SymTag.CONSTRUCTOR) == SymTag.CONSTRUCTOR && funcSymbol.type.tag == TypeTags.ERROR)) {
+            iExpr.symbol = funcSymbol;
+            iExpr.type = funcSymbol.type;
             checkErrorConstructorInvocation(iExpr);
             return;
         } else if (funcSymbol == symTable.notFoundSymbol || (funcSymbol.tag & SymTag.FUNCTION) != SymTag.FUNCTION) {
@@ -2712,6 +2722,8 @@ private void checkErrorConstructorInvocation(BLangInvocation iExpr) {
                 dlog.error(iExpr.pos, DiagnosticCode.CANNOT_INFER_ERROR_TYPE, expType);
                 resultType = symTable.semanticError;
                 return;
+            } else if ((iExpr.symbol.tag & SymTag.CONSTRUCTOR) == SymTag.CONSTRUCTOR) {
+                expType = iExpr.type;
             } else {
                 // var e = <error> error("r");
                 expType = symTable.errorType;
@@ -2725,6 +2737,12 @@ private void checkErrorConstructorInvocation(BLangInvocation iExpr) {
             return;
         }
 
+        if (nonNamedArgsGiven(iExpr) && (iExpr.symbol.tag & SymTag.CONSTRUCTOR) == SymTag.CONSTRUCTOR) {
+            dlog.error(iExpr.argExprs.get(0).pos, DiagnosticCode.INDIRECT_ERROR_CTOR_REASON_NOT_ALLOWED);
+            resultType = symTable.semanticError;
+            return;
+        }
+
         boolean reasonArgGiven = checkErrorReasonArg(iExpr, ctorType);
 
         if (ctorType.detailType.tag == TypeTags.RECORD) {
@@ -2757,6 +2775,10 @@ private void checkErrorConstructorInvocation(BLangInvocation iExpr) {
         iExpr.symbol = lhsErrorType.ctorSymbol;
     }
 
+    private boolean nonNamedArgsGiven(BLangInvocation iExpr) {
+        return iExpr.argExprs.stream().anyMatch(arg -> arg.getKind() != NodeKind.NAMED_ARGS_EXPR);
+    }
+
     private boolean checkErrorReasonArg(BLangInvocation iExpr, BErrorType ctorType) {
         if (iExpr.argExprs.isEmpty()) {
             return false;
@@ -2772,10 +2794,20 @@ private boolean checkErrorReasonArg(BLangInvocation iExpr, BErrorType ctorType)
     }
 
     private boolean checkNoArgErrorCtorInvocation(BErrorType errorType, DiagnosticPos pos) {
-        if (errorType.reasonType.getKind() == TypeKind.FINITE) {
+        if (errorType.reasonType.tag != TypeTags.FINITE) {
+            dlog.error(pos, DiagnosticCode.INDIRECT_ERROR_CTOR_NOT_ALLOWED_ON_NON_CONST_REASON,
+                    expType.tsymbol.name);
+            resultType = symTable.semanticError;
+            return true;
+        } else {
             BFiniteType finiteType = (BFiniteType) errorType.reasonType;
             if (finiteType.valueSpace.size() != 1) {
-                dlog.error(pos, DiagnosticCode.CANNOT_INFER_ERROR_TYPE, expType.tsymbol.name);
+                if (errorType == symTable.errorType) {
+                    dlog.error(pos, DiagnosticCode.CANNOT_INFER_ERROR_TYPE, expType.tsymbol.name);
+                } else {
+                    dlog.error(pos, DiagnosticCode.INDIRECT_ERROR_CTOR_NOT_ALLOWED_ON_NON_CONST_REASON,
+                            expType.tsymbol.name);
+                }
                 resultType = symTable.semanticError;
                 return true;
             }

diff --git a/compiler/ballerina-lang/src/main/resources/compiler.properties b/compiler/ballerina-lang/src/main/resources/compiler.properties
@@ -1040,7 +1040,13 @@ error.cannot.infer.error.type=\
   cannot infer type of the error from ''{0}''\
 
 error.invalid.error.detail.rec.does.not.match=\
-  invalid error constructor, error details does not match
+  invalid error constructor, error details does not match\
+
+error.invalid.error.reason.argument.to.indirect.error.constructor=\
+  indirect error constructor only accept error details as named arguments
+
+error.invalid.indirect.error.constructor.invocation=\
+  cannot infer reason type from error constructor: ''{0}''
 
 error.only.simple.literals.can.be.assigned.to.const=\
   only simple literals can be assigned to a constant

diff --git a/...s/jballerina-unit-test/src/test/java/org/ballerinalang/test/balo/types/ErrorTypeTest.java b/...s/jballerina-unit-test/src/test/java/org/ballerinalang/test/balo/types/ErrorTypeTest.java
@@ -52,6 +52,16 @@ public void errorFromAnotherPkg() {
                 "{ballerina/sql}ApplicationError {message:\"Client has been stopped\"}");
     }
 
+    @Test()
+    public void indirectErrorCtorFromAnotherPkg() {
+        BValue[] returns = BRunUtil.invoke(result, "getApplicationErrorIndirectCtor");
+        Assert.assertEquals(returns.length, 1);
+        Assert.assertNotNull(returns[0]);
+        Assert.assertTrue(returns[0] instanceof BError);
+        Assert.assertEquals(returns[0].stringValue(),
+                "{ballerina/sql}ApplicationError {message:\"Client has been stopped\"}");
+    }
+
     @AfterClass
     public void tearDown() {
         BaloCreator.clearPackageFromRepository(

diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/error/ErrorTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/error/ErrorTest.java
@@ -56,6 +56,16 @@ public void setup() {
         negativeCompileResult = BCompileUtil.compile("test-src/error/error_test_negative.bal");
     }
 
+    @Test
+    public void testIndirectErrorCtor() {
+        BValue[] errors = BRunUtil.invoke(errorTestResult, "testIndirectErrorConstructor");
+        Assert.assertEquals(errors.length, 4);
+        Assert.assertEquals(errors[0].stringValue(), "ErrNo-1 {\"detail1\":\"arg\"}");
+        Assert.assertEquals(errors[1].stringValue(), "ErrNo-1 {\"detail1\":\"arg\"}");
+        Assert.assertEquals(errors[2], errors[0]);
+        Assert.assertEquals(errors[3], errors[1]);
+    }
+
     @Test
     public void errorConstructReasonTest() {
         BValue[] returns = BRunUtil.invoke(errorTestResult, "errorConstructReasonTest");
@@ -218,7 +228,7 @@ public void testUnspecifiedErrorDetailFrozenness() {
 
     @Test
     public void testErrorNegative() {
-        Assert.assertEquals(negativeCompileResult.getErrorCount(), 10);
+        Assert.assertEquals(negativeCompileResult.getErrorCount(), 12);
         BAssertUtil.validateError(negativeCompileResult, 0,
                                   "incompatible types: expected 'reason one|reason two', found 'string'", 26, 31);
         BAssertUtil.validateError(negativeCompileResult, 1,
@@ -236,8 +246,11 @@ public void testErrorNegative() {
         BAssertUtil.validateError(negativeCompileResult, 7, "self referenced variable 'e3'", 53, 22);
         BAssertUtil.validateError(negativeCompileResult, 8, "self referenced variable 'e3'", 53, 41);
         BAssertUtil.validateError(negativeCompileResult, 9, "self referenced variable 'e4'", 54, 40);
+        BAssertUtil.validateError(negativeCompileResult, 10,
+                "cannot infer reason type from error constructor: 'UserDefErrorOne'", 55, 27);
+        BAssertUtil.validateError(negativeCompileResult, 11,
+                "cannot infer reason type from error constructor: 'MyError'", 56, 19);
     }
-
     @DataProvider(name = "userDefTypeAsReasonTests")
     public Object[][] userDefTypeAsReasonTests() {
         return new Object[][] {

diff --git a/...jballerina-unit-test/src/test/resources/test-src/balo/test_balo/types/error_type_test.bal b/...jballerina-unit-test/src/test/resources/test-src/balo/test_balo/types/error_type_test.bal
@@ -1,8 +1,28 @@
-import testorg/errors version v1 as er;
+// Copyright (c) 2019 WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
+//
+// WSO2 Inc. 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.
 
-int i = 0;
+import testorg/errors version v1 as er;
 
 function getApplicationError() returns error {
     er:ApplicationError e = error(message = "Client has been stopped");
     return e;
 }
+
+function getApplicationErrorIndirectCtor() returns error {
+    er:ApplicationError e = er:ApplicationError(message = "Client has been stopped");
+    error e1 = er:ApplicationError(message = "Client has been stopped");
+    return e;
+}
diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/error/error_test.bal b/tests/jballerina-unit-test/src/test/resources/test-src/error/error_test.bal
@@ -223,3 +223,9 @@ public function errorReasonSubType() returns [error, error, error, error] {
     UserDefErrorTwoB er_aBLit = error("ErrorNo-2");
     return [er_rA, er_rB, er_aALit, er_aBLit];
 }
+
+function testIndirectErrorConstructor() returns [UserDefErrorTwoA, UserDefErrorTwoA, error, error] {
+    var e0 = UserDefErrorTwoA(detail1="arg");
+    UserDefErrorTwoA e1 = UserDefErrorTwoA(detail1="arg");
+    return [e0, e1, e0, e1];
+}
diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/error/error_test_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/error/error_test_negative.bal
@@ -52,4 +52,6 @@ type MyError error<string, map<MyError>>;
 function testSelfReferencingErrorConstructor() {
     error e3 = error(e3.reason(), err = e3);
     MyError e4 = error("reason", err = e4);
+    UserDefErrorOne ue1 = UserDefErrorOne();
+    MyError me1 = MyError();
 }
diff --git a/...it-test/src/test/resources/test-src/taintchecking/propagation/service-level-variables.bal b/...it-test/src/test/resources/test-src/taintchecking/propagation/service-level-variables.bal
@@ -1,6 +1,6 @@
 import ballerina/http;
 
-listener http:Listener helloWorldEP = new (9090);
+listener http:Listener helloWorldEP = new (19090);
 
 any globalLevelVariable = "";
 service sample on helloWorldEP {