Fixed bug 346010: [model] strange initialization dependency in OptionTests

4 files changed, 73 insertions, 6 deletions

diff --git a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/OptionTests.java b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/OptionTests.java
index f47fa2fe25..4c9c42d9b4 100644
--- a/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/OptionTests.java
+++ b/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/OptionTests.java
@@ -7,11 +7,17 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Stephan Herrmann - Contribution for Bug 346010 - [model] strange initialization dependency in OptionTests
  *******************************************************************************/
 package org.eclipse.jdt.core.tests.model;
 
+import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.Hashtable;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import junit.framework.Test;
 
@@ -43,6 +49,7 @@ public OptionTests(String name) {
 	super(name);
 }
 static {
+//	TESTS_NAMES = new String[] {    "testBug346010" };
 //		TESTS_NUMBERS = new int[] { 125360 };
 //		TESTS_RANGE = new int[] { 4, -1 };
 }
@@ -758,6 +765,61 @@ public void testBug324987_Project02() throws CoreException {
 	}
 }
 /**
+ * @bug 346010 - [model] strange initialization dependency in OptionTests
+ * @test Verify that unfortunate order of map entries doesn't spoil intended semantics.
+ * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=346010"
+ * @deprecated As using deprecated constants
+ */
+public void testBug346010() throws CoreException {
+	class ForcedOrderMap extends Hashtable {
+		private static final long serialVersionUID = 8012963985718522218L;
+		Map original;
+		Map.Entry additionalEntry;
+		/* Force (additionalKey,additionalValue) to be served after all entries of original. */
+		public ForcedOrderMap(Map original, String additionalKey, String additionalValue) {
+			this.original = original;
+			// convert additionalKey->additionalValue to a Map.Entry without inserting into original:
+			Hashtable tmp = new Hashtable();
+			tmp.put(additionalKey, additionalValue);
+			this.additionalEntry = (Map.Entry) tmp.entrySet().iterator().next();
+		}
+		public Set entrySet() {
+			return new HashSet() {
+				private static final long serialVersionUID = 1L;
+				public Iterator iterator() {
+					List orderedEntries;
+					orderedEntries = new ArrayList(ForcedOrderMap.this.original.entrySet());
+					orderedEntries.add(ForcedOrderMap.this.additionalEntry);
+					return orderedEntries.iterator();
+				}
+			};
+		}
+		public synchronized boolean containsKey(Object key) {
+			return this.original.containsKey(key) || key.equals(this.additionalEntry.getKey());
+		}
+	}
+	try {
+		// Set the obsolete option using the IJavaProject API
+		JavaProject project = (JavaProject) createJavaProject("P");
+		final String obsoleteOption = DefaultCodeFormatterConstants.FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER;
+		Map testOptions = project.getOptions(true);
+		Map orderedOptions = new ForcedOrderMap(testOptions, obsoleteOption, JavaCore.DO_NOT_INSERT);
+		project.setOptions(orderedOptions);
+		// Verify that obsolete preference is not stored
+		assertNull(
+				"Unexpected value for formatter deprecated option 'org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_member'", 
+				project.getEclipsePreferences().get(obsoleteOption, null));
+		// Verify that project obsolete option is well retrieved
+		assertEquals(
+				"Unexpected value for formatter deprecated option 'org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_member'", 
+				JavaCore.INSERT,
+				project.getOption(obsoleteOption, true));
+	} finally {
+		deleteProject("P");
+	}
+}
+
+/**
  * @bug 324987: [formatter] API compatibility problem with Annotation Newline options
  * @test Verify that a deprecated option is well preserved when read through
  * 		the IEclipsePreferences (i.e. simulate reading project preferences of a project
diff --git a/org.eclipse.jdt.core/buildnotes_jdt-core.html b/org.eclipse.jdt.core/buildnotes_jdt-core.html
index 4c5c54c09a..f610a849d0 100644
--- a/org.eclipse.jdt.core/buildnotes_jdt-core.html
+++ b/org.eclipse.jdt.core/buildnotes_jdt-core.html
@@ -41,7 +41,6 @@
 	</td>
   </tr>
 </table>
-<<<<<<< HEAD
 <a name="v_B84_R36x_J7"></a>
 <hr><h1>
 Eclipse Platform Build Notes<br>
diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java
index db778e3085..c3e036e5d0 100644
--- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaModelManager.java
@@ -10,6 +10,7 @@
  *     Theodora Yeung (tyeung@bea.com) - ensure that JarPackageFragmentRoot make it into cache
  *                                                           before its contents
  *                                                           (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=102422)
+ *     Stephan Herrmann - Contribution for Bug 346010 - [model] strange initialization dependency in OptionTests
  *******************************************************************************/
 package org.eclipse.jdt.internal.core;
 
@@ -4803,10 +4804,11 @@ public class JavaModelManager implements ISaveParticipant, IContentTypeChangeLis
 	 * @param optionValue The value of the option. If <code>null</code>, then
 	 * 	the option will be removed from the preferences instead.
 	 * @param eclipsePreferences The eclipse preferences to be updated
+	 * @param otherOptions more options being stored, used to avoid conflict between deprecated option and its compatible
 	 * @return <code>true</code> if the preferences have been changed,
 	 * 	<code>false</code> otherwise.
 	 */
-	public boolean storePreference(String optionName, String optionValue, IEclipsePreferences eclipsePreferences) {
+	public boolean storePreference(String optionName, String optionValue, IEclipsePreferences eclipsePreferences, Map otherOptions) {
 		int optionLevel = this.getOptionLevel(optionName);
 		if (optionLevel == UNKNOWN_OPTION) return false; // unrecognized option
 		
@@ -4824,6 +4826,8 @@ public class JavaModelManager implements ISaveParticipant, IContentTypeChangeLis
 				eclipsePreferences.remove(optionName); // get rid off old preference
 				String[] compatibleOptions = (String[]) this.deprecatedOptions.get(optionName);
 				for (int co=0, length=compatibleOptions.length; co < length; co++) {
+					if (otherOptions != null && otherOptions.containsKey(compatibleOptions[co]))
+						continue; // don't overwrite explicit value of otherOptions at compatibleOptions[co]
 					if (optionValue == null) {
 						eclipsePreferences.remove(compatibleOptions[co]);
 					} else {
@@ -4877,7 +4881,7 @@ public class JavaModelManager implements ISaveParticipant, IContentTypeChangeLis
 					if (defaultValue != null && defaultValue.equals(value)) {
 						value = null;
 					}
-					storePreference(key, value, instancePreferences);
+					storePreference(key, value, instancePreferences, newOptions);
 				}
 				try {
 					// persist options
diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java
index 18057ac089..22cc0828a5 100644
--- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java
+++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/JavaProject.java
@@ -7,7 +7,9 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
- *     Stephan Herrmann <stephan@cs.tu-berlin.de> - inconsistent initialization of classpath container backed by external class folder, see https://bugs.eclipse.org/320618
+ *     Stephan Herrmann <stephan@cs.tu-berlin.de> - Contributions for
+ *     						Bug 320618 - inconsistent initialization of classpath container backed by external class folder
+ *     						Bug 346010 - [model] strange initialization dependency in OptionTests
  *******************************************************************************/
 package org.eclipse.jdt.internal.core;
 
@@ -2943,7 +2945,7 @@ public class JavaProject
 	public void setOption(String optionName, String optionValue) {
 		// Store option value
 		IEclipsePreferences projectPreferences = getEclipsePreferences();
-		boolean modified = JavaModelManager.getJavaModelManager().storePreference(optionName, optionValue, projectPreferences);
+		boolean modified = JavaModelManager.getJavaModelManager().storePreference(optionName, optionValue, projectPreferences, null);
 
 		// Write changes
 		if (modified) {
@@ -2972,7 +2974,7 @@ public class JavaProject
 					Map.Entry entry = (Map.Entry) entries.next();
 					String key = (String) entry.getKey();
 					String value = (String) entry.getValue();
-					javaModelManager.storePreference(key, value, projectPreferences);
+					javaModelManager.storePreference(key, value, projectPreferences, newOptions);
 				}
 
 				// reset to default all options not in new map