langium icon indicating copy to clipboard operation
langium copied to clipboard

Make the `$container` type more precise in case of guard conditions

Open JohannesMeierSE opened this issue 2 years ago • 2 comments

The following simple and artificial example uses guard conditions in order to switch between different cases (excerpt):

Teacher:
    'teacher' name=ID values=Reused<true>;

Student:
    'student' name=ID values=Reused<false>;

Reused<condition>:
    <!condition> OneValue | <condition> TwoValues;

OneValue:
    'values' value=ID;
TwoValues:
    'values' valueOne=ID valueTwo=ID;

(The whole grammar with an instance can be found in the playground.)

For this grammar, Langium generates the following types in the ast.ts (excerpt):

export type Reused = OneValue | TwoValues;

export interface OneValue extends AstNode {
    readonly $container: Student | Teacher;
    readonly $type: 'OneValue';
    value: string
}

export interface TwoValues extends AstNode {
    readonly $container: Student | Teacher;
    readonly $type: 'TwoValues';
    valueOne: string
    valueTwo: string
}

From my point of view, the types of the $container properties could be more precise, i.e. only Student for OneValue and only Teacher for TwoValues. The current $container types are not wrong, but as a user of Langium, I would expect the following types, since the grammar clearly shows, that these types are enough:

export type Reused = OneValue | TwoValues;

export interface OneValue extends AstNode {
    readonly $container: Student;
    readonly $type: 'OneValue';
    value: string
}

export interface TwoValues extends AstNode {
    readonly $container: Teacher;
    readonly $type: 'TwoValues';
    valueOne: string
    valueTwo: string
}

JohannesMeierSE avatar Nov 22 '23 13:11 JohannesMeierSE

If you think this issue is worth an improvement, I am willing to work on this issue!

JohannesMeierSE avatar Nov 22 '23 13:11 JohannesMeierSE

Yes for such clear cases it would make sense to restrict the container type.

spoenemann avatar Nov 23 '23 08:11 spoenemann