Add new Xtext editor quickfixes on *.xtext files

[miklossy]

The Xtext editor should provide the following quickfixes on *.xtext files:

• Provide quickfixes for illegal rule references (see #331943)
• Provide quickfixes for illegal hidden token definitions (see #331942)
• Provide more quickfixes for missing rules (see #331941)

@loradd Would you like to take care of this issue?

[nbhusare]

@miklossy I would like to work on this issue. Kindly let me know if that is ok.

[miklossy]

@nbhusare Would be awesome if you could take care of this issue. Thanks a lot in advance!

[loradd]

@miklossy sorry for the late reply!
I could also work on this, @nbhusare let me know if you need support.

[miklossy]

Thanks for your reply @loradd . I would sugges that @nbhusare will take care of this issue, for you @loradd I created a similar one to the Xtend repository: eclipse/xtext-xtend#1060 . Please comment on that issue if you could take care of that one. Thanks!

[nbhusare]

@szarnekow @miklossy @cdietrich please review
#1470
eclipse/xtext-core#1523

[nbhusare]

@szarnekow - The method XtextGrammarQuickfixProvider.fixUnresolvedRule(Issue, IssueResolutionAcceptor), on line 144, specifically gets a container of type ParserRule. Yet, on line 149, it checks if the if (abstractRule instanceof TerminalRule). I looked at the history (PS comment below) and I don't understand it. Could you please confirm if the code on line 149 is required?

Comment from history - Use textual modification instead of semantic modification for new parser rules as serialization / comment location preserving is to fragile for invalid models

[szarnekow]

@szarnekow - The method XtextGrammarQuickfixProvider.fixUnresolvedRule(Issue, IssueResolutionAcceptor), on line 144, specifically gets a container of type ParserRule. Yet, on line 149, it checks if the if (abstractRule instanceof TerminalRule). I looked at the history (PS comment below) and I don't understand it. Could you please confirm if the code on line 149 is required?

It's a bug. It should probably get a container of type AbstractRule in line 144.

[nbhusare]

@szarnekow I have replaced the existing PR's with new ones. I have addressed the review comments in the same. Kindly have a look.

[nbhusare]

@miklossy We have merged the PR's. Could you please close this issue. Tx

[miklossy]

Thanks for your contibution @nbhusare ! I would like to try out the new quickfixes before closing this issue.

[miklossy]

@nbhusare I took a look at the current state of the fix and found the following shortcomings:

• In some cases, the Convert terminal fragment to terminal rule quickfix is offered but has no effect:
  

• Did you fix the bug #331941 completely? E.g. the Create terminal rule quickfix is not offered where it should be:
  

• Would be awesome if you could add some words about this fix to the release notes, see [2020-09] create release notes xtext#1786

[nbhusare]

@miklossy Let me take a look at the above.

[nbhusare]

@miklossy I have raised a PR (xtext-eclipse/pull/1492) to address the first issue you reported above. For the second one, to show the two quick-fixes proposals (Create terminal..., Create terminal fragment...), I added the annotation @Fix(XtextLinkingDiagnosticMessageProvider.UNRESOLVED_RULE) on the methods fixUnresolvedTerminalRule(), and fixUnresolvedTerminalFragmentRule(), and also enhanced the method createNewRule(); however, it adds multiple "Change to 'XXX'" proposals in addition to the required ones. Is there a way to only show one "Change to 'XXX'" proposal?

[cdietrich]

@nbhusare why are there 3 quickfixed created at all?
how to reproduce this?

[miklossy]

Thanks @cdietrich for jumping in and taking over!

[nbhusare]

@cdietrich Sorry, by 3 quick-fixes do you mean the "Change to 'PQR'" ones, or the Create XX ones?
To reproduce the issue, please add annotation @Fix(XtextLinkingDiagnosticMessageProvider.UNRESOLVED_RULE) to the methods fixUnresolvedTerminalRule(), and fixUnresolvedTerminalFragmentRule() in XtextGrammarQuickFixProvider. Please use Model: a=ID2 grammar as an example.

[cdietrich]

if you call createLinkingIssueResolutions three times and dont filter the typo fixes then of course you get 3 fixes

[nbhusare]

@cdietrich Tx for the tip. Let me take a look.

[nbhusare]

@cdietrich @szarnekow I came across the following issues while working on this task. Please have a look.
Also filed jiras - https://bugs.eclipse.org/bugs/show_bug.cgi?id=565523, https://bugs.eclipse.org/bugs/show_bug.cgi?id=565524 respectively.

1. In the grammar below, the quick-fix "Remove Hidden token definition" fails with the below error. The serializer is failing while it tries to serialize the broken reference name=IDX

Model hidden(ModelA):  // Reference to a parser-rule
    name=IDX; //IDX is not defined yet
ModelA:
    name=ID;

java.lang.RuntimeException: Could not serialize RuleCall:
RuleCall.rule is required to have a value, but it does not.
Semantic Object: Grammar'org.xtext.example.mydsl.MyDsl'.rules[0]->ParserRule'Model'.alternatives->Assignment.terminal->RuleCall
URI: platform:/resource/org.xtext.example.mydsl/src/org/xtext/example/mydsl/MyDsl.xtext
Context: AssignableTerminal returns RuleCall

2. In the below grammar, for the hidden token A, I get three proposals - Change to B, Convert terminal fragment to terminal rule, and Remove hidden token definition. In the method XtextValidator#checkHiddenTokenIsNotAFragment(), the number of hidden tokens returned is one, even though we have two tokens defined (one has a broken reference). The index value that is passed to the call to addIssue() is 0, and as a side-effect, we see three proposals for the hidden token definition. In short, the proposals shown (at least the second and the third) applicable to the hidden token B are displayed for the token A.

Now, if I choose the proposal "Remove hidden token definition", all the tokens get removed. Proposal "Change to B" works, but it doesn't make sense since B is a terminal fragment. "Convert terminal Fragment to terminal rule" does not make sense either.

Model hidden(A, B):
	name=ID;
terminal fragment B:
	'b';

[cdietrich]

ping @szarnekow

[nbhusare]

Would be awesome if you could add some words about this fix to the release notes, see eclipse/xtext#1786
@miklossy Do you want me to add the details as a comment in the jira or directly to the 2020-09-01-version-2-23-0.md file?

[miklossy]

Directly to the 2020-09-01-version-2.23.md file with a Pull Request

[nbhusare]

@miklossy Kindly review - https://github.com/eclipse/xtext/pull/1823/files

[cdietrich]

@miklossy what is the state of this issue?

[cdietrich]

@miklossy @nbhusare is there anything left?

[cdietrich]

i assume this fixed. if there is anything left, please create a new issue