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

Add support for record literals with mapping binding patterns and fix using mapping-binding pattern within a list binding pattern against an open record #40283

Merged
merged 21 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ public enum DiagnosticErrorCode implements DiagnosticCode {
INCOMPATIBLE_TYPES_CONVERSION_WITH_SUGGESTION("BCE2503", "incompatible.types.conversion.with.suggestion"),
UNSAFE_CAST_ATTEMPT("BCE2504", "unsafe.cast.attempt"),

INVALID_LITERAL_FOR_TYPE("BCE2506", "invalid.literal.for.type"),
INCOMPATIBLE_MAPPING_CONSTRUCTOR("BCE2507", "incompatible.mapping.constructor.expression"),
MAPPING_CONSTRUCTOR_COMPATIBLE_TYPE_NOT_FOUND("BCE2508", "mapping.constructor.compatible.type.not.found"),
CANNOT_INFER_TYPES_FOR_TUPLE_BINDING("BCE2509", "cannot.infer.types.for.tuple.binding"),
Expand Down Expand Up @@ -333,7 +332,6 @@ public enum DiagnosticErrorCode implements DiagnosticCode {
INVALID_ANY_VAR_DEF("BCE2574", "invalid.any.var.def"),
INVALID_RECORD_LITERAL("BCE2575", "invalid.record.literal"),
INVALID_FIELD_IN_RECORD_BINDING_PATTERN("BCE2576", "invalid.field.in.record.binding.pattern"),
INVALID_RECORD_LITERAL_BINDING_PATTERN("BCE2577", "invalid.record.literal.in.binding.pattern"),
DUPLICATE_KEY_IN_MAPPING_CONSTRUCTOR("BCE2578", "duplicate.key.in.mapping.constructor"),
DUPLICATE_KEY_IN_TABLE_LITERAL("BCE2579", "duplicate.key.in.table.literal"),
DUPLICATE_KEY_IN_RECORD_LITERAL_SPREAD_OP("BCE2580", "duplicate.key.in.record.literal.spread.op"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2199,11 +2199,6 @@ public void visit(BLangSimpleVariableDef varDefNode, AnalyzerData data) {

@Override
public void visit(BLangRecordVariableDef varDefNode, AnalyzerData data) {
// TODO: 10/18/18 Need to support record literals as well
if (varDefNode.var.expr != null && varDefNode.var.expr.getKind() == RECORD_LITERAL_EXPR) {
dlog.error(varDefNode.pos, DiagnosticErrorCode.INVALID_LITERAL_FOR_TYPE, "record binding pattern");
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this DiagnosticErrorCode removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed via 08f5e57c

Copy link
Member

Choose a reason for hiding this comment

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

Where have the tests been added for this?

Copy link
Member

Choose a reason for hiding this comment

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

Noticed that they've been added in RecordVariableReferenceTest.java. That's not the correct class. Should go in RecordVariableDefinitionTest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack

return;
}
analyzeNode(varDefNode.var, data);
}

Expand Down Expand Up @@ -2386,15 +2381,9 @@ public void visit(BLangRecordDestructure recordDeStmt, AnalyzerData data) {
setTypeOfVarRef(recordDeStmt.varRef, data);

SymbolEnv currentEnv = data.env;
typeChecker.checkExpr(recordDeStmt.varRef, currentEnv, symTable.noType, data.prevEnvs,
data.typeChecker.checkExpr(recordDeStmt.varRef, currentEnv, symTable.noType, data.prevEnvs,
data.commonAnalyzerData);

if (recordDeStmt.expr.getKind() == RECORD_LITERAL_EXPR) {
// TODO: 10/18/18 Need to support record literals as well
dlog.error(recordDeStmt.expr.pos, DiagnosticErrorCode.INVALID_RECORD_LITERAL_BINDING_PATTERN);
dulajdilshan marked this conversation as resolved.
Show resolved Hide resolved
return;
}
typeChecker.checkExpr(recordDeStmt.expr, currentEnv, symTable.noType, data.prevEnvs,
data.typeChecker.checkExpr(recordDeStmt.expr, currentEnv, symTable.noType, data.prevEnvs,
data.commonAnalyzerData);
checkRecordVarRefEquivalency(recordDeStmt.pos, recordDeStmt.varRef, recordDeStmt.expr.getBType(),
recordDeStmt.expr.pos, data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3292,8 +3292,7 @@ public void visit(BLangRecordVarRef varRefExpr, AnalyzerData data) {
SOURCE);

if (restParam == null) {
bRecordType.sealed = true;
bRecordType.restFieldType = symTable.noType;
bRecordType.restFieldType = symTable.anyOrErrorType;
} else if (restParam.getBType() == symTable.semanticError) {
bRecordType.restFieldType = symTable.mapType;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,6 @@ error.array.literal.not.allowed=\
error.string.template.literal.not.allowed=\
string template literals not allowed here

error.invalid.literal.for.type=\
invalid literal for type ''{0}''

error.invalid.literal.for.match.pattern=\
invalid literal for match pattern; allowed literals are simple, tuple and record only

Expand Down Expand Up @@ -1277,9 +1274,6 @@ error.invalid.record.binding.pattern=\
error.invalid.field.in.record.binding.pattern=\
invalid record binding pattern; unknown field ''{0}'' in record type ''{1}''

error.invalid.record.literal.in.binding.pattern=\
record literal is not supported for record binding pattern

error.no.matching.record.ref.found=\
no matching record reference found for field ''{0}''

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

/**
Expand Down Expand Up @@ -136,43 +137,24 @@ public void testRestParameterType() {
Assert.assertTrue((Boolean) returns);
}

// TODO: Uncomment below tests once record literal is supported with var ref
//
// @Test(description = "Test simple record variable definition")
// public void testVarAssignmentOfRecordLiteral() {
// Object returns = JvmRunUtil.invoke(result, "testVarAssignmentOfRecordLiteral");
// Assert.assertEquals(returns.toString(), "Peter");
// Assert.assertTrue((Boolean) returns[1]]);
// Assert.assertEquals(returns[2], 12);
// Assert.assertEquals(returns[3].toString(), "Y");
// }
//
// @Test(description = "Test simple record variable definition")
// public void testVarAssignmentOfRecordLiteral2() {
// Object returns = JvmRunUtil.invoke(result, "testVarAssignmentOfRecordLiteral2");
// Assert.assertEquals(returns.toString(), "Peter");
// Assert.assertTrue((Boolean) returns[1]]);
// Assert.assertEquals(( ((BMap) returns[2]).get(StringUtils.fromString("age"))), 12);
// Assert.assertEquals(((BMap) returns[2]).get(StringUtils.fromString("format")).toString(), "Y");
// }
//
// @Test(description = "Test simple record variable definition")
// public void testVarAssignmentOfRecordLiteral3() {
// Object returns = JvmRunUtil.invoke(result, "testVarAssignmentOfRecordLiteral3");
// Assert.assertEquals(returns.toString(), "Peter");
// Assert.assertTrue((Boolean) returns[1]]);
// Assert.assertEquals(( ((BMap) returns[2]).get(StringUtils.fromString("age"))), 12);
// Assert.assertEquals(((BMap) returns[2]).get(StringUtils.fromString("format")).toString(), "Y");
// }
//
// @Test(description = "Test simple record variable definition")
// public void testVarAssignmentOfRecordLiteral4() {
// Object returns = JvmRunUtil.invoke(result, "testVarAssignmentOfRecordLiteral4");
// Assert.assertEquals(returns.toString(), "Peter");
// Assert.assertTrue((Boolean) returns[1]]);
// Assert.assertEquals(( ((BMap) returns[2]).get(StringUtils.fromString("age"))), 12);
// Assert.assertEquals(((BMap) returns[2]).get(StringUtils.fromString("format")).toString(), "Y");
// }
@Test(description = "Test variable assignment with record literal", dataProvider = "VarAssignmentOfRecordLiteral")
public void testVarAssignmentOfRecordLiteral(String funcName) {
BRunUtil.invoke(result, funcName);
}

@DataProvider(name = "VarAssignmentOfRecordLiteral")
public Object[] varAssignmentOfRecordLiteralProvider() {
return new Object[]{
"testVarAssignmentOfRecordLiteral1",
"testVarAssignmentOfRecordLiteral2",
"testVarAssignmentOfRecordLiteral3",
"testVarAssignmentOfRecordLiteral4",
"testVarAssignmentOfRecordLiteral5",
"testVarAssignmentOfRecordLiteral6",
"testVarAssignmentOfRecordLiteral7",
"testVarAssignmentOfRecordLiteral8",
};
}

@Test
public void testRecordFieldBindingPatternsWithIdentifierEscapes() {
Expand All @@ -189,6 +171,11 @@ public void testMappingBindingWithSingleNameFieldBinding() {
BRunUtil.invoke(result, "testMappingBindingWithSingleNameFieldBinding");
}

@Test
public void testMappingBindingPatternAgainstOpenRecordInTupleDestructuring() {
BRunUtil.invoke(result, "testMappingBindingPatternAgainstOpenRecordInTupleDestructuring");
}

@Test
public void testRecordVariablesSemanticsNegative() {
resultSemanticsNegative = BCompileUtil.compile(
Expand All @@ -210,7 +197,10 @@ public void testRecordVariablesSemanticsNegative() {
BAssertUtil.validateError(resultSemanticsNegative, ++i,
"incompatible types: expected 'record type', found 'int'", 96, 38);
BAssertUtil.validateError(resultSemanticsNegative, ++i,
"record literal is not supported for record binding pattern", 97, 38);
"incompatible types: expected 'Bar', found 'string'", 97, 12);
BAssertUtil.validateError(resultSemanticsNegative, ++i,
"incompatible types: expected 'string', found 'record {| int var1; [string,int,boolean] var2; |}'",
97, 27);
BAssertUtil.validateError(resultSemanticsNegative, ++i,
"incompatible types: expected 'Person', found 'Age'", 106, 19);
BAssertUtil.validateError(resultSemanticsNegative, ++i,
Expand Down Expand Up @@ -281,6 +271,34 @@ public void testNegativeRecordVariables() {
Assert.assertEquals(resultNegative.getDiagnostics().length, i);
}

@Test
public void testDestructuringWithRecordReferenceNegative() {
CompileResult resultNegative = BCompileUtil.compile(
"test-src/expressions/varref/record_bp_in_list_and_record_destructuring_assignment_negative.bal");
int i = 0;
BAssertUtil.validateError(resultNegative, i++, "incompatible types: " +
"expected '[record {| int a; int b; (any|error)...; |}]', " +
"found '[record {| int a; int b?; |}]'", 22, 15);
BAssertUtil.validateError(resultNegative, i++, "incompatible types: " +
"expected '[int,[record {| int b; (any|error)...; |}]]', " +
"found '[int,[record {| int b?; anydata...; |}]]'", 24, 18);
BAssertUtil.validateError(resultNegative, i++, "incompatible types: " +
"expected '[record {| int a; int b; int c; (any|error)...; |}]', " +
"found '[record {| int a; int b; anydata...; |}]'", 32, 18);
BAssertUtil.validateError(resultNegative, i++, "incompatible types: " +
"expected '[int,[record {| int b; int c; (any|error)...; |}]]', " +
"found '[int,[record {| int b; anydata...; |}]]'", 34, 21);
BAssertUtil.validateError(resultNegative, i++, "invalid field binding pattern; can only bind required fields",
41, 9);
BAssertUtil.validateError(resultNegative, i++, "invalid field binding pattern; can only bind required fields",
43, 14);
BAssertUtil.validateError(resultNegative, i++, "invalid record binding pattern; " +
"unknown field 'b' in record type 'record {| int a; anydata...; |}'", 50, 5);
BAssertUtil.validateError(resultNegative, i++, "invalid record binding pattern; " +
"unknown field 'b' in record type 'record {| |} & readonly'", 52, 13);
Assert.assertEquals(resultNegative.getDiagnostics().length, i);
}

@AfterClass
public void tearDown() {
result = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,66 +248,64 @@ public void testNegativeCases() {
65, 33);
BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected '[int,int]', found '[int...]'",
70, 36);
BAssertUtil.validateError(negativeResult, i++, "record literal is not supported for record binding pattern",
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't this test removed from the bal file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's removed. tests/jballerina-unit-test/src/test/resources/test-src/query/group_by_clause_negative.bal L74:82

79, 32);
BAssertUtil.validateError(negativeResult, i++, "invalid operation: type " +
"'seq record {| string name; int price1; |}' does not support field access", 87, 29);
"'seq record {| string name; int price1; |}' does not support field access", 78, 29);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 87, 29);
"list constructor or function invocation", 78, 29);
BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected 'int[]', found 'seq int?'",
108, 32);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 116, 40);
99, 32);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 107, 40);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element" +
" list constructor or function invocation", 116, 40);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 120, 33);
" list constructor or function invocation", 107, 40);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'int'", 111, 33);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element" +
" list constructor or function invocation", 120, 33);
" list constructor or function invocation", 111, 33);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
133, 36);
124, 36);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 133, 36);
"list constructor or function invocation", 124, 36);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 133, 45);
"list constructor or function invocation", 124, 45);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
136, 28);
127, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 136, 28);
"list constructor or function invocation", 127, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 136, 37);
"list constructor or function invocation", 127, 37);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
139, 28);
130, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 139, 28);
"list constructor or function invocation", 130, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 139, 37);
"list constructor or function invocation", 130, 37);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
142, 28);
133, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 142, 28);
"list constructor or function invocation", 133, 28);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 142, 37);
"list constructor or function invocation", 133, 37);
BAssertUtil.validateError(negativeResult, i++, "operator '+' not defined for 'seq int' and 'seq int'",
145, 26);
136, 26);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 145, 26);
"list constructor or function invocation", 136, 26);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 145, 35);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 148, 36);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after rest argument", 151, 39);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 154, 37);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 154, 40);
"list constructor or function invocation", 136, 35);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 139, 36);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after rest argument", 142, 39);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 145, 37);
BAssertUtil.validateError(negativeResult, i++, "arguments not allowed after seq argument", 145, 40);
BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected '(any|error)[]', found 'int'",
157, 37);
148, 37);
BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected " +
"'([int,int...]|record {| int n; |})', found '[int...]'", 167, 43);
"'([int,int...]|record {| int n; |})', found '[int...]'", 158, 43);
BAssertUtil.validateError(negativeResult, i++, "invalid grouping key type 'error', expected a subtype of " +
"'anydata'", 175, 26);
"'anydata'", 166, 26);
BAssertUtil.validateError(negativeResult, i++, "invalid grouping key type 'error', expected a subtype of " +
"'anydata'", 178, 26);
"'anydata'", 169, 26);
BAssertUtil.validateError(negativeResult, i++, "incompatible types: expected 'string', found 'seq string'",
200, 24);
191, 24);
BAssertUtil.validateError(negativeResult, i++, "sequence variable can be used in a single element " +
"list constructor or function invocation", 200, 24);
"list constructor or function invocation", 191, 24);
Assert.assertEquals(negativeResult.getErrorCount(), i);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ public void testIfStmtInsideDoClause() {
BRunUtil.invoke(result, "testIfStmtInsideDoClause");
}

@Test
public void testRecordDestructureWithRecordLiteralInsideDoClause() {
BRunUtil.invoke(result, "testRecordDestructureWithRecordLiteralInsideDoClause");
}

@Test(dataProvider = "dataToTestQueryActionWithVar")
public void testQueryActionWithVar(String functionName) {
BRunUtil.invoke(result, functionName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,11 @@ public void testResolvingRestField() {
BRunUtil.invoke(result, "testRestFieldResolving");
}

@Test
public void testRecordTypedBindingPatternAgainstRecordLiteralInRecordDestructuring() {
BRunUtil.invoke(result, "testRecordTypedBindingPatternAgainstRecordLiteralInRecordDestructuring");
}

@Test
public void testNegativeRecordVariables() {
String redeclaredSymbol = "redeclared symbol ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public void testTupleDestructure() {
Assert.assertEquals(returns.get(0).toString(), "true");
Assert.assertEquals(returns.get(1).toString(), "string value");
Assert.assertEquals(returns.get(2).toString(), "[25,12.5]");

BRunUtil.invoke(result, "tupleDestructureTest10");
}

@Test(description = "Test positive tuple destructure scenarios")
Expand Down
Loading