Add extension checker that suggests any/all statements from for loops (#5196)

* Add extension checker that suggests any/all statements from for loops

* Suggest all/not all: If there are negated conditions, we can suggest
an all/not all statement to move the 'not' out of the comprehension and
call it only once.

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>

9 files changed, 404 insertions, 0 deletions

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index fd5d627eb..545691405 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -558,3 +558,5 @@ contributors:
 * James DesLauriers: contributor
 
 * Youngsoo Sung: contributor
+
+* Arianna Yang: contributor
diff --git a/ChangeLog b/ChangeLog
index be49bc113..12365068a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,12 @@ Release date: TBA
 ..
   Put new features here and also in 'doc/whatsnew/2.12.rst'
 
+* Add an optional extension ``consider-using-any-or-all`` : Emitted when a ``for`` loop only
+  produces a boolean and could be replaced by ``any`` or ``all`` using a generator. Also suggests
+  a suitable any or all statement.
+
+  Closes #5008
+
 * Properly identify parameters with no documentation and add new message called ``missing-any-param-doc``
 
   Closes #3799
diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst
index 1126ebd53..8b5b5f0c2 100644
--- a/doc/whatsnew/2.12.rst
+++ b/doc/whatsnew/2.12.rst
@@ -50,7 +50,11 @@ Removed checkers
 
 Extensions
 ==========
+* Added an optional extension ``consider-using-any-or-all``: Emitted when a ``for`` loop only
+  produces a boolean and could be replaced by ``any`` or ``all`` using a generator. Also suggests
+  a suitable any/all statement if it is concise.
 
+  Closes #5008
 
 Other Changes
 =============
diff --git a/pylint/checkers/utils.py b/pylint/checkers/utils.py
index 43157d0f9..1d198e87e 100644
--- a/pylint/checkers/utils.py
+++ b/pylint/checkers/utils.py
@@ -1638,3 +1638,12 @@ def is_empty_str_literal(node: Optional[nodes.NodeNG]) -> bool:
     return (
         isinstance(node, nodes.Const) and isinstance(node.value, str) and not node.value
     )
+
+
+def returns_bool(node: nodes.NodeNG) -> bool:
+    """Returns true if a node is a return that returns a constant boolean"""
+    return (
+        isinstance(node, nodes.Return)
+        and isinstance(node.value, nodes.Const)
+        and node.value.value in (True, False)
+    )
diff --git a/pylint/extensions/for_any_all.py b/pylint/extensions/for_any_all.py
new file mode 100644
index 000000000..9c7274095
--- /dev/null
+++ b/pylint/extensions/for_any_all.py
@@ -0,0 +1,73 @@
+"""Check for use of for loops that only check for a condition."""
+from typing import TYPE_CHECKING
+
+from astroid import nodes
+
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages, returns_bool
+from pylint.interfaces import IAstroidChecker
+
+if TYPE_CHECKING:
+    from pylint.lint.pylinter import PyLinter
+
+
+class ConsiderUsingAnyOrAllChecker(BaseChecker):
+
+    __implements__ = (IAstroidChecker,)
+    name = "consider-using-any-or-all"
+    msgs = {
+        "C0501": (
+            "`for` loop could be '%s'",
+            "consider-using-any-or-all",
+            "A for loop that checks for a condition and return a bool can be replaced with any or all.",
+        )
+    }
+
+    @check_messages("consider-using-any-or-all")
+    def visit_for(self, node: nodes.For) -> None:
+        if len(node.body) != 1:  # Only If node with no Else
+            return
+        if not isinstance(node.body[0], nodes.If):
+            return
+
+        if_children = list(node.body[0].get_children())
+        if not len(if_children) == 2:  # The If node has only a comparison and return
+            return
+        if not returns_bool(if_children[1]):
+            return
+
+        # Check for terminating boolean return right after the loop
+        node_after_loop = node.next_sibling()
+        if returns_bool(node_after_loop):
+            final_return_bool = node_after_loop.value.value
+            suggested_string = self._build_suggested_string(node, final_return_bool)
+            self.add_message(
+                "consider-using-any-or-all", node=node, args=suggested_string
+            )
+
+    @staticmethod
+    def _build_suggested_string(node: nodes.For, final_return_bool: bool) -> str:
+        """When a nodes.For node can be rewritten as an any/all statement, return a suggestion for that statement
+        final_return_bool is the boolean literal returned after the for loop if all conditions fail
+        """
+        loop_var = node.target.as_string()
+        loop_iter = node.iter.as_string()
+        test_node = next(node.body[0].get_children())
+
+        if isinstance(test_node, nodes.UnaryOp) and test_node.op == "not":
+            # The condition is negated. Advance the node to the operand and modify the suggestion
+            test_node = test_node.operand
+            suggested_function = "all" if final_return_bool else "not all"
+        else:
+            suggested_function = "not any" if final_return_bool else "any"
+
+        test = test_node.as_string()
+        return f"{suggested_function}({test} for {loop_var} in {loop_iter})"
+
+
+def register(linter: "PyLinter") -> None:
+    """Required method to auto register this checker.
+
+    :param linter: Main interface object for Pylint plugins
+    """
+    linter.register_checker(ConsiderUsingAnyOrAllChecker(linter))
diff --git a/tests/extensions/test_for_any_all.py b/tests/extensions/test_for_any_all.py
new file mode 100644
index 000000000..d9cf46189
--- /dev/null
+++ b/tests/extensions/test_for_any_all.py
@@ -0,0 +1,247 @@
+"""Tests for the pylint checker in :mod:`pylint.extensions.for_any_all
+"""
+
+import astroid
+
+from pylint.extensions.for_any_all import ConsiderUsingAnyOrAllChecker
+from pylint.testutils import CheckerTestCase, MessageTest
+
+
+class TestForAnyAll(CheckerTestCase):
+
+    CHECKER_CLASS = ConsiderUsingAnyOrAllChecker
+
+    def test_basic_emit_for_any_all(self) -> None:
+        """Simple case where we expect the message to be emitted."""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(node):
+            for parent in node.node_ancestors():
+                if isinstance(parent, Decorators):
+                    return True
+            return False
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="any(isinstance(parent, Decorators) for parent in node.node_ancestors())",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_emit_not_any_for_any_all(self) -> None:
+        """Case where we expect the message to be emitted with a not any suggestion."""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(node):
+            for parent in node.node_ancestors():
+                if isinstance(parent, Decorators):
+                    return False
+            return True
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="not any(isinstance(parent, Decorators) for parent in node.node_ancestors())",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_emit_not_any_boolop_for_any_all(self) -> None:
+        """Case where we expect not any statement with a more complicated condition"""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(items):
+            for item in items:
+                if item % 2 == 0 and (item % 3 == 0 or item > 15):
+                    return False
+            return True
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="not any(item % 2 == 0 and (item % 3 == 0 or item > 15) for item in items)",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_emit_long_message_for_any_all(self) -> None:
+        """Case where we expect a particularly long message to be emitted."""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(node):
+            for ancestor in itertools.chain([node], ancestors):
+                if (
+                    ancestor.name in ("Exception", "BaseException")
+                    and ancestor.root().name == EXCEPTIONS_MODULE
+                ):
+                    return True
+            return False
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="any(ancestor.name in ('Exception', 'BaseException') "
+                "and ancestor.root().name == EXCEPTIONS_MODULE "
+                "for ancestor in itertools.chain([node], ancestors))",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_emit_all_for_any_all(self) -> None:
+        """Case where we expect an all statement because of negation in the condition"""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(items):
+            for item in items:
+                if not(item % 2 == 0 and (item % 3 == 0 or item > 15)):
+                    return False
+            return True
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="all(item % 2 == 0 and (item % 3 == 0 or item > 15) for item in items)",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_emit_not_all_for_any_all(self) -> None:
+        """Case where we expect a not all statement because of negation in the condition"""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(node):
+            for ancestor in itertools.chain([node], ancestors):
+                if not (
+                    ancestor.name in ("Exception", "BaseException")
+                    and ancestor.root().name == EXCEPTIONS_MODULE
+                ):
+                    return True
+            return False
+        """
+        ).body[0]
+
+        with self.assertAddsMessages(
+            MessageTest(
+                "consider-using-any-or-all",
+                node=node,
+                args="not all(ancestor.name in ('Exception', 'BaseException') "
+                "and ancestor.root().name == EXCEPTIONS_MODULE "
+                "for ancestor in itertools.chain([node], ancestors))",
+            )
+        ):
+            self.checker.visit_for(node)
+
+    def test_no_pattern(self) -> None:
+        """Do not emit if the for loop does not have the pattern we are looking for"""
+        node = astroid.extract_node(
+            """
+        def no_suggestion_if_not_if():
+            for x in range(1):
+                var = x
+                return var
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    def test_other_return(self) -> None:
+        """Do not emit if the if-statement does not return a bool"""
+        node = astroid.extract_node(
+            """
+        def no_suggestion_if_not_bool(item):
+            for parent in item.parents():
+                if isinstance(parent, str):
+                    return "True"
+            return "False"
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    def test_non_conditional_for(self) -> None:
+        """Do not emit if there is no If condition in the for loop."""
+        node = astroid.extract_node(
+            """
+        def print_items(items):
+            for item in items:
+                print(item)
+            return True
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    def test_returns_something_else(self) -> None:
+        """Do not emit if anything besides a boolean is returned."""
+        node = astroid.extract_node(
+            """
+        def print_items(items):
+            for item in items:
+                return items
+            return True
+
+        def print_items(items):
+            for item in items:
+                return False
+            return items
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    def test_additional_statements(self) -> None:
+        """Do not emit if there is more logic which can cause side effects
+        or become less readable in a list comprehension.
+        """
+        node = astroid.extract_node(
+            """
+        def print_items(items):
+            for item in items:
+                if isinstance(item, str):
+                    print(item)
+                    return False
+            return True
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    def test_for_if_else(self) -> None:
+        """Do not emit if the if has an else condition. Generally implies more complicated logic."""
+        node = astroid.extract_node(
+            """
+        def is_from_decorator(node):
+            for parent in node.node_ancestors():
+                if isinstance(parent, Decorators):
+                    return True
+                else:
+                    if parent in Annotations.selected_annotations:
+                        return False
+
+            return False
+        """
+        ).body[0]
+
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
diff --git a/tests/functional/ext/for_any_all.py b/tests/functional/ext/for_any_all.py
new file mode 100644
index 000000000..674bcf68b
--- /dev/null
+++ b/tests/functional/ext/for_any_all.py
@@ -0,0 +1,54 @@
+"""Functional test"""
+
+def any_even(items):
+    """Return True if the list contains any even numbers"""
+    for item in items: # [consider-using-any-or-all]
+        if item % 2 == 0:
+            return True
+    return False
+
+def all_even(items):
+    """Return True if the list contains all even numbers"""
+    for item in items: # [consider-using-any-or-all]
+        if not item % 2 == 0:
+            return False
+    return True
+
+def any_uneven(items):
+    """Return True if the list contains any uneven numbers"""
+    for item in items: # [consider-using-any-or-all]
+        if not item % 2 == 0:
+            return True
+    return False
+
+def all_uneven(items):
+    """Return True if the list contains all uneven numbers"""
+    for item in items: # [consider-using-any-or-all]
+        if item % 2 == 0:
+            return False
+    return True
+
+def is_from_string(item):
+    """Return True if one of parents of item is a string"""
+    for parent in item.parents(): # [consider-using-any-or-all]
+        if isinstance(parent, str):
+            return True
+    return False
+
+def is_not_from_string(item):
+    """Return True if one of parents of item isn't a string"""
+    for parent in item.parents(): # [consider-using-any-or-all]
+        if not isinstance(parent, str):
+            return True
+    return False
+
+def nested_check(items):
+    """Tests that for loops at deeper levels are picked up"""
+    if items and len(items) > 5:
+        print(items)
+        for item in items: # [consider-using-any-or-all]
+            if item in (1, 2, 3):
+                return False
+        return True
+    print(items)
+    return items[3] > 5
diff --git a/tests/functional/ext/for_any_all.rc b/tests/functional/ext/for_any_all.rc
new file mode 100644
index 000000000..41b0eaa71
--- /dev/null
+++ b/tests/functional/ext/for_any_all.rc
@@ -0,0 +1,2 @@
+[MASTER]
+load-plugins=pylint.extensions.for_any_all
diff --git a/tests/functional/ext/for_any_all.txt b/tests/functional/ext/for_any_all.txt
new file mode 100644
index 000000000..f5ed43799
--- /dev/null
+++ b/tests/functional/ext/for_any_all.txt
@@ -0,0 +1,7 @@
+consider-using-any-or-all:5:4:any_even:`for` loop could be 'any(item % 2 == 0 for item in items)'
+consider-using-any-or-all:12:4:all_even:`for` loop could be 'all(item % 2 == 0 for item in items)'
+consider-using-any-or-all:19:4:any_uneven:`for` loop could be 'not all(item % 2 == 0 for item in items)'
+consider-using-any-or-all:26:4:all_uneven:`for` loop could be 'not any(item % 2 == 0 for item in items)'
+consider-using-any-or-all:33:4:is_from_string:`for` loop could be 'any(isinstance(parent, str) for parent in item.parents())'
+consider-using-any-or-all:40:4:is_not_from_string:`for` loop could be 'not all(isinstance(parent, str) for parent in item.parents())'
+consider-using-any-or-all:49:8:nested_check:`for` loop could be 'not any(item in (1, 2, 3) for item in items)'