Skip to content

Commit d336140

Browse files
committed
Python: Modernise the py/non-iterable-in-for-loop query.
Also adds a small test case exhibiting the same false positive seen in ODASA-8042.
1 parent ef7984d commit d336140

File tree

4 files changed

+20
-4
lines changed

4 files changed

+20
-4
lines changed

python/ql/src/Statements/NonIteratorInForLoop.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313

1414
import python
1515

16-
from For loop, ControlFlowNode iter, ClassObject t, ControlFlowNode origin
16+
from For loop, ControlFlowNode iter, Value v, ClassValue t, ControlFlowNode origin
1717
where loop.getIter().getAFlowNode() = iter and
18-
iter.refersTo(_, t, origin) and
19-
not t.isIterable() and not t.failedInference() and
20-
not t = theNoneType() and
18+
iter.pointsTo(_, v, origin) and v.getClass() = t and
19+
not t.isIterable() and not t.failedInference(_) and
20+
not v = Value::named("None") and
2121
not t.isDescriptorType()
2222

2323
select loop, "$@ of class '$@' may be used in for-loop.", origin, "Non-iterator", t, t.getName()

python/ql/src/semmle/python/objects/ObjectAPI.qll

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,18 @@ class ClassValue extends Value {
346346
this.(ClassObjectInternal).lookup("__call__", _, _)
347347
}
348348

349+
/** Holds if this class is an iterable. */
350+
predicate isIterable() {
351+
this.hasAttribute("__iter__")
352+
or
353+
this.hasAttribute("__getitem__")
354+
}
355+
356+
/** Holds if this class is a descriptor. */
357+
predicate isDescriptorType() {
358+
this.hasAttribute("__get__")
359+
}
360+
349361
/** Gets the qualified name for this class.
350362
* Should return the same name as the `__qualname__` attribute on classes in Python 3.
351363
*/
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
| test.py:50:1:50:23 | For | $@ of class '$@' may be used in for-loop. | test.py:50:10:50:22 | ControlFlowNode for NonIterator() | Non-iterator | test.py:45:1:45:26 | class NonIterator | NonIterator |
2+
| test.py:170:10:170:22 | For | $@ of class '$@' may be used in for-loop. | test.py:170:10:170:22 | ControlFlowNode for .0 | Non-iterator | test.py:169:1:169:21 | class false_positive | false_positive |

python/ql/test/query-tests/Statements/general/test.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,4 +165,7 @@ def no_with():
165165
def assert_ok(seq):
166166
assert all(isinstance(element, (str, unicode)) for element in seq)
167167

168+
# False positive. ODASA-8042
169+
class false_positive:
170+
e = (x for x in [])
168171

0 commit comments

Comments
 (0)