Merge pull request #1121 from Thrameos/compareTo

Change to use .equals

doc/CHANGELOG.rst

@@ -5,11 +5,20 @@ This changelog *only* contains changes from the *first* pypi release (0.5.4.3) o
 
 Latest Changes:
 - **1.5.0_dev0 - 2023-04-03**
-- **1.4.2_dev0 - 2022-10-26**
+
+  - Switched ``__eq__`` and ``__ne__`` operator to use ``equals`` rather than
+    ``compareTo`` for comparable objects to avoid exception when comparing
+    object of different types.
+
+  - Fixed segmentation fault when comparing Java Comparable to primitives.
+
+  - Java exceptions that occur in inequality comparisons now map to Python
+    TypeError.
 
   - Fixed crash when calling subscript on JArray.
 
-  - Fixed direct byte buffers not reporting nbytes correctly when cast to memoryview.
+  - Fixed direct byte buffers not reporting nbytes correctly when cast to
+    memoryview.
 
   - Expand the defintion for Functional interface to include classes without
     FunctionInterface annotation.

native/common/jp_javaframe.cpp

@@ -1194,7 +1194,13 @@ jint JPJavaFrame::compareTo(jobject obj, jobject obj2)
 {
 	jvalue v;
 	v.l = obj2;
-	return CallIntMethodA(obj, m_Context->m_CompareToID, &v);
+	jint ret = m_Env->CallIntMethodA(obj, m_Context->m_CompareToID, &v);
+	if (m_Env->ExceptionOccurred())
+	{
+		m_Env->ExceptionClear();
+		JP_RAISE(PyExc_TypeError, "Unable to compare")
+	}
+	return ret;
 }
 
 jthrowable JPJavaFrame::getCause(jthrowable th)

native/python/pyjp_object.cpp

@@ -55,8 +55,6 @@ static PyObject *PyJPObject_compare(PyObject *self, PyObject *other, int op)
 	if (op == Py_NE)
 	{
 		PyObject *ret = PyJPObject_compare(self, other, Py_EQ);
-		if (ret == NULL)
-			return NULL;
 		int rc = (ret == Py_False);
 		Py_DECREF(ret);
 		return PyBool_FromLong(rc);
@@ -120,7 +118,7 @@ static PyObject *PyJPComparable_compare(PyObject *self, PyObject *other, int op)
 	bool null1 = false;
 
 	// First slot is Null
-	if (self == Py_None || javaSlot0 == NULL ||
+	if (javaSlot0 == NULL ||
 			(!javaSlot0->getClass()->isPrimitive() && javaSlot0->getValue().l == NULL))
 		null0  = true;
 	if (other == Py_None || (javaSlot1 != NULL &&
@@ -153,8 +151,16 @@ static PyObject *PyJPComparable_compare(PyObject *self, PyObject *other, int op)
 			Py_INCREF(out);
 			return out;
 		}
+		if (match.type < JPMatch::Type::_implicit)
+		{
+			if (op == Py_EQ || op == Py_NE)
+				return  PyBool_FromLong(op == Py_NE);
+			PyObject *out = Py_NotImplemented;
+			Py_INCREF(out);
+			return out;
+		}
 		obj1 = match.convert().l;
-	} else if (!null1 && javaSlot1 != NULL)
+	} else if (!null1 && javaSlot1 != NULL && !javaSlot1->getClass()->isPrimitive())
 		obj1 = javaSlot1->getValue().l;
 
 	switch (op)
@@ -164,14 +170,13 @@ static PyObject *PyJPComparable_compare(PyObject *self, PyObject *other, int op)
 				Py_RETURN_TRUE;
 			if (null0 || null1)
 				Py_RETURN_FALSE;
-			return PyBool_FromLong(frame.compareTo(obj0, obj1) == 0);
-
+			return PyBool_FromLong(frame.equals(obj0, obj1));
 		case Py_NE:
 			if (null0 && null1)
 				Py_RETURN_FALSE;
 			if (null0 || null1)
 				Py_RETURN_TRUE;
-			return PyBool_FromLong(frame.compareTo(obj0, obj1) != 0);
+			return PyBool_FromLong(!frame.equals(obj0, obj1));
 		case Py_LT:
 			if (null0 || null1)
 				break;

test/jpypetest/test_comparable.py

@@ -79,3 +79,43 @@ def testComparableNull(self):
             print(i3 > i3)
         with self.assertRaises(ValueError):
             print(i3 >= i3)
+
+    def testComparableObj(self):
+        C1 = jpype.JClass("java.time.temporal.ChronoUnit")
+        C2 = jpype.JClass("java.util.concurrent.TimeUnit")
+        O1 = C1.SECONDS
+        O2 = C2.SECONDS
+        N1 = jpype.JObject(None, C1)
+        N2 = jpype.JObject(None, C2)
+        V = jpype.JInt(1)
+        # Test dissimilar objects
+        self.assertTrue(O1 != O2)
+        self.assertFalse(O1 == O2)
+        # test Nulls
+        self.assertTrue(N1 == N2)
+        self.assertFalse(N1 != N2)
+        self.assertTrue(N1 == None)
+        self.assertFalse(N1 != None)
+        # test primitives
+        self.assertFalse(O1 == V)
+        self.assertFalse(V == O1)
+        # test null to primitives
+        self.assertFalse(N1 == V)
+        self.assertFalse(V == N1)
+
+        self.assertFalse(1 == O1)
+        self.assertFalse("M" == O1)
+        self.assertFalse(O1 == 1)
+        self.assertFalse(O1 == "M")
+
+        self.assertTrue(1 != O1)
+        self.assertTrue("M" != O1)
+        self.assertTrue(O1 != 1)
+        self.assertTrue(O1 != "M")
+
+        with self.assertRaises(TypeError):
+            self.assertTrue(O1 > 1)
+        with self.assertRaises(TypeError):
+            self.assertTrue(1 > O1)
+        with self.assertRaises(TypeError):
+            self.assertTrue(O1 > O2)

test/jpypetest/test_objectwrapper.py

@@ -113,6 +113,37 @@ class Fred(object):
         with self.assertRaises(TypeError):
             jpype.JObject(Fred())
 
+    def testObjectEq(self):
+        C1 = jpype.JClass("java.lang.StringBuffer")
+        C2 = jpype.JClass("java.lang.Exception")
+        O1 = C1()
+        O2 = C2()
+        N1 = jpype.JObject(None, C1)
+        N2 = jpype.JObject(None, C2)
+        V = jpype.JInt(1)
+        # Test dissimilar objects
+        self.assertTrue(O1 != O2)
+        self.assertFalse(O1 == O2)
+        self.assertTrue(O2 != O1)
+        self.assertFalse(O2 == O1)
+        # test Nulls
+        self.assertTrue(N1 == N2)
+        self.assertFalse(N1 != N2)
+        self.assertTrue(N1 == None)
+        self.assertFalse(N1 != None)
+        self.assertTrue(None == N1)
+        self.assertFalse(None != N1)
+        # test primitives
+        self.assertFalse(O1 == V)
+        self.assertFalse(V == O1)
+        self.assertFalse(O2 == V)
+        self.assertFalse(V == O2)
+        # test null to primitives
+        self.assertFalse(N1 == V)
+        self.assertFalse(V == N1)
+        self.assertFalse(N2 == V)
+        self.assertFalse(V == N2)
+
 
 #     def testMakeSureWeCanLoadAllClasses(self):
 #         def get_system_jars():