eclipse-langium / langium

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

`AbstractType`s need to include `InferredType`s #1328

Closed JohannesMeierSE closed 8 months ago

JohannesMeierSE commented 9 months ago

... since cross-references might use InferredTypes as their types.

This seems to be an edge-case in the Langium grammar: The type of a cross-reference does not point to the parser rule itself, if this parser rule has an explicitly declared inferred type. Instead, the cross-reference points to the corresponding InferredType.

But InferredType is not supported by AbstractType, which is used as type for cross-references in the Langium grammar:

type AbstractType = Interface | Type | Action | ParserRule;

CrossReference infers AbstractElement:
    {infer CrossReference} '[' type=[AbstractType] ((deprecatedSyntax?='|' | ':') terminal=CrossReferenceableTerminal )? ']';

The example from the created test case is available here in the playground.

At the moment, the TS compiler complains about this issue:

Screenshot 2024-01-02 at 11 55 50

Resolving this cross-reference to the InferredType works at runtime, since it is explicitly exported: https://github.com/eclipse-langium/langium/blob/2ca2b3adb452ad5048135af85e1510f84a71fd7b/packages/langium/src/grammar/references/grammar-scope.ts#L106

I am proposing to mark InferredTypes as AbstractTypes ...

type AbstractType = Interface | Type | Action | ParserRule | InferredType;

... but there might be a better solution.

Update 2024-01-05:

@pluralia Since we discussed about these topics, I am looking forward to your review as well!

Update 2024-01-12:

JohannesMeierSE commented 9 months ago

Yes, I was a bit surprised by the use of Interface in all cases, but I don't see a problem/bug in practise so far.

Additionally, I found another point: There might be an issue with multiple InferredTypes with the same name, since all cross-references for this type point to the first InferredType in the grammar. It seems to work, but that looks a bit strange to me.

Maybe, the source for these discussions is the terminology/understanding of the various "kinds of types"?

  1. We have "language types" like Person and Greeting in the hello world example.
  2. InferredType, ReturnType, ... define the wanted language types and are types for the AST nodes which represent the current Langium grammar.
  3. The language types are realized in TypeScript as type or as interface.

As an example, when we have an InferredType in a parser rule, it is an AST node of type "InferredType" which explicitly defines the name of the inferred (language) type of the parser rule.

As another consequence, when we want to store the language types in the global scope, all these types have the same type, which reflects the current situation (whether "interface" is the best name for this type is another discussion).

A problem might be, that the same language types can be defined multiple times.

spoenemann commented 9 months ago

Additionally, I found another point: There might be an issue with multiple InferredTypes with the same name, since all cross-references for this type point to the first InferredType in the grammar. It seems to work, but that looks a bit strange to me.

Yes, cross-references can be resolved only to one target element, but the same type can be inferred by multiple rules or actions in the grammar.

JohannesMeierSE commented 9 months ago

For implicitly inferred types (a rule with no returns and no infers), we could set it to InferredType – here we need to make a distinction between the parser rule and the corresponding type.

I am not yet convinced, since AST nodes of $type InferredType are used for implicitly and explicitly inferred types: Why should we use InferredType in scopes only for implicitly inferred types? That would confuse me.

As an alternative, we could use InferredType in scopes for all inferred types, i.e. explicitly inferred types in actions ("{infer ...}"), explicitly inferred types in parser rules ("infers"), and implicitly inferred types in parser rules (neither "infers" nor "returns"). That would distinguish inferred types from explicitly declared Types and Interfaces.