Attachment 180334 Details for Bug 325131 – Proposed fix + regression test

[patch] Proposed fix + regression test

patch_325131.txt (text/plain), 13.03 KB, created by Olivier Thomann

on 2010-10-06 10:29:40 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2010-10-06 10:29:40 EDT

Size: 13.03 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: dom/org/eclipse/jdt/core/dom/ASTNode.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/ASTNode.java,v
>retrieving revision 1.81
>diff -u -r1.81 ASTNode.java
>--- dom/org/eclipse/jdt/core/dom/ASTNode.java	2 Sep 2010 18:48:21 -0000	1.81
>+++ dom/org/eclipse/jdt/core/dom/ASTNode.java	6 Oct 2010 14:28:27 -0000
>@@ -966,9 +966,9 @@
> 	/**
> 	 * Primary field used in representing node properties efficiently.
> 	 * If <code>null</code>, this node has no properties.
>-	 * If a <code>String</code>, this is the name of this node's sole property,
>+	 * If a {@link String}, this is the name of this node's sole property,
> 	 * and <code>property2</code> contains its value.
>-	 * If a <code>HashMap</code>, this is the table of property name-value
>+	 * If a {@link Map}, this is the table of property name-value
> 	 * mappings; <code>property2</code>, if non-null is its unmodifiable
> 	 * equivalent.
> 	 * Initially <code>null</code>.
>@@ -2079,7 +2079,7 @@
> 	}
> 
> 	/**
>-	 * Returns the named property of this node, or <code>null</code> if none.
>+	 * Returns the value of the named property of this node, or <code>null</code> if none.
> 	 *
> 	 * @param propertyName the property name
> 	 * @return the property value, or <code>null</code> if none
>@@ -2124,6 +2124,7 @@
> 	 * @param propertyName the property name
> 	 * @param data the new property value, or <code>null</code> if none
> 	 * @see #getProperty(String)
>+	 * @throws IllegalArgumentException if the given property name is <code>null</code>
> 	 */
> 	public final void setProperty(String propertyName, Object data) {
> 		if (propertyName == null) {
>@@ -2147,11 +2148,12 @@
> 			// node has only a single property
> 			if (propertyName.equals(this.property1)) {
> 				// we're in luck
>-				this.property2 = data;
> 				if (data == null) {
> 					// just deleted last property
> 					this.property1 = null;
> 					this.property2 = null;
>+				} else {
>+					this.property2 = data;
> 				}
> 				return;
> 			}
>@@ -2161,7 +2163,7 @@
> 			}
> 			// node already has one property - getting its second
> 			// convert to more flexible representation
>-			HashMap m = new HashMap(2);
>+			Map m = new HashMap(3);
> 			m.put(this.property1, this.property2);
> 			m.put(propertyName, data);
> 			this.property1 = m;
>@@ -2170,7 +2172,7 @@
> 		}
> 
> 		// node has two or more properties
>-		HashMap m = (HashMap) this.property1;
>+		Map m = (Map) this.property1;
> 		if (data == null) {
> 			m.remove(propertyName);
> 			// check for just one property left
>Index: dom/org/eclipse/jdt/core/dom/rewrite/ASTRewrite.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/rewrite/ASTRewrite.java,v
>retrieving revision 1.44
>diff -u -r1.44 ASTRewrite.java
>--- dom/org/eclipse/jdt/core/dom/rewrite/ASTRewrite.java	25 Jun 2010 14:58:37 -0000	1.44
>+++ dom/org/eclipse/jdt/core/dom/rewrite/ASTRewrite.java	6 Oct 2010 14:28:27 -0000
>@@ -10,6 +10,7 @@
>  *******************************************************************************/
> package org.eclipse.jdt.core.dom.rewrite;
> 
>+import java.util.HashMap;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Map;
>@@ -90,7 +91,6 @@
>  * @noextend This class is not intended to be subclassed by clients.
>  */
> public class ASTRewrite {
>-
> 	/** root node for the rewrite: Only nodes under this root are accepted */
> 	private final AST ast;
> 
>@@ -103,6 +103,26 @@
> 	 * @since 3.1
> 	 */
> 	private TargetSourceRangeComputer targetSourceRangeComputer = null;
>+	
>+	/**
>+	 * Primary field used in representing rewrite properties efficiently.
>+	 * If <code>null</code>, this rewrite has no properties.
>+	 * If a {@link String}, this is the name of this rewrite's sole property,
>+	 * and <code>property2</code> contains its value.
>+	 * If a {@link Map}, this is the table of property name-value
>+	 * mappings.
>+	 * Initially <code>null</code>.
>+	 * 
>+	 * @see #property2
>+	 */
>+	private Object property1 = null;
>+
>+	/**
>+	 * Auxiliary field used in representing rewrite properties efficiently.
>+	 *
>+	 * @see #property1
>+	 */
>+	private Object property2 = null;
> 
> 	/**
> 	 * Creates a new instance for describing manipulations of
>@@ -495,6 +515,36 @@
> 	}
> 
> 	/**
>+	 * Returns the value of the named property of this rewrite, or <code>null</code> if none.
>+	 *
>+	 * @param propertyName the property name
>+	 * @return the property value, or <code>null</code> if none
>+	 * @see #setProperty(String,Object)
>+	 * @throws IllegalArgumentException if the given property name is <code>null</code>
>+	 * @since 3.7
>+	 */
>+	public final Object getProperty(String propertyName) {
>+		if (propertyName == null) {
>+			throw new IllegalArgumentException();
>+		}
>+		if (this.property1 == null) {
>+			// rewrite has no properties at all
>+			return null;
>+		}
>+		if (this.property1 instanceof String) {
>+			// rewrite has only a single property
>+			if (propertyName.equals(this.property1)) {
>+				return this.property2;
>+			} else {
>+				return null;
>+			}
>+		}
>+		// otherwise rewrite has table of properties
>+		Map m = (Map) this.property1;
>+		return m.get(propertyName);
>+	}
>+	
>+	/**
> 	 * Returns an object that tracks the source range of the given node
> 	 * across the rewrite to its AST. Upon return, the result object reflects
> 	 * the given node's current source range in the AST. After
>@@ -674,6 +724,87 @@
> 	}
> 
> 	/**
>+	 * Sets the named property of this rewrite to the given value,
>+	 * or to <code>null</code> to clear it.
>+	 * <p>
>+	 * Clients should employ property names that are sufficiently unique
>+	 * to avoid inadvertent conflicts with other clients that might also be
>+	 * setting properties on the same rewrite.
>+	 * </p>
>+	 * <p>
>+	 * Note that modifying a property is not considered a modification to the
>+	 * AST itself. This is to allow clients to decorate existing rewrites with
>+	 * their own properties without jeopardizing certain things (like the
>+	 * validity of bindings), which rely on the underlying tree remaining static.
>+	 * </p>
>+	 *
>+	 * @param propertyName the property name
>+	 * @param data the new property value, or <code>null</code> if none
>+	 * @see #getProperty(String)
>+	 * @throws IllegalArgumentException if the given property name is <code>null</code>
>+	 * @since 3.7
>+	 */
>+	public final void setProperty(String propertyName, Object data) {
>+		if (propertyName == null) {
>+			throw new IllegalArgumentException();
>+		}
>+		if (this.property1 == null) {
>+			// rewrite has no properties at all
>+			if (data == null) {
>+				// rewrite already knows this
>+				return;
>+			}
>+			// rewrite gets its fist property
>+			this.property1 = propertyName;
>+			this.property2 = data;
>+			return;
>+		}
>+		if (this.property1 instanceof String) {
>+			// rewrite has only a single property
>+			if (propertyName.equals(this.property1)) {
>+				// we're in luck
>+				if (data == null) {
>+					// just delete last property
>+					this.property1 = null;
>+					this.property2 = null;
>+				} else {
>+					this.property2 = data;
>+				}
>+				return;
>+			}
>+			if (data == null) {
>+				// we already know this
>+				return;
>+			}
>+			// rewrite already has one property - getting its second
>+			// convert to more flexible representation
>+			Map m = new HashMap(3);
>+			m.put(this.property1, this.property2);
>+			m.put(propertyName, data);
>+			this.property1 = m;
>+			this.property2 = null;
>+			return;
>+		}
>+		// rewrite has two or more properties
>+		Map m = (Map) this.property1;
>+		if (data == null) {
>+			m.remove(propertyName);
>+			// check for just one property left
>+			if (m.size() == 1) {
>+				// convert to more efficient representation
>+				Map.Entry[] entries = (Map.Entry[]) m.entrySet().toArray(new Map.Entry[1]);
>+				this.property1 = entries[0].getKey();
>+				this.property2 = entries[0].getValue();
>+			}
>+			return;
>+		} else {
>+			m.put(propertyName, data);
>+			// still has two or more properties
>+			return;
>+		}
>+	}
>+
>+	/**
> 	 * Sets a custom target source range computer for this AST rewriter. This is advanced feature to modify how
> 	 * comments are associated with nodes, which should be done only in special cases.
> 	 *
>#P org.eclipse.jdt.core.tests.model
>Index: src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritePropertyTest.java
>===================================================================
>RCS file: src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritePropertyTest.java
>diff -N src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritePropertyTest.java
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritePropertyTest.java	1 Jan 1970 00:00:00 -0000
>@@ -0,0 +1,84 @@
>+/*******************************************************************************
>+ * Copyright (c) 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
>+ * http://www.eclipse.org/legal/epl-v10.html
>+ *
>+ * Contributors:
>+ *     IBM Corporation - initial API and implementation
>+ *******************************************************************************/
>+package org.eclipse.jdt.core.tests.rewrite.describing;
>+
>+import junit.framework.Test;
>+import junit.framework.TestSuite;
>+
>+import org.eclipse.jdt.core.ICompilationUnit;
>+import org.eclipse.jdt.core.IPackageFragment;
>+import org.eclipse.jdt.core.dom.CompilationUnit;
>+import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
>+
>+public class ASTRewritePropertyTest extends ASTRewritingTest {
>+
>+	private static final Class THIS= ASTRewritePropertyTest.class;
>+
>+	public ASTRewritePropertyTest(String name) {
>+		super(name);
>+	}
>+	public static Test allTests() {
>+		return new Suite(THIS);
>+	}
>+
>+	public static Test setUpTest(Test someTest) {
>+		TestSuite suite= new Suite("one test");
>+		suite.addTest(someTest);
>+		return suite;
>+	}
>+
>+	public static Test suite() {
>+		return buildModelTestSuite(THIS);
>+	}
>+
>+	public void testProperties() throws Exception {
>+		IPackageFragment pack1= this.sourceFolder.createPackageFragment("test1", false, null);
>+		StringBuffer buf= new StringBuffer();
>+		buf.append("package test1;\n");
>+		buf.append("public class C {}");
>+		ICompilationUnit cu= pack1.createCompilationUnit("C.java", buf.toString(), false, null);
>+
>+		CompilationUnit astRoot= createAST(cu);
>+		ASTRewrite rewrite= ASTRewrite.create(astRoot.getAST());
>+		final String propertyName1 = "test.propertyName1";
>+		final String propertyName2 = "test.propertyName2";
>+		assertNull(rewrite.getProperty(propertyName1));
>+		try {
>+			rewrite.getProperty(null);
>+			assertTrue("Should not be reached", false);
>+		} catch(IllegalArgumentException e) {
>+			// ignore
>+		}
>+		rewrite.setProperty(propertyName1, "value");
>+		rewrite.setProperty(propertyName2, new Integer(1));
>+		try {
>+			rewrite.setProperty(null, "");
>+			assertTrue("Should not be reached", false);
>+		} catch(IllegalArgumentException e) {
>+			// ignore
>+		}
>+		Object value1 = rewrite.getProperty(propertyName1);
>+		assertTrue("Not a String", value1 instanceof String);
>+		assertTrue("Wrong value", "value".equals(value1));
>+		
>+		Object value2 = rewrite.getProperty(propertyName2);
>+		assertTrue("Not an Integer", value2 instanceof Integer);
>+		assertTrue("Wrong value", new Integer(1).equals(value2));
>+
>+		rewrite.setProperty(propertyName1, null);
>+		value1 = rewrite.getProperty(propertyName1);
>+		assertNull("Not null", value1);
>+
>+		rewrite.setProperty(propertyName2, null);
>+		value2 = rewrite.getProperty(propertyName2);
>+		assertNull("Not null", value2);
>+	}
>+}
>Index: src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingTest.java,v
>retrieving revision 1.25
>diff -u -r1.25 ASTRewritingTest.java
>--- src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingTest.java	28 Apr 2009 17:46:13 -0000	1.25
>+++ src/org/eclipse/jdt/core/tests/rewrite/describing/ASTRewritingTest.java	6 Oct 2010 14:28:27 -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
>@@ -50,6 +50,7 @@
> 		suite.addTest(ImportRewriteTest.allTests());
> 		suite.addTest(LineCommentOffsetsTest.allTests());
> 		suite.addTest(ASTRewritingWithStatementsRecoveryTest.allTests());
>+		suite.addTest(ASTRewritePropertyTest.allTests());
> 		return suite;
> 	}
>

Actions: View | Diff

Attachments on bug 325131: 179620 | 179676 | 179735 | 180334