alpha-asp/Alpha

String-typed ConstantTerms for strings and constant symbols mistakenly equal

madmike200590 opened this issue · 2 comments

It seems that the symbolic flag for ConstantTerm<String>s isn't taken into account when checking equality.
The following test illustrating this currently fails (see branch bugfix_term_equality):

@Test
public void testStringTermEquality() {
	ConstantTerm<String> stringConstant = ConstantTerm.getInstance("string");
	ConstantTerm<String> constantSymbol = ConstantTerm.getSymbolicInstance("string");
	Assert.assertNotEquals(stringConstant, constantSymbol);
}

The term called stringConstant represents the quoted string "symbol" in ASP code,
while the other term, constantSymbol, refers to the constant symbol symbol in code. These are not the same thing and should therefore not be equal.

Further analysis showed that at least the "internalizing" still works correctly - stringConstant and constantSymbol are distinct instances in the term cache. This is due to the fact that the value of symbolic is taken into account by ConstantTerm#hashCode.

As a preliminary fix, I added the following check to ConstantTerm#equals:

if (this.symbolic != that.symbolic) {
	return false;
}

However, the above fix still leaves one further issue:
The assertion Assert.assertNotEquals(0, stringConstant.compareTo(constantSymbol)); currently fails, meaning both terms are equal with respect to compareTo, which should not be the case

I guess this is a bug in ConstantTerm.compareTo where in line 120 we have

if (other.object.getClass() == this.object.getClass()) {
  return this.object.compareTo((T) other.object);
}

I guess what is missing there is && other.symbolic == this.symbolic in the if.