Attachment 57147 Details for Bug 171066 – proposed fix

[patch] proposed fix

fix_171066.txt (text/plain), 19.02 KB, created by Benno Baumgartner

on 2007-01-19 11:21:37 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Benno Baumgartner

Created: 2007-01-19 11:21:37 EST

Size: 19.02 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: model/org/eclipse/jdt/core/util/CompilationUnitSorter.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/model/org/eclipse/jdt/core/util/CompilationUnitSorter.java,v
>retrieving revision 1.35
>diff -u -r1.35 CompilationUnitSorter.java
>--- model/org/eclipse/jdt/core/util/CompilationUnitSorter.java	10 May 2006 18:03:49 -0000	1.35
>+++ model/org/eclipse/jdt/core/util/CompilationUnitSorter.java	19 Jan 2007 16:19:55 -0000
>@@ -7,6 +7,7 @@
> *
> * Contributors:
> * IBM Corporation - initial API and implementation
>+ * Alex Blewitt - alex_blewitt@yahoo.com https://bugs.eclipse.org/bugs/show_bug.cgi?id=171066
> *******************************************************************************/
> 
> package org.eclipse.jdt.core.util;
>@@ -15,9 +16,13 @@
> 
> import org.eclipse.core.runtime.IProgressMonitor;
> import org.eclipse.jdt.core.ICompilationUnit;
>+import org.eclipse.jdt.core.IJavaElement;
> import org.eclipse.jdt.core.JavaModelException;
> import org.eclipse.jdt.core.dom.AST;
>+import org.eclipse.jdt.core.dom.CompilationUnit;
> import org.eclipse.jdt.internal.core.SortElementsOperation;
>+import org.eclipse.text.edits.TextEdit;
>+import org.eclipse.text.edits.TextEditGroup;
> 
> /**
> * Operation for sorting members within a compilation unit.
>@@ -329,4 +334,135 @@
> operation.runOperation(monitor);
> }
> 
>+	/**
>+	 * Reorders the declarations in the given compilation unit according to the
>+	 * specified comparator. The caller is responsible for arranging in advance
>+	 * that the given compilation unit is a working copy, and for applying the
>+	 * returned TextEdit afterwards.
>+	 * 
>+	 * Note: Reordering the members within a type declaration might be
>+	 * more than a cosmetic change and could have potentially serious
>+	 * repercussions. Firstly, the order in which the fields of a type are
>+	 * initialized is significant in the Java language; reordering fields and
>+	 * initializers may result in compilation errors or change the execution
>+	 * behavior of the code. Secondly, reordering a class's members may affect
>+	 * how its instances are serialized. This operation should therefore be used
>+	 * with caution and due concern for potential negative side effects.
>+	 * 
>+	 * 
>+	 * The <code>compare</code> method of the given comparator is passed pairs
>+	 * of body declarations (subclasses of <code>BodyDeclaration</code>)
>+	 * representing body declarations at the same level. The nodes are from an
>+	 * AST of the specified level ({@link org.eclipse.jdt.core.dom.ASTParser#newParser(int)}.
>+	 * Clients will generally use AST.JLS3 since that will cover all
>+	 * constructs found in Java 1.0, 1.1, 1.2, 1.3, 1.4, and 1.5 source code.
>+	 * The comparator is called on body declarations of nested classes,
>+	 * including anonymous and local classes, but always at the same level.
>+	 * Clients need to provide a comparator implementation (there is no standard
>+	 * comparator). The <code>RELATIVE_ORDER</code> property attached to these
>+	 * AST nodes affords the comparator a way to preserve the original relative
>+	 * order.
>+	 * 
>+	 * 
>+	 * The body declarations passed as parameters to the comparator always carry
>+	 * at least the following minimal signature information: 
>+	 * <table border="1" width="80%" cellpadding="5">
>+	 * <tr>
>+	 * <td width="20%"><code>TypeDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, isInterface, name, superclass,
>+	 * superInterfaces, typeParameters 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>FieldDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, type, fragments
>+	 * (VariableDeclarationFragments
>+	 * with name only) 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>MethodDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, isConstructor, returnType, name,
>+	 * typeParameters, parameters
>+	 * (SingleVariableDeclarations with name, type, and modifiers only),
>+	 * thrownExceptions 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>Initializer</code></td>
>+	 * <td width="50%"><code>modifiers 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>AnnotationTypeDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, name 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>AnnotationTypeMemberDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, name, type, default 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>EnumDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, name, superInterfaces 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * <tr>
>+	 * <td width="20%"><code>EnumConstantDeclaration</code></td>
>+	 * <td width="50%"><code>modifiers, name, arguments 
>+	 * RELATIVE_ORDER property</code></td>
>+	 * </tr>
>+	 * </table> Clients should not rely on the AST nodes being properly parented
>+	 * or on having source range information. (Future releases may provide
>+	 * options for requesting additional information like source positions, full
>+	 * ASTs, non-recursive sorting, etc.)
>+	 * 
>+	 * 
>+	 * @param unit
>+	 * the CompilationUnit to sort
>+	 * @param comparator
>+	 * the comparator capable of ordering
>+	 * <code>BodyDeclaration</code>s; this comparator is passed
>+	 * AST nodes from an AST of the specified AST level
>+	 * @param options
>+	 * bitwise-or of option flags; <code>0</code> for default
>+	 * behavior (reserved for future growth)
>+	 * @param group
>+	 * the text edit group to use when generating text edits, or <code>null</code>
>+	 * @param monitor
>+	 * the progress monitor to notify, or <code>null</code> if none
>+	 * @return a TextEdit describing the required edits to do the sort, or <code>null</code> 
>+	 * if sorting is not required
>+	 * @exception JavaModelException
>+	 * if the compilation unit could not be sorted. Reasons
>+	 * include:
>+	 * <ul>
>+	 * <li> The given compilation unit does not exist
>+	 * (ELEMENT_DOES_NOT_EXIST)</li>
>+	 * <li> The given compilation unit is not a working copy
>+	 * (INVALID_ELEMENT_TYPES)</li>
>+	 * <li> A <code>CoreException</code> occurred while
>+	 * accessing the underlying resource
>+	 * </ul>
>+	 * @exception IllegalArgumentException
>+	 * if the given compilation unit is null or if the given
>+	 * comparator is null, or if <code>options</code> is not one
>+	 * of the supported levels.
>+	 * @see org.eclipse.jdt.core.dom.BodyDeclaration
>+	 * @see #RELATIVE_ORDER
>+	 * @since 3.3
>+	 */
>+	public static TextEdit sort(CompilationUnit unit,
>+			Comparator comparator,
>+			int options,
>+			TextEditGroup group,
>+			IProgressMonitor monitor) throws JavaModelException {
>+		if (unit == null || comparator == null) {
>+			throw new IllegalArgumentException();
>+		}
>+		
>+		SortElementsOperation operation = new SortElementsOperation(AST.JLS3, new IJavaElement[] {unit.getJavaElement()}, null, comparator);
>+		return operation.calculateEdit(unit, group);
>+	}
> }
>Index: model/org/eclipse/jdt/internal/core/SortElementsOperation.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SortElementsOperation.java,v
>retrieving revision 1.36
>diff -u -r1.36 SortElementsOperation.java
>--- model/org/eclipse/jdt/internal/core/SortElementsOperation.java	10 May 2006 18:03:47 -0000	1.36
>+++ model/org/eclipse/jdt/internal/core/SortElementsOperation.java	19 Jan 2007 16:19:55 -0000
>@@ -7,6 +7,7 @@
> *
> * Contributors:
> * IBM Corporation - initial API and implementation
>+ * Alex Blewitt - alex_blewitt@yahoo.com https://bugs.eclipse.org/bugs/show_bug.cgi?id=171066
> *******************************************************************************/
> package org.eclipse.jdt.internal.core;
> 
>@@ -23,7 +24,6 @@
> import org.eclipse.jdt.core.IJavaModelStatusConstants;
> import org.eclipse.jdt.core.JavaModelException;
> import org.eclipse.jdt.core.compiler.CharOperation;
>-import org.eclipse.jdt.core.dom.AST;
> import org.eclipse.jdt.core.dom.ASTNode;
> import org.eclipse.jdt.core.dom.ASTParser;
> import org.eclipse.jdt.core.dom.ASTVisitor;
>@@ -43,6 +43,7 @@
> import org.eclipse.jface.text.Document;
> import org.eclipse.text.edits.RangeMarker;
> import org.eclipse.text.edits.TextEdit;
>+import org.eclipse.text.edits.TextEditGroup;
> 
> /**
> * This operation is used to sort elements in a compilation unit according to
>@@ -112,6 +113,29 @@
> 			done();
> 		}
> 	}
>+	
>+	/**
>+	 * Calculates the required text edits to sort the <code>unit</code>
>+	 * @param group 
>+	 * @return the edit or null if no sorting is required
>+	 */
>+	public TextEdit calculateEdit(org.eclipse.jdt.core.dom.CompilationUnit unit, TextEditGroup group) throws JavaModelException {
>+		try {
>+			beginTask(Messages.operation_sortelements, getMainAmountOfWork());
>+			
>+			ASTRewrite rewrite= sortCompilationUnit(unit, group);
>+			if (rewrite == null)
>+				return null;
>+			
>+			ICompilationUnit cu= (ICompilationUnit)this.elementsToProcess[0];
>+			String content= cu.getBuffer().getContents();
>+			Document document= new Document(content);
>+			
>+			return rewrite.rewriteAST(document, null);
>+		} finally {
>+			done();
>+		}
>+	}
> 
> 	/**
> 	 * Method processElement.
>@@ -119,14 +143,45 @@
> 	 * @param source
> 	 */
> 	private String processElement(ICompilationUnit unit, char[] source) {
>+		Document document = new Document(new String(source));
> 		CompilerOptions options = new CompilerOptions(unit.getJavaProject().getOptions(true));
> 		ASTParser parser = ASTParser.newParser(this.apiLevel);
> 		parser.setCompilerOptions(options.getMap());
> 		parser.setSource(source);
> 		parser.setKind(ASTParser.K_COMPILATION_UNIT);
> 		parser.setResolveBindings(false);
>-		org.eclipse.jdt.core.dom.CompilationUnit domUnit = (org.eclipse.jdt.core.dom.CompilationUnit) parser.createAST(null);
>-		domUnit.accept(new ASTVisitor() {
>+		org.eclipse.jdt.core.dom.CompilationUnit ast = (org.eclipse.jdt.core.dom.CompilationUnit) parser.createAST(null);
>+ 
>+		ASTRewrite rewriter= sortCompilationUnit(ast, null);
>+		if (rewriter == null)
>+			return document.get();
>+		
>+		TextEdit edits = rewriter.rewriteAST(document, null);
>+		
>+		RangeMarker[] markers = null;
>+		if (this.positions != null) {
>+			markers = new RangeMarker[this.positions.length];
>+			for (int i = 0, max = this.positions.length; i < max; i++) {
>+				markers[i]= new RangeMarker(this.positions[i], 0);
>+				insert(edits, markers[i]);
>+			}
>+		}
>+		try {
>+			edits.apply(document, TextEdit.UPDATE_REGIONS);
>+			if (this.positions != null) {
>+				for (int i= 0, max = markers.length; i < max; i++) {
>+					this.positions[i]= markers[i].getOffset();
>+				}
>+			}
>+		} catch (BadLocationException e) {
>+			// ignore
>+		}
>+		return document.get();
>+	}
>+	
>+	
>+	private ASTRewrite sortCompilationUnit(org.eclipse.jdt.core.dom.CompilationUnit ast, final TextEditGroup group) {
>+		ast.accept(new ASTVisitor() {
> 			public boolean visit(org.eclipse.jdt.core.dom.CompilationUnit compilationUnit) {
> 				List types = compilationUnit.types();
> 				for (Iterator iter = types.iterator(); iter.hasNext();) {
>@@ -182,52 +237,45 @@
> 				return true;
> 			}
> 		});
>-		final AST localAst = domUnit.getAST();
>-		final ASTRewrite rewriter = ASTRewrite.create(localAst);
>-		RangeMarker[] markers = null;
> 		
>-		final boolean needPositionsMapping = this.positions != null;
>-		if (needPositionsMapping) {
>-			markers = new RangeMarker[this.positions.length];
>-			for (int i= 0; i < this.positions.length; i++) {
>-				markers[i]= new RangeMarker(this.positions[i], 0);
>+		final ASTRewrite rewriter= ASTRewrite.create(ast.getAST()); 
>+		final boolean[] hasChanges= new boolean[] {false}; 
>+		
>+		ast.accept(new ASTVisitor() {
>+		
>+			private void sortElements(List elements, ListRewrite listRewrite) {
>+				if (elements.size() == 0)
>+					return;
>+				
>+				final List myCopy = new ArrayList();
>+				myCopy.addAll(elements);
>+				Collections.sort(myCopy, SortElementsOperation.this.comparator);
>+				
>+				for (int i = 0; i < elements.size(); i++) {
>+					ASTNode oldNode= (ASTNode) elements.get(i);
>+					ASTNode newNode= (ASTNode) myCopy.get(i);
>+					if (oldNode != newNode) {
>+						listRewrite.replace(oldNode, rewriter.createMoveTarget(newNode), group);
>+						hasChanges[0]= true;
>+					}
>+				}
> 			}
>-		}
>-		String generatedSource = new String(source);
>-		Document document = new Document(generatedSource);
>-		domUnit.accept(new ASTVisitor() {
>+
> 			public boolean visit(org.eclipse.jdt.core.dom.CompilationUnit compilationUnit) {
> 				if (checkMalformedNodes(compilationUnit)) {
> 					return true; // abort sorting of current element
> 				}
>-				ListRewrite listRewrite = rewriter.getListRewrite(compilationUnit, org.eclipse.jdt.core.dom.CompilationUnit.TYPES_PROPERTY);
>-				List types = compilationUnit.types();
>-				final int length = types.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(types);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) types.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>+				
>+				sortElements(compilationUnit.types(), rewriter.getListRewrite(compilationUnit, org.eclipse.jdt.core.dom.CompilationUnit.TYPES_PROPERTY));
> 				return true;
> 			}
>+
> 			public boolean visit(AnnotationTypeDeclaration annotationTypeDeclaration) {
> 				if (checkMalformedNodes(annotationTypeDeclaration)) {
> 					return true; // abort sorting of current element
> 				}
>-				ListRewrite listRewrite = rewriter.getListRewrite(annotationTypeDeclaration, AnnotationTypeDeclaration.BODY_DECLARATIONS_PROPERTY);
>-				List bodyDeclarations = annotationTypeDeclaration.bodyDeclarations();
>-				final int length = bodyDeclarations.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(bodyDeclarations);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) bodyDeclarations.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>+				
>+				sortElements(annotationTypeDeclaration.bodyDeclarations(), rewriter.getListRewrite(annotationTypeDeclaration, AnnotationTypeDeclaration.BODY_DECLARATIONS_PROPERTY));
> 				return true;
> 			}
> 
>@@ -235,17 +283,8 @@
> 				if (checkMalformedNodes(anonymousClassDeclaration)) {
> 					return true; // abort sorting of current element
> 				}
>-				ListRewrite listRewrite = rewriter.getListRewrite(anonymousClassDeclaration, AnonymousClassDeclaration.BODY_DECLARATIONS_PROPERTY);
>-				List bodyDeclarations = anonymousClassDeclaration.bodyDeclarations();
>-				final int length = bodyDeclarations.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(bodyDeclarations);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) bodyDeclarations.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>+				
>+				sortElements(anonymousClassDeclaration.bodyDeclarations(), rewriter.getListRewrite(anonymousClassDeclaration, AnonymousClassDeclaration.BODY_DECLARATIONS_PROPERTY));	
> 				return true;
> 			}
> 			
>@@ -253,17 +292,8 @@
> 				if (checkMalformedNodes(typeDeclaration)) {
> 					return true; // abort sorting of current element
> 				}
>-				ListRewrite listRewrite = rewriter.getListRewrite(typeDeclaration, TypeDeclaration.BODY_DECLARATIONS_PROPERTY);
>-				List bodyDeclarations = typeDeclaration.bodyDeclarations();
>-				final int length = bodyDeclarations.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(bodyDeclarations);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) bodyDeclarations.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>+				
>+				sortElements(typeDeclaration.bodyDeclarations(), rewriter.getListRewrite(typeDeclaration, TypeDeclaration.BODY_DECLARATIONS_PROPERTY));
> 				return true;
> 			}
> 
>@@ -271,49 +301,17 @@
> 				if (checkMalformedNodes(enumDeclaration)) {
> 					return true; // abort sorting of current element
> 				}
>-				ListRewrite listRewrite = rewriter.getListRewrite(enumDeclaration, EnumDeclaration.BODY_DECLARATIONS_PROPERTY);
>-				List bodyDeclarations = enumDeclaration.bodyDeclarations();
>-				int length = bodyDeclarations.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(bodyDeclarations);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) bodyDeclarations.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>-				listRewrite = rewriter.getListRewrite(enumDeclaration, EnumDeclaration.ENUM_CONSTANTS_PROPERTY);
>-				List enumConstants = enumDeclaration.enumConstants();
>-				length = enumConstants.size();
>-				if (length > 1) {
>-					final List myCopy = new ArrayList();
>-					myCopy.addAll(enumConstants);
>-					Collections.sort(myCopy, SortElementsOperation.this.comparator);
>-					for (int i = 0; i < length; i++) {
>-						listRewrite.replace((ASTNode) enumConstants.get(i), rewriter.createMoveTarget((ASTNode) myCopy.get(i)), null);
>-					}
>-				}
>+				
>+				sortElements(enumDeclaration.bodyDeclarations(), rewriter.getListRewrite(enumDeclaration, EnumDeclaration.BODY_DECLARATIONS_PROPERTY));
>+				sortElements(enumDeclaration.enumConstants(), rewriter.getListRewrite(enumDeclaration, EnumDeclaration.ENUM_CONSTANTS_PROPERTY));
> 				return true;
> 			}
>-		});			
>-		TextEdit edits = rewriter.rewriteAST(document, null);
>-		if (needPositionsMapping) {
>-			for (int i = 0, max = markers.length; i < max; i++) {
>-				insert(edits, markers[i]);
>-			}
>-		}
>-		try {
>-			edits.apply(document, TextEdit.UPDATE_REGIONS);
>-			generatedSource = document.get();
>-			if (needPositionsMapping) {
>-				for (int i= 0, max = markers.length; i < max; i++) {
>-					this.positions[i]= markers[i].getOffset();
>-				}
>-			}
>-		} catch (BadLocationException e) {
>-			// ignore
>-		}
>-		return generatedSource;
>+		});
>+		
>+		if (!hasChanges[0])
>+			return null;
>+		
>+		return rewriter;
> 	}
> 
> 	/**

Actions: View | Diff

Attachments on bug 171066: 57147 | 57341