Skip to content

Commit

Permalink
Merge pull request #39600 from ushirask/issue_39341
Browse files Browse the repository at this point in the history 
Throw error when field access is used with a map of `xml`
  • Loading branch information
@hasithaa
hasithaa authored Aug 15, 2023
2 parents 4925404 + d85ed69 commit f6430ee
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 58 deletions.
2 changes: 1 addition & 1 deletion compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
8 changes: 4 additions & 4 deletions ...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<BLangExpression> argExprs = new ArrayList<>();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 2 additions & 10 deletions ...allerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,9 @@ public BType checkType(Location pos,
return symTable.semanticError;
}

public boolean isLax(BType type) {
public boolean isLaxFieldAccessAllowed(BType type) {
Set<BType> 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
Expand All @@ -289,8 +285,6 @@ public int isLaxType(BType type, Set<BType> 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);
Expand Down Expand Up @@ -323,8 +317,6 @@ public boolean isLaxType(BType type, Map<BType, Boolean> visited) {
}
switch (type.tag) {
case TypeTags.JSON:
case TypeTags.XML:
case TypeTags.XML_ELEMENT:
visited.put(type, true);
return true;
case TypeTags.MAP:
Expand Down
20 changes: 20 additions & 0 deletions ...na-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<xml>' does not support field access"
, 382, 19);
validateError(negativeResult, i++, "invalid operation: type 'map<xml>' does not support field access"
, 387, 19);
validateError(negativeResult, i++, "invalid operation: type 'map<xml>' 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<xml>' does not support " +
"optional field access", 404, 13);
validateError(negativeResult, i++, "invalid operation: type 'map<xml>' does not support " +
"optional field access", 409, 14);
validateError(negativeResult, i++, "invalid operation: type 'map<xml>' does not support " +
"optional field access", 414, 13);

Assert.assertEquals(negativeResult.getErrorCount(), i);
}

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions ...test/src/test/java/org/ballerinalang/test/expressions/access/OptionalFieldAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public void testNegativeCases() {
"access", 58, 14);
validateError(negativeResult, i++, "invalid operation: type 'map<string>?' does not support optional field " +
"access", 61, 19);
validateError(negativeResult, i++, "invalid operation: type '(map<xml>|map<json>)' 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);
Expand Down
15 changes: 0 additions & 15 deletions ...rina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,14 @@ function validateJSONOperationErrorMsg(json|error val) {
assertEquals(<anydata> checkpanic err.detail()["message"], "JSON value is not a mapping");
}

function testValidXMLmapFieldAccess() {
map<xml> 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);
Expand Down
37 changes: 37 additions & 0 deletions ...lerina-unit-test/src/test/resources/test-src/expressions/access/field_access_negative.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,40 @@ function testInvalidBoundMethodAccessWithRemoteMethod(ServiceClass a,

_ = e.ser.fn;
}

function testInvalidXMLMapFieldAccess1() returns error? {
map<xml> m = {a: xml `foo`};
xml x = check m.a; // error
}

function testInvalidXMLMapFieldAccess2() returns error? {
map<xml> m = {a: xml `foo`};
xml x = check m.b; // error
}

function testInvalidXMLMapFieldAccess3() returns error? {
map<xml> m = {};
m["a"] = xml `foo`;
xml x = check m.a; // error
}

function testInvalidXMLMapFieldAccess4() returns error? {
map<xml|json> m = {};
m["a"] = xml `foo`;
xml|json x = check m.a; // error
}

function testInvalidXMLMapFieldAccess5() returns error? {
map<xml> m = {a: xml `foo`};
xml x = m?.a; // error
}

function testInvalidXMLMapFieldAccess6() returns error? {
record {|map<xml> a; xml c;|} m = {a: {b: xml `foo`}, c: xml `bar`};
xml? x = m["a"]?.b.c;
}

function testInvalidXMLMapFieldAccess7() returns error? {
map<xml> m = {a: xml `foo`};
xml x = m?.b; // error
}
33 changes: 5 additions & 28 deletions ...ina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-lax-behavior.bal
View file
Original file line number Diff line number Diff line change
@@ -1,36 +1,13 @@
import ballerina/lang.'xml;

function testXMLAsMapContent() returns [string|error?, string|error?, boolean] {
map<xml> 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<xml> xmap = getXMLMap();
string|error val = xmap.b.attr;
return val;
}

function testXMLAttributeWithNSPrefix() returns
[string|error?, string|error?, string|error?, boolean, boolean] {
map<xml> xmap = {};
xmap["a"] = xml `<elem xmlns="ns-uri" attr="val" xml:space="preserve"></elem>`;
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 `<elem xmlns="ns-uri" attr="val" xml:space="preserve"></elem>`;
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 `<elem xmlns="ns-uri" attr="val" xml:space="default"></elem>`;
return [x.attr is string, x?.attr is string, x.attrNon is error, x?.attrNon is ()];
}

function getXMLMap() returns map<xml> {
map<xml> xmap = {};
xmap["a"] = xml `<elem attr="val"></elem>`;
return xmap;
}

0 comments on commit f6430ee

Please sign in to comment.