langium icon indicating copy to clipboard operation
langium copied to clipboard

Support for properties in the parent type

Open pluralia opened this issue 3 years ago • 8 comments

Unassigned rule calls create a parent-child relation, and in the example below, AbstractToken is a parent of TokenA, TokenB and TokenC. AbstractToken also has own property cardinality.

AbstractToken:
  ( TokenA
  | TokenB
  | TokenC
  ) cardinality=('?'|'+'|'*')?;

However, types generation only reflects reflects ownership, not inheritance. Generated types:

export interface AbstractToken extends AstNode {
    cardinality?: '*' | '+' | '?'
}

// similar to `TokenB` and `TokenC`
export TokenA extends AstNode {
	...
}

Since inheritance is reflected by union type for the purpose of property lifting, we should generate the types in the above case as follows:

export type AbstractToken = (TokenA | TokenB | TokenC) & { cardinality?: '*' | '+' | '?' }

pluralia avatar May 23 '22 05:05 pluralia

Closing this issue should includes supporting union types here.

pluralia avatar May 23 '22 05:05 pluralia

With the solution you proposed, we would have to repeat the cardinality property in all three subtypes, so declaring it in the supertype is not necessary. I'd propose to change this to

export interface AbstractToken extends AstNode {
    cardinality?: '*' | '+' | '?'
}

export interface TokenA extends AbstractToken {
    ...

This means: use a type alternative only if the supertype does not declare its own properties. Otherwise fall back to an interface.

spoenemann avatar May 23 '22 06:05 spoenemann

With the solution you proposed, we would have to repeat the cardinality property in all three subtypes, so declaring it in the supertype is not necessary.

You are right.

use a type alternative only if the supertype does not declare its own properties. Otherwise fall back to an interface.

This is the current solution. However, in this case when the parent has own properties and represented by an interface, we have to manually lift the children properties, and thus we go back to necessity to implement an algorithm that lifts properties. Duplicating parent properties in children can save from it.

pluralia avatar May 23 '22 07:05 pluralia

In fact, when deciding on inheritance (type or/and interface), we must keep in mind:

  1. the declared types get into ast.ts as they are with the inheritance hierarchy expressed by interfaces
  2. #441 forces us to implement a "conversion" of inferred types to declared types, so if inferred types express inheritance by types and declared types by interfaces, it will be harder to convert; besides, this conversion will force us to implement a property-lifting algorithm anyway

pluralia avatar May 23 '22 07:05 pluralia

In conclusion, we have 2 possible solutions:

  1. leave the current solution with types and interfaces where the parent has its own properties, and reintroduce the algorithm for raising properties
  2. duplicate parent properties in children

pluralia avatar May 23 '22 07:05 pluralia

@pluralia I agree with the notion that we probably have to propagate child properties up to parents when inheritance is a factor, as we currently don't allow duplicate properties in any form. This matches what would be written explicitly with our types, and it makes sense to me that we would do the same with the generated component of the type system.

If we wanted to allow duplicates we would have to change things, but after some talks last week it seems like that's not desired. Also duplicating parent properties in children doesn't feel quite right.

~@spoenemann I have a question about the example above. You have TokenA inheriting from AbstractToken, giving each token its own cardinality. I think, given the grammar, an abstract token just contains one of 3 concrete token types, in addition to a cardinality. Whereas you have each token inheriting the cardinality property. This makes sense from the perspective of using interfaces/types effectively for abstraction, but not from the perspective of the grammar rules.~. The aforementioned was based on a false assumption. There is inheritance going on here, but leaving for context.

edit: As an addition, if I'm in the right direction, then it makes sense to me to adopt @pluralia 's mention of using type intersections, and we can retain the union that would otherwise be lost. As is it seems kind of odd that adding a single property can change a generated type to an interface and remove inheritance in the process.

montymxb avatar May 23 '22 11:05 montymxb

we have to manually lift the children properties, and thus we go back to necessity to implement an algorithm that lifts properties.

@pluralia I think now I understood what you mean. But nobody forces us to do this lifting, it's really just a convenience feature. We could simply decide not to do any property lifting ourselves here and see whether it's a real problem in languages that have such constructs. WDYT?

spoenemann avatar May 23 '22 13:05 spoenemann

We could simply decide not to do any property lifting ourselves here and see whether it's a real problem in languages that have such constructs. WDYT?

Yes, that makes sense. At least we can take a pause to think more about it. I move the milestone.

pluralia avatar May 23 '22 14:05 pluralia