Community
Participate
Working Groups
Build ID: I20080330-1350 (3.4M6) Steps To Reproduce: 1. Please select the text on lines 5-7. 2. On the "Surround With" menu, choose "runnable". See crash below... --------------- Bug.java ------------------ class Bug{{ final Integer x=0, y=1; new Object(){ void method(){ if(x==y)//LINE 5 return; toString();//LINE 7 }}; }} --------------------------------------------- java.lang.ArrayIndexOutOfBoundsException: 1 at org.eclipse.jdt.internal.corext.refactoring.code.flow.LocalFlowInfo.<init>(LocalFlowInfo.java:24) at org.eclipse.jdt.internal.corext.refactoring.code.flow.FlowAnalyzer.endVisit(FlowAnalyzer.java:766) at org.eclipse.jdt.core.dom.SimpleName.accept0(SimpleName.java:148) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2478) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2525) at org.eclipse.jdt.core.dom.InfixExpression.accept0(InfixExpression.java:367) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2478) at org.eclipse.jdt.core.dom.ASTNode.acceptChild(ASTNode.java:2525) at org.eclipse.jdt.core.dom.IfStatement.accept0(IfStatement.java:188) at org.eclipse.jdt.core.dom.ASTNode.accept(ASTNode.java:2478) at org.eclipse.jdt.internal.corext.refactoring.code.flow.InOutFlowAnalyzer.perform(InOutFlowAnalyzer.java:40) at org.eclipse.jdt.internal.ui.text.correction.SurroundWith.getReads(SurroundWith.java:337) at org.eclipse.jdt.internal.ui.text.correction.SurroundWith.getVariableDeclarationReadsInside(SurroundWith.java:277) at org.eclipse.jdt.internal.ui.text.template.contentassist.SurroundWithTemplateProposal$SurroundWithTemplate.getVariableDeclarationReadsInside(SurroundWithTemplateProposal.java:79) at org.eclipse.jdt.internal.ui.text.correction.SurroundWith.getRewrite(SurroundWith.java:221) at org.eclipse.jdt.internal.ui.text.template.contentassist.SurroundWithTemplateProposal.createNewContext(SurroundWithTemplateProposal.java:244) at org.eclipse.jdt.internal.ui.text.template.contentassist.SurroundWithTemplateProposal.apply(SurroundWithTemplateProposal.java:198)
No regression, this is also in 3.3.2
reproduced in I20080715-1015 Should fix in 3.5
Created attachment 108057 [details] patch Here is a little patch
Comment on attachment 108057 [details] patch Thanks for the patch. I released the test case, but not the actual fix. While your fix fixed the reported problem, it's not at the right spot. The underlying problem is that LocalVariableIndex.perform(BodyDeclaration) does not follow the parent chain long enough if the affected declaration is inside an initializer. My fix in LocalVariableIndex also fixes the same problem for Extract Method and Inline Method.
And also thanks to Brian for reporting. It's always amazing to see how code can look like if it's not taken from a CS 101 script ;-)
I thought something like that but was not sure. At least the testcases helped you :) btw: are there any documents about the flow analysis stuff besides the code itself? It's a really interesting topic but not a little hard to have a good insight as a contributor. Thanks!
> btw: are there any documents about the flow analysis stuff besides the code > itself? It's a really interesting topic but not a little hard to have a good > insight as a contributor. No, all we have is the code. I agree it's a hard topic, and the pitfalls are many. My way to cope with the complexities in refactoring code it is to - have the type hierarchy and possible nesting hierarchy of all ASTNode subclasses in my head, and - use search and the call hierarchy to see who else uses a helper method, and - be suspicious whenever some code uses ASTNodes.getParent(ASTNode, ...): Usually, this is wrong, since it either misses possible parent types, or fails to stop at other node types (e.g. there's no enclosing method for a statement in an initializer, and walking further up would not be correct).
> use search and the call hierarchy to see who else uses a helper method That's how I do it at the moment ;) Thanks anyway!