Summary: | Problem in nodes removal/replacement for ASTRewrite.rewriteAST(...). Suspecting incorrect offsets calculation | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | smakady | ||||||||
Component: | Core | Assignee: | Olivier Thomann <Olivier_Thomann> | ||||||||
Status: | VERIFIED INVALID | QA Contact: | |||||||||
Severity: | normal | ||||||||||
Priority: | P3 | CC: | satyam.kandula, srikanth_sankaran | ||||||||
Version: | 3.7 | ||||||||||
Target Milestone: | 3.7 M1 | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows XP | ||||||||||
Whiteboard: | |||||||||||
Attachments: |
|
Description
smakady
2010-06-29 05:12:55 EDT
Please follow up, Thanks. Could you please attach the test case rather than pasting it in? That would preserve the correct encoding for source files ? What encoding are you using ? Thanks. Created attachment 173035 [details]
The test file that gets screwed on certain nodes removal
I am attaching the test case file that is causing the problem. I set the Eclipse workspace encoding to UTF-8 for all the java files, so this Java file is opened in UTF-8 within Eclipse when I run my code (which causes the problems). Does that answer your question about encoding?
yes, this is fine. Now that I have the test case, I'll try to reproduce once I get the code that is calling: ListRewrite list = rewrite.getListRewrite(s.getParent(), Block.STATEMENTS_PROPERTY); //where s is the statement to remove list.remove(s, null); Thanks. In fact the code size it large, so I tried to put the part that is directly preceding the node removal to make it more readable, and decrease the confusion. The idea is that I look for certain method invocations, retrieve their parent statements, and then remove those statements, or comment them out. The test method that I wanted to apply that idea on, was "testFieldAndFormat", and the invocations that I wanted to remove were for Util.getFieldAndFormat(..), which is invoked 8 times within testFieldAndFormat. I successfully captured the 8 statements containing those 8 method invocations, but the problem happens on deletion. I am pasting that code below. Thanks. import java.util.ArrayList; import java.util.Vector; import org.eclipse.jdt.core.IMember; import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.ASTVisitor; import org.eclipse.jdt.core.dom.Block; import org.eclipse.jdt.core.dom.IMethodBinding; import org.eclipse.jdt.core.dom.IfStatement; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.Statement; import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jdt.core.dom.rewrite.ListRewrite; public class FindAndRemoveMethodCalls { private static final String REWRITE = "NODE_REWRITE"; public void methodB(ASTNode sourceMethodNode /*Stands for the test method to be modified */, ASTNode node /*Stands for the TestClass's ASTNode*/, String callToRemove, final IMember targetMemberToRemove) { final Vector<ASTNode> matchedInvocations = new Vector<ASTNode>(); final ArrayList<MethodInvocation> removedMethodInvocationNodes = new ArrayList<MethodInvocation>(); Vector<ASTNode> doneContainers = new Vector<ASTNode>(); // null SMN just means the method isn't in the node if (sourceMethodNode != null) sourceMethodNode.accept(new ASTVisitor() { public boolean visit(MethodInvocation node) { IMethodBinding mBinding = node.resolveMethodBinding(); if (mBinding == null) { ; } if (mBinding != null) // A user-defined comparison to know if the current invocation matches the target call to remove //For the test method testFieldAndFormat, I wanted to remove all the statements that include invocations of Util.getFieldAndFormat(..) method if (ModelUtilities.matchMethod(mBinding, (IMethod) targetMemberToRemove)) { matchedInvocations.add(node); removedMethodInvocationNodes.add(node); } return super.visit(node); } }); ASTRewrite rewrite = (ASTRewrite) node.getProperty(REWRITE); /*This property "REWRITE" is set in an earlier part of the code by the following: ASTRewrite newRewrite = ASTRewrite.create(node.getAST()); */ if(matchedInvocations.isEmpty()) return; //The code works properly till here, where it captures all the method invocations inside the testFieldAndFormat test case for (ASTNode n : matchedInvocations) { Statement s = getContainingStatement(n); if (!doneContainers.contains(s)) { doneContainers.add(s); if (s instanceof IfStatement) ; //Option 1 //START: Code for node comment out ASTNode placeHolder = rewrite.createStringPlaceholder( "/** Removed node: \n", ASTNode.EMPTY_STATEMENT); ListRewrite list = rewrite.getListRewrite(s.getParent(), Block.STATEMENTS_PROPERTY); list.insertBefore(placeHolder, s, null); placeHolder = rewrite.createStringPlaceholder("\n*/", ASTNode.EMPTY_STATEMENT); list = rewrite.getListRewrite(s.getParent(), Block.STATEMENTS_PROPERTY); list.insertAfter(placeHolder, s, null); //END code for node comment out //Option 2 //START: Code for node removal //ListRewrite list = rewrite.getListRewrite(s.getParent(),Block.STATEMENTS_PROPERTY); //where s is the statement to remove //list.remove(s, null); //END: Code for node removal } } } private Statement getContainingStatement(ASTNode n) { if (n instanceof Statement) return (Statement) n; else return getContainingStatement(n.getParent()); } } I just want to add that I am using Eclipse version: 3.5.2, and JDT version: 3.5.2.v_981_R35x, not 3.7. What is the code for: ModelUtilities.matchMethod(mBinding,(IMethod) targetMemberToRemove) ? Thanks. Would it be possible to speed up the resolution of this bug to get a self-containing example that produces the wrong output? So some code that calls the methodB(...) and also all the necessary code to compile the given code snippet (in comment 5). Thanks. Created attachment 173249 [details]
A plugin project that shows the shows the bug
Hello,
Attached, please find a plug-in project that I created and that shows the bug. The project has one view "ASTBug view" in the "ASTBug" category. In that view, there is one action called "BugAction". The project assumes that you have JabRef2.5 project source code (and test code) imported in the runtime eclipse instance. Also, the project assumes that the source code and test code are grouped in the same packages. For instance, UtilTest.java will be in the package net.sf.jabref, not in the package test.net.sf.jabref
Once you click on the "Bug action", it does the following:
1. Searches for the "UtilTest.java" compilation unit
2. Located the MethodDeclaration node for "testFieldAndFormat" test method
3. Retrieves all the method invocations for "getFieldAndFormat".
4. Removes those invocations.
Hello, I just want to know if you managed to reproduce the bug or not, and if this problem would it be a high priority to solve. If it would not be solved soon, is there a certain workaround to be able to remove nodes without screwing the file's AST? Thanks. I'll work on it this week. I had a couple of days off. Reproduced. I am investigating. Created attachment 174402 [details]
Patched version of writeCUDown(..)
Your problem is in the writeCUDown(..). When you retrieve the source of the file, you don't take the encoding into account so you apply the rewrite on a different source than the source used to create the rewrite. This leads to the problem you are having with a corrupted source.
The attached version works, but it hardcoded the encoding of the file.
I recommend that you save the ICompilationUnit source when you create the CompilationUnit node. Like this you will make sure that both are consistent.
I will close as INVALID as the problem doesn't come from ASTRewrite.
Closing as INVALID. Let me know if you have any question. Thanks a lot for your detailed reply. It helped a lot. I just have a quick question: If I run into similar problems like that, what would be the appropriate forum to post to (so that I don't open it as a bug). For this one, I posted it before to this forum (http://www.eclipse.org/forums/index.php?t=thread&frm_id=13&) but I don't if it was the correct forum, or there is a more specific forum. Many thanks for your help. (In reply to comment #15) > Thanks a lot for your detailed reply. It helped a lot. I just have a quick > question: If I run into similar problems like that, what would be the > appropriate forum to post to (so that I don't open it as a bug). For this one, > I posted it before to this forum > (http://www.eclipse.org/forums/index.php?t=thread&frm_id=13&) but I don't if it > was the correct forum, or there is a more specific forum. This is the right place to ask questions. I just haven't checked it for a while. I'll check it more often starting today. > Many thanks for your help. You're welcome. In general, make sure you are making modifications on the same source than the one used to create the AST. Verified for 3.7M1 |