Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Playground displays misleading linker errors #226

Closed
Rophuine opened this issue Apr 20, 2024 · 5 comments · Fixed by #230
Closed

Playground displays misleading linker errors #226

Rophuine opened this issue Apr 20, 2024 · 5 comments · Fixed by #230
Labels
enhancement New feature or request playground Issues related to the playground functionalits

Comments

@Rophuine
Copy link

When using the Langium Playground, you're really only editing a grammar and seeing how it parses a sample input into an AST - the linking stage doesn't seem relevant. But linker errors are displayed, which as someone experimenting with Langium for the first time was very confusing until I realised what was happening.

I spent over an hour trying to understand why my grammar and content were producing errors, including diving into the XText docs. I nearly gave up on Langium as it just seemed broken, even in very simple scenarios. It turns out these errors just aren't resolvable in the Playground, because you're only working on the grammar and not doing anything related to resolution or scope.

The Playground is very likely to be used by people trying out Langium for the first time, and displaying these errors is very likely to lead to frustration and people giving up.

Steps To Reproduce

  1. Load the Playground at https://langium.org/playground/
  2. Put the following into the Grammar pane:
grammar MyLang

entry Model:
    body+=Statement+;

Statement: 'const' assignment=ConstantAssignment ';';
ConstantAssignment: name=ID '=' value=Expression;
Expression: value=NUMBER | reference=[ConstantAssignment];

hidden terminal WS: /\s+/;
terminal ID: /[_a-zA-Z][\w_]*/;
terminal NUMBER returns number: /[0-9]+(\.[0-9]*)?/;
  1. Put the following into the Content pane:
const n=5;
const y=n;

The current behavior

image

The expected behavior

No error should be displayed - the grammar is valid and has been successfully parsed into an AST. The reference hasn't been resolved, but we don't expect it to be - you can't write linker code in the playground, so many valid examples will result in errors.

@Rophuine Rophuine added the bug Something isn't working label Apr 20, 2024
@msujew msujew transferred this issue from eclipse-langium/langium Apr 21, 2024
@msujew
Copy link
Contributor

msujew commented Apr 21, 2024

Thank you for the discussion @Rophuine, though I'm not sure I can agree with this. You're completely right in the regard that the inability to write custom scoping code will lead to unintended error messages about unresolved cross references and is actually something we're investigating how to address. Maybe @Lotes can chime in on that. However, IMO at this point you'd be past using the playground as a means to develop/iterate your language. I think it's fairly valuable to have the full cross reference support in there, including error messages in case the linking failed.

@spoenemann Do you have an opinion on this?

@spoenemann
Copy link
Contributor

Idea: enable to switch linking errors on/off. They're useful even in the playground sometimes to demonstrate how the default scoping/linking works and e.g. how renaming a symbol without the automatic rename feature breaks the reference. But I'd be ok with turning that switch off by default.

@spoenemann spoenemann added enhancement New feature or request playground Issues related to the playground functionalits and removed bug Something isn't working labels Apr 22, 2024
@Lotes
Copy link
Contributor

Lotes commented Apr 22, 2024

The playground will get an update soon (waiting for some depending PRs). The perfect opportunity for me to add such a switch.

@Lotes
Copy link
Contributor

Lotes commented May 15, 2024

If we turn linking errors entirely off, any identifier can be put in as a cross-reference and nothing complains. This looks more broken to me than having a linking error in red.
I would like to propose a third 'middle' way: Since we will never implement a custom scope provider by user, the user is not able to remove the error. It would be ok to raise warnings instead, that explains that a custom scope provider needs to be injected and that this is out of scope for the playground. I would vote against a switch because it does not change the possible actions for the user. A switch on off just ignores linking, which is not so helpful for testing Langium out. Langium is more than just a parser.

WDYT? @Rophuine @spoenemann

Here is an alternative view (preview):
Bildschirmfoto 2024-05-15 um 16 32 45

Linking failed. Please inject a custom scope provider. This is a limitation of the playground. To overcome this issue, please study the learning section in the Langium documentation ...

I am open for other wordings if we want to go this route :)

@Rophuine
Copy link
Author

A warning sounds perfect, especially with some explanation. I wasted quite a lot of time in the playground thinking that Langium just wasn't processing a perfectly valid grammar before I realised that it was actually a scope error - I nearly assumed the parsing was just broken and moved on to other options. I'm glad I persevered because Langium has been great!

Having a warning that pointed me to a custom scope provider as a possible solution would have been a much better experience as a beginner!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request playground Issues related to the playground functionalits
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants