From bedd44a2877e2b64b0e0717e49213fc5d46c8c02 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Wed, 21 May 2025 11:02:24 +0100 Subject: [PATCH 01/10] Update query and add case for iter(self.__next__, None) --- python/ql/src/Functions/IterReturnsNonSelf.ql | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.ql b/python/ql/src/Functions/IterReturnsNonSelf.ql index 385677a57630..38da2954e1c7 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.ql +++ b/python/ql/src/Functions/IterReturnsNonSelf.ql @@ -4,6 +4,7 @@ * @kind problem * @tags reliability * correctness + * quality * @problem.severity error * @sub-severity low * @precision high @@ -11,20 +12,46 @@ */ import python +import semmle.python.dataflow.new.DataFlow +import semmle.python.ApiGraphs -Function iter_method(ClassValue t) { result = t.lookup("__iter__").(FunctionValue).getScope() } +/** Holds if `f` is a method of the class `c`. */ +private predicate methodOfClass(Function f, Class c) { + exists(FunctionDef d | d.getDefinedFunction() = f and d.getScope() = c) +} + +Function iterMethod(Class c) { methodOfClass(result, c) and result.getName() = "__iter__" } + +Function nextMethod(Class c) { methodOfClass(result, c) and result.getName() = "__next__" } -predicate is_self(Name value, Function f) { value.getVariable() = f.getArg(0).(Name).getVariable() } +predicate isSelfVar(Function f, Name var) { var.getVariable() = f.getArg(0).(Name).getVariable() } + +predicate isGoodReturn(Function f, Expr e) { + isSelfVar(f, e) + or + exists(DataFlow::CallCfgNode call, DataFlow::AttrRead read, DataFlow::Node selfNode | + e = call.asExpr() + | + call = API::builtin("iter").getACall() and + call.getArg(0) = read and + read.accesses(selfNode, "__next__") and + isSelfVar(f, selfNode.asExpr()) and + call.getArg(1).asExpr() instanceof None + ) +} -predicate returns_non_self(Function f) { +predicate returnsNonSelf(Function f) { exists(f.getFallthroughNode()) or - exists(Return r | r.getScope() = f and not is_self(r.getValue(), f)) + exists(Return r | r.getScope() = f and not isGoodReturn(f, r.getValue())) or exists(Return r | r.getScope() = f and not exists(r.getValue())) } -from ClassValue t, Function iter -where t.isIterator() and iter = iter_method(t) and returns_non_self(iter) -select t, "Class " + t.getName() + " is an iterator but its $@ method does not return 'self'.", - iter, iter.getName() +from Class c, Function iter +where + exists(nextMethod(c)) and + iter = iterMethod(c) and + returnsNonSelf(iter) +select iter, "Iter method of iterator $@ does not return `" + iter.getArg(0).getName() + "`.", c, + c.getName() From 7b452a1611c961d504620462fcfb42be71be5f7a Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Thu, 22 May 2025 09:01:15 +0100 Subject: [PATCH 02/10] Add case for wrappers --- python/ql/src/Functions/IterReturnsNonSelf.ql | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.ql b/python/ql/src/Functions/IterReturnsNonSelf.ql index 38da2954e1c7..e1672d25272a 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.ql +++ b/python/ql/src/Functions/IterReturnsNonSelf.ql @@ -20,12 +20,16 @@ private predicate methodOfClass(Function f, Class c) { exists(FunctionDef d | d.getDefinedFunction() = f and d.getScope() = c) } +/** Gets the __iter__ method of `c`. */ Function iterMethod(Class c) { methodOfClass(result, c) and result.getName() = "__iter__" } +/** Gets the `__next__` method of `c`. */ Function nextMethod(Class c) { methodOfClass(result, c) and result.getName() = "__next__" } +/** Holds if `var` is a variable referring to the `self` parameter of `f`. */ predicate isSelfVar(Function f, Name var) { var.getVariable() = f.getArg(0).(Name).getVariable() } +/** Holds if `e` is an expression that an iter function `f` should return. */ predicate isGoodReturn(Function f, Expr e) { isSelfVar(f, e) or @@ -40,6 +44,7 @@ predicate isGoodReturn(Function f, Expr e) { ) } +/** Holds if the iter method `f` does not return `self` or an equivalent. */ predicate returnsNonSelf(Function f) { exists(f.getFallthroughNode()) or @@ -48,10 +53,46 @@ predicate returnsNonSelf(Function f) { exists(Return r | r.getScope() = f and not exists(r.getValue())) } -from Class c, Function iter +/** Holds if `iter` and `next` methods are wrappers around some field. */ +predicate iterWrapperMethods(Function iter, Function next) { + exists(string field | + exists(Return r, DataFlow::Node self, DataFlow::AttrRead read | + r.getScope() = iter and + r.getValue() = iterCall(read).asExpr() and + read.accesses(self, field) and + isSelfVar(iter, self.asExpr()) + ) and + exists(Return r, DataFlow::Node self, DataFlow::AttrRead read | + r.getScope() = next and + r.getValue() = nextCall(read).asExpr() and + read.accesses(self, field) and + isSelfVar(next, self.asExpr()) + ) + ) +} + +DataFlow::CallCfgNode iterCall(DataFlow::Node arg) { + result.(DataFlow::MethodCallNode).calls(arg, "__iter__") + or + result = API::builtin("iter").getACall() and + arg = result.getArg(0) and + not exists(result.getArg(1)) + or + result = arg // assume the wrapping field is already an iterator +} + +DataFlow::CallCfgNode nextCall(DataFlow::Node arg) { + result.(DataFlow::MethodCallNode).calls(arg, "__next__") + or + result = API::builtin("next").getACall() and + arg = result.getArg(0) +} + +from Class c, Function iter, Function next where - exists(nextMethod(c)) and + next = nextMethod(c) and iter = iterMethod(c) and - returnsNonSelf(iter) + returnsNonSelf(iter) and + not iterWrapperMethods(iter, next) select iter, "Iter method of iterator $@ does not return `" + iter.getArg(0).getName() + "`.", c, c.getName() From f27057a747a3a0e308eaf5627fe3275642f62c3b Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 10:56:43 +0100 Subject: [PATCH 03/10] Update qhelp --- .../ql/src/Functions/IterReturnsNonSelf.qhelp | 27 +++++++------------ .../{ => examples}/IterReturnsNonSelf.py | 6 ++--- 2 files changed, 13 insertions(+), 20 deletions(-) rename python/ql/src/Functions/{ => examples}/IterReturnsNonSelf.py (65%) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.qhelp b/python/ql/src/Functions/IterReturnsNonSelf.qhelp index f614d912ff0a..94b6c30fbfec 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.qhelp +++ b/python/ql/src/Functions/IterReturnsNonSelf.qhelp @@ -3,34 +3,27 @@ "qhelp.dtd"> -

The __iter__ method of an iterator should return self. -This is important so that iterators can be used as sequences in any context -that expect a sequence. To do so requires that __iter__ is -idempotent on iterators.

- -

-Note that sequences and mapping should return a new iterator, it is just the returned -iterator that must obey this constraint. +

Iterator classes (classes defining a __next__ method) should have an __iter__ that returns the iterator itself. +This ensures that the object is also an iterable; and behaves as expected when used anywhere an iterator or iterable is expected, such as in for loops.

+ + -

Make the __iter__ return self unless the class should not be an iterator, -in which case rename the next (Python 2) or __next__ (Python 3) -to something else.

+

Ensure that the __iter__ method returns self, or is otherwise equivalent as an iterator to self.

 -

In this example the Counter class's __iter__ method does not -return self (or even an iterator). This will cause the program to fail when anyone attempts -to use the iterator in a for loop or in statement.

- +

In the following example, the MyRange class's __iter__ method does not return self. +This would lead to unexpected results when used with a for loop or in statement. + -

  • Python Language Reference: object.__iter__.
    • -
  • Python Standard Library: Iterators.
    • +
  • Python Language Reference: object.__iter__.
    • +
  • Python Standard Library: Iterators.
    • diff --git a/python/ql/src/Functions/IterReturnsNonSelf.py b/python/ql/src/Functions/examples/IterReturnsNonSelf.py similarity index 65% rename from python/ql/src/Functions/IterReturnsNonSelf.py rename to python/ql/src/Functions/examples/IterReturnsNonSelf.py index 6251b87aba7b..20ba5eb18215 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.py +++ b/python/ql/src/Functions/examples/IterReturnsNonSelf.py @@ -4,10 +4,10 @@ def __init__(self, low, high): self.high = high def __iter__(self): - return self.current + return (self.current, self.high) # BAD: does not return `self`. - def next(self): + def __next__(self): if self.current > self.high: - raise StopIteration + return None self.current += 1 return self.current - 1 \ No newline at end of file From 06504f2cb650feb6adc00d6a02ce08b1dfc39176 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 13:04:56 +0100 Subject: [PATCH 04/10] Update tests --- .../python-code-quality.qls.expected | 1 + .../general/IterReturnsNonSelf.expected | 1 - .../iterators/IterReturnsNonSelf.expected | 0 .../IterReturnsNonSelf.qlref | 0 .../query-tests/Functions/iterators/test.py | 43 +++++++++++++++++++ 5 files changed, 44 insertions(+), 1 deletion(-) delete mode 100644 python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.expected create mode 100644 python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected rename python/ql/test/query-tests/Functions/{general => iterators}/IterReturnsNonSelf.qlref (100%) create mode 100644 python/ql/test/query-tests/Functions/iterators/test.py diff --git a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected index b81d300d0241..7f1bb89365e4 100644 --- a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected +++ b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected @@ -2,5 +2,6 @@ ql/python/ql/src/Functions/NonCls.ql ql/python/ql/src/Functions/NonSelf.ql ql/python/ql/src/Functions/ReturnConsistentTupleSizes.ql ql/python/ql/src/Functions/SignatureSpecialMethods.ql +ql/python/ql/src/Functions/IterReturnsNonSelf.ql ql/python/ql/src/Resources/FileNotAlwaysClosed.ql ql/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql diff --git a/python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.expected b/python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.expected deleted file mode 100644 index 9fd22c1df612..000000000000 --- a/python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.expected +++ /dev/null @@ -1 +0,0 @@ -| protocols.py:54:1:54:29 | class AlmostIterator | Class AlmostIterator is an iterator but its $@ method does not return 'self'. | protocols.py:62:5:62:23 | Function __iter__ | __iter__ | diff --git a/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.qlref b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.qlref similarity index 100% rename from python/ql/test/query-tests/Functions/general/IterReturnsNonSelf.qlref rename to python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.qlref diff --git a/python/ql/test/query-tests/Functions/iterators/test.py b/python/ql/test/query-tests/Functions/iterators/test.py new file mode 100644 index 000000000000..be334be77b7d --- /dev/null +++ b/python/ql/test/query-tests/Functions/iterators/test.py @@ -0,0 +1,43 @@ +class Bad1: + def __next__(self): + return 0 + + def __iter__(self): # BAD: Iter does not return self + yield 0 + +class Good1: + def __next__(self): + return 0 + + def __iter__(self): # GOOD: iter returns self + return self + +class Good2: + def __init__(self): + self._it = iter([0,0,0]) + + def __next__(self): + return next(self._it) + + def __iter__(self): # GOOD: iter and next are wrappers around a field + return self._it.__iter__() + +class Good3: + def __next__(self): + return 0 + + def __iter__(self): # GOOD: this is an equivalent iterator to `self`. + return iter(self.__next__, None) + +class FalsePositive1: + def __init__(self): + self._it = None + + def __next__(self): + if self._it is None: + self._it = iter(self) + return next(self._it) + + def __iter__(self): # SPURIOUS, GOOD: implementation of next ensures the iterator is equivalent to the one returned by iter, but this is not detected. + yield 0 + yield 0 \ No newline at end of file From 44a678a3f4f1b49dcfe87c2a1eb04fc8c963bb81 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 13:16:13 +0100 Subject: [PATCH 05/10] remove redundant import --- python/ql/src/Functions/IterReturnsNonSelf.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.ql b/python/ql/src/Functions/IterReturnsNonSelf.ql index e1672d25272a..275e2a1ebf6e 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.ql +++ b/python/ql/src/Functions/IterReturnsNonSelf.ql @@ -12,7 +12,6 @@ */ import python -import semmle.python.dataflow.new.DataFlow import semmle.python.ApiGraphs /** Holds if `f` is a method of the class `c`. */ From b15fec0fb97e53411ca0cb10f098bdb2c947f7b5 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 14:17:21 +0100 Subject: [PATCH 06/10] Fix qhelp and tests --- .../query-suite/python-code-quality.qls.expected | 2 +- python/ql/src/Functions/IterReturnsNonSelf.qhelp | 2 +- .../query-tests/Functions/iterators/IterReturnsNonSelf.expected | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected index 7f1bb89365e4..c2168cab937b 100644 --- a/python/ql/integration-tests/query-suite/python-code-quality.qls.expected +++ b/python/ql/integration-tests/query-suite/python-code-quality.qls.expected @@ -1,7 +1,7 @@ +ql/python/ql/src/Functions/IterReturnsNonSelf.ql ql/python/ql/src/Functions/NonCls.ql ql/python/ql/src/Functions/NonSelf.ql ql/python/ql/src/Functions/ReturnConsistentTupleSizes.ql ql/python/ql/src/Functions/SignatureSpecialMethods.ql -ql/python/ql/src/Functions/IterReturnsNonSelf.ql ql/python/ql/src/Resources/FileNotAlwaysClosed.ql ql/python/ql/src/Variables/LoopVariableCapture/LoopVariableCapture.ql diff --git a/python/ql/src/Functions/IterReturnsNonSelf.qhelp b/python/ql/src/Functions/IterReturnsNonSelf.qhelp index 94b6c30fbfec..b668e3509672 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.qhelp +++ b/python/ql/src/Functions/IterReturnsNonSelf.qhelp @@ -11,7 +11,7 @@ This ensures that the object is also an iterable; and behaves as expected when u -

    Ensure that the __iter__ method returns self, or is otherwise equivalent as an iterator to self.

    +

    Ensure that the __iter__ method returns self, or is otherwise equivalent as an iterator to self.

     diff --git a/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected index e69de29bb2d1..f07c637241e9 100644 --- a/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected +++ b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected @@ -0,0 +1,2 @@ +| test.py:5:5:5:23 | Function __iter__ | Iter method of iterator $@ does not return `self`. | test.py:1:1:1:11 | Class Bad1 | Bad1 | +| test.py:41:5:41:23 | Function __iter__ | Iter method of iterator $@ does not return `self`. | test.py:32:1:32:21 | Class FalsePositive1 | FalsePositive1 | From e933a27cd95a3bd6d33d880aae157db09b406e9d Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 14:25:38 +0100 Subject: [PATCH 07/10] Add changenote --- python/ql/src/change-notes/2025-05-23-iter-not-return-self.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 python/ql/src/change-notes/2025-05-23-iter-not-return-self.md diff --git a/python/ql/src/change-notes/2025-05-23-iter-not-return-self.md b/python/ql/src/change-notes/2025-05-23-iter-not-return-self.md new file mode 100644 index 000000000000..80b8313a72b8 --- /dev/null +++ b/python/ql/src/change-notes/2025-05-23-iter-not-return-self.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `py/iter-returns-non-self` query has been modernized, and no longer alerts for certain cases where an equivalent iterator is returned. \ No newline at end of file From c070d04231772818887d0747acee27bdc70d2265 Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 23 May 2025 14:31:13 +0100 Subject: [PATCH 08/10] Fix qhelp --- python/ql/src/Functions/IterReturnsNonSelf.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.qhelp b/python/ql/src/Functions/IterReturnsNonSelf.qhelp index b668e3509672..5b37e8cffeee 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.qhelp +++ b/python/ql/src/Functions/IterReturnsNonSelf.qhelp @@ -16,7 +16,7 @@ This ensures that the object is also an iterable; and behaves as expected when u

    In the following example, the MyRange class's __iter__ method does not return self. -This would lead to unexpected results when used with a for loop or in statement. +This would lead to unexpected results when used with a for loop or in statement.

     From f3a5608b06454b3f0e57e07de7c223141f4b858a Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Tue, 27 May 2025 13:35:13 +0100 Subject: [PATCH 09/10] Apply review suggestions - remove methodOfClass, fix qhelp typo; additionally add some more doc comments --- .../ql/src/Functions/IterReturnsNonSelf.qhelp | 2 +- python/ql/src/Functions/IterReturnsNonSelf.ql | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.qhelp b/python/ql/src/Functions/IterReturnsNonSelf.qhelp index 5b37e8cffeee..0ad5a05fdf48 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.qhelp +++ b/python/ql/src/Functions/IterReturnsNonSelf.qhelp @@ -3,7 +3,7 @@ "qhelp.dtd"> -

    Iterator classes (classes defining a __next__ method) should have an __iter__ that returns the iterator itself. +

    Iterator classes (classes defining a __next__ method) should have an __iter__ method that returns the iterator itself. This ensures that the object is also an iterable; and behaves as expected when used anywhere an iterator or iterable is expected, such as in for loops.

    diff --git a/python/ql/src/Functions/IterReturnsNonSelf.ql b/python/ql/src/Functions/IterReturnsNonSelf.ql index 275e2a1ebf6e..229204e8c9d9 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.ql +++ b/python/ql/src/Functions/IterReturnsNonSelf.ql @@ -14,16 +14,11 @@ import python import semmle.python.ApiGraphs -/** Holds if `f` is a method of the class `c`. */ -private predicate methodOfClass(Function f, Class c) { - exists(FunctionDef d | d.getDefinedFunction() = f and d.getScope() = c) -} - /** Gets the __iter__ method of `c`. */ -Function iterMethod(Class c) { methodOfClass(result, c) and result.getName() = "__iter__" } +Function iterMethod(Class c) { result = c.getAMethod() and result.getName() = "__iter__" } /** Gets the `__next__` method of `c`. */ -Function nextMethod(Class c) { methodOfClass(result, c) and result.getName() = "__next__" } +Function nextMethod(Class c) { result = c.getAMethod() and result.getName() = "__next__" } /** Holds if `var` is a variable referring to the `self` parameter of `f`. */ predicate isSelfVar(Function f, Name var) { var.getVariable() = f.getArg(0).(Name).getVariable() } @@ -48,8 +43,6 @@ predicate returnsNonSelf(Function f) { exists(f.getFallthroughNode()) or exists(Return r | r.getScope() = f and not isGoodReturn(f, r.getValue())) - or - exists(Return r | r.getScope() = f and not exists(r.getValue())) } /** Holds if `iter` and `next` methods are wrappers around some field. */ @@ -70,7 +63,8 @@ predicate iterWrapperMethods(Function iter, Function next) { ) } -DataFlow::CallCfgNode iterCall(DataFlow::Node arg) { +/** Gets a call to `iter(arg)`, `arg.__iter__()`, or `arg` itself (which we assume may already be an iterator). */ +private DataFlow::CallCfgNode iterCall(DataFlow::Node arg) { result.(DataFlow::MethodCallNode).calls(arg, "__iter__") or result = API::builtin("iter").getACall() and @@ -80,7 +74,8 @@ DataFlow::CallCfgNode iterCall(DataFlow::Node arg) { result = arg // assume the wrapping field is already an iterator } -DataFlow::CallCfgNode nextCall(DataFlow::Node arg) { +/** Gets a call to `next(arg)` or `arg.__next__()`. */ +private DataFlow::CallCfgNode nextCall(DataFlow::Node arg) { result.(DataFlow::MethodCallNode).calls(arg, "__next__") or result = API::builtin("next").getACall() and From 73f2770acb2f2c7cc53c93a8d7171694201f476b Mon Sep 17 00:00:00 2001 From: Joe Farebrother Date: Fri, 30 May 2025 11:24:06 +0100 Subject: [PATCH 10/10] Fix handling for some wrappers + add test case --- python/ql/src/Functions/IterReturnsNonSelf.ql | 6 ++---- .../Functions/iterators/IterReturnsNonSelf.expected | 2 +- .../ql/test/query-tests/Functions/iterators/test.py | 12 +++++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/python/ql/src/Functions/IterReturnsNonSelf.ql b/python/ql/src/Functions/IterReturnsNonSelf.ql index 229204e8c9d9..d6501a803a30 100644 --- a/python/ql/src/Functions/IterReturnsNonSelf.ql +++ b/python/ql/src/Functions/IterReturnsNonSelf.ql @@ -50,7 +50,7 @@ predicate iterWrapperMethods(Function iter, Function next) { exists(string field | exists(Return r, DataFlow::Node self, DataFlow::AttrRead read | r.getScope() = iter and - r.getValue() = iterCall(read).asExpr() and + r.getValue() = [iterCall(read).asExpr(), read.asExpr()] and read.accesses(self, field) and isSelfVar(iter, self.asExpr()) ) and @@ -63,15 +63,13 @@ predicate iterWrapperMethods(Function iter, Function next) { ) } -/** Gets a call to `iter(arg)`, `arg.__iter__()`, or `arg` itself (which we assume may already be an iterator). */ +/** Gets a call to `iter(arg)` or `arg.__iter__()`. */ private DataFlow::CallCfgNode iterCall(DataFlow::Node arg) { result.(DataFlow::MethodCallNode).calls(arg, "__iter__") or result = API::builtin("iter").getACall() and arg = result.getArg(0) and not exists(result.getArg(1)) - or - result = arg // assume the wrapping field is already an iterator } /** Gets a call to `next(arg)` or `arg.__next__()`. */ diff --git a/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected index f07c637241e9..a21f8de68a59 100644 --- a/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected +++ b/python/ql/test/query-tests/Functions/iterators/IterReturnsNonSelf.expected @@ -1,2 +1,2 @@ | test.py:5:5:5:23 | Function __iter__ | Iter method of iterator $@ does not return `self`. | test.py:1:1:1:11 | Class Bad1 | Bad1 | -| test.py:41:5:41:23 | Function __iter__ | Iter method of iterator $@ does not return `self`. | test.py:32:1:32:21 | Class FalsePositive1 | FalsePositive1 | +| test.py:51:5:51:23 | Function __iter__ | Iter method of iterator $@ does not return `self`. | test.py:42:1:42:21 | Class FalsePositive1 | FalsePositive1 | diff --git a/python/ql/test/query-tests/Functions/iterators/test.py b/python/ql/test/query-tests/Functions/iterators/test.py index be334be77b7d..ced389967e41 100644 --- a/python/ql/test/query-tests/Functions/iterators/test.py +++ b/python/ql/test/query-tests/Functions/iterators/test.py @@ -21,8 +21,18 @@ def __next__(self): def __iter__(self): # GOOD: iter and next are wrappers around a field return self._it.__iter__() - + class Good3: + def __init__(self): + self._it = iter([0,0,0]) + + def __next__(self): + return self._it.__next__() + + def __iter__(self): # GOOD: iter and next are wrappers around a field + return self._it + +class Good4: def __next__(self): return 0