Skip to content

Commit 233ae5a

Browse files
authored
Python: Fix FP in py/unused-local-variable
This is only a temporary fix, as indicated by the TODO comment. The real underlying issue is the fact that `isUnused` is defined in terms of the underlying SSA variables (as these are only created for variables that are actually used), and the fact that annotated assignments are always considered to redefine their targets, which may not actually be the case. Thus, the correct fix would be to change the extractor to _disregard_ mere type annotations for the purposes of figuring out whether an SSA variable should be created or not. However, in the short term the present fix is likely sufficient.
1 parent 8b3fa78 commit 233ae5a

File tree

2 files changed

+12
-1
lines changed

2 files changed

+12
-1
lines changed

python/ql/src/Variables/UnusedLocalVariable.ql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,25 @@ predicate unused_local(Name unused, LocalVariable v) {
1919
def.getVariable() = v and
2020
def.isUnused() and
2121
not exists(def.getARedef()) and
22+
not exists(annotation_without_assignment(v)) and
2223
def.isRelevant() and
2324
not v = any(Nonlocal n).getAVariable() and
2425
not exists(def.getNode().getParentNode().(FunctionDef).getDefinedFunction().getADecorator()) and
2526
not exists(def.getNode().getParentNode().(ClassDef).getDefinedClass().getADecorator())
2627
)
2728
}
2829

30+
/**
31+
* Gets any annotation of the local variable `v` that does not also reassign its value.
32+
*
33+
* TODO: This predicate should not be needed. Rather, annotated "assignments" that do not actually
34+
* assign a value should not result in the creation of an SSA variable (which then goes unused).
35+
*/
36+
private AnnAssign annotation_without_assignment(LocalVariable v) {
37+
result.getTarget() = v.getAStore() and
38+
not exists(result.getValue())
39+
}
40+
2941
from Name unused, LocalVariable v
3042
where
3143
unused_local(unused, v) and

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
| type_annotation_fp.py:5:5:5:7 | foo | The value assigned to local variable 'foo' is never used. |
21
| variables_test.py:29:5:29:5 | x | The value assigned to local variable 'x' is never used. |
32
| variables_test.py:89:5:89:5 | a | The value assigned to local variable 'a' is never used. |
43
| variables_test.py:89:7:89:7 | b | The value assigned to local variable 'b' is never used. |

0 commit comments

Comments
 (0)