Add new checker `bad-chained-comparison` (#7990)

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

11 files changed, 150 insertions, 2 deletions

diff --git a/doc/data/messages/b/bad-chained-comparison/bad.py b/doc/data/messages/b/bad-chained-comparison/bad.py
new file mode 100644
index 000000000..eeca75f13
--- /dev/null
+++ b/doc/data/messages/b/bad-chained-comparison/bad.py
@@ -0,0 +1,3 @@
+def xor_check(*, left=None, right=None):
+    if left is None != right is None:  # [bad-chained-comparison]
+        raise ValueError('Either both left= and right= need to be provided or none should.')
diff --git a/doc/data/messages/b/bad-chained-comparison/good.py b/doc/data/messages/b/bad-chained-comparison/good.py
new file mode 100644
index 000000000..115f4a9db
--- /dev/null
+++ b/doc/data/messages/b/bad-chained-comparison/good.py
@@ -0,0 +1,3 @@
+def xor_check(*, left=None, right=None):
+    if (left is None) != (right is None):
+        raise ValueError('Either both left= and right= need to be provided or none should.')
diff --git a/doc/data/messages/b/bad-chained-comparison/related.rst b/doc/data/messages/b/bad-chained-comparison/related.rst
new file mode 100644
index 000000000..620ba6df3
--- /dev/null
+++ b/doc/data/messages/b/bad-chained-comparison/related.rst
@@ -0,0 +1 @@
+- `Comparison Chaining <https://docs.python.org/3/reference/expressions.html#comparisons>`_
diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst
index ac17fabb6..d5bceae57 100644
--- a/doc/user_guide/checkers/features.rst
+++ b/doc/user_guide/checkers/features.rst
@@ -31,6 +31,19 @@ Async checker Messages
   function. This message can't be emitted when using Python < 3.5.
 
 
+Bad-Chained-Comparison checker
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Verbatim name of the checker is ``bad-chained-comparison``.
+
+Bad-Chained-Comparison checker Messages
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+:bad-chained-comparison (W3501): *Suspicious %s-part chained comparison using semantically incompatible operators (%s)*
+  Used when there is a chained comparison where one expression is part of two
+  comparisons that belong to different semantic groups ("<" does not mean the
+  same thing as "is", chaining them in "0 < x is None" is probably a mistake).
+
+
 Basic checker
 ~~~~~~~~~~~~~
 
diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst
index 6c4405b42..a1962f8e0 100644
--- a/doc/user_guide/messages/messages_overview.rst
+++ b/doc/user_guide/messages/messages_overview.rst
@@ -209,6 +209,7 @@ All messages in the warning category:
    warning/assert-on-tuple
    warning/attribute-defined-outside-init
    warning/bad-builtin
+   warning/bad-chained-comparison
    warning/bad-dunder-name
    warning/bad-format-string
    warning/bad-format-string-key
diff --git a/doc/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check
new file mode 100644
index 000000000..fa4f146e3
--- /dev/null
+++ b/doc/whatsnew/fragments/6559.new_check
@@ -0,0 +1,4 @@
+Add a ``bad-chained-comparison`` check that emits a warning when
+there is a chained comparison where one expression is semantically incompatible with the other.
+
+Closes #6559
diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py
new file mode 100644
index 000000000..fa75c52b2
--- /dev/null
+++ b/pylint/checkers/bad_chained_comparison.py
@@ -0,0 +1,61 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
+# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from astroid import nodes
+
+from pylint.checkers import BaseChecker
+from pylint.interfaces import HIGH
+
+if TYPE_CHECKING:
+    from pylint.lint import PyLinter
+
+COMPARISON_OP = frozenset(("<", "<=", ">", ">=", "!=", "=="))
+IDENTITY_OP = frozenset(("is", "is not"))
+MEMBERSHIP_OP = frozenset(("in", "not in"))
+
+
+class BadChainedComparisonChecker(BaseChecker):
+    """Checks for unintentional usage of chained comparison."""
+
+    name = "bad-chained-comparison"
+    msgs = {
+        "W3501": (
+            "Suspicious %s-part chained comparison using semantically incompatible operators (%s)",
+            "bad-chained-comparison",
+            "Used when there is a chained comparison where one expression is part "
+            "of two comparisons that belong to different semantic groups "
+            '("<" does not mean the same thing as "is", chaining them in '
+            '"0 < x is None" is probably a mistake).',
+        )
+    }
+
+    def _has_diff_semantic_groups(self, operators: list[str]) -> bool:
+
+        # Check if comparison operators are in the same semantic group
+        for semantic_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP):
+            if operators[0] in semantic_group:
+                group = semantic_group
+        return not all(o in group for o in operators)
+
+    def visit_compare(self, node: nodes.Compare) -> None:
+        operators = sorted({op[0] for op in node.ops})
+        if self._has_diff_semantic_groups(operators):
+            num_parts = f"{len(node.ops)}"
+            incompatibles = (
+                ", ".join(f"'{o}'" for o in operators[:-1]) + f" and '{operators[-1]}'"
+            )
+            self.add_message(
+                "bad-chained-comparison",
+                node=node,
+                args=(num_parts, incompatibles),
+                confidence=HIGH,
+            )
+
+
+def register(linter: PyLinter) -> None:
+    linter.register_checker(BadChainedComparisonChecker(linter))
diff --git a/tests/functional/b/bad_chained_comparison.py b/tests/functional/b/bad_chained_comparison.py
new file mode 100644
index 000000000..c1bdab980
--- /dev/null
+++ b/tests/functional/b/bad_chained_comparison.py
@@ -0,0 +1,51 @@
+# pylint: disable=invalid-name
+"""Checks for chained comparisons with comparisons belonging to different groups"""
+
+primes = set(2, 3, 5, 7, 11)
+
+def valid(x, y, z):
+    """valid usage of chained comparisons"""
+    if x < y <= z:
+        pass
+    elif x > y >= z:
+        pass
+    elif x == y != z:
+        pass
+    elif x is y is not z:
+        pass
+    elif x in y not in z:
+        pass
+
+def id_comparison_invalid(*, left=None, right=None):
+    """identity mixed with comparison"""
+    if left is None != right is None:  # [bad-chained-comparison]
+        raise ValueError('Either both left= and right= need to be provided or none should.')
+    if left is not None == right is not None:  # [bad-chained-comparison]
+        pass
+
+def member_comparison_invalid(x:int, y:int, z:int):
+    """membership mixed with comparison"""
+    if x in primes == y in primes:  # [bad-chained-comparison]
+        pass
+    elif x in primes == z not in primes:  # [bad-chained-comparison]
+        pass
+    elif x not in primes == y in primes != z not in primes:  # [bad-chained-comparison]
+        pass
+    elif x not in primes <= y not in primes > z in primes:  # [bad-chained-comparison]
+        pass
+
+def id_member_invalid(x:int, y:int, z:int):
+    """identity mixed with membership"""
+    if x in primes is y in primes:  # [bad-chained-comparison]
+        pass
+    elif x in primes is z not in primes:  # [bad-chained-comparison]
+        pass
+    elif x not in primes is y in primes is not z not in primes:  # [bad-chained-comparison]
+        pass
+
+def complex_invalid(x:int, y:int, z:int):
+    """invalid complex chained comparisons"""
+    if x in primes == y not in primes is not z in primes:  # [bad-chained-comparison]
+        pass
+    elif x < y <= z != x not in primes is y in primes:  # [bad-chained-comparison]
+        pass
diff --git a/tests/functional/b/bad_chained_comparison.txt b/tests/functional/b/bad_chained_comparison.txt
new file mode 100644
index 000000000..276a1debc
--- /dev/null
+++ b/tests/functional/b/bad_chained_comparison.txt
@@ -0,0 +1,11 @@
+bad-chained-comparison:21:7:21:36:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('!=' and 'is'):HIGH
+bad-chained-comparison:23:7:23:44:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('==' and 'is not'):HIGH
+bad-chained-comparison:28:7:28:33:member_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('==' and 'in'):HIGH
+bad-chained-comparison:30:9:30:39:member_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('==', 'in' and 'not in'):HIGH
+bad-chained-comparison:32:9:32:58:member_comparison_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('!=', '==', 'in' and 'not in'):HIGH
+bad-chained-comparison:34:9:34:57:member_comparison_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('<=', '>', 'in' and 'not in'):HIGH
+bad-chained-comparison:39:7:39:33:id_member_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('in' and 'is'):HIGH
+bad-chained-comparison:41:9:41:39:id_member_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('in', 'is' and 'not in'):HIGH
+bad-chained-comparison:43:9:43:62:id_member_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('in', 'is', 'is not' and 'not in'):HIGH
+bad-chained-comparison:48:7:48:56:complex_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('==', 'in', 'is not' and 'not in'):HIGH
+bad-chained-comparison:50:9:50:53:complex_invalid:Suspicious 6-part chained comparison using semantically incompatible operators ('!=', '<', '<=', 'in', 'is' and 'not in'):HIGH
diff --git a/tests/functional/c/consider/consider_iterating_dictionary.py b/tests/functional/c/consider/consider_iterating_dictionary.py
index 8c75b4e3e..91402bf77 100644
--- a/tests/functional/c/consider/consider_iterating_dictionary.py
+++ b/tests/functional/c/consider/consider_iterating_dictionary.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring, expression-not-assigned, too-few-public-methods
+# pylint: disable=missing-docstring, expression-not-assigned, too-few-public-methods, bad-chained-comparison
 # pylint: disable=no-member, import-error, line-too-long
 # pylint: disable=unnecessary-comprehension, use-dict-literal, use-implicit-booleaness-not-comparison
 
diff --git a/tests/functional/ext/set_membership/use_set_membership.py b/tests/functional/ext/set_membership/use_set_membership.py
index 7872d7f98..df5cd961b 100644
--- a/tests/functional/ext/set_membership/use_set_membership.py
+++ b/tests/functional/ext/set_membership/use_set_membership.py
@@ -1,4 +1,4 @@
-# pylint: disable=invalid-name,missing-docstring,pointless-statement,unnecessary-comprehension,undefined-variable
+# pylint: disable=invalid-name,missing-docstring,pointless-statement,unnecessary-comprehension,undefined-variable,bad-chained-comparison
 
 x = 1
 var = frozenset({1, 2, 3})