Merge pull request #1089 from markshannon/python-fix-redundant-comparison-complex-test

Fix false positive for redundant comparison query

Author: taus-semmle

change-notes/1.20/analysis-python.md

@@ -30,6 +30,7 @@ The API has been improved to declutter the global namespace and improve discover
 | Comparison using is when operands support \_\_eq\_\_ (`py/comparison-using-is`) | Fewer false positive results | Results where one of the objects being compared is an enum member are no longer reported. |
 | Modification of parameter with default (`py/modification-of-default-value`) | More true positive results | Instances where the mutable default value is mutated inside other functions are now also reported. |
 | Mutation of descriptor in \_\_get\_\_ or \_\_set\_\_ method (`py/mutable-descriptor`) | Fewer false positive results | Results where the mutation does not occur when calling one of the  `__get__`, `__set__` or `__delete__` methods are no longer reported. |
+| Redundant comparison (`py/redundant-comparison`) | Fewer false positive results | Results in chained comparisons are no longer reported. |
 | Unused import (`py/unused-import`) | Fewer false positive results | Results where the imported module is used in a `doctest` string are no longer reported. |
 | Unused import (`py/unused-import`) | Fewer false positive results | Results where the imported module is used in a type-hint comment are no longer reported. |
 

python/ql/src/Expressions/Comparisons/UselessComparisonTest.ql

@@ -15,14 +15,22 @@
 import python
 import semmle.python.Comparisons
 
+/* Holds if the comparison `comp` is of the complex form `a op b op c` and not of
+ * the simple form `a op b`.
+ */
+private predicate is_complex(Expr comp) {
+    exists(comp.(Compare).getOp(1))
+    or
+    is_complex(comp.(UnaryExpr).getOperand())
+}
 
 /** A test is useless if for every block that it controls there is another test that is at least as
  * strict and also controls that block.
  */
 private predicate useless_test(Comparison comp, ComparisonControlBlock controls, boolean isTrue) {
     controls.impliesThat(comp.getBasicBlock(), comp, isTrue) and
     /* Exclude complex comparisons of form `a < x < y`, as we do not (yet) have perfect flow control for those */
-    not exists(controls.getTest().getNode().(Compare).getOp(1))
+    not is_complex(controls.getTest().getNode())
 }
 
 private predicate useless_test_ast(AstNode comp, AstNode previous, boolean isTrue) {

python/ql/test/query-tests/Expressions/comparisons/test.py

@@ -86,3 +86,8 @@ def odasa6782_v3(protocol):
         pass
     else:
         raise ValueError()
+
+#Inverted complex test
+if not (0 > stop >= step) and stop < 0:
+    pass
+