eclipse-langium / langium

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

When determining implemented interfaces in union types langium should honor inheritance hierarchy #1544

Open m-novikov opened 3 weeks ago

m-novikov commented 3 weeks ago

Langium version: 3.0, 3.1 Package name: langium

Steps To Reproduce

  1. Given the following grammar:
    
    grammar HelloWorld

entry Model: (entities+=Entity | receptors+=Receptor)*;

interface Base {} interface Person extends Base {name: string} interface Receptor {in: Base}

Person returns Person: 'person' name=ID;

Bot returns Base: {infer Bot} 'id' id=ID;

Entity: Bot | Person;

Receptor returns Receptor: 'receptor' in=Entity;

hidden terminal WS: /\s+/; terminal ID: /[a-zA-Z][\w]*/;

hidden terminal ML_COMMENT: /\/*[\s\S]?*\//; hidden terminal SL_COMMENT: /\/\/[^\n\r]/;



2. Try to generate using "langium:generate" command. 

## The current behavior

Compilation fails with message:

> The assigned type 'Entity' is not compatible with the declared property 'in' of type 'Base'.

## The expected behavior

Compilation takes into account that interface Person also extends Base and compilation completes.
msujew commented 3 weeks ago

Hey @m-novikov,

I know it can be a little confusing, but this is working as designed. Please do not mix & match inferred and declared data types within the same inheritance structure. This doesn't work, as the inference mechanism infers different types than whatever is declared as your types. This leads to a mismatch down in the Base type which is declared as an empty interface, but gets inferred as type Base = Person | Receptor | Bot. Note that empty interfaces in Langium always get inferred as union types, as empty interfaces in TypeScript are virtually useless.

This modified grammar will work instead:

grammar HelloWorld

entry Model:
    (entities+=Entity | receptors+=Receptor)*;

interface Base {}
interface Person extends Base {name: string}
interface Receptor {in: Base}

Person returns Person:
    'person' name=ID;

interface Bot extends Base {id: string}

Bot returns Bot:
    'id' id=ID;

type Entity = Bot | Person;

Entity returns Entity: Bot | Person;

Receptor returns Receptor:
    'receptor' in=Entity;

hidden terminal WS: /\s+/;
terminal ID: /[_a-zA-Z][\w_]*/;

hidden terminal ML_COMMENT: /\/\*[\s\S]*?\*\//;
hidden terminal SL_COMMENT: /\/\/[^\n\r]*/;

We might be able to improve on this, but the type system is already hugely complex as it is, so I'm not sure it's actually feasible. I'll keep this issue open for now.

m-novikov commented 3 weeks ago

My original use case is to avoid forward declaring all the types, to have them in other file as partial interfaces.

grammar HelloWorld

entry Model:
    (entities+=Entity | receptors+=Receptor)*;

// Interfaces below located in other file for reuse
interface Entity {}
interface Receptor {in: Entity}

Person:
    'person' name=ID;

Bot:
    'id' id=ID;

Entity returns Entity: Bot | Person;

Receptor returns Receptor:
    'receptor' in=Entity;

hidden terminal WS: /\s+/;
terminal ID: /[_a-zA-Z][\w_]*/;

hidden terminal ML_COMMENT: /\/\*[\s\S]*?\*\//;
hidden terminal SL_COMMENT: /\/\/[^\n\r]*/;

But then resulting AST has following

export interface Entity extends AstNode {
    readonly $type: 'Bot' | 'Entity' | 'Person';
}

Which is incorrect, because "Entity" would be never instantiated and my goal to enforce that switch/case is exhaustive using typescript when applied to Entity union type.

msujew commented 3 weeks ago

Which is incorrect, because "Entity" would be never instantiated and my goal to enforce that switch/case is exhaustive using typescript when applied to Entity union type.

I guess I can see that, but why not just use type Entity: Bot | Person? That would be an even more type safe way to perform an exhaustive switch/case. Note that cyclic dependencies between Langium files are no issue for the framework.

m-novikov commented 3 weeks ago

Note that cyclic dependencies between Langium files are no issue for the framework.

While introducing cycling dependencies does work, thank you for that. It has it's own problems due to the fact that it not only imports types, but also all the other rules from the grammar.

msujew commented 3 weeks ago

While introducing cycling dependencies does work, thank you for that. It has it's own problems due to the fact that it not only imports types, but also all the other rules from the grammar.

Right, but that's really only problematic for hidden terminal rules - everything else gets pulled into the grammar in an on-demand basis. If you don't use an imported grammar rule, it won't be in your final, generated grammar.