Bug 199668 - IAE in ASTNode.setSourceRange while editing a class
Summary: IAE in ASTNode.setSourceRange while editing a class
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.3.1   Edit
Assignee: David Audel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 200080 201285 (view as bug list)
Depends on: 203579
Blocks:
  Show dependency tree
 
Reported: 2007-08-12 06:14 EDT by Frederic Fusier CLA
Modified: 2007-10-29 08:27 EDT (History)
5 users (show)

See Also:


Attachments
Woraround and regression test (4.97 KB, patch)
2007-08-16 07:47 EDT, Jerome Lanneluc CLA
no flags Details | Diff
Proposed patch for 3.3.1 (6.02 KB, patch)
2007-08-30 09:42 EDT, David Audel CLA
no flags Details | Diff
Proposed fix + regression test (8.98 KB, patch)
2007-09-20 15:12 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2007-08-12 06:14:14 EDT
Using build 3.4 M1, I got following IllegalArgumentException in ASTNode:

VariableDeclarationFragment(ASTNode).setSourceRange(int, int) line: 2609	
ASTConverter.convertToVariableDeclarationFragment(LocalDeclaration) line: 3012	
ASTConverter.convert(ForStatement) line: 1743	
ASTConverter.convert(Statement) line: 2401	
ASTConverter.convert(AbstractMethodDeclaration) line: 511	
ASTConverter.buildBodyDeclarations(TypeDeclaration, AbstractTypeDeclaration) line: 179	
ASTConverter.convert(TypeDeclaration) line: 2661	
ASTConverter.convert(CompilationUnitDeclaration, char[]) line: 1248	
AST.convertCompilationUnit(int, CompilationUnitDeclaration, char[], Map, boolean, CompilationUnit, int, IProgressMonitor) line: 278	
CompilationUnit.buildStructure(OpenableElementInfo, IProgressMonitor, Map, IResource) line: 201	
CompilationUnit(Openable).generateInfos(Object, HashMap, IProgressMonitor) line: 229	
CompilationUnit(JavaElement).openWhenClosed(Object, IProgressMonitor) line: 505	
CompilationUnit.makeConsistent(int, boolean, int, HashMap, IProgressMonitor) line: 1020	
ReconcileWorkingCopyOperation.makeConsistent(CompilationUnit) line: 170	
ReconcileWorkingCopyOperation.executeOperation() line: 89	
ReconcileWorkingCopyOperation(JavaModelOperation).run(IProgressMonitor) line: 720	
ReconcileWorkingCopyOperation(JavaModelOperation).runOperation(IProgressMonitor) line: 780	
CompilationUnit.reconcile(int, int, WorkingCopyOwner, IProgressMonitor) line: 1169	

Test case to reproduce this problem:
public class Test {
	void foo() {
		for (int i=0,; i<10; i++) {
		}
	}
}

May be a recovery problem as there's a syntax error (the ',' after the i=0).

Looks similar than bug 144450 as the line where the exception occurred matches exactly the line of ASTNode 3.2 RC6 as described in this bug. However, I entered a new bug because the exception occurred during conversion of a VariableDeclarationFragment although it was in a method body declaration for bug  144450...
Comment 1 Frederic Fusier CLA 2007-08-12 06:17:57 EDT
Increase severity to 'major' as the reconciling stops to work in the edited file which is definitely annoying. I was obliged to use the debugger to find the origin of the problem and fix the syntax error to have the reconciler back working, so I guess that standard user would never get it...
Comment 2 Jerome Lanneluc CLA 2007-08-16 06:11:09 EDT
*** Bug 200080 has been marked as a duplicate of this bug. ***
Comment 3 Jerome Lanneluc CLA 2007-08-16 07:47:58 EDT
Created attachment 76210 [details]
Woraround and regression test
Comment 4 Jerome Lanneluc CLA 2007-08-16 10:54:25 EDT
Workaround and test released in HEAD.

David, please check the workaround and make the necessary modifications (or rewrite the fix if needed).
Comment 5 Frederic Fusier CLA 2007-08-27 12:55:15 EDT
*** Bug 201285 has been marked as a duplicate of this bug. ***
Comment 6 Philipe Mulet CLA 2007-08-28 06:18:51 EDT
+1 for 3.3.1
Comment 7 David Audel CLA 2007-08-30 09:41:18 EDT
The workaround works but we can do a better simple fix.

The bug is caused by the statements recovery.
The syntax error is: Syntax error on token ",", VariableDeclarator expected after this token.
Then a fake Identifier is added by recovery as a VariableDeclarator and the 'sourceEnd' of the corresponding ast node is equals to his 'sourceStart - 1'.
ASTConverter#convertToVariableDeclarationFragment use this 'sourceEnd' to find the position of the following comma or semicolon but in this case the previous comma is found instead and this cause the IllegalArgumentException.

For 3.3.1, to do the smallest and less dangerous fix we should use the 'sourceStart' instead of the the 'sourceEnd' when the variable name is a fake token.

For 3.4, we could do a more general fix because there is some other inconsistency in convertToVariableDeclarationFragment() behavior that require a bigger fix.
Comment 8 David Audel CLA 2007-08-30 09:42:34 EDT
Created attachment 77374 [details]
Proposed patch for 3.3.1
Comment 9 Jerome Lanneluc CLA 2007-08-30 10:04:19 EDT
+1 for 3.3.1 with the patch from comment 8
Comment 10 David Audel CLA 2007-08-30 10:12:30 EDT
Released for 3.3.1.

Test added
  ASTConverterTest2#test0608() -> test0610()
Comment 11 Jerome Lanneluc CLA 2007-09-03 07:48:39 EDT
Verified for 3.3.1 using M20070831-2000.
Comment 12 David Audel CLA 2007-09-17 07:27:59 EDT
Reopen to fix the bug in 3.4
Comment 13 David Audel CLA 2007-09-19 05:53:47 EDT
The fix for the bug 203579 will be probably to remove whitespace from the fragment. So the call to retrievePositionBeforeNextCommaOrSemiColon() will be removed and this bug will disappear.

I will remove the released workaround from 3.4 once the bug 203579 will be fixed
Comment 14 Olivier Thomann CLA 2007-09-20 15:12:49 EDT
Created attachment 78886 [details]
Proposed fix + regression test
Comment 15 Olivier Thomann CLA 2007-09-20 15:13:41 EDT
David,

Please review last patch. Thanks.
Comment 16 David Audel CLA 2007-09-24 07:40:01 EDT
The patch  for 3.4 is good. It remove the useless workaround.

Released for 3.4M3.

Test updated
  ASTConverterTest2#test0608()
Comment 17 Jerome Lanneluc CLA 2007-10-29 08:23:37 EDT
Verified for 3.4M3 using I20071029-0019. Resetting target to 3.3.1 as this is the lowest target where the fix can be found.