Merge pull request #245 from alpha-asp/bugfix_term_equality

Bugfix: String-typed ConstantTerms for strings and constant symbols mistakenly equal (Issue #244)

src/main/java/at/ac/tuwien/kr/alpha/common/terms/ConstantTerm.java

@@ -7,7 +7,7 @@
 import java.util.List;
 
 /**
- * Copyright (c) 2016, the Alpha Team.
+ * Copyright (c) 2016-2020, the Alpha Team.
  */
 public class ConstantTerm<T extends Comparable<T>> extends Term {
 	private static final Interner<ConstantTerm> INTERNER = new Interner<>();
@@ -67,7 +67,10 @@ public boolean equals(Object o) {
 			return false;
 		}
 
-		ConstantTerm that = (ConstantTerm) o;
+		ConstantTerm<?> that = (ConstantTerm<?>) o;
+		if (this.symbolic != that.symbolic) {
+			return false;
+		}
 
 		return object.equals(that.object);
 	}
@@ -103,7 +106,7 @@ public int compareTo(Term o) {
 			return super.compareTo(o);
 		}
 
-		ConstantTerm other = (ConstantTerm) o;
+		ConstantTerm<?> other = (ConstantTerm<?>) o;
 
 		// We will perform an unchecked cast.
 		// Because of type erasure, we cannot know the exact type
@@ -117,7 +120,7 @@ public int compareTo(Term o) {
 		// wrong if we have some bug that generates strange
 		// ConstantTerms at runtime, bypassing the check for T
 		// at compile-time.
-		if (other.object.getClass() == this.object.getClass()) {
+		if (other.object.getClass() == this.object.getClass() && other.symbolic == this.symbolic) {
 			return this.object.compareTo((T) other.object);
 		}
 

src/test/java/at/ac/tuwien/kr/alpha/common/TermTest.java

@@ -4,6 +4,8 @@
 import at.ac.tuwien.kr.alpha.common.terms.FunctionTerm;
 import at.ac.tuwien.kr.alpha.common.terms.Term;
 import at.ac.tuwien.kr.alpha.common.terms.VariableTerm;
+
+import org.junit.Assert;
 import org.junit.Test;
 
 import java.util.LinkedList;
@@ -20,7 +22,8 @@ public class TermTest {
 
 	@Test
 	public void testTermReferenceEquality() {
-		// Terms must have a unique representation so that reference comparison is sufficient to check
+		// Terms must have a unique representation so that reference comparison is
+		// sufficient to check
 		// whether two terms are equal.
 		ConstantTerm ta1 = ConstantTerm.getInstance("a");
 		ConstantTerm ta2 = ConstantTerm.getInstance("a");
@@ -66,4 +69,23 @@ public void testTermOrdering() throws Exception {
 		assertTrue(cto.compareTo(ft) < 0);
 		assertTrue(ft.compareTo(cto) > 0);
 	}
+
+	@Test
+	public void testStringVsConstantSymbolEquality() {
+		String theString = "string";
+		ConstantTerm<String> stringConstant = ConstantTerm.getInstance(theString);
+		ConstantTerm<String> constantSymbol = ConstantTerm.getSymbolicInstance(theString);
+		// Reference equality must hold for both the string constant and the constant
+		// symbol.
+		Assert.assertTrue(stringConstant == ConstantTerm.getInstance(theString));
+		ConstantTerm<String> sameConstantSymbol = ConstantTerm.getSymbolicInstance(theString);
+		Assert.assertTrue(constantSymbol == sameConstantSymbol);
+		// Make sure both hashCode and equals understand that stringConstant and
+		// constantSymbol are NOT the same thing!
+		Assert.assertNotEquals(stringConstant.hashCode(), constantSymbol.hashCode());
+		Assert.assertNotEquals(stringConstant, constantSymbol);
+		// This also applies to compareTo - it must behave in sync with equals and
+		// hashCode, i.e. return a non-zero result for non-equal objects
+		Assert.assertNotEquals(0, stringConstant.compareTo(constantSymbol));
+	}
 }