Skip to content

Python: Modernize iter not returning self query #19554

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

joefarebrother
Copy link
Contributor

Removes dependence on pointsto and adresses a couple common FP cases.

Copy link
Contributor

github-actions bot commented May 23, 2025

QHelp previews:

python/ql/src/Functions/IterReturnsNonSelf.qhelp

Iterator does not return self from __iter__ method

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.

Recommendation

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

Example

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.

class MyRange(object):
    def __init__(self, low, high):
        self.current = low
        self.high = high

    def __iter__(self):
        return (self.current, self.high) # BAD: does not return `self`.

    def __next__(self):
        if self.current > self.high:
            return None
        self.current += 1
        return self.current - 1

References

@joefarebrother joefarebrother marked this pull request as ready for review May 23, 2025 15:28
@joefarebrother joefarebrother requested a review from a team as a code owner May 23, 2025 15:28
tausbn
tausbn previously approved these changes May 26, 2025
Copy link
Contributor

@tausbn tausbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments, but otherwise this looks good to me. 👍

Comment on lines +92 to +95
next = nextMethod(c) and
iter = iterMethod(c) and
returnsNonSelf(iter) and
not iterWrapperMethods(iter, next)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This where clause requires that there be both an __iter__ and a __next__ method present. What if only the __next__ method is present? It might make sense to also flag this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially useful. The alert for that case would have to be different (doesn't link to the __iter__ method) which is a little trickier to handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants