Merge pull request #6336 from tausbn/python-make-annotated-assignment-a-definitionnode

Python: Two fixes regarding annotated assignments

Author: RasmusWL

python/ql/lib/semmle/python/Flow.qll

@@ -653,6 +653,8 @@ class DefinitionNode extends ControlFlowNode {
   DefinitionNode() {
     exists(Assign a | a.getATarget().getAFlowNode() = this)
     or
+    exists(AnnAssign a | a.getTarget().getAFlowNode() = this and exists(a.getValue()))
+    or
     exists(Alias a | a.getAsname().getAFlowNode() = this)
     or
     augstore(_, this)
@@ -795,6 +797,9 @@ private AstNode assigned_value(Expr lhs) {
   /* lhs = result */
   exists(Assign a | a.getATarget() = lhs and result = a.getValue())
   or
+  /* lhs : annotation = result */
+  exists(AnnAssign a | a.getTarget() = lhs and result = a.getValue())
+  or
   /* import result as lhs */
   exists(Alias a | a.getAsname() = lhs and result = a.getValue())
   or

python/ql/src/Variables/UnusedLocalVariable.ql

@@ -19,13 +19,25 @@ predicate unused_local(Name unused, LocalVariable v) {
     def.getVariable() = v and
     def.isUnused() and
     not exists(def.getARedef()) and
+    not exists(annotation_without_assignment(v)) and
     def.isRelevant() and
     not v = any(Nonlocal n).getAVariable() and
     not exists(def.getNode().getParentNode().(FunctionDef).getDefinedFunction().getADecorator()) and
     not exists(def.getNode().getParentNode().(ClassDef).getDefinedClass().getADecorator())
   )
 }
 
+/**
+ * Gets any annotation of the local variable `v` that does not also reassign its value.
+ *
+ * TODO: This predicate should not be needed. Rather, annotated "assignments" that do not actually
+ * assign a value should not result in the creation of an SSA variable (which then goes unused).
+ */
+private AnnAssign annotation_without_assignment(LocalVariable v) {
+  result.getTarget() = v.getAStore() and
+  not exists(result.getValue())
+}
+
 from Name unused, LocalVariable v
 where
   unused_local(unused, v) and

python/ql/test/query-tests/Variables/undefined/UninitializedLocal.py

@@ -288,3 +288,8 @@ def avoid_redundant_split(a):
         var = False
     if var:
         foo.bar() #foo is defined here.
+
+def type_annotation_fp():
+    annotated : annotation = [1,2,3]
+    for x in annotated:
+        print(x)

python/ql/test/query-tests/Variables/unused/UnusedLocalVariable.expected

@@ -1,4 +1,3 @@
-| type_annotation_fp.py:5:5:5:7 | foo | The value assigned to local variable 'foo' is never used. |
 | variables_test.py:29:5:29:5 | x | The value assigned to local variable 'x' is never used. |
 | variables_test.py:89:5:89:5 | a | The value assigned to local variable 'a' is never used. |
 | variables_test.py:89:7:89:7 | b | The value assigned to local variable 'b' is never used. |

python/ql/test/query-tests/Variables/unused/type_annotation_fp.py

@@ -9,3 +9,8 @@ def type_annotation(x):
     else:
         foo : float
         do_other_stuff_with(foo)
+
+def type_annotation_fn():
+    # False negative: the value of `bar` is never used, but this is masked by the presence of the type annotation.
+    bar = 5
+    bar : int