Bug 207754

Summary:

[AST][DOM] source range of ParenthesizedExpression does not include the parenthesis

Product:

[Eclipse Project] JDT

Reporter:

Benno Baumgartner <benno.baumgartner>

Component:

Core

Assignee:

Frederic Fusier <frederic_fusier>

Status:

VERIFIED FIXED

QA Contact:

Severity:

major

Priority:

CC:

daniel_megert, eric_jodet, martinae, Olivier_Thomann, roland.illig

Version:

3.3

Target Milestone:

3.4 M4

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
test case for a)	none
Proposed fix	none
Regression test	none
Proposed patch	none

Description Benno Baumgartner

2007-10-29 07:05:36 EDT

I20071029-0010

Fup of bug 207332

Given:
package test1;
public class E1 {
	protected String foo(String string) {
		return ("" + string + "") + ("");
	}   
}
Several test cases:

a) 
1. Run clean up with option 'remove extra parenthesis'
Is:
return ("" + string + "") + "";
Should:
return "" + string + "" + ""; 

b) 
1. Select '("" + string + "")'
2. Ctrl-1
3. Remove extra parenthesis
Is:
return ("" + string + "") + ("");
Should:
return "" + string + "" + ("");

c)
1. Set caret at + between the two parenthesized expressions
2. Ctrl-1
3. Exchange left and right operant
Is:
 return ("" + "" + string + "";
Should: 
 return ("") + ("" + string + "");

I've verified my code. In test case b I call (with pExpr=("" + string + ""))

ASTNode moveTarget= rewrite.createMoveTarget(pExpr.getExpression());
rewrite.replace(pExpr, moveTarget, null);

This results in following text edit:
[{MultiTextEdit} [85,16]
  {MoveTargetEdit} [85,0]
  {DeleteEdit} [85,16]
    {MoveSourceEdit} [85,16]]

But I guess MoveSourceEdit should be [86,14] ?

Comment 1 Benno Baumgartner

2007-10-29 07:18:24 EDT

Created attachment 81436 [details]
test case for a)

Comment 2 Martin Aeschlimann

2007-10-29 07:34:34 EDT

Looks like a AST problem: The ParentheziedExpression's range does not include the parenthesis

Comment 3 Olivier Thomann

2007-10-29 08:30:08 EDT

I'll take a look.

Comment 4 Olivier Thomann

2007-10-29 08:34:30 EDT

Reproduced.
I am investigating.

Comment 5 Olivier Thomann

2007-10-29 11:25:54 EDT

Created attachment 81468 [details]
Proposed fix

Comment 6 Olivier Thomann

2007-10-29 11:26:13 EDT

Created attachment 81469 [details]
Regression test

Comment 7 Olivier Thomann

2007-10-29 11:26:52 EDT

Frederic,

Please review.

Comment 8 Frederic Fusier

2007-10-31 12:51:17 EDT

Created attachment 81740 [details]
Proposed patch

I would prefer to add new constructor on BinaryExpression and CombinedBinaryExpression to avoid missing the duplication of this important information (sourceStart, sourceEnd, bits) anywhere this pattern is encountered. 

This does not add extra time cost and, IMO, make the code safer. What do you think?

Comment 9 Olivier Thomann

2007-11-05 20:04:02 EST

+1.
Patch look good.

Comment 10 Frederic Fusier

2007-11-06 06:12:08 EST

Released for 3.4M4 in HEAD stream.

Comment 11 Eric Jodet

2007-12-12 03:00:02 EST

Verified for 3.4 M4 using build I20071211-0010

Comment 12 Roland Illig

2008-08-11 03:16:50 EDT

Will this bug be fixed in an update to Eclipse 3.3? Or do I have to upgrade to 3.4?