Attachment 172349 Details for Bug 306524 – improved fix + tests

[patch] improved fix + tests

patch306524.txt (text/plain), 18.46 KB, created by Ayushman Jain

on 2010-06-21 14:01:44 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ayushman Jain

Created: 2010-06-21 14:01:44 EDT

Size: 18.46 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: dom/org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java,v
>retrieving revision 1.74
>diff -u -r1.74 ASTRewriteAnalyzer.java
>--- dom/org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java	12 Apr 2010 16:07:26 -0000	1.74
>+++ dom/org/eclipse/jdt/internal/core/dom/rewrite/ASTRewriteAnalyzer.java	21 Jun 2010 17:37:21 -0000
>@@ -585,6 +585,32 @@
> 					ASTNode node= (ASTNode) currEvent.getOriginalValue();
> 					TextEditGroup editGroup= getEditGroup(currEvent);
> 					int currEnd= getEndOfNode(node);
>+					// https://bugs.eclipse.org/bugs/show_bug.cgi?id=306524
>+					// Check for leading comments that are not part of extended range, and prevent them
>+					// from getting removed.
>+					try {
>+						TokenScanner scanner = getScanner();
>+						int newOffset = prevEnd;
>+						int extendedOffset = getExtendedOffset(node);
>+						// Try to find the end of the last comment which is not part of extended source
>+						// range of the node.
>+						while (TokenScanner.isComment(scanner.readNext(newOffset, false))) {
>+							int tempOffset = scanner.getNextEndOffset(newOffset, false);
>+							// check whether the comment is part of extended source range of the node.
>+							// If it is then we need to stop.
>+							if (tempOffset < extendedOffset) {
>+								newOffset = tempOffset;
>+							} else {
>+								break;
>+							}
>+						}
>+						if (currPos < newOffset) {
>+							currPos = extendedOffset;
>+						} 
>+						prevEnd = newOffset;		
>+					} catch (CoreException e) {
>+						// ignore
>+					}
> 					if (i > lastNonDelete && separatorState == EXISTING) {
> 						// is last, remove previous separator: split delete to allow range copies
> 						doTextRemove(prevEnd, currPos - prevEnd, editGroup); // remove separator
>@@ -598,6 +624,25 @@
> 						
> 						// remove element and next separator
> 						int end= getStartOfNextNode(nextIndex, currEnd); // start of next
>+						// https://bugs.eclipse.org/bugs/show_bug.cgi?id=306524
>+						// Check for trailing comments that are not part of extended range, and prevent them
>+						// from getting removed.
>+						try {
>+							TokenScanner scanner = getScanner();
>+							int nextToken= scanner.readNext(currEnd, false);
>+							if (TokenScanner.isComment(nextToken)) {
>+								// the separator also has comments that are not part of extended
>+								// source range of this node or the next node. So dont remove the separator
>+								if (end != scanner.getNextStartOffset(currEnd, false)) {
>+									// If this condition were true, comments just found as part of the separator would've basically been
>+									// part of the extended source range of the next node. So 'end' wud've safely been set to the correct position
>+									// and no change is needed.
>+									end = currEnd;
>+								}
>+							}
>+						} catch (CoreException e) {
>+							// ignore
>+						}
> 						doTextRemoveAndVisit(currPos, currEnd - currPos, node, getEditGroup(currEvent)); // remove node
> 						if (mustRemoveSeparator(currPos, i)) {
> 							doTextRemove(currEnd, end - currEnd, editGroup); // remove separator
>#P org.eclipse.jdt.core.tests.model
>Index: src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingInsertBoundTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingInsertBoundTest.java,v
>retrieving revision 1.16
>diff -u -r1.16 ASTRewritingInsertBoundTest.java
>--- src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingInsertBoundTest.java	28 Apr 2009 17:49:28 -0000	1.16
>+++ src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingInsertBoundTest.java	21 Jun 2010 17:37:24 -0000
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2009 IBM Corporation and others.
>+ * Copyright (c) 2000, 2010 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -19,7 +19,6 @@
> 
> import org.eclipse.jdt.core.ICompilationUnit;
> import org.eclipse.jdt.core.IPackageFragment;
>-
> import org.eclipse.jdt.core.dom.AST;
> import org.eclipse.jdt.core.dom.ASTNode;
> import org.eclipse.jdt.core.dom.CompilationUnit;
>@@ -522,8 +521,15 @@
> 		buf.append("public class C {\n");
> 		buf.append("//c1\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public void foo2();\n");
> 		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
>+		buf.append("\n");
> 		buf.append("//c4\n");
> 		buf.append("}\n");
> 
>@@ -573,6 +579,10 @@
> 		buf.append("\n");
> 		buf.append("//c2\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
> 		buf.append("    public void foo3();\n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
>@@ -623,6 +633,12 @@
> 		buf.append("\n");
> 		buf.append("    public void foo1();\n");
> 		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
>+		buf.append("\n");
> 		buf.append("//c4\n");
> 		buf.append("}\n");
> 
>@@ -668,6 +684,17 @@
> 		buf= new StringBuffer();
> 		buf.append("package test1;\n");
> 		buf.append("public class C {\n");
>+		buf.append("//c1\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
>+		buf.append("    \n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
> 		buf.append("}\n");
>@@ -726,6 +753,10 @@
> 		buf.append("\n");
> 		buf.append("    void new1();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public void foo2();\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -797,6 +828,10 @@
> 		buf.append("\n");
> 		buf.append("    void new2();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public void foo2();\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -868,6 +903,10 @@
> 		buf.append("\n");
> 		buf.append("    void new2();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
> 		buf.append("    public void foo3();\n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
>@@ -928,6 +967,10 @@
> 		buf.append("\n");
> 		buf.append("    int new1;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public int x2;\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -998,6 +1041,10 @@
> 		buf.append("\n");
> 		buf.append("    int new2;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public int x2;\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -1069,6 +1116,10 @@
> 		buf.append("\n");
> 		buf.append("    int new2;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
> 		buf.append("    public int x3;\n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
>@@ -1131,6 +1182,10 @@
> 		buf.append("\n");
> 		buf.append("    void new1();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public void foo2();\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -1196,6 +1251,10 @@
> 		buf.append("\n");
> 		buf.append("    int new1;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public int x2;\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -1269,6 +1328,10 @@
> 		buf.append("\n");
> 		buf.append("    void new2();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public void foo2();\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -1343,6 +1406,10 @@
> 		buf.append("\n");
> 		buf.append("    int new2;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    public int x2;\n");
> 		buf.append("\n");
> 		buf.append("//c3\n");
>@@ -1409,6 +1476,14 @@
> 		buf.append("\n");
> 		buf.append("    void new1();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
> 		buf.append("    void new2();\n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
>@@ -1468,6 +1543,14 @@
> 		buf.append("\n");
> 		buf.append("    int new1;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
> 		buf.append("    int new2;\n");
> 		buf.append("\n");
> 		buf.append("//c4\n");
>@@ -1530,8 +1613,16 @@
> 		buf.append("\n");
> 		buf.append("    void new1();\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    void new2();\n");
> 		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
> 		buf.append("//c4\n");
> 		buf.append("}\n");
> 
>@@ -1591,8 +1682,16 @@
> 		buf.append("\n");
> 		buf.append("    int new1;\n");
> 		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("//c2\n");
>+		buf.append("\n");
> 		buf.append("    int new2;\n");
> 		buf.append("\n");
>+		buf.append("//c3\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
> 		buf.append("//c4\n");
> 		buf.append("}\n");
> 
>@@ -1650,10 +1749,14 @@
> 		rewrite.remove(f1, null);
> 
> 		String preview= evaluateRewrite(cu, rewrite);
>-
>+		// Note that c1 is absent because source range has been calculated from c1
> 		buf= new StringBuffer();
> 		buf.append("package test1;\n");
> 		buf.append("public class C {\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("    //c2\n");
>+		buf.append("\n");
> 		buf.append("    public int x2;\n");
> 		buf.append("\n");
> 		buf.append("    //c3\n");
>Index: src/org/eclipse/jdt/core/tests/rewrite/modifying/ASTRewritingModifyingRemoveTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/rewrite/modifying/ASTRewritingModifyingRemoveTest.java,v
>retrieving revision 1.9
>diff -u -r1.9 ASTRewritingModifyingRemoveTest.java
>--- src/org/eclipse/jdt/core/tests/rewrite/modifying/ASTRewritingModifyingRemoveTest.java	28 Apr 2009 17:46:12 -0000	1.9
>+++ src/org/eclipse/jdt/core/tests/rewrite/modifying/ASTRewritingModifyingRemoveTest.java	21 Jun 2010 17:37:24 -0000
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2009 IBM Corporation and others.
>+ * Copyright (c) 2000, 2010 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -16,8 +16,11 @@
> 
> import org.eclipse.jdt.core.ICompilationUnit;
> import org.eclipse.jdt.core.IPackageFragment;
>-
>-import org.eclipse.jdt.core.dom.*;
>+import org.eclipse.jdt.core.dom.AST;
>+import org.eclipse.jdt.core.dom.Block;
>+import org.eclipse.jdt.core.dom.CompilationUnit;
>+import org.eclipse.jdt.core.dom.MethodDeclaration;
>+import org.eclipse.jdt.core.dom.TypeDeclaration;
> 
> public class ASTRewritingModifyingRemoveTest extends ASTRewritingModifyingTest {
> 	private static final Class THIS = ASTRewritingModifyingRemoveTest.class;
>@@ -407,6 +410,9 @@
> 		buf.append("package test0010;\n");
> 		buf.append("\n");
> 		buf.append("public class X {\n");
>+		buf.append("    // comment1\n");
>+		buf.append("\n");
>+		buf.append("    \n");
> 		buf.append("\n");
> 		buf.append("    // comment5\n");
> 		buf.append("}\n");
>@@ -458,4 +464,174 @@
> 		buf.append("}\n");
> 		assertEqualString(preview, buf.toString());
> 	}
>+	
>+	/*
>+	 * https://bugs.eclipse.org/bugs/show_bug.cgi?id=306524
>+	 * To test that when types are removed, only the comments in the extended source range are
>+	 * removed, and the rest are left untouched.
>+	 */
>+	public void test0012() throws Exception {
>+		IPackageFragment pack1= this.sourceFolder.createPackageFragment("test0012", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test0012;\n");
>+		buf.append("public class X {\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment2\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment3\n");
>+		buf.append("    class X1{\n");
>+		buf.append("    }\n");
>+		buf.append("    // one line comment4\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment5\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment6\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment7\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment8\n");
>+		buf.append("    // one line comment9\n");
>+		buf.append("    class X2{\n");
>+		buf.append("    }\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment10\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment11\n");
>+		buf.append("    // one line comment12\n");
>+		buf.append("}\n");
>+		ICompilationUnit cu= pack1.createCompilationUnit("X.java", buf.toString(), false, null);
>+
>+		CompilationUnit astRoot= createCU(cu, false);
>+
>+		astRoot.recordModifications();
>+
>+		List types = astRoot.types();
>+		TypeDeclaration typeDeclaration = (TypeDeclaration)types.get(0);
>+		TypeDeclaration [] members = typeDeclaration.getTypes();
>+		typeDeclaration.bodyDeclarations().remove(members[0]);
>+		typeDeclaration.bodyDeclarations().remove(members[1]);
>+
>+		String preview = evaluateRewrite(cu, astRoot);
>+
>+		buf= new StringBuffer();
>+		buf.append("package test0012;\n");
>+		buf.append("public class X {\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment2\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment5\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment6\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment7\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("    // one line comment11\n");
>+		buf.append("    // one line comment12\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
>+	
>+	/*
>+	 * https://bugs.eclipse.org/bugs/show_bug.cgi?id=306524
>+	 * To test that when types are removed, only the comments in the extended source range are
>+	 * removed, and the rest are left untouched. This test is for cases where type to be removed is the 
>+	 * last one in file and there are more than one leading comments that are not part of its 
>+	 * extended source range
>+	 */
>+	public void test0013() throws Exception {
>+		IPackageFragment pack1= this.sourceFolder.createPackageFragment("test0013", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test0013;\n");
>+		buf.append("public class X {\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1a\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment2\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1b\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment3\n");
>+		buf.append("    class X1{\n");
>+		buf.append("    }\n");
>+		buf.append("    // one line comment4\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment5\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment6\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment7\n");
>+		buf.append("     */\n");
>+		buf.append("}\n");
>+		ICompilationUnit cu= pack1.createCompilationUnit("X.java", buf.toString(), false, null);
>+
>+		CompilationUnit astRoot= createCU(cu, false);
>+
>+		astRoot.recordModifications();
>+
>+		List types = astRoot.types();
>+		TypeDeclaration typeDeclaration = (TypeDeclaration)types.get(0);
>+		TypeDeclaration [] members = typeDeclaration.getTypes();
>+		typeDeclaration.bodyDeclarations().remove(members[0]);
>+
>+		String preview = evaluateRewrite(cu, astRoot);
>+		buf= new StringBuffer();
>+		buf.append("package test0013;\n");
>+		buf.append("public class X {\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1a\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment2\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment1b\n");
>+		buf.append("\n");
>+		buf.append("    \n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    // one line comment5\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment6\n");
>+		buf.append("     */\n");
>+		buf.append("\n");
>+		buf.append("\n");
>+		buf.append("    /*\n");
>+		buf.append("     * comment7\n");
>+		buf.append("     */\n");
>+		buf.append("}\n");
>+		assertEqualString(preview, buf.toString());
>+	}
> }

Actions: View | Diff

Attachments on bug 306524: 172148 | 172349