eclipse-langium / langium

Next-gen language engineering / DSL framework
https://langium.org/
MIT License
754 stars 68 forks source link

Add the ability to use parentheses when declaring `TypeAlternatives` #511

Closed pluralia closed 1 year ago

pluralia commented 2 years ago

The Langium grammar allows to create the rule X, where the property x is an array of type alternatives, and it has to be possible to declare the corresponding type XType, as in the example. We should support parentheses in TypeAlternatives.

X returns XType:
    x+=(ID | NUMBER)*;

interface XType {
    x: (number | string)[];
}
spoenemann commented 2 years ago

Proposal: add a validation to mark such alternatives with different types as error. We can relax this when we see a use case where it's really necessary, but until then I'd try to avoid making things too complicated.

pluralia commented 2 years ago

In my opinion, we can't relax this. The property of declared types is ability to "lift" all the types from ast.ts to the level of grammar, so the user can operate them. The grammar rule X

X infers XType:
    x+=(ID | NUMBER)*;

will give us such XType in the ast.ts:

export interface XType extends AstNode {
    x: Array<number | string>
}

We should be able to write the declared types in the grammar so that it can generate the same XType, and we need braces for that.

spoenemann commented 2 years ago

My proposal is to mark x+=(ID | NUMBER) with an error if ID and NUMBER have different types (string and number). You can simply solve this by introducing a data type rule that returns a string:

X returns XType:
    x+=XPropertyType*;

XPropertyType returns string:
    ID | NUMBER;
pluralia commented 2 years ago

The inferred type will be type XPropertyType = string and information about number will be lost. Also, the generated XType will has property x: Array<XPropertyType> instead of x: Array<string | number>. Would it be okay?

An important point I should mention: property types in the current implementation are able to support brackets, and it won't be that "expensive" to implement.

spoenemann commented 2 years ago

Would it be okay?

I would personally prefer an array of strings over mixed strings and numbers, so yes I think the restriction should be ok. This is related to https://github.com/langium/langium/issues/494#issuecomment-1125811597, where I proposed to disallow multiple assignments with different types to the same property. But maybe I'm too strict here and there could be valid use cases to write explicit alternatives to an assignment.

An important point I should mention: property types in the current implementation are able to support brackets, and it won't be that "expensive" to implement.

If it's a "low hanging fruit", we could still implement this, independently of how we decide about validation.

pluralia commented 2 years ago

I would personally prefer an array of strings over mixed strings and numbers, so yes I think the restriction should be ok.

The ability to have an array of strings will remain after the parentheses are added, also I think it's a "low hanging fruit", so I'd suggest to implement it.