Special handling of properties in equals() to account for default values

core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/language/settings/providers/LanguageSettingsSerializableTests.java

@@ -56,9 +56,13 @@ public class LanguageSettingsSerializableTests extends BaseTestCase {
 	private static final String PROVIDER_NAME_2 = "test.provider.2.name";
 	private static final String ATTR_PARAMETER = "parameter";
 	private static final String VALUE_PARAMETER = "custom.parameter";
-	private static final String ATTR_STORE_ENTRIES = "store-entries";
-	private static final String VALUE_PROJECT = "project";
 	private static final String ELEM_TEST = "test";
+	private static final String ATTR_PROPERTY = "custom-property";
+	private static final String ATTR_PROPERTY_BOOL = "custom-property-bool";
+	private static final String VALUE_PROPERTY = "custom.property";
+
+	// This value must match that of LanguageSettingsProvidersSerializer.ATTR_STORE_ENTRIES_WITH_PROJECT
+	private static final String ATTR_STORE_ENTRIES_WITH_PROJECT = "store-entries-with-project";
 
 	/**
 	 * Constructor.
@@ -357,8 +361,8 @@ public class LanguageSettingsSerializableTests extends BaseTestCase {
 			Element rootElement = XmlUtil.appendElement(doc, ELEM_TEST);
 			elementProvider = provider.serialize(rootElement);
 			String xmlString = XmlUtil.toString(doc);
-			assertTrue(xmlString.contains(ATTR_STORE_ENTRIES));
+			assertTrue(xmlString.contains(ATTR_STORE_ENTRIES_WITH_PROJECT));
-			assertTrue(xmlString.contains(VALUE_PROJECT));
+			assertTrue(xmlString.contains(ATTR_STORE_ENTRIES_WITH_PROJECT+"=\"true\""));
 		}
 		{
 			// re-load and check storing mode of the newly loaded provider
@@ -1189,46 +1193,144 @@ public class LanguageSettingsSerializableTests extends BaseTestCase {
 		provider1.setSettingEntries(MOCK_CFG, MOCK_RC, LANG_ID, sampleEntries_1);
 		provider1.setSettingEntries(null, null, LANG_ID, sampleEntries_2);
 
+		// create another provider with the same data
+		LanguageSettingsSerializableProvider provider2 = new LanguageSettingsSerializableProvider(PROVIDER_1, PROVIDER_NAME_1);
 		{
-			// create another provider with the same data
-			LanguageSettingsSerializableProvider provider2 = new LanguageSettingsSerializableProvider(PROVIDER_1, PROVIDER_NAME_1);
-			assertFalse(provider1.equals(provider2));
-			assertFalse(provider1.hashCode()==provider2.hashCode());
-
-			provider2.setSettingEntries(MOCK_CFG, MOCK_RC, LANG_ID, sampleEntries_1);
-			assertFalse(provider1.equals(provider2));
-			assertFalse(provider1.hashCode()==provider2.hashCode());
-
-			provider2.setSettingEntries(null, null, LANG_ID, sampleEntries_2);
-			assertFalse(provider1.equals(provider2));
-			assertFalse(provider1.hashCode()==provider2.hashCode());
-
 			provider2.setLanguageScope(sampleLanguages);
-			assertFalse(provider1.equals(provider2));
-			assertFalse(provider1.hashCode()==provider2.hashCode());
-
 			provider2.setProperty(ATTR_PARAMETER, VALUE_PARAMETER);
-			assertFalse(provider1.equals(provider2));
-			assertFalse(provider1.hashCode()==provider2.hashCode());
-
 			LanguageSettingsManager.setStoringEntriesInProjectArea(provider2, true);
-
+			provider2.setSettingEntries(MOCK_CFG, MOCK_RC, LANG_ID, sampleEntries_1);
+			provider2.setSettingEntries(null, null, LANG_ID, sampleEntries_2);
 			// All set now, so they should be equal
-			assertTrue(provider1.equals(provider2));
 			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+		}
 
-			// check different ID
+		{
-			provider2.setId(PROVIDER_2);
+			// start with provider with the same data
-			assertFalse(provider1.equals(provider2));
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+			// replace languages
+			List<String> sampleLanguages2 = new ArrayList<String>();
+			sampleLanguages2.add(LANG_ID_1);
+			provider2.setLanguageScope(sampleLanguages2);
 			assertFalse(provider1.hashCode()==provider2.hashCode());
+			assertFalse(provider1.equals(provider2));
+			// restore provider
+			provider2.setLanguageScope(sampleLanguages);
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+		}
+
+		{
+			// start with provider with the same data
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+			// replace property
+			provider2.setProperty(ATTR_PARAMETER, "changed-parameter");
+			// hash is not calculated for properties
+			assertFalse(provider1.equals(provider2));
+			// restore provider
+			provider2.setProperty(ATTR_PARAMETER, VALUE_PARAMETER);
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+		}
+		{
+			// start with provider with the same data
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+			// replace property
+			LanguageSettingsManager.setStoringEntriesInProjectArea(provider2, false);
+			// hash is not calculated for properties
+			assertFalse(provider1.equals(provider2));
+			// restore provider
+			LanguageSettingsManager.setStoringEntriesInProjectArea(provider2, true);
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+		}
+		{
+			// start with provider with the same data
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+			// replace entries
+			List<ICLanguageSettingEntry> changedEntries = new ArrayList<ICLanguageSettingEntry>();
+			changedEntries.add(new CMacroEntry("MACROX", "valueX",1));
+			provider2.setSettingEntries(MOCK_CFG, MOCK_RC, LANG_ID, changedEntries);
+			assertFalse(provider1.hashCode()==provider2.hashCode());
+			assertFalse(provider1.equals(provider2));
+			// restore provider
+			provider2.setSettingEntries(MOCK_CFG, MOCK_RC, LANG_ID, sampleEntries_1);
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+		}
+		{
+			// start with provider with the same data
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
+			// replace default entries
+			List<ICLanguageSettingEntry> changedEntries = new ArrayList<ICLanguageSettingEntry>();
+			changedEntries.add(new CIncludePathEntry("pathX", 1));
+			provider2.setSettingEntries(null, null, LANG_ID, changedEntries);
+			assertFalse(provider1.hashCode()==provider2.hashCode());
+			assertFalse(provider1.equals(provider2));
+			// restore provider
+			provider2.setSettingEntries(null, null, LANG_ID, sampleEntries_2);
+			assertTrue(provider1.hashCode()==provider2.hashCode());
+			assertTrue(provider1.equals(provider2));
 		}
 
 		{
 			// check that subclasses are not equal
 			LanguageSettingsSerializableProvider providerSub1 = new LanguageSettingsSerializableProvider() {};
 			LanguageSettingsSerializableProvider providerSub2 = new LanguageSettingsSerializableProvider() {};
-			assertFalse(providerSub1.equals(providerSub2));
 			assertFalse(providerSub1.hashCode()==providerSub2.hashCode());
+			assertFalse(providerSub1.equals(providerSub2));
+		}
+	}
+
+	/**
+	 */
+	public void testEquals_DefaultProperties() throws Exception {
+		// create model providers
+		LanguageSettingsSerializableProvider provider1 = new LanguageSettingsSerializableProvider(PROVIDER_1, PROVIDER_NAME_1);
+		LanguageSettingsSerializableProvider provider2 = new LanguageSettingsSerializableProvider(PROVIDER_1, PROVIDER_NAME_1);
+
+		// equality for setProperty(String, String)
+		{
+			// equality for missing property
+			assertTrue(provider1.equals(provider2));
+			// equality for default empty value (missing in provider2)
+			provider1.setProperty(ATTR_PROPERTY, "");
+			assertTrue(provider1.equals(provider2));
+			// just for kicks disturb equality
+			provider1.setProperty(ATTR_PROPERTY, VALUE_PROPERTY);
+			assertFalse(provider1.equals(provider2));
+			// equality for default null value (missing in provider2)
+			provider1.setProperty(ATTR_PROPERTY, null);
+			assertTrue(provider1.equals(provider2));
+		}
+
+		// equality for setPropertyBool(String, boolean)
+		{
+			// equality for missing property
+			assertEquals(false, provider1.getPropertyBool(ATTR_PROPERTY_BOOL));
+			assertTrue(provider1.equals(provider2));
+			// equality for default empty value (missing in provider2)
+			provider1.setPropertyBool(ATTR_PROPERTY_BOOL, false);
+			assertEquals(false, provider1.getPropertyBool(ATTR_PROPERTY_BOOL));
+			assertTrue(provider1.equals(provider2));
+			// just for kicks disturb equality
+			provider1.setPropertyBool(ATTR_PROPERTY_BOOL, true);
+			assertEquals(true, provider1.getPropertyBool(ATTR_PROPERTY_BOOL));
+			assertFalse(provider1.equals(provider2));
+			// equality for true value in both
+			provider2.setPropertyBool(ATTR_PROPERTY_BOOL, true);
+			assertEquals(true, provider2.getPropertyBool(ATTR_PROPERTY_BOOL));
+			assertTrue(provider1.equals(provider2));
+			// switch provider1 back to false
+			provider1.setPropertyBool(ATTR_PROPERTY_BOOL, false);
+			provider1.setPropertyBool(ATTR_PROPERTY_BOOL, false);
+			assertFalse(provider1.equals(provider2));
 		}
 	}
 

core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/language/settings/providers/LanguageSettingsBaseProvider.java

@@ -14,8 +14,10 @@ package org.eclipse.cdt.core.language.settings.providers;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.eclipse.cdt.core.AbstractExecutableExtensionBase;
 import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
@@ -135,6 +137,10 @@ public class LanguageSettingsBaseProvider extends AbstractExecutableExtensionBas
 	 * so extenders of this class can customize the provider. The properties
 	 * of {@code LanguageSettingsBaseProvider} come from the extension in plugin.xml
 	 * although the extenders can provide their own method.
+	 * <br><br>
+	 * Please note that empty string value is treated as "default" value and
+	 * the same as {@code null} and the same as missing property, which allows
+	 * {@link #equals(Object)} evaluate the property as equal while comparing providers.
 	 *
 	 * @param key - property to check the value.
 	 * @return value of the property.
@@ -143,6 +149,17 @@ public class LanguageSettingsBaseProvider extends AbstractExecutableExtensionBas
 		return properties.get(key);
 	}
 
+	/**
+	 * Convenience method to get boolean property.
+	 * @see #getProperty(String)
+	 *
+	 * @param key - property to check the value.
+	 * @return boolean value of the property.
+	 */
+	public boolean getPropertyBool(String key) {
+		return Boolean.parseBoolean(properties.get(key));
+	}
+
 	private List<ICLanguageSettingEntry> getPooledList(List<ICLanguageSettingEntry> entries) {
 		if (entries != null) {
 			return LanguageSettingsStorage.getPooledList(entries);
@@ -183,4 +200,84 @@ public class LanguageSettingsBaseProvider extends AbstractExecutableExtensionBas
 			return null;
 		return Collections.unmodifiableList(languageScope);
 	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = 1;
+		result = prime * result + ((getId() == null) ? 0 : getId().hashCode());
+		result = prime * result + ((getName() == null) ? 0 : getName().hashCode());
+		result = prime * result + ((entries == null) ? 0 : entries.hashCode());
+		result = prime * result + ((languageScope == null) ? 0 : languageScope.hashCode());
+		// exclude field "properties" because of special rules for equals()
+		result = prime * result + getClass().hashCode();
+		return result;
+	}
+
+	/**
+	 * @return {@code true} if the objects are equal, {@code false } otherwise.
+	 * @see java.lang.Object#equals(java.lang.Object)
+	 */
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		LanguageSettingsBaseProvider other = (LanguageSettingsBaseProvider) obj;
+
+		String id = getId();
+		String otherId = other.getId();
+		if (id == null) {
+			if (otherId != null)
+				return false;
+		} else if (!id.equals(otherId))
+			return false;
+
+		String name = getName();
+		String otherName = other.getName();
+		if (name == null) {
+			if (otherName != null)
+				return false;
+		} else if (!name.equals(otherName))
+			return false;
+
+		if (entries == null) {
+			if (other.entries != null)
+				return false;
+		} else if (!entries.equals(other.entries))
+			return false;
+
+		if (languageScope == null) {
+			if (other.languageScope != null)
+				return false;
+		} else if (!languageScope.equals(other.languageScope))
+			return false;
+
+		if (properties == null) {
+			if (other.properties != null)
+				return false;
+		} else if (other.properties == null) {
+			return false;
+		} else {
+			// The trouble to ensure default properties are equal to missing ones.
+			Set<String> keys = new HashSet<String>(properties.keySet());
+			keys.addAll(other.properties.keySet());
+			for (String key : keys) {
+				String value = properties.get(key);
+				if (value == null)
+					value = ""; //$NON-NLS-1$
+				String otherValue = other.properties.get(key);
+				if (otherValue == null)
+					otherValue = ""; //$NON-NLS-1$
+				if (!value.equals(otherValue))
+					return false;
+			}
+		}
+
+		return true;
+	}
+
 }

core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/language/settings/providers/LanguageSettingsSerializableProvider.java

@@ -306,9 +306,9 @@ public class LanguageSettingsSerializableProvider extends LanguageSettingsBasePr
 				if (!key.equals(ATTR_ID) && !key.equals(ATTR_NAME) && !key.equals(ATTR_CLASS)) {
 					String value = attr.getNodeValue();
 					properties.put(key, value);
+					}
 				}
 			}
-		}
 
 		this.setId(providerId);
 		this.setName(providerName);
@@ -336,6 +336,12 @@ public class LanguageSettingsSerializableProvider extends LanguageSettingsBasePr
 
 	/**
 	 * Set a custom property of the provider.
+	 * <br><br>
+	 * A note of caution - do not use default values for a provider which are different
+	 * from empty or {@code null} value. When providers are checked for equality
+	 * (during internal operations in core) the missing properties are evaluated as
+	 * empty ones.
+	 *
 	 * @see LanguageSettingsBaseProvider#getProperty(String)
 	 *
 	 * @param key - name of the property.
@@ -343,10 +349,23 @@ public class LanguageSettingsSerializableProvider extends LanguageSettingsBasePr
 	 *    If value is {@code null} the property is removed from the list.
 	 */
 	public void setProperty(String key, String value) {
-		if (value != null) {
+		properties.put(key, value);
-			properties.put(key, value);
+	}
+
+	/**
+	 * Set a custom boolean property of the provider.
+	 * <br>Please, note that default value is always {@code false}.
+	 * @see LanguageSettingsBaseProvider#getProperty(String)
+	 *
+	 * @param key - name of the property.
+	 * @param value - {@code boolean} value of the property.
+	 */
+	public void setPropertyBool(String key, boolean value) {
+		if (value == true) {
+			properties.put(key, Boolean.TRUE.toString());
 		} else {
-			properties.remove(key);
+			// Keep "false" values in default representation and preserve the key in the list
+			properties.put(key, null);
 		}
 	}
 
@@ -382,69 +401,6 @@ public class LanguageSettingsSerializableProvider extends LanguageSettingsBasePr
 		return clone;
 	}
 
-	@Override
-	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + ((getId() == null) ? 0 : getId().hashCode());
-		result = prime * result + ((getName() == null) ? 0 : getName().hashCode());
-		result = prime * result + ((languageScope == null) ? 0 : languageScope.hashCode());
-		result = prime * result + ((properties == null) ? 0 : properties.hashCode());
-		result = prime * result + ((fStorage == null) ? 0 : fStorage.hashCode());
-		result = prime * result + getClass().hashCode();
-		return result;
-	}
-
-	/**
-	 * @return {@code true} if the objects are equal, {@code false } otherwise.
-	 * @see java.lang.Object#equals(java.lang.Object)
-	 */
-	@Override
-	public boolean equals(Object obj) {
-		if (this == obj)
-			return true;
-		if (obj == null)
-			return false;
-		if (getClass() != obj.getClass())
-			return false;
-		LanguageSettingsSerializableProvider other = (LanguageSettingsSerializableProvider) obj;
-
-		String id = getId();
-		String otherId = other.getId();
-		if (id == null) {
-			if (otherId != null)
-				return false;
-		} else if (!id.equals(otherId))
-			return false;
-
-		String name = getName();
-		String otherName = other.getName();
-		if (name == null) {
-			if (otherName != null)
-				return false;
-		} else if (!name.equals(otherName))
-			return false;
-
-		if (languageScope == null) {
-			if (other.languageScope != null)
-				return false;
-		} else if (!languageScope.equals(other.languageScope))
-			return false;
-
-		if (properties == null) {
-			if (other.properties != null)
-				return false;
-		} else if (!properties.equals(other.properties))
-			return false;
-
-		if (fStorage == null) {
-			if (other.fStorage != null)
-				return false;
-		} else if (!fStorage.equals(other.fStorage))
-			return false;
-		return true;
-	}
-
 	@Override
 	public LanguageSettingsStorage copyStorage() {
 		try {
@@ -454,4 +410,30 @@ public class LanguageSettingsSerializableProvider extends LanguageSettingsBasePr
 		}
 		return null;
 	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = super.hashCode();
+		result = prime * result + ((fStorage == null) ? 0 : fStorage.hashCode());
+		return result;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (!super.equals(obj))
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		LanguageSettingsSerializableProvider other = (LanguageSettingsSerializableProvider) obj;
+
+		if (fStorage == null) {
+			if (other.fStorage != null)
+				return false;
+		} else if (!fStorage.equals(other.fStorage))
+			return false;
+		return true;
+	}
 }

core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/language/settings/providers/LanguageSettingsProvidersSerializer.java

@@ -83,10 +83,7 @@ public class LanguageSettingsProvidersSerializer {
 	private static final String ELEM_PROJECT = "project"; //$NON-NLS-1$
 	private static final String ELEM_CONFIGURATION = "configuration"; //$NON-NLS-1$
 	private static final String ELEM_PROVIDER_REFERENCE = "provider-reference"; //$NON-NLS-1$
-
+	private static final String ATTR_STORE_ENTRIES_WITH_PROJECT = "store-entries-with-project"; //$NON-NLS-1$
-	private static final String ATTR_STORE_ENTRIES = "store-entries"; //$NON-NLS-1$
-	private static final String VALUE_WORKSPACE = "workspace"; //$NON-NLS-1$
-	private static final String VALUE_PROJECT = "project"; //$NON-NLS-1$
 
 	/** Cache of true (raw) workspace providers */
 	private static Map<String, ILanguageSettingsProvider> rawGlobalWorkspaceProviders = new HashMap<String, ILanguageSettingsProvider>();
@@ -284,8 +281,7 @@ public class LanguageSettingsProvidersSerializer {
 	 * @return {@code true} if LSE persisted with the project or {@code false} if in the workspace.
 	 */
 	public static boolean isStoringEntriesInProjectArea(LanguageSettingsSerializableProvider provider) {
-		String value = provider.getProperty(ATTR_STORE_ENTRIES);
+		return provider.getPropertyBool(ATTR_STORE_ENTRIES_WITH_PROJECT);
-		return VALUE_PROJECT.equals(value);
 	}
 
 	/**
@@ -296,7 +292,7 @@ public class LanguageSettingsProvidersSerializer {
 	 *    {@code false} if in workspace area.
 	 */
 	public static void setStoringEntriesInProjectArea(LanguageSettingsSerializableProvider provider, boolean storeEntriesWithProject) {
-		provider.setProperty(ATTR_STORE_ENTRIES, storeEntriesWithProject ? VALUE_PROJECT : VALUE_WORKSPACE);
+		provider.setPropertyBool(ATTR_STORE_ENTRIES_WITH_PROJECT, storeEntriesWithProject);
 	}
 
 	/**