Skip to content

Commit

Permalink
Merge pull request #22928 from mohanvive/new-table-impl-2
Browse files Browse the repository at this point in the history 
Improve Table isAssignable check
  • Loading branch information
@mohanvive
mohanvive authored Apr 28, 2020
2 parents 485595c + ed235ba commit 3d6b5a7
Show file tree
Hide file tree
Showing 16 changed files with 136 additions and 90 deletions.
45 changes: 12 additions & 33 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 @@ -746,15 +746,11 @@ public void visit(BLangTableConstructorExpr tableConstructorExpr) {
}

BType actualType = checkExpr(tableConstructorExpr, env, symTable.noType);
if (types.isAssignable(actualType, expType)) {
BTableType actualTableType = (BTableType) actualType;
BTableType expectedTableType = (BTableType) expType;
if (expectedTableType.fieldNameList != null && actualTableType.fieldNameList == null) {
actualTableType.fieldNameList = expectedTableType.fieldNameList;
resultType = actualType;
}
} else {
resultType = symTable.semanticError;
BTableType actualTableType = (BTableType) actualType;
BTableType expectedTableType = (BTableType) expType;
if (expectedTableType.fieldNameList != null && actualTableType.fieldNameList == null) {
actualTableType.fieldNameList = expectedTableType.fieldNameList;
resultType = actualType;
}
} else {
resultType = symTable.semanticError;
Expand Down Expand Up @@ -824,7 +820,7 @@ private BLangRecordKeyValueField getRecordKeyValueField(BLangRecordLiteral recor
private boolean validateKeySpecifier(List<String> fieldNameList, BType constraint,
DiagnosticPos pos) {
for (String fieldName : fieldNameList) {
BField field = getTableConstraintField(constraint, fieldName);
BField field = types.getTableConstraintField(constraint, fieldName);
if (field == null) {
dlog.error(pos,
DiagnosticCode.INVALID_FIELD_NAMES_IN_KEY_SPECIFIER, fieldName, constraint);
Expand Down Expand Up @@ -902,14 +898,8 @@ private boolean validateTableConstructorExpr(BLangTableConstructorExpr tableCons

int index = 0;
for (BLangIdentifier identifier : fieldNameIdentifierList) {
BField field = getTableConstraintField(constraintType, identifier.value);
if (field == null) {
//NOT POSSIBLE
return false;
}

BType fieldType = field.type;
if (memberTypes.get(index).tag != fieldType.tag) {
BField field = types.getTableConstraintField(constraintType, identifier.value);
if (!types.isAssignable(field.type, memberTypes.get(index))) {
dlog.error(tableConstructorExpr.tableKeySpecifier.pos,
DiagnosticCode.KEY_SPECIFIER_MISMATCH_WITH_KEY_CONSTRAINT,
fieldNameIdentifierList.toString(), memberTypes.toString());
Expand All @@ -923,17 +913,6 @@ private boolean validateTableConstructorExpr(BLangTableConstructorExpr tableCons
return true;
}

private BField getTableConstraintField(BType constraintType, String fieldName) {
List<BField> fieldList = ((BRecordType) constraintType).getFields();

for (BField field : fieldList) {
if (field.name.toString().equals(fieldName)) {
return field;
}
}

return null;
}

private List<String> getTableKeyNameList(BLangTableKeySpecifier tableKeySpecifier) {
List<String> fieldNamesList = new ArrayList<>();
Expand All @@ -951,7 +930,8 @@ private BType createTableKeyConstraint(List<String> fieldNames, BType constraint

List<BType> memTypes = new ArrayList<>();
for (String fieldName : fieldNames) {
BType fieldType = getTableConstraintField(constraintType, fieldName).type; //null is not possible for field
//null is not possible for field
BType fieldType = types.getTableConstraintField(constraintType, fieldName).type;
memTypes.add(fieldType);
}

Expand Down Expand Up @@ -2003,8 +1983,7 @@ public void visit(BLangIndexBasedAccess indexBasedAccessExpr) {
indexBasedAccessExpr.compoundAssignmentLhsVar;
checkExpr(indexBasedAccessExpr.expr, this.env, symTable.noType);

if (indexBasedAccessExpr.multiKeyExpr != null && !types.isAssignable(indexBasedAccessExpr.expr.type,
symTable.tableType)) {
if (indexBasedAccessExpr.multiKeyExpr != null && indexBasedAccessExpr.expr.type.tag != TypeTags.TABLE) {
dlog.error(indexBasedAccessExpr.pos, DiagnosticCode.MULTI_KEY_MEMBER_ACCESS_NOT_SUPPORTED,
indexBasedAccessExpr.expr.type);
resultType = symTable.semanticError;
Expand Down Expand Up @@ -5283,7 +5262,7 @@ private BType checkIndexAccessExpr(BLangIndexBasedAccess indexBasedAccessExpr) {
// hence, this needs to be set to xml type
actualType = varRefType;
indexBasedAccessExpr.originalType = actualType;
} else if (types.isAssignable(varRefType, symTable.tableType)) {
} else if (varRefType.tag == TypeTags.TABLE) {
BTableType tableType = (BTableType) indexBasedAccessExpr.expr.type;
BType keyTypeConstraint = tableType.keyTypeConstraint;
if (tableType.keyTypeConstraint == null) {
Expand Down
16 changes: 14 additions & 2 deletions ...g/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeParamAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,20 @@ private void findTypeParamInStream(DiagnosticPos pos, BStreamType expType, BStre
private void findTypeParamInTable(DiagnosticPos pos, BTableType expType, BTableType actualType, SymbolEnv env,
HashSet<BType> resolvedTypes, FindTypeParamResult result) {
findTypeParam(pos, expType.constraint, actualType.constraint, env, resolvedTypes, result);
if (expType.keyTypeConstraint != null && actualType.keyTypeConstraint != null) {
findTypeParam(pos, expType.keyTypeConstraint, actualType.keyTypeConstraint, env, resolvedTypes, result);
if (expType.keyTypeConstraint != null) {
if (actualType.keyTypeConstraint != null) {
findTypeParam(pos, expType.keyTypeConstraint, actualType.keyTypeConstraint, env, resolvedTypes, result);
} else if (actualType.fieldNameList != null) {
List<BType> memberTypes = new ArrayList<>();
actualType.fieldNameList.forEach(field -> memberTypes
.add(types.getTableConstraintField(actualType.constraint, field).type));
if (memberTypes.size() == 1) {
findTypeParam(pos, expType.keyTypeConstraint, memberTypes.get(0), env, resolvedTypes, result);
} else {
BTupleType tupleType = new BTupleType(memberTypes);
findTypeParam(pos, expType.keyTypeConstraint, tupleType, env, resolvedTypes, result);
}
}
}
}

Expand Down
53 changes: 48 additions & 5 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 @@ -549,17 +549,13 @@ private boolean isAssignable(BType source, BType target, Set<TypePair> unresolve

//TODO Need to check the key specifier
if (targetTag == TypeTags.TABLE && sourceTag == TypeTags.TABLE) {
return isAssignable(((BTableType) source).constraint, ((BTableType) target).constraint, unresolvedTypes);
return isAssignableTableType((BTableType) source, (BTableType) target);
}

if (targetTag == TypeTags.STREAM && sourceTag == TypeTags.STREAM) {
return isAssignable(((BStreamType) source).constraint, ((BStreamType) target).constraint, unresolvedTypes);
}

if (targetTag == TypeTags.TABLE && sourceTag == TypeTags.TABLE) {
return isAssignable(((BTableType) source).constraint, ((BTableType) target).constraint, unresolvedTypes);
}

if (isBuiltInTypeWidenPossible(source, target) == TypeTestResult.TRUE) {
return true;
}
Expand Down Expand Up @@ -663,6 +659,53 @@ private boolean recordFieldsAssignableToType(BRecordType recordType, BType targe
return true;
}

private boolean isAssignableTableType(BTableType sourceTableType, BTableType targetTableType) {
if (!isAssignable(sourceTableType.constraint, targetTableType.constraint)) {
return false;
}

if (targetTableType.keyTypeConstraint == null && targetTableType.fieldNameList == null) {
return true;
}

if (targetTableType.keyTypeConstraint != null) {
if (sourceTableType.keyTypeConstraint != null &&
(isAssignable(sourceTableType.keyTypeConstraint, targetTableType.keyTypeConstraint))) {
return true;
}

if (sourceTableType.fieldNameList == null) {
return false;
}

List<BType> fieldTypes = new ArrayList<>();
sourceTableType.fieldNameList.forEach(field -> fieldTypes
.add(getTableConstraintField(sourceTableType.constraint, field).type));

if (fieldTypes.size() == 1) {
return isAssignable(fieldTypes.get(0), targetTableType.keyTypeConstraint);
}

BTupleType tupleType = new BTupleType(fieldTypes);
return isAssignable(tupleType, targetTableType.keyTypeConstraint);
}

return targetTableType.fieldNameList.equals(sourceTableType.fieldNameList);
}


BField getTableConstraintField(BType constraintType, String fieldName) {
List<BField> fieldList = ((BRecordType) constraintType).getFields();

for (BField field : fieldList) {
if (field.name.toString().equals(fieldName)) {
return field;
}
}

return null;
}

private boolean isAssignableMapType(BMapType sourceMapType, BRecordType targetRecType) {
if (targetRecType.sealed) {
return false;
Expand Down
17 changes: 14 additions & 3 deletions ...-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/model/types/BTableType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,21 @@ public <T, R> R accept(BTypeVisitor<T, R> visitor, T t) {
public String toString() {
if (constraint == null) {
return super.toString();
} else {
return super.toString() + "<" + constraint + ">"
+ ((this.keyTypeConstraint == null) ? "" : ", key<" + this.keyTypeConstraint + ">");
}

StringBuilder keyStringBuilder = new StringBuilder();
if (fieldNameList != null) {
for (String fieldName : fieldNameList) {
if (!keyStringBuilder.toString().equals("")) {
keyStringBuilder.append(", ");
}
keyStringBuilder.append(fieldName);
}
return super.toString() + "<" + constraint + "> key(" + keyStringBuilder.toString() + ")";
}

return (super.toString() + "<" + constraint + "> " +
((keyTypeConstraint != null) ? ("key<" + keyTypeConstraint + ">") : "")).trim();
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions ...ina-lang/src/main/java/org/wso2/ballerinalang/compiler/tree/types/BLangTableTypeNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ public NodeKind getKind() {

@Override
public String toString() {
return "table<" + this.constraint.toString() + "> " +
return ("table<" + this.constraint.toString() + "> " +
((this.tableKeySpecifier != null) ? this.tableKeySpecifier.toString() :
((this.tableKeyTypeConstraint != null) ? this.tableKeyTypeConstraint.toString() : ""));
((this.tableKeyTypeConstraint != null) ? this.tableKeyTypeConstraint.toString() : ""))).trim();
}
}
24 changes: 7 additions & 17 deletions langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,6 @@ public void testGetKeyList() {
assertEquals(returns[3].stringValue(), "Granier");
}

@Test
public void getKeysFromKeyLessTbl() {
BValue[] returns = BRunUtil.invoke(compileResult, "getKeysFromKeyLessTbl");
Assert.assertTrue(((BBoolean) returns[0]).booleanValue());
}

@Test
public void testRemoveAllFromTable() {
BValue[] returns = BRunUtil.invoke(compileResult, "removeAllFromTable");
Expand All @@ -142,14 +136,6 @@ public void testNextKey() {
assertEquals(((BInteger) returns[0]).intValue(), 101);
}

@Test(expectedExceptions = BLangRuntimeException.class,
expectedExceptionsMessageRegExp = "error: OperationNotSupported message=Defined key sequence "
+ "is not supported with nextKey\\(\\). The key sequence should only have an Integer field.*")
public void testNextKeyNegative() {
BRunUtil.invoke(compileResult, "testNextKeyNegative");
Assert.fail();
}

@Test(expectedExceptions = BLangRuntimeException.class,
expectedExceptionsMessageRegExp =
"error: \\{ballerina/lang.table\\}KeyNotFound message=cannot find key 'AAA'.*")
Expand All @@ -168,14 +154,18 @@ public void removeWithInvalidKey() {

@Test
public void testCompilerNegativeCases() {
validateError(negativeResult, 0, "incompatible types: expected 'table<Employee>', " +
"found 'table<Person>, key<other>'", 66, 36);
validateError(negativeResult, 0, "incompatible types: expected 'table<Employee> " +
"key(name)', found 'table<Person> key<string>'", 66, 36);
validateError(negativeResult, 1, "incompatible types: expected 'Employee', " +
"found 'Person'", 66, 47);
validateError(negativeResult, 2, "incompatible types: expected " +
"'object { public function next () returns (record {| Employee value; |}?); }', found " +
"'object { public function next () returns (record {| Person value; |}?); }'",
75, 92);
assertEquals(negativeResult.getErrorCount(), 3);
validateError(negativeResult, 3, "incompatible types: expected 'table<(any|error)> " +
"key<int>', found 'table<Person> key(name)'", 82, 12);
validateError(negativeResult, 4, "incompatible types: expected 'table<(any|error)> " +
"key<anydata>', found 'table<Person>'", 94, 12);
assertEquals(negativeResult.getErrorCount(), 5);
}
}
25 changes: 5 additions & 20 deletions langlib/langlib-test/src/test/resources/test-src/tablelib_test.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ type PersonalTable table<Person> key(name);

type EmployeeTable table<Employee> key(name);

type PersonalKeyLessTable table<Person>;

type CustomerTable table<Customer> key(id);

PersonalTable tab = table key(name)[
Expand Down Expand Up @@ -113,17 +111,18 @@ function testMap() returns boolean {
boolean testPassed = true;
Person[] personList = getPersonList();

EmployeeTable empTab = tab.'map(function (Person person) returns Employee {
table<Employee> empTab = tab.'map(function (Person person) returns Employee {
return {name: person.name, department : "HR"};
});

var personTblKeys = tab.keys();
var empTblKeys = empTab.keys();
var castedEmpTab = <table<Employee> key(name)> empTab;
var empTblKeys = castedEmpTab.keys();
testPassed = testPassed && personTblKeys.length() == empTblKeys.length();
//check keys of both tables are equal. Cannot check it until KeyType[] is returned

int index = 0;
empTab.forEach(function (Employee emp) {
castedEmpTab.forEach(function (Employee emp) {
testPassed = testPassed && emp.name == personList[index].name;
testPassed = testPassed && emp.department == "HR";
index+=1;
Expand All @@ -145,7 +144,7 @@ function testForeach() returns string {
}

function testFilter() returns boolean {
PersonalTable filteredTable = tab.filter(function (Person person) returns boolean {
table<Person> key<string> filteredTable = tab.filter(function (Person person) returns boolean {
return person.age < 35;
});
return filteredTable.length() == 2;
Expand Down Expand Up @@ -223,17 +222,3 @@ function testNextKey() returns int {
];
return custTbl.nextKey();
}

function testNextKeyNegative() returns int {
return tab.nextKey();
}

function getKeysFromKeyLessTbl() returns boolean {
PersonalKeyLessTable keyless = table [
{ name: "Chiran", age: 33 },
{ name: "Mohan", age: 37 },
{ name: "Gima", age: 38 },
{ name: "Granier", age: 34 }
];
return keyless.keys().length() == 0;
}
16 changes: 16 additions & 0 deletions langlib/langlib-test/src/test/resources/test-src/tablelib_test_negative.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,19 @@ function testIteratorNegative() returns boolean {
testPassed = testPassed && emp?.name == personList[0].name;
return testPassed;
}

function testNextKeyNegative() returns int {
return tab.nextKey();
}

type PersonalKeyLessTable table<Person>;

function getKeysFromKeyLessTbl() returns boolean {
PersonalKeyLessTable keyless = table [
{ name: "Chiran", age: 33 },
{ name: "Mohan", age: 37 },
{ name: "Gima", age: 38 },
{ name: "Granier", age: 34 }
];
return keyless.keys().length() == 0;
}
4 changes: 3 additions & 1 deletion ...it-test/src/test/java/org/ballerinalang/test/functions/FunctionSignatureNegativeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void testFuncSignatureSemanticsNegative() {
BAssertUtil.validateError(result, i++, "redeclared symbol 'c'", 1, 73);
BAssertUtil.validateError(result, i++, "redeclared argument 'a'", 17, 19);
BAssertUtil.validateError(result, i++, "undefined defaultable parameter 'c'", 21, 19);
BAssertUtil.validateError(result, i++, "invalid rest arguments", 29, 23);
BAssertUtil.validateError(result, i++, "incompatible types: expected 'int', found 'float'", 29, 20);
BAssertUtil.validateError(result, i++, "incompatible types: expected 'json', found 'xml:Text'", 40, 61);
BAssertUtil.validateError(result, i++, "missing required parameter 'a' in call to " +
"'functionWithOnlyPositionalParams'()", 57, 9);
Expand Down Expand Up @@ -67,6 +67,8 @@ public void testFuncSignatureSemanticsNegative() {
BAssertUtil.validateError(result, i++, "required parameter not allowed after defaultable parameters", 75, 60);
BAssertUtil.validateError(result, i++, "incompatible types: expected 'boolean', found 'boolean[]'", 85, 33);
BAssertUtil.validateError(result, i++, "incompatible types: expected 'float', found 'boolean[]'", 87, 28);
BAssertUtil.validateError(result, i++, "incompatible types: expected '[float,boolean...]', found " +
"'boolean[]'", 88, 31);
BAssertUtil.validateError(result, i++, "incompatible types: expected 'boolean', found 'boolean[]'", 89, 45);
BAssertUtil.validateError(result, i++, "positional argument not allowed after named arguments", 89, 45);
BAssertUtil.validateError(result, i++, "rest argument not allowed after named arguments", 90, 45);
Expand Down
2 changes: 1 addition & 1 deletion ...ring-unit-test/src/test/java/org/ballerinalang/test/taintchecking/connectors/SQLTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void testSelectWithTaintedQueryNegative() {
}


@Test
@Test(enabled = false)
public void testSelectWithUntaintedQueryProducingTaintedReturn() {
CompileResult result = BCompileUtil
.compile("test-src/taintchecking/connectors/sql-select-untainted-query-tainted-return.bal");
Expand Down
Loading

0 comments on commit 3d6b5a7

Please sign in to comment.