diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java index e02b7df1b9e7..5f6a58d66e27 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java @@ -6094,7 +6094,7 @@ private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr) { targetVarRef = new BLangStructFieldAccessExpr(fieldAccessExpr.pos, fieldAccessExpr.expr, stringLit, (BVarSymbol) fieldAccessExpr.symbol, false, fieldAccessExpr.isStoreOnCreation); } - } else if (types.isLax(refType)) { + } else if (types.isLaxFieldAccessAllowed(refType)) { if (!(refType.tag == TypeTags.XML || refType.tag == TypeTags.XML_ELEMENT)) { if (refType.tag == TypeTags.MAP && TypeTags.isXMLTypeTag(((BMapType) refType).constraint.tag)) { result = rewriteExpr(rewriteLaxMapAccess(fieldAccessExpr)); diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java index 49729107ff97..66a62db40fb5 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java @@ -6177,7 +6177,7 @@ private void rewriteWithEnsureTypeFunc(BLangCheckedExpr checkedExpr, BType type, rhsType = getCandidateType(checkedExpr, rhsType, data); } BType candidateLaxType = getCandidateLaxType(checkedExpr.expr, rhsType); - if (!types.isLax(candidateLaxType)) { + if (!types.isLaxFieldAccessAllowed(candidateLaxType)) { return; } ArrayList argExprs = new ArrayList<>(); @@ -8209,7 +8209,7 @@ private BType checkFieldAccessExpr(BLangFieldBasedAccess fieldAccessExpr, BType fieldName, varRefType.getKind() == TypeKind.UNION ? "union" : varRefType.getKind().typeName(), varRefType); } - } else if (types.isLax(varRefType)) { + } else if (types.isLaxFieldAccessAllowed(varRefType)) { if (fieldAccessExpr.isLValue) { dlog.error(fieldAccessExpr.pos, DiagnosticErrorCode.OPERATION_DOES_NOT_SUPPORT_FIELD_ACCESS_FOR_ASSIGNMENT, @@ -8267,7 +8267,7 @@ private void resolveXMLNamespace(BLangFieldBasedAccess.BLangNSPrefixedFieldBased } private boolean hasLaxOriginalType(BLangFieldBasedAccess fieldBasedAccess) { - return fieldBasedAccess.originalType != null && types.isLax(fieldBasedAccess.originalType); + return fieldBasedAccess.originalType != null && types.isLaxFieldAccessAllowed(fieldBasedAccess.originalType); } private BType getLaxFieldAccessType(BType exprType) { @@ -8331,7 +8331,7 @@ private BType checkOptionalFieldAccessExpr(BLangFieldBasedAccess fieldAccessExpr fieldAccessExpr.nilSafeNavigation = nillableExprType; fieldAccessExpr.originalType = fieldAccessExpr.leafNode || !nillableExprType ? actualType : types.getTypeWithoutNil(actualType); - } else if (types.isLax(effectiveType)) { + } else if (types.isLaxFieldAccessAllowed(effectiveType)) { BType laxFieldAccessType = getLaxFieldAccessType(effectiveType); actualType = accessCouldResultInError(effectiveType) ? BUnionType.create(null, laxFieldAccessType, symTable.errorType) : laxFieldAccessType; diff --git a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java index 54f2a6d778b9..c4a0acf3b34f 100644 --- a/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java +++ b/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java @@ -273,13 +273,9 @@ public BType checkType(Location pos, return symTable.semanticError; } - public boolean isLax(BType type) { + public boolean isLaxFieldAccessAllowed(BType type) { Set visited = new HashSet<>(); - int result = isLaxType(type, visited); - if (result == 1) { - return true; - } - return false; + return isLaxType(type, visited) == 1 || type.tag == TypeTags.XML || type.tag == TypeTags.XML_ELEMENT; } // TODO : clean @@ -289,8 +285,6 @@ public int isLaxType(BType type, Set visited) { } switch (type.tag) { case TypeTags.JSON: - case TypeTags.XML: - case TypeTags.XML_ELEMENT: return 1; case TypeTags.MAP: return isLaxType(((BMapType) type).constraint, visited); @@ -323,8 +317,6 @@ public boolean isLaxType(BType type, Map visited) { } switch (type.tag) { case TypeTags.JSON: - case TypeTags.XML: - case TypeTags.XML_ELEMENT: visited.put(type, true); return true; case TypeTags.MAP: diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java index cf07af6d528e..1ffa1a276409 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java @@ -139,6 +139,21 @@ public void testNegativeCases() { "expression", 375, 15); validateError(negativeResult, i++, "'remote' methods of an object cannot be accessed using the field access " + "expression", 377, 15); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support field access" + , 382, 19); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support field access" + , 387, 19); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support field access" + , 393, 19); + validateError(negativeResult, i++, "invalid operation: type 'map<(xml|json)>' does not support field access" + , 399, 24); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support " + + "optional field access", 404, 13); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support " + + "optional field access", 409, 14); + validateError(negativeResult, i++, "invalid operation: type 'map' does not support " + + "optional field access", 414, 13); + Assert.assertEquals(negativeResult.getErrorCount(), i); } @@ -290,6 +305,11 @@ public void testAccessingMethodOnUnionObjectType() { BRunUtil.invoke(result, "testAccessingMethodOnUnionObjectType"); } + @Test + public void testValidXMLmapFieldAccess() { + BRunUtil.invoke(result, "testValidXMLmapFieldAccess"); + } + @Test(dataProvider = "fieldAccessOnJsonTypedRecordFields") public void testFieldAccessOnJsonTypedRecordFields(String function) { BRunUtil.invoke(result, function); diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/OptionalFieldAccessTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/OptionalFieldAccessTest.java index fd044f6816d2..e99424a48d36 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/OptionalFieldAccessTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/expressions/access/OptionalFieldAccessTest.java @@ -60,6 +60,8 @@ public void testNegativeCases() { "access", 58, 14); validateError(negativeResult, i++, "invalid operation: type 'map?' does not support optional field " + "access", 61, 19); + validateError(negativeResult, i++, "invalid operation: type '(map|map)' does not support" + + " optional field access", 65, 20); validateError(negativeResult, i++, "incompatible types: expected 'json', found '(json|error)'", 71, 15); validateError(negativeResult, i++, "invalid operation: type 'Qux' does not support optional field access", 87 , 9); diff --git a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java index 71f4ba6ba030..b3d67756482f 100644 --- a/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java +++ b/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java @@ -83,26 +83,11 @@ public void testXMLAttributeAccessNegative() { BAssertUtil.validateError(negative, 1, "invalid character ':' in field access expression", 10, 13); } - @Test - public void testXMLAsMapContent() { - BArray result = (BArray) BRunUtil.invoke(lexCompileRes, "testXMLAsMapContent"); - Assert.assertEquals(result.get(0).toString(), "val"); - Assert.assertEquals(result.get(1).toString(), "val"); - Assert.assertEquals(result.get(2).toString(), "true"); - } - @Test public void testXMLAttributeWithNSPrefix() { BArray result = (BArray) BRunUtil.invoke(lexCompileRes, "testXMLAttributeWithNSPrefix"); Assert.assertEquals(result.get(0).toString(), "preserve"); Assert.assertEquals(result.get(1).toString(), "preserve"); - Assert.assertEquals(result.get(2).toString(), "error(\"{lang.map}InvalidKey\",key=\"b\")"); - } - - @Test - public void testXMLASMapContentInvalidKey() { - Object result = BRunUtil.invoke(lexCompileRes, "testXMLASMapContentInvalidKey"); - Assert.assertEquals(result.toString(), "error(\"{lang.map}InvalidKey\",key=\"b\")"); } @Test diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal b/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal index 88a2337b75ea..18d4c3ebc944 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal @@ -536,6 +536,14 @@ function validateJSONOperationErrorMsg(json|error val) { assertEquals( checkpanic err.detail()["message"], "JSON value is not a mapping"); } +function testValidXMLmapFieldAccess() { + map m = {a: xml `foo`, b: xml `bar`}; + xml? x = m["a"]; + xml? y = m["c"]; + assertEquals(x, xml `foo`); + assertEquals(y, null); +} + isolated function isEqual(anydata|error val1, anydata|error val2) returns boolean { if (val1 is anydata && val2 is anydata) { return (val1 == val2); diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access_negative.bal b/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access_negative.bal index e1b3b5d261cd..5657ee785102 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access_negative.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access_negative.bal @@ -376,3 +376,40 @@ function testInvalidBoundMethodAccessWithRemoteMethod(ServiceClass a, _ = e.ser.fn; } + +function testInvalidXMLMapFieldAccess1() returns error? { + map m = {a: xml `foo`}; + xml x = check m.a; // error +} + +function testInvalidXMLMapFieldAccess2() returns error? { + map m = {a: xml `foo`}; + xml x = check m.b; // error +} + +function testInvalidXMLMapFieldAccess3() returns error? { + map m = {}; + m["a"] = xml `foo`; + xml x = check m.a; // error +} + +function testInvalidXMLMapFieldAccess4() returns error? { + map m = {}; + m["a"] = xml `foo`; + xml|json x = check m.a; // error +} + +function testInvalidXMLMapFieldAccess5() returns error? { + map m = {a: xml `foo`}; + xml x = m?.a; // error +} + +function testInvalidXMLMapFieldAccess6() returns error? { + record {|map a; xml c;|} m = {a: {b: xml `foo`}, c: xml `bar`}; + xml? x = m["a"]?.b.c; +} + +function testInvalidXMLMapFieldAccess7() returns error? { + map m = {a: xml `foo`}; + xml x = m?.b; // error +} diff --git a/tests/jballerina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-lax-behavior.bal b/tests/jballerina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-lax-behavior.bal index 5cf64bd32463..267334994fba 100644 --- a/tests/jballerina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-lax-behavior.bal +++ b/tests/jballerina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-lax-behavior.bal @@ -1,36 +1,13 @@ import ballerina/lang.'xml; -function testXMLAsMapContent() returns [string|error?, string|error?, boolean] { - map xmap = getXMLMap(); - string|error val = xmap.a.attr; - string|error? val2 = xmap.a?.attr; - string|error? val3 = xmap.a?.attr2; - return [val, val2, val3 is ()]; -} - -function testXMLASMapContentInvalidKey() returns string|error? { - map xmap = getXMLMap(); - string|error val = xmap.b.attr; - return val; -} - -function testXMLAttributeWithNSPrefix() returns - [string|error?, string|error?, string|error?, boolean, boolean] { - map xmap = {}; - xmap["a"] = xml ``; - string|error val = xmap.a.'xml:space; - string|error? val2 = xmap.a?.'xml:space; - string|error? val3 = xmap.b?.'xml:space; - return [val, val2, val3]; +function testXMLAttributeWithNSPrefix() returns [string|error?, string|error?] { + xml a = xml ``; + string|error val = a.'xml:space; + string|error? val2 = a?.'xml:space; + return [val, val2]; } function testXMLDirectAttributeAccess() returns [boolean, boolean, boolean, boolean] { xml x = xml ``; return [x.attr is string, x?.attr is string, x.attrNon is error, x?.attrNon is ()]; } - -function getXMLMap() returns map { - map xmap = {}; - xmap["a"] = xml ``; - return xmap; -}