Change cross-reference syntax

[spoenemann]

The cross-reference syntax of Xtext is [Type|TOKEN], where Type is the AST type of the reference and TOKEN is a terminal rule or datatype rule that shall be parsed here. This syntax is misleading because in the context of TypeScript | means an alternative.

Proposals: [Type: TOKEN], [TOKEN: Type]

Of course omitting the token shall still be possible: [Type] (the token defaults to ID)

[spoenemann]

Note: We should still allow the old syntax, but mark it with a validation error. This way we can offer a code action to fix it.

[dhuebner]

but mark it with a validation error

Let use at least warning

because in the context of TypeScript

I can't recall that we were worried about clashes with Java syntax as the Xtext grammar language was invented

[spoenemann]

I understand why that decision was made: it's like the mathematical notation for sets {a | a > 0}. The same reasoning is behind the lambda syntax of Xtend. The point is that it's inconsistent with all major programming languages that we use. We should keep the good parts of the Xtext syntax, but change the bad parts. Easing migration is a separate topic, we could even offer a CLI to convert .xtext to .langium.

[debovema]

A CLI to convert XText to Langium would be so nice 😍

[spoenemann]

If we change the syntax as proposed, we could enable alternative types for cross-references: [Type1 | Type2 | Type3] (or [Type1 | Type2 | Type3: TOKEN])

[pluralia]

The idea to have alternatives [Type1 | Type2 | Type3] is very nice! Should we open a new issue for that?

However, I have concerns about [Type1 | Type2 | Type3: TOKEN] syntax:

1. On the first view TOKEN belongs only to Type3 -- it can confuse
2. What if we want reference on different tokens for different alternatives?

I'd propose to replace it with [(Type1 | Type2 | Type3): TOKEN]. Also, for the case, when Type1 and Type2 are referenced by TOKEN_X and Type3 -- by TOKEN_Y: [(Type1 | Type2): TOKEN_X | Type3: TOKEN_Y].

[spoenemann]

Should we open a new issue for that?

I'd wait for real-world use cases first.

[sailingKieler]

I just stumbled over this issue. In the spirit of

We should keep the good parts of the Xtext syntax, but change the bad parts. Easing migration is a separate topic

I vote for switching [Type: TOKEN] to [TOKEN: Type]. (Miro's 2nd proposal in the description)
That would IMO improve explainability of that syntax feature and it's similar to Typescript's property definition syntax
propName : PropType.

What do you think?

[spoenemann]

A problematic aspect of that is that in Xtext the token name is optional, while in TypeScript the type is optional. We could solve that by making the token name mandatory, but that would increase verbosity.

[sailingKieler]

A problematic aspect of that is that in Xtext the token name is optional, while in TypeScript the type is optional. We could solve that by making the token name mandatory, but that would increase verbosity.

Making the token rule name mandatory is not necessary IMO.
I think the crucial question is whether users would expect the opportunity of prepending Type in [Type] with something like TOKEN:.

From the teaching perspective - e.g. in the xtext workshops - the difference in the meaning of name=ID/transitions+=Transition and targetState=[State] is always an issue for the trainees.
(i.e. understanding which definition is actually referenced by Transition and State)

While typing I just got the idea of having both optional, i.e. allowing something like targetState=[], the type of targetState should be AstNode in that case. Maybe the "implicit token and explicit type version" should then be written like targetState=[:State]. Hmm...

[dhuebner]

Typescript's property definition syntax
propName : PropType

The similarity to TypeScript is still very confusing to me. [TOKEN: Type] <- Type here is not the TOKEN type, but referencing type, which will confuse TS people even more than [TOKEN| Type]

[spoenemann]

Right. What about this?

property=TOKEN[Type] // explicit token
property=[Type] // implicit token

[dhuebner]

If we stay with statemachine example:

State:
	'state' name=ID
		transitions+=Transition*
	'end';

Transition:
	event=[Event] '=>' state=[State];

state=[State] means take the property named name of State
state=[State|ID] means take first property of State with token type ID

Why not be more explicit or more clear and express what we are currently want to know:
state=[State] <- remain optional as above
state=[State.name] <- link by name
state=[State by name] <- link by name

Property State.name must be validated to be a terminal token type.

I don't recall it why in Xtext it was important to have TOKEN explicitly listed in the cross ref syntax, but maybe we can do it different now

[sailingKieler]

I don't recall it why in Xtext it was important to have TOKEN explicitly listed in the cross ref syntax

Image you want allow referencing entities by a different string than it's declared name/id, maybe from a different DSL, because the original name/id syntax would cause a conflict.
Maybe you need special qualifier characters in the cross-ref strings like #, @, ... that are not used/permitted in the original name/id definition. Maybe there is no original definition in a DSL because you need to cross reference external or built-in entities...

I advocate a simple and pragmatic notation that is as concise as possible but as verbose as necessary to keep users on the right track.

[sailingKieler]

Right. What about this?

property=TOKEN[Type] // explicit token
property=[Type] // implicit token

I don't really have an opinion on your proposal yet.
It might happen that user write it like this

 .................................... property=TOKEN [Type]

or even like

 .................................... property=TOKEN
    [Type]

so having everything wrapped by the bracket keeps belonging things together (I never thought about this before).

We could also consider adding the target type to the property name (LHS) instead to the RHS, like

targetState[State] = ID
targetState:State = ID
targetState->State = ID
targetState[State]   // implicit token
targetState:State    // implicit token
targetState->State   // implicit token

I actually don't like the : version, as it doesn't suggest the cross-ref nature at all to me.
And my remark from above somehow applies here, too ;-)

[dhuebner]

@sailingKieler
True, there are some fancy cases where one need to use another lexical rule to crosslink as the origin uses. Well, TOKEN must be configurable.

Both suggestions @spoenemann targetState =ID[State] and @sailingKieler targetState[State] = ID feels more natural to me because of property = TOKEN syntax, which is the same as for non referencing properties.

Moving the Type to the LHS in other hand will break a lot as we will need to move CrossReference.type to Assignment feature type.

[spoenemann]

targetState[State] = ID is fine for me, too, but I don't like the implicit variant omitting the RHS.

@sailingKieler being able to put arbitrary white space between tokens applies to all syntaxes.

[spoenemann]

This is related to the discussion at #341 (comment). Now I'm inclined to make the cross-reference token mandatory and add a code action that inserts ID if it was previously implicit.

[spoenemann]

We had an internal chat about this and decided to keep the current syntax. Feel free to drop new thoughts here anyway.