Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 351324 - Content assist queries the scope in a wrong manner when rule actions are used
Summary: Content assist queries the scope in a wrong manner when rule actions are used
Status: CLOSED DUPLICATE of bug 276619
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 2.0.0   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-06 08:49 EDT by Mark Christiaens CLA
Modified: 2011-07-07 04:36 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Christiaens CLA 2011-07-06 08:49:05 EDT
Build Identifier: 20110615-0604

This is a bit complicated so I'll try to explain with a fake parser rule containing a rule action:

A: B ('.' {Concatenation.left=current} right=C)?; 

So the idea is that you have a rule A that can result in an EMF object of class B or in a EMF tree with the "Concatenation" as root and objects of class B and C as resp. left and right nodes.

Now, let's assume that B and C are rules that simply consume an identifier.  Suppose I type "identifier1." in my editor (notice the dot!).  When I now request content assist, something unexpected happens: the content assist code is presented with an EMF model that consists of just a B object (representing identifier1). That's not really the right context to do content assist.  The right context to make suggestions for C is a root Concatenation object with B as its left child and null as its right child.  

As a result, content assist will go to the scope provider to get all possible candidates for C and will launch a scope query that is unexpected.  In my case, this results in the (declarative) scope provider not finding any rule that matches the query and eventually bubbling up the query to the global scope.  The net result is nonsense.

Just to confirm that the rule action is indeed the culprit: if I type "identifier.a" (notice the additional "a") then the rule action is executed and the tree is restructured as expected.  As a result, the content assist proposals are fine.  

I know that the rule actions are executed by the parser when there is a complete match but in the case of content assist, some prescience is necessary to obtain the right results.  I think the fix is probably to perform the rule action prior to trying to figure out a sensible completion for C.  Then undo the action. 


Reproducible: Always
Comment 1 Sebastian Zarnekow CLA 2011-07-06 11:29:49 EDT
Hi Mark,

the commonly used pattern is 

A: B ({Concatenation.left=current} '.' right=C)?; 

instead of 

A: B ('.' {Concatenation.left=current} right=C)?; 

Note the dot after the action. You have to enforce object instantiation prior to consuming the selective token. 

Closing as duplicate of bug 276619 since its basically the same problem.

*** This bug has been marked as a duplicate of bug 276619 ***
Comment 2 Mark Christiaens CLA 2011-07-06 11:43:18 EDT
(In reply to comment #1)
> Hi Mark,
> 
> the commonly used pattern is 
> 
> A: B ({Concatenation.left=current} '.' right=C)?; 
> 
> instead of 
> 
> A: B ('.' {Concatenation.left=current} right=C)?; 
> 
> Note the dot after the action. You have to enforce object instantiation prior
> to consuming the selective token. 
> 
> Closing as duplicate of bug 276619 since its basically the same problem.
> 
> *** This bug has been marked as a duplicate of bug 276619 ***

That's an oversight in my bug report (serves me well for not using the actual grammar rules :) ).  In my real grammar, I'm using the common pattern.  So I'm not quite convinced this is a duplicate.  I do see with my debugger that the model that is presented to content assist and to scoping has not undergone the rule call yet.  

Is that not a different issue?  I don't see a mention of a rule call in https://bugs.eclipse.org/bugs/show_bug.cgi?id=276619
Comment 3 Sebastian Zarnekow CLA 2011-07-06 11:53:35 EDT
Do you have backtracking enabled in your grammar?
Comment 4 Mark Christiaens CLA 2011-07-06 15:10:37 EDT
(In reply to comment #3)
> Do you have backtracking enabled in your grammar?
Yes I do.
Comment 5 Sebastian Zarnekow CLA 2011-07-06 15:19:01 EDT
That's most likely related to the issue. I'd assume that the production parser is in backtracking mode when it tries to consume the '.' followed by right=C. It fails and simply consumes the '.' as a syntax error and does not execute the action.
I'd recommend to use syntactic predicates instead of backtracking.
Comment 6 Mark Christiaens CLA 2011-07-07 04:36:54 EDT
(In reply to comment #5)
> That's most likely related to the issue. I'd assume that the production parser
> is in backtracking mode when it tries to consume the '.' followed by right=C.
> It fails and simply consumes the '.' as a syntax error and does not execute the
> action.
> I'd recommend to use syntactic predicates instead of backtracking.

I've been looking at the rule in question for 15 minutes and I'm convinced there is no backtracking in this rule.  I'd like to verify it using the debug grammar but sadly the 
     fragment = parser.antlr.DebugAntlrGeneratorFragment {}
throws an exception:

61811 [main] ERROR enerator.CompositeGeneratorFragment  - [XtextSyntaxDiagnostic: null:1476 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1476 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1497 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1499 extraneous input ')' expecting ';', XtextSyntaxDiagnostic: null:1933 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1933 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1951 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1953 extraneous input ')' expecting ';']
java.lang.RuntimeException: [XtextSyntaxDiagnostic: null:1476 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1476 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1497 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1499 extraneous input ')' expecting ';', XtextSyntaxDiagnostic: null:1933 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1933 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1951 no viable alternative at input '(', XtextSyntaxDiagnostic: null:1953 extraneous input ')' expecting ';']
	at org.eclipse.xtext.generator.parser.antlr.DebugAntlrGeneratorFragment.prettyPrint(DebugAntlrGeneratorFragment.java:54)
	at org.eclipse.xtext.generator.parser.antlr.DebugAntlrGeneratorFragment.generate(DebugAntlrGeneratorFragment.java:44)
	at org.eclipse.xtext.generator.CompositeGeneratorFragment.generate(CompositeGeneratorFragment.java:81)
	at org.eclipse.xtext.generator.LanguageConfig.generate(LanguageConfig.java:69)
	at org.eclipse.xtext.generator.Generator.generate(Generator.java:351)
	at org.eclipse.xtext.generator.Generator.invokeInternal(Generator.java:125)
	at org.eclipse.emf.mwe.core.lib.AbstractWorkflowComponent.invoke(AbstractWorkflowComponent.java:126)
	at org.eclipse.emf.mwe.core.lib.Mwe2Bridge.invoke(Mwe2Bridge.java:34)
	at org.eclipse.emf.mwe.core.lib.AbstractWorkflowComponent.invoke(AbstractWorkflowComponent.java:201)
	at org.eclipse.emf.mwe2.runtime.workflow.AbstractCompositeWorkflowComponent.invoke(AbstractCompositeWorkflowComponent.java:35)
	at org.eclipse.emf.mwe2.runtime.workflow.Workflow.run(Workflow.java:19)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:97)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:73)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:64)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Runner.run(Mwe2Runner.java:55)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Launcher.run(Mwe2Launcher.java:74)
	at org.eclipse.emf.mwe2.launch.runtime.Mwe2Launcher.main(Mwe2Launcher.java:35)

Anyway, I've restructured the rule by moving as much of the rule calls and the '.' to the left. 

SelectedNamePathExpression returns SelectedNamePathExpression:
	SelectedNamePathIdentifier 
	(
		({SelectedNamePathConcatenation.left=current} '.' right=SelectedNamePathPostFix) |
		(({SelectedNamePathConcatenation.left=current} '.' right=SelectedNamePathIdentifier)
			(
				({SelectedNamePathConcatenation.left=current} '.' right=SelectedNamePathPostFix) |
				({SelectedNamePathConcatenation.left=current} '.' right=SelectedNamePathIdentifier)
			)?
		)
	); 

becomes

SelectedNamePathExpression returns SelectedNamePathExpression:
	{SelectedNamePathConcatenation} left=SelectedNamePathIdentifier '.'
	(
		right=SelectedNamePathPostFix |
		(right=SelectedNamePathIdentifier
			({SelectedNamePathConcatenation.left=current} '.'  
				(right=SelectedNamePathPostFix | right=SelectedNamePathIdentifier)
			)?
		)
	); 

Then the problem is gone.  

I also tried just left-factoring the '.'.  

SelectedNamePathExpression returns SelectedNamePathExpression:
	SelectedNamePathIdentifier '.' 
	(
		({SelectedNamePathConcatenation.left=current} right=SelectedNamePathPostFix) |
		(({SelectedNamePathConcatenation.left=current} right=SelectedNamePathIdentifier)
			('.'
				({SelectedNamePathConcatenation.left=current} right=SelectedNamePathPostFix) |
				({SelectedNamePathConcatenation.left=current} right=SelectedNamePathIdentifier)
			)?
		)
	); 


The problem then still persists.  So I'm still thinking I'm not dealing with a backtracking issue but with a rule action that should be performed earlier before presenting the model to the content assist code.