Attachment 176666 Details for Bug 252556 – Proposed patch

[patch] Proposed patch

v10.txt (text/plain), 19.14 KB, created by Frederic Fusier

on 2010-08-16 07:22:26 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Frederic Fusier

Created: 2010-08-16 07:22:26 EDT

Size: 19.14 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: formatter/org/eclipse/jdt/internal/formatter/OptimizedReplaceEdit.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/OptimizedReplaceEdit.java,v
>retrieving revision 1.8
>diff -u -r1.8 OptimizedReplaceEdit.java
>--- formatter/org/eclipse/jdt/internal/formatter/OptimizedReplaceEdit.java	7 Mar 2009 01:08:09 -0000	1.8
>+++ formatter/org/eclipse/jdt/internal/formatter/OptimizedReplaceEdit.java	16 Aug 2010 09:02:14 -0000
>@@ -12,6 +12,7 @@
> 
> public class OptimizedReplaceEdit {
> 
>+	boolean valid = true;
> 	int offset;
> 	int length;
> 	String replacement;
>@@ -23,6 +24,6 @@
> 	}
> 
> 	public String toString() {
>-		return "(" + this.offset + ", length " + this.length + " :>" + this.replacement + "<"; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$//$NON-NLS-4$
>+		return (this.valid ? "(" : "X(") + this.offset + ", length " + this.length + " :>" + this.replacement + "<"; //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$//$NON-NLS-4$ //$NON-NLS-5$
> 	}
> }
>Index: formatter/org/eclipse/jdt/internal/formatter/Scribe.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/Scribe.java,v
>retrieving revision 1.214
>diff -u -r1.214 Scribe.java
>--- formatter/org/eclipse/jdt/internal/formatter/Scribe.java	12 Aug 2010 07:12:18 -0000	1.214
>+++ formatter/org/eclipse/jdt/internal/formatter/Scribe.java	16 Aug 2010 09:02:14 -0000
>@@ -219,26 +219,12 @@
> 				// the offset of the region is inside a comment => restart the region from the comment start
> 				adaptedOffset = this.commentPositions[index][0];
> 				if (adaptedOffset >= 0) {
>-					// adapt only javadoc or block commments. Since fix for bug
>+					// adapt only javadoc or block comments. Since fix for bug
> 					// https://bugs.eclipse.org/bugs/show_bug.cgi?id=238210
> 					// edits in line comments only concerns whitespaces hence can be
> 					// treated as edits in code
> 					adaptedLength = length + offset - adaptedOffset;
> 					commentIndex = index;
>-					// include also the indentation edit just before the comment if any
>-					for (int j=0; j<this.editsIndex; j++) {
>-						int editOffset = this.edits[j].offset;
>-						int editEnd = editOffset + this.edits[j].length;
>-						if (editEnd == adaptedOffset) {
>-							if (j > 0 && this.edits[j].replacement.trim().length() == 0) {
>-								adaptedLength += adaptedOffset - this.edits[j].offset;
>-								adaptedOffset = editOffset;
>-								break;
>-							}
>-						} else if (editEnd > adaptedOffset) {
>-							break;
>-						}
>-					}
> 				}
> 			}
> 			index = getCommentIndex(commentIndex, offset+length-1);
>@@ -299,6 +285,14 @@
> 				currentEdit = index;
> 			}
> 		}
>+
>+    	// Set invalid all edits outside the region
>+		if (currentEdit != -1) {
>+			int length = sortedEdits.length;
>+	    	for (int e=currentEdit; e<length; e++) {
>+	    		sortedEdits[e].valid = false;
>+	    	}
>+    	}
> 	}
> 
> 	/*
>@@ -316,12 +310,12 @@
>      * region.
>      */
>     private int adaptEdit(OptimizedReplaceEdit[] sortedEdits, int start, int regionStart, int regionEnd) {
>-    	int bottom = start==-1?0:start, top = sortedEdits.length - 1;
>+    	int initialStart = start==-1 ? 0 : start;
>+		int bottom = initialStart, top = sortedEdits.length - 1;
>     	int topEnd = top;
>     	int i = 0;
>     	OptimizedReplaceEdit edit = null;
>     	int overlapIndex = -1;
>-        int linesOutside= -1;
> 
>     	// Look for an edit overlapping the region start
>     	while (bottom <= top) {
>@@ -329,28 +323,37 @@
>     		edit = sortedEdits[i];
>     		int editStart = edit.offset;
>    			int editEnd = editStart + edit.length;
>-    		if (regionStart < editStart) {  // the edit starts after the region's start => no possible overlap of region's start
>+    		if (editStart > regionStart) {  // the edit starts after the region's start => no possible overlap of region's start
>     			top = i-1;
>-    			if (regionEnd < editStart) { // the edit starts after the region's end => no possible overlap of region's end
>+    			if (editStart > regionEnd) { // the edit starts after the region's end => no possible overlap of region's end
>     				topEnd = top;
>     			}
>     		} else {
>-    			if (regionStart >= editEnd) { // the edit ends before the region's start => no possible overlap of region's start
>+    			if (editEnd < regionStart) { // the edit ends before the region's start => no possible overlap of region's start
> 	    			bottom = i+1;
> 				} else {
> 					// Count the lines of the edit which are outside the region
>-					linesOutside = 0;
>+					int linesOutside = 0;
>+					StringBuffer spacesOutside = new StringBuffer();
> 					this.scanner.resetTo(editStart, editEnd-1);
>-					while (!this.scanner.atEnd()) {
>-						boolean before = this.scanner.currentPosition < regionStart;
>-	                    char ch = (char) this.scanner.getNextChar();
>-                    	if (ch == '\n' ) {
>-                    		if (before) linesOutside++;
>-                    	}
>-                    }
>+					while (this.scanner.currentPosition < regionStart && !this.scanner.atEnd()) {
>+						char ch = (char) this.scanner.getNextChar();
>+						switch (ch) {
>+							case '\n':
>+								linesOutside++;
>+								spacesOutside.setLength(0);
>+								break;
>+							case '\r':
>+								break;
>+							default:
>+								spacesOutside.append(ch);
>+								break;
>+						}
>+					}
> 
> 					// Restart the edit at the beginning of the line where the region start
> 					edit.offset = regionStart;
>+					int editLength = edit.length;
> 					edit.length -= edit.offset - editStart;
> 
> 					// Cut replacement string if necessary
>@@ -363,36 +366,64 @@
> 							if (edit.replacement.charAt(idx) == '\n') linesReplaced++;
> 						}
> 
>-						// As the edit starts outside the region, remove first lines from edit string if any
>-						if (linesReplaced > 0) {
>-					    	int linesCount = linesOutside >= linesReplaced ? linesReplaced : linesOutside;
>-					    	if (linesCount > 0) {
>-					    		int idx=0;
>-					    		loop: while (idx < length) {
>-					    			char ch = edit.replacement.charAt(idx);
>-					    			switch (ch) {
>-					    				case '\n':
>-						    				linesCount--;
>-						    				if (linesCount == 0) {
>-						    					idx++;
>-						    					break loop;
>-						    				}
>-						    				break;
>-					    				case '\r':
>-					    				case ' ':
>-					    				case '\t':
>-					    					break;
>-					    				default:
>-					    					break loop;
>-					    			}
>-					    			idx++;
>-					    		}
>-					    		if (idx >= length) {
>-					    			edit.replacement = ""; //$NON-NLS-1$
>-					    		} else {
>-					    			edit.replacement = edit.replacement.substring(idx);
>-					    		}
>-					    	}
>+						// If the edit was a replacement but become an insertion due to the length reduction
>+						// and if the edit finishes just before the region starts and if there's no line to replace
>+						// then there's no replacement to do...
>+						if (editLength > 0 && edit.length == 0 && editEnd == regionStart && linesReplaced == 0 && linesOutside== 0) {
>+							edit.valid = false;
>+						} else {
>+
>+							// As the edit starts outside the region, remove first lines from edit string if any
>+							if (linesReplaced > 0) {
>+								int linesCount = linesOutside >= linesReplaced ? linesReplaced : linesOutside;
>+								if (linesCount > 0) {
>+									int idx = 0;
>+									loop: while (idx < length) {
>+										char ch = edit.replacement.charAt(idx);
>+										switch (ch) {
>+											case '\n':
>+												linesCount--;
>+												if (linesCount == 0) {
>+													idx++;
>+													break loop;
>+												}
>+												break;
>+											case '\r':
>+											case ' ':
>+											case '\t':
>+												break;
>+											default:
>+												break loop;
>+										}
>+										idx++;
>+									}
>+									// Compare spaces outside the region and the beginning
>+									// of the replacement string to remove the common part
>+									int spacesOutsideLength = spacesOutside.length();
>+									int replacementStart = idx;
>+									for (int o=0, r=0; o < spacesOutsideLength && r<(length-idx); o++) {
>+										char rch = edit.replacement.charAt(idx + r);
>+										char och = spacesOutside.charAt(o);
>+										if (rch == och) {
>+											replacementStart++;
>+											r++;
>+										} else if (rch == '\t' && (this.tabLength > 0 && och == ' ')) {
>+											if ((o+1)%this.tabLength == 0) {
>+												replacementStart++;
>+												r++;
>+											}
>+										} else {
>+											break;
>+										}
>+									}
>+									// Update the replacement string
>+									if (replacementStart >= length) {
>+										edit.valid = false;
>+									} else {
>+										edit.replacement = edit.replacement.substring(replacementStart);
>+									}
>+								}
>+							}
> 						}
> 					}
> 					overlapIndex = i;
>@@ -400,6 +431,7 @@
> 				}
> 			}
>     	}
>+    	int validIndex = (overlapIndex != -1) ? overlapIndex : bottom;
> 
>     	// Look for an edit overlapping the region end
>     	if (overlapIndex != -1) bottom = overlapIndex;
>@@ -411,11 +443,11 @@
>     		if (regionEnd < editStart) {	// the edit starts after the region's end => no possible overlap of region's end
>     			topEnd = i-1;
>     		} else {
>-    			if (regionEnd >= editEnd) {	// the edit ends before the region's end => no possible overlap of region's end
>+    			if (editEnd <= regionEnd) {	// the edit ends before the region's end => no possible overlap of region's end
> 	    			bottom = i+1;
> 				} else {
> 					// Count the lines of the edit which are outside the region
>-					linesOutside = 0;
>+					int linesOutside = 0;
> 					this.scanner.resetTo(editStart, editEnd-1);
> 					while (!this.scanner.atEnd()) {
> 						boolean after = this.scanner.currentPosition >= regionEnd;
>@@ -450,11 +482,25 @@
> 						}
> 					}
> 					edit.length -= editEnd - regionEnd;
>-					return i;
>+
>+			    	// Set invalid all edits outside the region
>+			    	for (int e=initialStart; e<validIndex; e++) {
>+			    		sortedEdits[e].valid = false;
>+			    	}
>+			    	
>+			    	// Return the index of next edit to look at
>+					return i + 1;
> 				}
> 			}
>     	}
>-    	return overlapIndex;
>+
>+    	// Set invalid all edits outside the region
>+    	for (int e=initialStart; e<validIndex; e++) {
>+    		sortedEdits[e].valid = false;
>+    	}
>+    	
>+    	// Return the index of next edit to look at
>+    	return topEnd+1;
>     }
> 
> 	private final void addDeleteEdit(int start, int end) {
>@@ -1204,15 +1250,6 @@
> 		return getNewLine();
> 	}
> 
>-	private IRegion getAdaptedRegionAt(int offset) {
>-		int index = getIndexOfAdaptedRegionAt(offset);
>-		if (index < 0) {
>-			return null;
>-		}
>-
>-		return this.adaptedRegions[index];
>-	}
>-
> 	public TextEdit getRootEdit() {
> 		// https://bugs.eclipse.org/bugs/show_bug.cgi?id=208541
> 		adaptRegions();
>@@ -1245,7 +1282,7 @@
> 		}
> 		for (int i= 0, max = this.editsIndex; i < max; i++) {
> 			OptimizedReplaceEdit currentEdit = this.edits[i];
>-			if (isValidEdit(currentEdit)) {
>+			if (currentEdit.valid && isValidEdit(currentEdit)) {
> 				try {
> 					edit.addChild(new ReplaceEdit(currentEdit.offset, currentEdit.length, currentEdit.replacement));
> 				}
>@@ -1475,23 +1512,6 @@
> 				return true;
> 			}
> 
>-			IRegion starting = getAdaptedRegionAt(editOffset + editLength);
>-			if (starting != null) {
>-				int i = editOffset;
>-				for (int max = editOffset + editLength; i < max; i++) {
>-					int replacementStringIndex = i - editOffset;
>-					if (replacementStringIndex >= editReplacementLength || this.scanner.source[i] != edit.replacement.charAt(replacementStringIndex)) {
>-						break;
>-					}
>-				}
>-				if (i - editOffset != editReplacementLength && i != editOffset + editLength - 1) {
>-					edit.offset = starting.getOffset();
>-					edit.length = 0;
>-					edit.replacement = edit.replacement.substring(i - editOffset);
>-					return true;
>-				}
>-			}
>-
> 			return false;
> 		}
> 
>#P org.eclipse.jdt.core.tests.model
>Index: src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java,v
>retrieving revision 1.33
>diff -u -r1.33 FormatterBugsTests.java
>--- src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java	12 Aug 2010 17:58:22 -0000	1.33
>+++ src/org/eclipse/jdt/core/tests/formatter/FormatterBugsTests.java	16 Aug 2010 09:02:16 -0000
>@@ -1690,7 +1690,6 @@
>  * @bug 252556: [formatter] Spaces removed before formatted region of a compilation unit.
>  * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=252556"
>  */
>-// TODO Fix the bug... this test currently verifies that the problem still occurs!
> public void testBug252556() {
> 	String source =
> 		"package a;\n" + 
>@@ -1711,8 +1710,7 @@
> 		"public class Test {\n" + 
> 		"\n" + 
> 		"	private int field;\n" + 
>-//		"	\n" + // this is the expected untouched line
>-		"\n" + // instead the tab is removed although it is outside the selection...
>+		"	\n" + 
> 		"	/**\n" + 
> 		"	 * fds\n" + 
> 		"	 */\n" + 
>@@ -1721,6 +1719,134 @@
> 		"}\n"
> 	);
> }
>+// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=95340
>+public void testBug252556a() {
>+	String source =
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"int foo() {[#\n" + 
>+		"return 0;\n" + 
>+		"#]}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"int foo() {\n" + 
>+		"		return 0;\n" + 
>+		"	}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n"
>+	);
>+}
>+// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=95340
>+public void testBug252556b() {
>+	String source =
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"int [#foo() {\n" + 
>+		"return 0;\n" + 
>+		"#]}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"int foo() {\n" + 
>+		"		return 0;\n" + 
>+		"	}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n"
>+	);
>+}
>+// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=95340
>+public void testBug252556c() {
>+	String source =
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"[#int foo() {\n" + 
>+		"return 0;\n" + 
>+		"#]}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"	int foo() {\n" + 
>+		"		return 0;\n" + 
>+		"	}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n"
>+	);
>+}
>+// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=95340
>+public void testBug252556d() {
>+	String source =
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"[#int foo() {\n" + 
>+		"return 0;\n" + 
>+		"}#]\n" + 
>+		"void bar(){}\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"	int foo() {\n" + 
>+		"		return 0;\n" + 
>+		"	}\n" + 
>+		"void bar(){}\n" + 
>+		"}\n"
>+	);
>+}
>+// see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=95340
>+public void testBug252556e() {
>+	String source =
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"[#int foo() {\n" + 
>+		"return 0;\n" + 
>+		"}\n" + 
>+		"#]void bar(){}\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"public class Test {\n" + 
>+		"\n" + 
>+		"	int foo() {\n" + 
>+		"		return 0;\n" + 
>+		"	}\n" + 
>+		"\n" + 
>+		"	void bar(){}\n" + 
>+		"}\n"
>+	);
>+}
>+// see org.eclipse.jdt.ui.tests.core.CodeFormatterUtilTest.testFormatSubstring()
>+public void testBug252556f() {
>+	String source =
>+		"package test1;\n" + 
>+		"\n" + 
>+		"import java.util.Vector;\n" + 
>+		"\n" + 
>+		"public class A {\n" + 
>+		"    public void foo() {\n" + 
>+		"    [#Runnable runnable= new Runnable() {};#]\n" + 
>+		"    runnable.toString();\n" + 
>+		"    }\n" + 
>+		"}\n";
>+	formatSource(source,
>+		"package test1;\n" + 
>+		"\n" + 
>+		"import java.util.Vector;\n" + 
>+		"\n" + 
>+		"public class A {\n" + 
>+		"    public void foo() {\n" + 
>+		"    	Runnable runnable = new Runnable() {\n" + 
>+		"		};\n" + 
>+		"    runnable.toString();\n" + 
>+		"    }\n" + 
>+		"}\n"
>+	);
>+}
> 
> /**
>  * @bug 281655: [formatter] "Never join lines" does not work for annotations.
>Index: src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java,v
>retrieving revision 1.69
>diff -u -r1.69 FormatterCommentsBugsTest.java
>--- src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java	24 Jun 2010 07:45:53 -0000	1.69
>+++ src/org/eclipse/jdt/core/tests/formatter/FormatterCommentsBugsTest.java	16 Aug 2010 09:02:17 -0000
>@@ -714,7 +714,7 @@
> 		"public class Test {\r\n" +
> 		"\r\n" +
> 		"	private int field;\r\n" +
>-		"\r\n" +
>+		"	\r\n" +
> 		"	/**\r\n" +
> 		"	 * fds\r\n" +
> 		"	 */\r\n" +
>@@ -954,8 +954,8 @@
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
> 		"public class C {\n" +
>-		"\n" +
>-		"	/**\n" +
>+		"	\n" +
>+		"        /**\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"	void m1() {\n" +
>@@ -987,8 +987,8 @@
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
> 		"public class C {\n" +
>-		"\n" +
>-		"	/**\n" +
>+		"	\n" +
>+		"        /**\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"	void m1  (   )   {\n" +
>@@ -1083,9 +1083,9 @@
> 	// Note that the incorrect indentation before the javadoc is fixed in this test case...
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
>-		"   public     class			C{\n" +
>-		"\n" +
>-		"	/**\n" +
>+		"   public     class			C{    \n" +
>+		"	\n" +
>+		"        /**\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"        void		m1  (   )   {\n" +
>@@ -1147,8 +1147,8 @@
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
> 		"public class D {\n" +
>-		"\n" +
>-		"	/*\n" +
>+		"	\n" +
>+		"        /*\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"	void m2() {\n" +
>@@ -1180,8 +1180,8 @@
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
> 		"public class D {\n" +
>-		"\n" +
>-		"	/*\n" +
>+		"	\n" +
>+		"        /*\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"	void m2  (   )   {\n" +
>@@ -1276,9 +1276,9 @@
> 	// Note that the incorrect indentation before the javadoc is fixed in this test case...
> 	// This is due to the fact that the region is adapted to include the edit just before the comment
> 	formatSource(source,
>-		"   public     class			D{\n" +
>-		"\n" +
>-		"	/*\n" +
>+		"   public     class			D{    \n" +
>+		"	\n" +
>+		"        /*\n" +
> 		"	 * a b c d .\n" +
> 		"	 */\n" +
> 		"        void		m2  (   )   {\n" +
>@@ -1553,7 +1553,7 @@
> 	formatSource(source,
> 		"\n" +
> 		"public class E01 {\n" +
>-		"	/**\n" +
>+		"        /**\n" +
> 		"	 * Javadoc comment\n" +
> 		"	 */\n" +
> 		"        /*\n" +

Actions: View | Diff

Attachments on bug 252556: 154671 | 176666 | 178159