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>

4 files changed, 310 insertions, 0 deletions

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)'