Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 326948 - ProposalProvider's methods not called as expected
Summary: ProposalProvider's methods not called as expected
Status: CLOSED FIXED
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 1.0.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: M3   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 327142
Blocks:
  Show dependency tree
 
Reported: 2010-10-04 12:44 EDT by Henrik Lindberg CLA
Modified: 2017-09-19 17:27 EDT (History)
2 users (show)

See Also:
sebastian.zarnekow: helios+
sebastian.zarnekow: indigo+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Henrik Lindberg CLA 2010-10-04 12:44:12 EDT
In b3, the Proposal Provider is not called as expected for this rule:
InfixExpression returns be::BExpression :
	CallExpression ( 
	  	({be::BCallFeature.funcExpr=current} "." name=ID_or_KW  "(" (parameterList = ParameterList)? ")")
	  |	({be::BAtExpression.objExpr=current} '[' indexExpr=Expression ']' )
	  | ({be::BFeatureExpression.objExpr=current} "." featureName=ID_or_KW)
	)*;

With b3 input like this:
   function foo() { "".| }

Where | is the caret where ctrl+space is pressed, the following complete methods are called:

completeInfixExpression_Name for model: BChainedExpressionImpl
complete_ID_or_KW for model: BChainedExpressionImpl
complete_ID_or_KW for model: BChainedExpressionImpl

Note that:
- the model object is BChainedExpressionImpl, which is the container of the "".feature expression
- no call is made to completeInfixExpression.FeatureName

If the input is modified to:
    function foo() { "".| ; }

(i.e. a semicolon at the end to complete the expression, the called complete methods gets the container of the BChainedExpressionImpl which is a B3FunctionImpl.

Seems like two separate bugs:
- the wrong model instance is computed and passed to the complete methods
- the completeInfixExpression_FeatureName is never called (not even with wrong model)

I suspected that the problem could be that there are two alternatives after the ".", but regenerating the grammer without the second alternative still produced the wrong model instances in the complete calls.
Comment 1 Henrik Lindberg CLA 2010-10-07 09:17:40 EDT
Raising the importance of this, it really prevents me from delivering an important feature.
Comment 2 Sven Efftinge CLA 2010-10-07 09:50:05 EDT
I think the problem with the wrong model instance is due to https://bugs.eclipse.org/bugs/show_bug.cgi?id=276619
Comment 3 Sven Efftinge CLA 2010-10-07 09:51:58 EDT
Actually the model should be the string literal.
Comment 4 Henrik Lindberg CLA 2010-10-07 10:04:49 EDT
(In reply to comment #3)
> Actually the model should be the string literal.

What I need to find is the BCallFeature.funcExpr, or BFeatureExpression.objExpr - so if the model is the String literal, I would find this in it's eContainer - right?

Is there some way that you can think of that allows me to work around the problem?
Comment 5 Sven Efftinge CLA 2010-10-07 10:20:53 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > Actually the model should be the string literal.
> 
> What I need to find is the BCallFeature.funcExpr, or BFeatureExpression.objExpr

There's no chance you can get them, because the parser is in look-ahad mode, since for the dot.
If you only had one alternative starting with a dot, you'ld get the expected model element, because the parser had already executed the corresponding action.

If you could refactor your InfixExpression to something like the following things would work (but you wouldn't have two different types for feature BCallE... and BFeatureE...:

InfixExpression returns be::BExpression :
    CallExpression ( 
          {be::BCallFeature.funcExpr=current} "." name=ID_or_KW ( "("
(parameterList = ParameterList)? ")")?
      |    {be::BAtExpression.objExpr=current} '[' indexExpr=Expression ']' 
    )*;

> - so if the model is the String literal, I would find this in it's eContainer -
> right?

Sorry, it is not the StringLiteral, beause that one has not been set to any containment reference in your resource.
Comment 6 Henrik Lindberg CLA 2010-10-07 12:35:42 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Actually the model should be the string literal.
> > 
> > What I need to find is the BCallFeature.funcExpr, or BFeatureExpression.objExpr
> 
> There's no chance you can get them, because the parser is in look-ahad mode,
> since for the dot.
> If you only had one alternative starting with a dot, you'ld get the expected
> model element, because the parser had already executed the corresponding
> action.
> 
> If you could refactor your InfixExpression to something like the following
> things would work (but you wouldn't have two different types for feature
> BCallE... and BFeatureE...:
> 
> InfixExpression returns be::BExpression :
>     CallExpression ( 
>           {be::BCallFeature.funcExpr=current} "." name=ID_or_KW ( "("
> (parameterList = ParameterList)? ")")?
>       |    {be::BAtExpression.objExpr=current} '[' indexExpr=Expression ']' 
>     )*;
> 
> > - so if the model is the String literal, I would find this in it's eContainer -
> > right?
> 
> Sorry, it is not the StringLiteral, beause that one has not been set to any
> containment reference in your resource.

I tried that but I then get the container of the to be returned InfixExpression as the model element. I need to get the CallExpression. Any suggested work around for that?
Comment 7 Sven Efftinge CLA 2010-10-07 13:32:14 EDT
(In reply to comment #6)
> I tried that but I then get the container of the to be returned InfixExpression
> as the model element.

Ye, that's as I said, because the parser hasn't yet decided which alternative to take.

> I need to get the CallExpression. Any suggested work
> around for that?

Only the recommendation to refactor your rule and give up the two distinct types for features calls and method invocations.
Comment 8 Henrik Lindberg CLA 2010-10-07 18:39:10 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > I tried that but I then get the container of the to be returned InfixExpression
> > as the model element.
> 
> Ye, that's as I said, because the parser hasn't yet decided which alternative
> to take.
> 
> > I need to get the CallExpression. Any suggested work
> > around for that?
> 
> Only the recommendation to refactor your rule and give up the two distinct
> types for features calls and method invocations.

That is what I tried - i.e. remove the second rule that also starts with "." - the result is the same.
I am starting to suspect that this has to do with the definition of REAL which can start without a leading digit and is expressed as a rule instead of a terminal. - i.e. parser does not know if this is a feature reference or the start of a REAL value.

I made the leading INT in real required, but I still had the same issue.
Only after also changing the name to be optional (like below) do I get the expected model instance BCallFeature:

InfixExpression returns be::BExpression :
    CallExpression ( 
          {be::BCallFeature.funcExpr=current} "."  (name=ID_or_KW  "("
(parameterList = ParameterList)? ")")?
      |    {be::BAtExpression.objExpr=current} '[' indexExpr=Expression ']' 
    )*;

This seems like an issue, but I am at least able to work around the problem now.
Comment 9 Sebastian Zarnekow CLA 2010-10-09 13:35:36 EDT
I'll add JavaDoc to the ContentAssistContext to describe the semantics of the various properties. Furthermore I'll make the most likely previous model available for such cases. 

At least a simplified example does not reproduce the missing invocation of completeInfixExpression_FeatureName.
Comment 10 Sebastian Zarnekow CLA 2010-10-11 10:09:19 EDT
Pushed to master.
Please note that complex backtracking scenarious may still confuse the content assistant.

It's likely that the fix will be backported to 1.0.2
Comment 11 Sebastian Zarnekow CLA 2010-10-11 10:11:06 EDT
Reopened as a reminder for the maintenance branch.
Comment 12 Sebastian Zarnekow CLA 2010-11-09 09:35:25 EST
Backported fix to Helio_Maintenance
Comment 13 Karsten Thoms CLA 2017-09-19 17:15:34 EDT
Closing all bugs that were set to RESOLVED before Neon.0
Comment 14 Karsten Thoms CLA 2017-09-19 17:27:00 EDT
Closing all bugs that were set to RESOLVED before Neon.0