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

Converting json to simple types #487

Closed
jclark opened this issue Apr 13, 2020 · 10 comments
Closed

Converting json to simple types #487

jclark opened this issue Apr 13, 2020 · 10 comments
Assignees
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks status/pending Design is agreed and waiting to be added

Comments

@jclark
Copy link
Collaborator

jclark commented Apr 13, 2020

Consider the example in #350.

if (orderPayload is json) {
             string orderId = orderPayload.Order.ID.toString();
}

Because of lax static typing, the type of orderPayload.Order.ID is json|error.

Apart from the problem mentioned in #350, there's another problem with using toString here: if orderPayload.Order.ID is a map or an array, we want to get an error, not try to convert the map or array to a string. If it's a number, string, boolean or (), it's not unreasonable to convert to a string using toString; an error would also be OK in these case.

There's currently no easy way to do this properly. Using a cast <string> has other problems. Not only will it hide the error, but it will also panic if it's not a string, rather than returning an error.

This problem applies not just to conversion to string but conversion to any simple type.

For numbers, this relates to #8.

A consistent way to do this would be for lang.string to provide:

public function fromJson(json j) returns string|error = external;

But this feels too inconvenient for such a common use case.

@jclark jclark added Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks labels Apr 13, 2020
@jclark jclark added this to the 2020R2 milestone Apr 13, 2020
@jclark jclark self-assigned this Apr 13, 2020
@jclark jclark added the status/discuss Needs discussion outside github comments label Aug 6, 2020
@jclark
Copy link
Collaborator Author

jclark commented Aug 9, 2020

We can do this now with:

string orderId = check orderPayload.Order.ID.fromJsonWithType();

except we may want some more conversions (e.g. between numbers and strings), but we probably want those with fromJsonWithType anyway.

We also have that the type of E is lax, so one approach is to make check do this automatically, as follows. Given an expression check Expr in a context that expects type S, where Expr has type T|E, and

  • E is a subtype of error
  • Expr is laxly typed
  • T is a subtype of json
  • S is a subtype of anydata
  • it would be a compile-time error with normal rules, because T is not a subtype of S

then this turns into

check (check Expr).fromJsonWithType(S)

So in the above case, the code would be simply:

string orderId = check orderPayload.Order.ID;

Note that if we were dealing with strongly typed data rather than just json, you could simply write:

string orderId = orderPayload.Order.ID;

so it’s not such a stretch to make lax typing do this.

Also note that fromJsonWithType already does numeric conversions. We could also make it do some of the conversions that JavaScript does automatically e.g. from and to strings.

@sameerajayasoma
Copy link
Contributor

sameerajayasoma commented Aug 10, 2020

As per the above comment, I assume that the following is also supported. As per the definition, this syntax works for any S that is a subtype of anydata.

xml order = check orderPayload.order;

If that is the case, the following should also be supported:

var orderJson = check orderPayload.order;
xml order = check orderJson;

One concern here is that the expression orderJson is not laxly typed.

@jclark
Copy link
Collaborator Author

jclark commented Aug 11, 2020

I would say orderJson is lax, since the inferred type-descriptor is json, which is lax, although our current approach to lax typing is not very well defined.

@jclark
Copy link
Collaborator Author

jclark commented Aug 11, 2020

@sameerajayasoma Your example before you edited was a good one, which I want to work:

Order order = check orderPayload.order; // "Order" is a record

@jclark
Copy link
Collaborator Author

jclark commented Aug 11, 2020

We should probably make fromJsonWithType lift errors i.e. allow the first arg to be an error, so that it works better with laxly typed member access.

@sameerajayasoma
Copy link
Contributor

According to @jclark's comment #487 (comment), any expression whose inferred type-descriptor is json will be lax.

By making fromJsonWithType to lift errors, we can make the following work:

Order order = check orderPayload.order.fromJsonWithType()

@jclark
Copy link
Collaborator Author

jclark commented Aug 14, 2020

Revised proposal following discussions today.

Add new langlib function to lang.value:

function requireType(any|error value, typedesc<any> t = <>) returns t|error {
  trap<t>check value; // not actually valid Ballerina, but hopefully semantics are clear
}

Then given an expression check Expr in a context that expects type S, where Expr has type T|E, and

  • E is a subtype of error
  • Expr is laxly typed
  • T is a subtype of json
  • S is a subtype of ()|int|float|decimal|string (json without structures)
  • it would be a compile-time error with normal rules, because T is not a subtype of S

then this turns into

check Expr.requireType(S)

So the original case can be written like this:

if orderPayload is json {
             string orderId = check orderPayload.Order.ID;
}

On the other hand, this

Order order = check orderPayload.order; // "Order" is a record

wouldn't work (it would get a compile-time error). And it shouldn't work because it's too much for check to do an implicit copy.

@jclark
Copy link
Collaborator Author

jclark commented Aug 16, 2020

We are OK with this, but prefer ensureType to requireType.

@jclark jclark added status/pending Design is agreed and waiting to be added and removed status/discuss Needs discussion outside github comments labels Aug 16, 2020
@chiranSachintha
Copy link
Member

Is this a valid scenario?

if orderPayload is json {
             string|int orderId = check orderPayload.Order.ID;
}

@jclark
Copy link
Collaborator Author

jclark commented Sep 7, 2020

@chiranSachintha Yes: string|int is a subtype of ()|int|float|decimal|string

@jclark jclark removed this from the Swan Lake September preview milestone Sep 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Lang Relates to the Ballerina language specification design/usability Design does not work well for some tasks status/pending Design is agreed and waiting to be added
Projects
None yet
Development

No branches or pull requests

3 participants