diff --git a/python/ql/src/Classes/IncompleteOrdering.ql b/python/ql/src/Classes/IncompleteOrdering.ql index d6cd1230ece..bbb6ca5cf6d 100644 --- a/python/ql/src/Classes/IncompleteOrdering.ql +++ b/python/ql/src/Classes/IncompleteOrdering.ql @@ -2,7 +2,8 @@ * @name Incomplete ordering * @description Class defines one or more ordering method but does not define all 4 ordering comparison methods * @kind problem - * @tags reliability + * @tags quality + * reliability * correctness * @problem.severity warning * @sub-severity low @@ -11,63 +12,46 @@ */ import python +import semmle.python.dataflow.new.internal.DataFlowDispatch +import semmle.python.ApiGraphs -predicate total_ordering(Class cls) { - exists(Attribute a | a = cls.getADecorator() | a.getName() = "total_ordering") +predicate totalOrdering(Class cls) { + cls.getADecorator() = + API::moduleImport("functools").getMember("total_ordering").asSource().asExpr() +} + +Function getMethod(Class cls, string name) { + result = cls.getAMethod() and + result.getName() = name +} + +predicate definesStrictOrdering(Class cls, Function meth) { + meth = getMethod(cls, "__lt__") or - exists(Name n | n = cls.getADecorator() | n.getId() = "total_ordering") + not exists(getMethod(cls, "__lt__")) and + meth = getMethod(cls, "__gt__") } -string ordering_name(int n) { - result = "__lt__" and n = 1 +predicate definesNonStrictOrdering(Class cls, Function meth) { + meth = getMethod(cls, "__le__") or - result = "__le__" and n = 2 + not exists(getMethod(cls, "__le__")) and + meth = getMethod(cls, "__ge__") +} + +predicate missingComparison(Class cls, Function defined, string missing) { + definesStrictOrdering(cls, defined) and + not definesNonStrictOrdering(getADirectSuperclass*(cls), _) and + missing = "__le__ or __ge__" or - result = "__gt__" and n = 3 - or - result = "__ge__" and n = 4 + definesNonStrictOrdering(cls, defined) and + not definesStrictOrdering(getADirectSuperclass*(cls), _) and + missing = "__lt__ or __gt__" } -predicate overrides_ordering_method(ClassValue c, string name) { - name = ordering_name(_) and - ( - c.declaresAttribute(name) - or - exists(ClassValue sup | sup = c.getASuperType() and not sup = Value::named("object") | - sup.declaresAttribute(name) - ) - ) -} - -string unimplemented_ordering(ClassValue c, int n) { - not c = Value::named("object") and - not overrides_ordering_method(c, result) and - result = ordering_name(n) -} - -string unimplemented_ordering_methods(ClassValue c, int n) { - n = 0 and result = "" and exists(unimplemented_ordering(c, _)) - or - exists(string prefix, int nm1 | n = nm1 + 1 and prefix = unimplemented_ordering_methods(c, nm1) | - prefix = "" and result = unimplemented_ordering(c, n) - or - result = prefix and not exists(unimplemented_ordering(c, n)) and n < 5 - or - prefix != "" and result = prefix + " or " + unimplemented_ordering(c, n) - ) -} - -Value ordering_method(ClassValue c, string name) { - /* If class doesn't declare a method then don't blame this class (the superclass will be blamed). */ - name = ordering_name(_) and result = c.declaredAttribute(name) -} - -from ClassValue c, Value ordering, string name +from Class cls, Function defined, string missing where - not c.failedInference(_) and - not total_ordering(c.getScope()) and - ordering = ordering_method(c, name) and - exists(unimplemented_ordering(c, _)) -select c, - "Class " + c.getName() + " implements $@, but does not implement " + - unimplemented_ordering_methods(c, 4) + ".", ordering, name + not totalOrdering(cls) and + missingComparison(cls, defined, missing) +select cls, "This class implements $@, but does not implement an " + missing + " method.", defined, + defined.getName()