Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 315660 - [content assist] Insert of assist proposal fails inside nested object literal
Summary: [content assist] Insert of assist proposal fails inside nested object literal
Status: RESOLVED FIXED
Alias: None
Product: JSDT
Classification: WebTools
Component: General (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5.1   Edit
Assignee: Project Inbox CLA
QA Contact: Chris Jaun CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-03 15:43 EDT by Chris Jaun CLA
Modified: 2013-08-14 15:51 EDT (History)
4 users (show)

See Also:
cmjaun: review+


Attachments
proposed patch (7.03 KB, patch)
2012-05-15 02:33 EDT, Wooyoung Cho CLA
no flags Details | Diff
testcase (4.47 KB, application/zip)
2012-05-15 02:33 EDT, Wooyoung Cho CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Jaun CLA 2010-06-03 15:43:16 EDT
1. Create a JS file in a JS Project.
2. Add the following...

var point = new Object();
point.x = 2.3;
point.y = -1.2;

var square = {"upperLeft": {x:point.x, y:point.y}, 'upperRight': {x:(p)} };

3. Attempt content assist directly after the 'p'.

A list of proposals pops up, including the proposal for "point".

4. Double click to select the proposal. Nothing is inserted.
Comment 1 Chris Jaun CLA 2010-08-25 14:50:42 EDT
Can be reproduced with a simple statement:

var ptt = {};
(p*)

attempt content assist on the *

The error seems to be in updateSourcePosition() in the parser which changes the start location of the completionNode to the (.
Comment 2 Wooyoung Cho CLA 2012-05-15 02:33:03 EDT
Created attachment 215614 [details]
proposed patch

This patch adjusts the replacement position of the completion proposal.

The start position of ast node includes parentheses('('). For example the start position of '(p)' expression is set to '(' not 'p'.
As Chris said, the position is determined while parsing by updateSourcePosition(). And Content Assist for '(p)' fails because AbstractJavaCompletionProposal.validate() checks the prefix of ast node with the proposal which are '(p' and 'p' respectively.

I'm not sure whether updateSourcePosition() is wrong or not. But ASTConverter which converts internal AST to public AST is handling this issue explicitly by introducing ParenthesizedExpression node. So the proposed patch handles this issue at the use site(content assist) not at the generation site(parser).

This patch sets the new replacement position of the proposal by searching actual start position backward from the cursor position.
Comment 3 Wooyoung Cho CLA 2012-05-15 02:33:58 EDT
Created attachment 215615 [details]
testcase
Comment 4 Jon Houghton CLA 2013-06-19 11:38:36 EDT
I have reviewed this patch and recommend that it be included in 3.5.1.  Chris, can you officially review it.
Comment 5 Chris Jaun CLA 2013-07-09 14:23:53 EDT
Wooyoung,

Eclipse is putting in some updated policies regarding accepting patches.

See point #1 and #2 in the Bugzilla section at this link: http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions

If you haven't yet, you need to sign the Contributor License Agreement for us to pull in your patch.

Thanks,
Chris
Comment 6 Chris Jaun CLA 2013-08-14 15:51:20 EDT
Pushed to 3.5.1 and master.