Python: Modernize 3 quality queries for comparison methods

python/ql/src/Classes/Comparisons/Comparisons.qll

@@ -5,6 +5,8 @@ import semmle.python.ApiGraphs
 
 /** Holds if `cls` has the `functools.total_ordering` decorator. */
 predicate totalOrdering(Class cls) {
-  cls.getADecorator() =
-    API::moduleImport("functools").getMember("total_ordering").asSource().asExpr()
+  API::moduleImport("functools")
+      .getMember("total_ordering")
+      .asSource()
+      .flowsTo(DataFlow::exprNode(cls.getADecorator()))
 }

python/ql/test/query-tests/Classes/equals-hash/EqualsOrHash.expected

@@ -0,0 +1,2 @@
+| equalsHash.py:13:1:13:8 | Class C | This class implements $@, but does not implement __eq__. | equalsHash.py:14:5:14:23 | Function __hash__ | __hash__ |
+| equalsHash.py:17:1:17:11 | Class D | This class implements $@, but does not implement __eq__. | equalsHash.py:18:5:18:23 | Function __hash__ | __hash__ |

python/ql/test/query-tests/Classes/equals-hash/EqualsOrHash.qlref

@@ -0,0 +1,2 @@
+query: Classes/Comparisons/EqualsOrHash.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

python/ql/test/query-tests/Classes/equals-hash/equalsHash.py

@@ -0,0 +1,19 @@
+class A:
+    def __eq__(self, other):
+        return True 
+
+    def __hash__(self, other):
+        return 7 
+
+# B is automatically non-hashable - so eq without hash never needs to alert
+class B:
+    def __eq__(self, other):
+        return True 
+
+class C: # $ Alert
+    def __hash__(self):
+        return 5
+
+class D(A): # $ Alert 
+    def __hash__(self):
+        return 4

python/ql/test/query-tests/Classes/equals-not-equals/EqualsOrNotEquals.expected

@@ -0,0 +1,2 @@
+| EqualsOrNotEquals.py:14:1:14:8 | Class B | This class implements $@, but does not implement __eq__. | EqualsOrNotEquals.py:19:5:19:28 | Function __ne__ | __ne__ |
+| EqualsOrNotEquals.py:37:1:37:11 | Class D | This class implements $@, but does not implement __ne__. | EqualsOrNotEquals.py:43:5:43:28 | Function __eq__ | __eq__ |

python/ql/test/query-tests/Classes/equals-not-equals/EqualsOrNotEquals.py

@@ -0,0 +1,147 @@
+class A:
+    def __init__(self, a):
+        self.a = a
+
+    # OK: __ne__ if not defined delegates to eq automatically
+    def __eq__(self, other):
+        return self.a == other.a 
+
+assert (A(1) == A(1))
+assert not (A(1) == A(2))
+assert not (A(1) != A(1))
+assert (A(1) != A(2))
+
+class B: # $ Alert 
+    def __init__(self, b):
+        self.b = b
+
+    # BAD: eq defaults to `is`
+    def __ne__(self, other):
+        return self.b != other.b 
+
+assert not (B(1) == B(1)) # potentially unexpected
+assert not (B(2) == B(2))
+assert not (B(1) != B(1))
+assert (B(1) != B(2))
+
+class C:
+    def __init__(self, c):
+        self.c = c 
+
+    def __eq__(self, other):
+        return self.c == other.c 
+
+    def __ne__(self, other):
+        return self.c != other.c 
+
+class D(C): # $ Alert 
+    def __init__(self, c, d):
+        super().__init__(c)
+        self.d = d
+
+    # BAD: ne is not defined, but the superclass ne is used instead of delegating, which may be incorrect
+    def __eq__(self, other):
+        return self.c == other.c and self.d == other.d
+
+assert (D(1,2) == D(1,2))
+assert not (D(1,2) == D(1,3))
+assert (D(1,2) != D(3,2)) 
+assert not (D(1,2) != D(1,3)) # Potentially unexpected 
+
+class E:
+    def __init__(self, e):
+        self.e = e 
+
+    def __eq__(self, other):
+        return self.e == other.e 
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+class F(E): 
+    def __init__(self, e, f):
+        super().__init__(e)
+        self.f = f
+
+    # OK: superclass ne delegates to eq
+    def __eq__(self, other):
+        return self.e == other.e and self.f == other.f
+
+assert (F(1,2) == F(1,2))
+assert not (F(1,2) == F(1,3))
+assert (F(1,2) != F(3,2)) 
+assert (F(1,2) != F(1,3))
+
+# Variations 
+
+class E2:
+    def __init__(self, e):
+        self.e = e 
+
+    def __eq__(self, other):
+        return self.e == other.e 
+
+    def __ne__(self, other):
+        return not self == other
+
+class F2(E2): 
+    def __init__(self, e, f):
+        super().__init__(e)
+        self.f = f
+
+    # OK: superclass ne delegates to eq
+    def __eq__(self, other):
+        return self.e == other.e and self.f == other.f
+
+assert (F2(1,2) == F2(1,2))
+assert not (F2(1,2) == F2(1,3))
+assert (F2(1,2) != F2(3,2)) 
+assert (F2(1,2) != F2(1,3))
+
+class E3:
+    def __init__(self, e):
+        self.e = e 
+
+    def __eq__(self, other):
+        return self.e == other.e 
+
+    def __ne__(self, other):
+        return not other.__eq__(self)
+
+class F3(E3): 
+    def __init__(self, e, f):
+        super().__init__(e)
+        self.f = f
+
+    # OK: superclass ne delegates to eq
+    def __eq__(self, other):
+        return self.e == other.e and self.f == other.f
+
+assert (F3(1,2) == F3(1,2))
+assert not (F3(1,2) == F3(1,3))
+assert (F3(1,2) != F3(3,2)) 
+assert (F3(1,2) != F3(1,3))
+
+class E4:
+    def __init__(self, e):
+        self.e = e 
+
+    def __eq__(self, other):
+        return self.e == other.e 
+
+    def __ne__(self, other):
+        return not other == self
+
+class F4(E4): 
+    def __init__(self, e, f):
+        super().__init__(e)
+        self.f = f
+
+    # OK: superclass ne delegates to eq
+    def __eq__(self, other):
+        return self.e == other.e and self.f == other.f
+
+assert (F4(1,2) == F4(1,2))
+assert not (F4(1,2) == F4(1,3))
+assert (F4(1,2) != F4(3,2)) 
+assert (F4(1,2) != F4(1,3))

python/ql/test/query-tests/Classes/equals-not-equals/EqualsOrNotEquals.qlref

@@ -0,0 +1,2 @@
+query: Classes/Comparisons/EqualsOrNotEquals.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

python/ql/test/query-tests/Classes/incomplete-ordering/IncompleteOrdering.expected

@@ -1 +1,2 @@
-| incomplete_ordering.py:3:1:3:26 | class PartOrdered | Class PartOrdered implements $@, but does not implement __le__ or __gt__ or __ge__. | incomplete_ordering.py:13:5:13:28 | Function PartOrdered.__lt__ | __lt__ |
+| incomplete_ordering.py:3:1:3:26 | Class LtWithoutLe | This class implements $@, but does not implement __le__ or __ge__. | incomplete_ordering.py:13:5:13:28 | Function __lt__ | __lt__ |
+| incomplete_ordering.py:28:1:28:17 | Class LendGeNoLt | This class implements $@, but does not implement __lt__ or __gt__. | incomplete_ordering.py:29:5:29:28 | Function __le__ | __le__ |

python/ql/test/query-tests/Classes/incomplete-ordering/IncompleteOrdering.qlref

@@ -1 +1,2 @@
-Classes/IncompleteOrdering.ql
+query: Classes/Comparisons/IncompleteOrdering.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

python/ql/test/query-tests/Classes/incomplete-ordering/incomplete_ordering.py

@@ -1,6 +1,6 @@
 #Incomplete ordering
 
-class PartOrdered(object):
+class LtWithoutLe(object): # $ Alert
     def __eq__(self, other):
         return self is other
 
@@ -13,6 +13,28 @@ def __hash__(self):
     def __lt__(self, other):
         return False
 
-#Don't blame a sub-class for super-class's sins.
-class DerivedPartOrdered(PartOrdered):
-    pass
+# Don't alert on subclass
+class LtWithoutLeSub(LtWithoutLe):
+    pass
+
+class LeSub(LtWithoutLe):
+    def __le__(self, other):
+        return self < other or self == other 
+
+class GeSub(LtWithoutLe):
+    def __ge__(self, other):
+        return self > other or self == other 
+
+class LendGeNoLt: # $ Alert
+    def __le__(self, other):
+        return True 
+
+    def __ge__(self, other):
+        return other <= self 
+
+from functools import total_ordering
+
+@total_ordering
+class Total:
+    def __le__(self, other):
+        return True