ballerina-platform / ballerina-spec

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

Should type inclusion copy metadata? #1311

Open MaryamZi opened 3 weeks ago

MaryamZi commented 3 weeks ago

Description: Consider the following simple example for record type inclusion.

annotation Attribute on field;

type Parent record {
    @Attribute
    string parentAttr;
};

type Child record {
    *Parent;

    @Attribute
    string childAttr;
};

Right now, the jBallerina implementation does not copy metadata when copying the fields, but it probably should (there have been multiple requirements for this for library modules too).

Since we require the including record to explicitly define the field when the same field is defined in two included records (https://github.com/ballerina-platform/ballerina-spec/issues/696#issuecomment-763312787), I believe we wouldn't run into conflicts.

annotation record {| string name; |} Attribute on field;

type Root record {
    @Attribute {name: "root"}
    string parentAttr;
};

type Parent record {
    @Attribute {name: "parent"}
    string parentAttr;
};

// Compile-time error anyway since `parentAttr` is not explicitly defined.
// Once fixed, metadata will be what is explicitly specified on the field defined here.
type Child record {
    *Root;
    *Parent;

    @Attribute {name: "child"}
    string childAttr;
};

Documentation

I believe this is generally straightforward, but BFM allows referring to "Ballerina-defined names from within documentation strings". When the included record comes from an imported module, there could be

So just copying as is may not produce the expected results?

Annotations

https://ballerina.io/spec/lang/master/#record-type-inclusion

For default values, the closure rather than the expression is copied in.

We can then use the result in a spread field.

jclark commented 3 weeks ago

I agree with your conclusions

The tricky bit is the occurrence of qualified names in the BFM. I would suggest that the abstract data model for the documentation string needs to include not just the string itself, but also the import context i.e. the imports declared at the top of the source file. Does that make sense?