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 the invalid error for readonly record types #45

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
# See: https://help.github.com/articles/about-codeowners/

# These owners will be the default owners for everything in the repo.
* @gimantha @MaryamZi @hasithaa
* @gimantha @MaryamZi @hasithaa @SasinduDilshara
31 changes: 31 additions & 0 deletions ballerina/tests/from_json_string_test.bal
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,37 @@ isolated function testSimpleJsonStringToRecord() returns Error? {
test:assertEquals(recC.get("b"), 1);
}

type ReadOnlyUser readonly & record {|
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
int id;
|};

@test:Config
isolated function testSimpleJsonStringToRecord2() returns Error? {
string user = string `{"id": 4012}`;
ReadOnlyUser r = check parseString(user);
test:assertEquals(r, {id: 4012});
}

Copy link
Member

Choose a reason for hiding this comment

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

Can we add more tests with

  • members of selectively-immutable types that become immutable because the type it is in is immutable - e.g., like the id field, but with a selectively-immutable type
  • field of a readonly intersection type in a mutable type

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 these scenarios.

Copy link
Member

Choose a reason for hiding this comment

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

Can you please point to the relevant tests for each of these cases?

public type UserId readonly & int;

public type UserName readonly & record {
string firstname;
string lastname;
};

type ReadOnlyUser2 readonly & record {|
UserId id;
UserName name;
int age;
|};

@test:Config
isolated function testSimpleJsonStringToRecord3() returns Error? {
string user = string `{"id": 4012, "name": {"firstname": "John", "lastname": "Doe"}, "age": 27}`;
ReadOnlyUserRecord2 r = check parseString(user);
test:assertEquals(r, {id: 4012, age: 27, name: {firstname: "John", lastname: "Doe"}});
}

@test:Config
isolated function testSimpleJsonStringToRecordWithProjection() returns Error? {
string str = string `{"a": "hello", "b": 1}`;
Expand Down
49 changes: 49 additions & 0 deletions ballerina/tests/from_json_test.bal
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,55 @@ isolated function testSimpleJsonToRecord() returns Error? {
test:assertEquals(recC.get("b"), 1);
}

type ReadOnlyUserRecord readonly & record {|
int id;
|};

@test:Config
isolated function testSimpleJsonToRecord2() returns Error? {
json user = {id: 4012};
ReadOnlyUserRecord r = check parseAsType(user);
test:assertEquals(r, {id: 4012});
}

public type ReadonlyUserId readonly & int;

public type ReadonlyUserName readonly & record {
string firstname;
string lastname;
};

type ReadOnlyUserRecord2 readonly & record {|
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
ReadonlyUserId id;
ReadonlyUserName name;
int age;
|};

@test:Config
isolated function testSimpleJsonToRecord3() returns Error? {
json user = {id: 4012, name: {firstname: "John", lastname: "Doe"}, age: 27};
ReadOnlyUserRecord2 r = check parseAsType(user);
test:assertEquals(r, {id: 4012, age: 27, name: {firstname: "John", lastname: "Doe"}});
}

type UserRecord3 ReadOnlyUserRecord2;

@test:Config
isolated function testSimpleJsonToRecord4() returns Error? {
json user = {id: 4012, name: {firstname: "John", lastname: "Doe"}, age: 27};
UserRecord3 r = check parseAsType(user);
test:assertEquals(r, {id: 4012, age: 27, name: {firstname: "John", lastname: "Doe"}});
}

@test:Config
isolated function testSimpleJsonToRecord5() returns Error? {
ReadonlyUserName username = {firstname: "John", lastname: "Doe"};
record{string firstname; string lastname;} nameRec = username;
json user = {id: 4012, name: nameRec.toJson(), age: 27};
ReadOnlyUserRecord2 r = check parseAsType(user);
test:assertEquals(r, {id: 4012, age: 27, name: {firstname: "John", lastname: "Doe"}});
}

@test:Config
isolated function testSimpleJsonToRecordWithProjection() returns Error? {
json j = {"a": "hello", "b": 1};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public Object execute(Reader reader, BMap<BString, Object> options, Type type) t
}
case TypeTags.INTERSECTION_TAG -> {
Type effectiveType = ((IntersectionType) type).getEffectiveType();
if (!SymbolFlags.isFlagOn(SymbolFlags.READONLY, effectiveType.getFlags())) {
SasinduDilshara marked this conversation as resolved.
Show resolved Hide resolved
if (!effectiveType.isReadOnly()) {
throw DiagnosticLog.error(DiagnosticErrorCode.UNSUPPORTED_TYPE, type);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private Object traverseJson(Object json, Type type) {
}
case TypeTags.INTERSECTION_TAG -> {
Type effectiveType = ((IntersectionType) referredType).getEffectiveType();
if (!SymbolFlags.isFlagOn(SymbolFlags.READONLY, effectiveType.getFlags())) {
if (!effectiveType.isReadOnly()) {
throw DiagnosticLog.error(DiagnosticErrorCode.UNSUPPORTED_TYPE, type);
}
for (Type constituentType : ((IntersectionType) referredType).getConstituentTypes()) {
Expand Down
Loading