From 28c20a3216446359997bb3f361cc9e66b1d97320 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 12 Mar 2019 15:05:39 +0000 Subject: [PATCH 1/2] Python: Fix false positive for redundant comparison query when a complex comparison is negated. --- .../Expressions/Comparisons/UselessComparisonTest.ql | 10 +++++++++- .../test/query-tests/Expressions/comparisons/test.py | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/python/ql/src/Expressions/Comparisons/UselessComparisonTest.ql b/python/ql/src/Expressions/Comparisons/UselessComparisonTest.ql index 35277300e25c..1a1083884ea7 100644 --- a/python/ql/src/Expressions/Comparisons/UselessComparisonTest.ql +++ b/python/ql/src/Expressions/Comparisons/UselessComparisonTest.ql @@ -15,6 +15,14 @@ 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. @@ -22,7 +30,7 @@ import semmle.python.Comparisons 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) { diff --git a/python/ql/test/query-tests/Expressions/comparisons/test.py b/python/ql/test/query-tests/Expressions/comparisons/test.py index b0bb709e2c34..aac73f4932eb 100644 --- a/python/ql/test/query-tests/Expressions/comparisons/test.py +++ b/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 + From 3fbe3c37aaa1155080d71193bcae369ad4839f7e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 13 Mar 2019 12:00:42 +0000 Subject: [PATCH 2/2] Add change note. --- change-notes/1.20/analysis-python.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.20/analysis-python.md b/change-notes/1.20/analysis-python.md index ead230e11cb5..149dcb6a7ec7 100644 --- a/change-notes/1.20/analysis-python.md +++ b/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. |