Python: Exclude iterators guarded by isinstance checks

A common pattern is to check `isinstance(it, (list, tuple)` before
proceeding with the iteration.

python/ql/src/Statements/NonIteratorInForLoop.ql

@@ -13,14 +13,47 @@
 
 import python
 private import semmle.python.dataflow.new.internal.DataFlowDispatch
+private import semmle.python.ApiGraphs
 
-from For loop, Expr iter, Class cls
+/**
+ * Holds if `cls_arg` references a known iterable builtin type, either directly
+ * (e.g. `list`) or as an element of a tuple (e.g. `(list, tuple)`).
+ */
+private predicate isIterableTypeArg(DataFlow::Node cls_arg) {
+  cls_arg =
+    API::builtin([
+        "list", "tuple", "set", "frozenset", "dict", "str", "bytes", "bytearray", "range",
+        "memoryview"
+      ]).getAValueReachableFromSource()
+  or
+  isIterableTypeArg(DataFlow::exprNode(cls_arg.asExpr().(Tuple).getAnElt()))
+}
+
+/**
+ * Holds if `iter` is guarded by an `isinstance` check that tests for
+ * an iterable type (e.g. `list`, `tuple`, `set`, `dict`).
+ */
+predicate guardedByIsinstanceIterable(DataFlow::Node iter) {
+  exists(
+    ConditionBlock guard, DataFlow::CallCfgNode isinstance_call, DataFlow::LocalSourceNode src
+  |
+    isinstance_call = API::builtin("isinstance").getACall() and
+    src.flowsTo(isinstance_call.getArg(0)) and
+    src.flowsTo(iter) and
+    isIterableTypeArg(isinstance_call.getArg(1)) and
+    guard.getLastNode() = isinstance_call.asCfgNode() and
+    guard.controls(iter.asCfgNode().getBasicBlock(), true)
+  )
+}
+
+from For loop, DataFlow::Node iter, Class cls
 where
-  iter = loop.getIter() and
-  classInstanceTracker(cls).asExpr() = iter and
+  iter.asExpr() = loop.getIter() and
+  iter = classInstanceTracker(cls) and
   not DuckTyping::isIterable(cls) and
   not DuckTyping::isDescriptor(cls) and
   not (loop.isAsync() and DuckTyping::hasMethod(cls, "__aiter__")) and
-  not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls))
-select loop, "This for-loop may attempt to iterate over a $@ of class $@.", iter,
+  not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls)) and
+  not guardedByIsinstanceIterable(iter)
+select loop, "This for-loop may attempt to iterate over a $@ of class $@.", iter.asExpr(),
   "non-iterable instance", cls, cls.getName()

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

@@ -174,3 +174,16 @@ def assert_ok(seq):
 # False positive. ODASA-8042. Fixed in PR #2401.
 class false_positive:
     e = (x for x in [])
+
+# isinstance guard should suppress non-iterable warning
+def guarded_iteration(x):
+    ni = NonIterator()
+    if isinstance(ni, (list, tuple)):
+        for item in ni:
+            pass
+
+def guarded_iteration_single(x):
+    ni = NonIterator()
+    if isinstance(ni, list):
+        for item in ni:
+            pass