Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several CCE issues in query expressions that has string and xml types as the expected type. #40041

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,14 @@ BLangStatementExpression desugar(BLangQueryExpr queryExpr, SymbolEnv env,
onConflictExpr = null;
} else {
BType refType = Types.getReferredType(queryExpr.getBType());
if (isXml(refType)) {
BType safeType = types.getSafeType(refType, true, true);
if (isXml(safeType)) {
if (types.isSubTypeOfReadOnly(refType, env)) {
isReadonly.value = true;
}
result = getStreamFunctionVariableRef(queryBlock, QUERY_TO_XML_FUNCTION,
Lists.of(streamRef, isReadonly), pos);
} else if (TypeTags.isStringTypeTag(refType.tag)) {
} else if (TypeTags.isStringTypeTag(safeType.tag)) {
result = getStreamFunctionVariableRef(queryBlock, QUERY_TO_STRING_FUNCTION, Lists.of(streamRef), pos);
} else {
BType arrayType = refType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,13 +323,20 @@ void solveSelectTypeAndResolveType(BLangQueryExpr queryExpr, BLangExpression sel
resolvedType = getResolvedType(selectType, type, isReadonly, env);
break;
case TypeTags.STRING:
selectType = checkExpr(selectExp, env, type, data);
resolvedType = symTable.stringType;
break;
case TypeTags.XML:
case TypeTags.XML_COMMENT:
case TypeTags.XML_ELEMENT:
case TypeTags.XML_PI:
case TypeTags.XML_TEXT:
selectType = checkExpr(selectExp, env, type, data);
resolvedType = selectType;
if (types.isAssignable(selectType, symTable.xmlType)) {
resolvedType = getResolvedType(new BXMLType(selectType, null), type, isReadonly, env);
} else {
resolvedType = selectType;
}
break;
case TypeTags.INTERSECTION:
type = ((BIntersectionType) type).effectiveType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@
*/
public class StringQueryExpressionTest {

private CompileResult result;
private CompileResult result, negativeResult;

@BeforeClass
public void setup() {
result = BCompileUtil.compile("test-src/query/string-query-expression.bal");
negativeResult = BCompileUtil.compile("test-src/query/string-query-expression-negative.bal");
}

@Test(description = "Test simple query expression with string result")
Expand All @@ -48,6 +49,11 @@ public void testSimpleQueryExprForStringResult() {
Assert.assertEquals(returnValues.toString(), "Alex Ranjan John ");
}

@Test(description = "Test simple query expression with string result")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add a negative test for ambiguous types also.

 string:Char[]|string strValue = check from Book _ in bookStream
       select chr;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add it to xml|xml[] case too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added tests for ambiguous situations in both string,xml.
But those tests will uncomment after fixing #40012

public void testSimpleQueryExprForStringResult2() {
BRunUtil.invoke(result, "testSimpleQueryExprForStringResult2");
}

@Test(description = "Test query expression with where giving string result")
public void testQueryExprWithWhereForStringResult() {
Object returnValues = BRunUtil.invoke(result, "testQueryExprWithWhereForStringResult");
Expand Down Expand Up @@ -172,6 +178,15 @@ public void testQueryExprWithLimitForStringResultV2() {
Assert.assertEquals(returnValues.toString(), "Ranjan John ");
}

// issue - #40012
// @Test(description = "Negative Query expr for String tests")
// public void testNegativeQueryExprForXML() {
// int index = 0;
// validateError(negativeResult, index++, "ambiguous type '[string:Char, string:Char]'", 46, 16);
// validateError(negativeResult, index++, "ambiguous type '[string:Char, string:Char, string:Char]'", 48, 16);
// Assert.assertEquals(negativeResult.getErrorCount(), index);
// }

@AfterClass
public void tearDown() {
result = null;
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,16 @@ public void testNegativeQueryExprForXML() {
validateError(negativeResult, index++,
"incompatible types: expected 'xml<(xml:ProcessingInstruction & readonly)> & readonly', " +
"found 'xml:ProcessingInstruction'", 51, 16);
Assert.assertEquals(negativeResult.getErrorCount(), index);
validateError(negativeResult, index++,
"incompatible types: expected '(xml:Element|error)', found '(xml<xml:Element>|error)'", 81, 27);
// issue - #40012
// validateError(negativeResult, index++,
// "ambiguous type '[xml:Element, xml:Element]'", 88, 16);
// validateError(negativeResult, index++,
// "incompatible types: expected 'xml:Text', found 'xml:Element'", 94, 16);
// validateError(negativeResult, index++,
// "ambiguous type '[xml:Element, xml:Element]'", 99, 16);
Assert.assertEquals(negativeResult.getErrorCount(), index + 3);
}

@Test(description = "Test simple query expression for XMLs - #1")
Expand Down Expand Up @@ -101,6 +110,11 @@ public void testSimpleQueryExprForXML3() {
"<price>30.00</price><price>29.99</price><price>49.99</price><price>39.95</price>");
}

@Test(description = "Test simple query expression for XMLs - #4")
public void testSimpleQueryExprForXML4() {
BRunUtil.invoke(result, "testSimpleQueryExprForXML4");
}

@Test(description = "Test simple query expression with limit clause for XMLs")
public void testQueryExprWithLimitForXML() {
Object returnValues = BRunUtil.invoke(result, "testQueryExprWithLimitForXML");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) 2023 WSO2 Inc. (http://www.wso2.org) All Rights Reserved.
//
// WSO2 Inc. licenses this file to you under the Apache License,
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
// 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.

type Book record {|
string title;
string author;
|};

class BookGenerator {
int i = 0;

public isolated function next() returns record {|Book value;|}|error? {
self.i += 1;
if (self.i < 0) {
return error("Error");
} else if (self.i >= 3) {
return ();
}
return {
value: {
title: "Title " + self.i.toString(), author: "Author " + self.i.toString()
}
};
}
}

function testSimpleQueryExprForStringNegative() returns error? {
string:Char chr = "a";
BookGenerator bookGenerator = new ();
stream<Book, error?> bookStream = new (bookGenerator);

string:Char[]|string _ = check from Book _ in bookStream
select chr;
string:Char[]|int|string _ = check from Book _ in bookStream
select chr;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,77 @@ function testSimpleQueryExprForStringResult() returns string {
return outputNameString;
}

type Book record {|
string title;
string author;
|};

class BookGenerator {
int i = 0;

public isolated function next() returns record {|Book value;|}|error? {
self.i += 1;
if (self.i < 0) {
return error("Error");
} else if (self.i >= 3) {
return ();
}
return {
value: {
title: "Title " + self.i.toString(), author: "Author " + self.i.toString()
}
};
}
}

function testSimpleQueryExprForStringResult2() {
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
error? e = trap simpleQueryExprForStringResult();
assertFalse(e is error);

e = trap simpleQueryExprForStringResult2();
assertFalse(e is error);

e = trap simpleQueryExprForStringResult3();
assertFalse(e is error);
}

function simpleQueryExprForStringResult() returns error? {
string:Char chr = "a";
BookGenerator bookGenerator = new ();
stream<Book, error?> bookStream = new (bookGenerator);
string strValue = check from Book _ in bookStream
select chr;
string expectedValue = "aa";
assertTrue(strValue is string);
assertEquality(strValue, expectedValue);
}

function simpleQueryExprForStringResult2() returns error? {
stream<Book, error?> bookStream = [{ author: "Author 1", title: "Title 1" },
{author: "Author 2", title: "Title 2" }].toStream();
string strValue = check from Book _ in bookStream
select <string:Char> "a";
string expectedValue = "aa";
assertTrue(strValue is string);
assertEquality(strValue, expectedValue);
}

function simpleQueryExprForStringResult3() returns error? {
string:Char chr = "a";
BookGenerator bookGenerator = new ();
stream<Book, error?> bookStream = new (bookGenerator);
string:Char[] strValue = check from Book _ in bookStream
select chr;
assertTrue(strValue is string:Char[]);
assertEquality(strValue[0], chr);
assertEquality(strValue[1], chr);
}

function testQueryExprWithWhereForStringResult() returns string {
Person p1 = {firstName: "Alex", lastName: "George", age: 23};
Person p2 = {firstName: "Ranjan", lastName: "Fonseka", age: 30};
Person p3 = {firstName: "John", lastName: "David", age: 33};

Person[] personList = [p1, p2, p3];

string outputNameString =
from var person in personList
where person.age >= 30
Expand Down Expand Up @@ -336,3 +400,25 @@ function testQueryExprWithLimitForStringResultV2() returns string {

return outputNameString;
}

function assertEquality(any|error actual, any|error expected) {
if expected is anydata && actual is anydata && expected == actual {
return;
}

if expected === actual {
return;
}
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved

string expectedValAsString = expected is error ? expected.toString() : expected.toString();
string actualValAsString = actual is error ? actual.toString() : actual.toString();
panic error("expected '" + expectedValAsString + "', found '" + actualValAsString + "'");
}

function assertTrue(any|error actual) {
assertEquality(actual, true);
}

function assertFalse(any|error actual) {
assertEquality(actual, false);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,54 @@ function testSimpleQueryExprForXMLWithReadonly1() {
xml<xml:ProcessingInstruction> & readonly _ = from xml:ProcessingInstruction x in processingInstructions
select x;
}

type Book record {|
string title;
string author;
|};

class BookGenerator {
int i = 0;

public isolated function next() returns record {|Book value;|}|error? {
self.i += 1;
if (self.i < 0) {
return error("Error");
} else if (self.i >= 3) {
return ();
}
return {
value: {
title: "Title " + self.i.toString(), author: "Author " + self.i.toString()
}
};
}
}

BookGenerator bookGenerator = new ();
stream<Book, error?> bookStream = new (bookGenerator);

function testSimpleQueryExprForXML() {
xml:Element _ = check from Book book in bookStream
select xml `<Book>
<Author>${book.author}</Author>
<Title>${book.title}</Title>
</Book>`;

xml:Element|xml _ = check from Book book in bookStream
select xml `<Book>
<Author>${book.author}</Author>
<Title>${book.title}</Title>
</Book>`;

xml:Text|xml _ = check from Book book in bookStream
select xml `<Book>
<Author>${book.author}</Author>
<Title>${book.title}</Title>
</Book>`;
xml:Element[]|xml _ = check from Book book in bookStream
select xml `<Book>
<Author>${book.author}</Author>
<Title>${book.title}</Title>
</Book>`;
}
Loading