ballerina-platform / ballerina-spec

Ballerina Language and Platform Specifications
Other
168 stars 53 forks source link

Use field types in mapping constructor when choosing inherent type #962

Open MaryamZi opened 3 years ago

MaryamZi commented 3 years ago

Description: For a mapping constructor expression the spec says

If there is a contextually expected type, then the inherent type of the newly created mapping is derived from the applicable contextually expected type. If the applicable contextually expected type is a mapping type descriptor, then that used as the inherent type. If the applicable contextually expected type is a union type descriptor, then any members of the union that are inconsistent with the field names specified in a specific-field in the mapping-constructor-expr will be ignored; it is a compile-time error if this does not leave a single mapping type descriptor, which is then used as the inherent type.

But when the applicable contextually-expected type is a union T1|T2|T3 and the mapping constructor is valid for only one of the types in the union (i.e., as when defined as Tn _ = {....} where n is 1, 2, or 3) can't we use that type as the type to be used in the construction?

Basically, in addition to the current checks, can't there be a type compatibility check for the fields as well?

This would also be applicable to langlib functions such as cloneWithType since they follow similar rules.

The implementation currently has a deviation where it does these.

import ballerina/io;

type StatusOk record {|
    readonly 200 code = 200;
    string body?;
|};

type StatusNotFound record {|
    readonly 404 code = 404;
    string body?;
    string kind?;
|};

function fn() returns StatusOk|StatusNotFound {
    // ...
    return {code: 200}; // Currently allowed, but the spec doesn't allow this.
}

public function main() returns error? {
    json payload = {code: 200};

    StatusOk|StatusNotFound status = check payload.cloneWithType(); // Currently succeeds and creates a `StatusOk`.

    io:println(status is StatusOk); // true
    io:println(status is StatusNotFound); // false
}

Real-world example with HTTP

import ballerina/http;

service / on new http:Listener(8080) {
    resource function get getResource() returns http:Ok|http:NotFound {
        return {status: new http:StatusOK()};
    }
}

The implementation currently does this for all kinds of constructor expressions including list constructor expressions.

int[]|boolean[] arr = [1, 2, 3]; // allowed and constructed as an `int[]`

Related Issues: https://github.com/ballerina-platform/ballerina-spec/issues/949

jclark commented 2 years ago

We can do the cloneWithType/fromJsonWithType part of this separately as part of #1088. This will be just for the mapping constructor part.