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.

Sign up for GitHub

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

Improve the error message for invalid table key values #41992

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Improve the error message for invalid table key values #41992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3f55f4
Provide better error message for table keys
nipunayf Jan 16, 2024
9901b72
Add comment to ignored default case
nipunayf Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 57 additions & 10 deletions ...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,28 +1522,76 @@
switch(expression.getKind()) {
case LITERAL:
case NUMERIC_LITERAL:
case STRING_TEMPLATE_LITERAL:
case XML_ELEMENT_LITERAL:
case XML_TEXT_LITERAL:
case LIST_CONSTRUCTOR_EXPR:
case TABLE_CONSTRUCTOR_EXPR:
case RECORD_LITERAL_EXPR:
case TYPE_CONVERSION_EXPR:
case UNARY_EXPR:
case BINARY_EXPR:
case TYPE_TEST_EXPR:
case TERNARY_EXPR:
case REG_EXP_TEMPLATE_LITERAL:
return true;
case SIMPLE_VARIABLE_REF:
return (((BLangSimpleVarRef) expression).symbol.tag & SymTag.CONSTANT) == SymTag.CONSTANT;
BSymbol varSymbol = ((BLangSimpleVarRef) expression).symbol;
return varSymbol == null || (varSymbol.tag & SymTag.CONSTANT) == SymTag.CONSTANT;
case STRING_TEMPLATE_LITERAL:
return checkNestedConstExpr(((BLangStringTemplateLiteral) expression).exprs);
case TERNARY_EXPR:
BLangTernaryExpr ternaryExpr = (BLangTernaryExpr) expression;
return isConstExpression(ternaryExpr.expr) && isConstExpression(ternaryExpr.thenExpr) &&
isConstExpression(ternaryExpr.elseExpr);
case UNARY_EXPR:
return isConstExpression(((BLangUnaryExpr) expression).expr);
case BINARY_EXPR:
BLangBinaryExpr binaryExpr = (BLangBinaryExpr) expression;
return isConstExpression(binaryExpr.lhsExpr) && isConstExpression(binaryExpr.rhsExpr);
case GROUP_EXPR:
return isConstExpression(((BLangGroupExpr) expression).expression);
case TYPE_CONVERSION_EXPR:
return isConstExpression(((BLangTypeConversionExpr) expression).expr);
case TYPE_TEST_EXPR:
return isConstExpression(((BLangTypeTestExpr) expression).expr);
case LIST_CONSTRUCTOR_EXPR:
return checkNestedConstExpr(((BLangListConstructorExpr) expression).exprs);
case LIST_CONSTRUCTOR_SPREAD_OP:
return isConstExpression(((BLangListConstructorExpr.BLangListConstructorSpreadOpExpr) expression).expr);
case RECORD_LITERAL_EXPR:
BLangRecordLiteral recordLiteral = (BLangRecordLiteral) expression;
List<RecordLiteralNode.RecordField> fields = recordLiteral.getFields();
for (RecordLiteralNode.RecordField field : fields) {
switch (field.getKind()) {
case RECORD_LITERAL_KEY_VALUE -> {
if (!isConstExpression(((BLangRecordKeyValueField) field).valueExpr) ||
!isConstExpression(((BLangRecordKeyValueField) field).key.expr)) {
return false;
}
}
case RECORD_LITERAL_SPREAD_OP -> {
if (!isConstExpression(((BLangRecordLiteral.BLangRecordSpreadOperatorField) field).expr)) {
return false;

Check warning on line 1567 in compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java

View check run for this annotation

Codecov / codecov/patch

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java#L1567 

Added line #L1567 was not covered by tests
KavinduZoysa marked this conversation as resolved.
Show resolved Hide resolved
}
}
case SIMPLE_VARIABLE_REF -> {
if (!isConstExpression(((BLangRecordVarNameField) field))) {
return false;
}
}
default -> {
// Ignore the default case
}
}
}
return true;
default:
return false;
}
}

private boolean checkNestedConstExpr(List<? extends BLangExpression> expressions) {
for (BLangExpression expr : expressions) {
if (!isConstExpression(expr)) {
return false;
}
}
return true;
}

private BLangExpression getRecordKeyValueField(BLangRecordLiteral recordLiteral,
String fieldName) {
for (RecordLiteralNode.RecordField recordField : recordLiteral.fields) {
Expand All @@ -1559,7 +1607,6 @@
}
}
}

return null;
}

Expand Down
72 changes: 66 additions & 6 deletions ...llerina-unit-test/src/test/java/org/ballerinalang/test/types/table/TableNegativeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,66 @@ public void testTableNegativeCases() {
validateError(compileResult, index++, "incompatible types: expected " +
"'table<ballerina/lang.table:0.0.0:MapType> key<ballerina/lang.table:0.0.0:KeyType>', " +
"found 'table<record {| int id; |}>'", 566, 9);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
576, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
586, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
587, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
588, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
596, 23);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
605, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
606, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
607, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
608, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
616, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
625, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
626, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
627, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
628, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
629, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
630, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
631, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
632, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
633, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
634, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
642, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
643, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
644, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
645, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
646, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
647, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
648, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
649, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
650, 5);
validateError(compileResult, index++, "value expression of key specifier 'k' must be a constant expression",
651, 5);
Assert.assertEquals(compileResult.getErrorCount(), index);
}

Expand All @@ -215,17 +275,17 @@ public void testTableKeyViolations() {
"</BLangXMLQName: (p) empId> [][5005]]'",
54, 9);
validateError(compileResult, index++, "duplicate key found in table row key('firstName') : '<string> " +
"(name is string && ! invalid?(BLangStringTemplateLiteral: [Hello , name, !!!]):James)'",
64, 9);
"(val > 10 && ! (val < 10)?(BLangStringTemplateLiteral: [Hello , name, !!!]):James)'",
67, 9);
validateError(compileResult, index++, "duplicate key found in table row key('id') : '[5005, 5006]'",
76, 5);
79, 5);
validateError(compileResult, index++, "duplicate key found in table row key('id') : ' '",
102, 9);
105, 9);
validateError(compileResult, index++, "duplicate key found in table row key('id') : " +
"'<(byte[] & readonly)> (base16 `5A`)'",
128, 9);
131, 9);
validateError(compileResult, index, "duplicate key found in table row key('id') : 'ID2'",
136, 9);
139, 9);
}

@Test
Expand Down
85 changes: 85 additions & 0 deletions tests/jballerina-unit-test/src/test/resources/test-src/types/table/table-negative.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,3 +565,88 @@ function testKeyConstraint() {
var ids = from var {id} in keylessTable select {id};
_ = ids.remove(0); // error
}

string strValue = "abc";
type Row6 record {
readonly string k;
int value;
};

table<Row6> key(k) tbl5 = table [
{k: string `A${idValue}C${strValue}`, value: 17}
];

type Row7 record {
readonly int[] k;
int value;
};
int[] intArr = [12, 12];

table<Row7> key(k) tbl7 = table [
{k: [12, 12, intVal], value: 5},
{k: [intVal, intVal], value: 17},
{k: [12, ...intArr], value: 17}
];

type Row8 record {
readonly table<Row6> key(k) k;
int value;
};
table<Row8> key(k) tbl8 = table [
{k: table key(k) [{k: strValue, value: 17}], value: 17}
];

type Row9 record {
readonly map<anydata> k;
int value;
};
Row9 mapVal = {k: {idValue, "C": [13.5, 24.3]}, value: 17};
table<Row9> key(k) tbl9 = table [
{k: {"A": strValue, "B": idValue, "C": [13.5, 24.3]}, value: 17},
{k: {strValue, "B": "idValue", "C": [13.5, 24.3]}, value: 17},
{k: {[strValue]: "a", "B": "idValue", "C": [13.5, 24.3]}, value: 17},
{...mapVal}
];

type Row10 record {
readonly float k;
int value;
};
table<Row10> key(k) tbl10 = table [
{k: <float>idValue, value: 17}
];

int? intOrNull = 12;
type Row11 record {
readonly int k;
int value;
};
table<Row11> key(k) tbl11 = table [
{k: +idValue, value: 17},
{k: idValue * 12, value: 19},
{k: idValue + 12, value: 11},
{k: idValue >> 13, value: 11},
{k: idValue | 13, value: 11},
{k: idValue & 13, value: 11},
{k: idValue is int ? 12 : 1, value: 11},
{k: idValue, value: 11},
{k: intOrNull ?: 1, value: 11},
{k: (idValue is int ? (<int> 10.5) : 20), value: 11}
];

type Row12 record {
readonly boolean k;
int value;
};
table<Row12> key(k) tbl12 = table [
{k: 5 < idValue, value: 17},
{k: idValue is int, value: 19},
{k: idValue !is int, value: 11},
{k: idValue == 15, value: 11},
{k: idValue != 15, value: 11},
{k: idValue === 15, value: 11},
{k: idValue !== 15, value: 11},
{k: idValue == 15 || false, value: 11},
{k: idValue == 15 && true, value: 11},
{k: !(idValue == 15 && true), value: 11}
];
Loading
Loading