From 842d4599c7468ca6ed8c84684948ec593d0fb5b7 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 27 Dec 2022 14:11:43 +0800 Subject: [PATCH 01/21] Add bad chained comparison extension --- pylint/extensions/bad_chained_comparison.py | 44 +++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 pylint/extensions/bad_chained_comparison.py diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py new file mode 100644 index 0000000000..8a33cc7037 --- /dev/null +++ b/pylint/extensions/bad_chained_comparison.py @@ -0,0 +1,44 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from astroid import nodes + +from pylint.checkers import BaseChecker + +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 = { + "W2401": ( + "Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.", + "bad-chained-comparison", + "Used when there is a chained comparison where one expression is part of two comparisons that belong to different groups.", + ) + } + + def _has_diff_comparison_groups(self, node: nodes.Compare) -> bool: + operators = [op[0] for op in node.ops] + # Check if comparison operators are in the same group + for comparison_group in [COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP]: + if operators[0] in comparison_group: + group = comparison_group + for operator in operators: + if operator not in group: + return True + return False + + def visit_compare(self, node: nodes.Compare) -> None: + if self._has_diff_comparison_groups(node): + num_pieces = f"{len(node.ops)}" + self.add_message("bad-chained-comparison", node=node, args=(num_pieces)) + +def register(linter: PyLinter) -> None: + linter.register_checker(BadChainedComparisonChecker(linter)) From efe18b1480e35e36b529bd880e3ead9cf61851f2 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 27 Dec 2022 14:12:50 +0800 Subject: [PATCH 02/21] Add tests for bad chained comparison --- .../bad_chained_comparison.py | 51 +++++++++++++++++++ .../bad_chained_comparison.rc | 2 + .../bad_chained_comparison.txt | 11 ++++ 3 files changed, 64 insertions(+) create mode 100644 tests/functional/ext/bad_chained_comparison/bad_chained_comparison.py create mode 100644 tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc create mode 100644 tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.py b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.py new file mode 100644 index 0000000000..df30401742 --- /dev/null +++ b/tests/functional/ext/bad_chained_comparison/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/ext/bad_chained_comparison/bad_chained_comparison.rc b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc new file mode 100644 index 0000000000..1ad1f67a16 --- /dev/null +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc @@ -0,0 +1,2 @@ +[MAIN] +load-plugins=pylint.extensions.bad_chained_comparison \ No newline at end of file diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt new file mode 100644 index 0000000000..5a1eb6f9c7 --- /dev/null +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt @@ -0,0 +1,11 @@ +bad-chained-comparison:21:7:21:36:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:23:7:23:44:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:28:7:28:33:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:30:9:30:39:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:32:9:32:58:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:34:9:34:57:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:39:7:39:33:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:41:9:41:39:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:43:9:43:62:id_member_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:48:7:48:56:complex_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED +bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED \ No newline at end of file From dbb7d080e16f9be4dd564b7591739018f1c9093d Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 27 Dec 2022 14:13:37 +0800 Subject: [PATCH 03/21] Add doc for bad chained comparison --- doc/data/messages/b/bad-chained-comparison/bad.py | 3 +++ doc/data/messages/b/bad-chained-comparison/good.py | 3 +++ doc/data/messages/b/bad-chained-comparison/pylintrc | 2 ++ doc/data/messages/b/bad-chained-comparison/related.rst | 1 + 4 files changed, 9 insertions(+) create mode 100644 doc/data/messages/b/bad-chained-comparison/bad.py create mode 100644 doc/data/messages/b/bad-chained-comparison/good.py create mode 100644 doc/data/messages/b/bad-chained-comparison/pylintrc create mode 100644 doc/data/messages/b/bad-chained-comparison/related.rst 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 0000000000..62e83401c6 --- /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 0000000000..412a716773 --- /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/pylintrc b/doc/data/messages/b/bad-chained-comparison/pylintrc new file mode 100644 index 0000000000..1ad1f67a16 --- /dev/null +++ b/doc/data/messages/b/bad-chained-comparison/pylintrc @@ -0,0 +1,2 @@ +[MAIN] +load-plugins=pylint.extensions.bad_chained_comparison \ No newline at end of file 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 0000000000..3cf876f33a --- /dev/null +++ b/doc/data/messages/b/bad-chained-comparison/related.rst @@ -0,0 +1 @@ +- `Comparison Chaining `_ \ No newline at end of file From 4c117154ec4871206b7044f06624582894a93ca6 Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 27 Dec 2022 14:24:24 +0800 Subject: [PATCH 04/21] Add fragment for bad chained comparison --- doc/whatsnew/fragments/6559.new_check | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 doc/whatsnew/fragments/6559.new_check diff --git a/doc/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check new file mode 100644 index 0000000000..72d7d092d4 --- /dev/null +++ b/doc/whatsnew/fragments/6559.new_check @@ -0,0 +1,4 @@ +Add a ``bad-chained-comparison`` extension that emits a warning when +there is a chained comparison where one expression is part of two comparisons that belong to different groups. + +Closes #6559 \ No newline at end of file From 38fc79edaf33622171d9cb937d0e71fe19513eed Mon Sep 17 00:00:00 2001 From: Zen Date: Tue, 27 Dec 2022 14:35:23 +0800 Subject: [PATCH 05/21] Update docs --- doc/user_guide/checkers/extensions.rst | 16 ++++++++++++++++ doc/user_guide/messages/messages_overview.rst | 1 + 2 files changed, 17 insertions(+) diff --git a/doc/user_guide/checkers/extensions.rst b/doc/user_guide/checkers/extensions.rst index 563633e62b..6a01c231b0 100644 --- a/doc/user_guide/checkers/extensions.rst +++ b/doc/user_guide/checkers/extensions.rst @@ -7,6 +7,7 @@ Optional checkers Pylint provides the following optional plugins: - :ref:`pylint.extensions.bad_builtin` +- :ref:`pylint.extensions.bad_chained_comparison` - :ref:`pylint.extensions.broad_try_clause` - :ref:`pylint.extensions.check_elif` - :ref:`pylint.extensions.code_style` @@ -37,6 +38,21 @@ You can activate any or all of these extensions by adding a ``load-plugins`` lin load-plugins=pylint.extensions.docparams,pylint.extensions.docstyle +.. _pylint.extensions.bad_chained_comparison: + +Bad-Chained-Comparison checker +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This checker is provided by ``pylint.extensions.bad_chained_comparison``. +Verbatim name of the checker is ``bad-chained-comparison``. + +Bad-Chained-Comparison checker Messages +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +:bad-chained-comparison (W2401): *Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.* + Used when there is a chained comparison where one expression is part of two + comparisons that belong to different groups. + + .. _pylint.extensions.broad_try_clause: Broad Try Clause checker diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst index 74b8dbb8a9..6cf3f05714 100644 --- a/doc/user_guide/messages/messages_overview.rst +++ b/doc/user_guide/messages/messages_overview.rst @@ -208,6 +208,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 From d1a683e77399cfd75f144c0ddeca1b52c2943639 Mon Sep 17 00:00:00 2001 From: Zen Date: Wed, 28 Dec 2022 13:38:48 +0800 Subject: [PATCH 06/21] Code style improvements for bad-chained-comparison --- pylint/extensions/bad_chained_comparison.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py index 8a33cc7037..6670e5caac 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/extensions/bad_chained_comparison.py @@ -27,13 +27,10 @@ class BadChainedComparisonChecker(BaseChecker): def _has_diff_comparison_groups(self, node: nodes.Compare) -> bool: operators = [op[0] for op in node.ops] # Check if comparison operators are in the same group - for comparison_group in [COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP]: + for comparison_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP): if operators[0] in comparison_group: group = comparison_group - for operator in operators: - if operator not in group: - return True - return False + return not all(o in group for o in operators) def visit_compare(self, node: nodes.Compare) -> None: if self._has_diff_comparison_groups(node): From 28e28bce267d01247d9a75095fc1f84584c0bfa0 Mon Sep 17 00:00:00 2001 From: Zen Date: Wed, 28 Dec 2022 13:52:41 +0800 Subject: [PATCH 07/21] Update confidence level in bad chained comparison --- pylint/extensions/bad_chained_comparison.py | 3 ++- .../bad_chained_comparison.txt | 22 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py index 6670e5caac..517c1f3bdd 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/extensions/bad_chained_comparison.py @@ -5,6 +5,7 @@ from astroid import nodes from pylint.checkers import BaseChecker +from pylint.interfaces import HIGH if TYPE_CHECKING: from pylint.lint import PyLinter @@ -35,7 +36,7 @@ def _has_diff_comparison_groups(self, node: nodes.Compare) -> bool: def visit_compare(self, node: nodes.Compare) -> None: if self._has_diff_comparison_groups(node): num_pieces = f"{len(node.ops)}" - self.add_message("bad-chained-comparison", node=node, args=(num_pieces)) + self.add_message("bad-chained-comparison", node=node, args=(num_pieces), confidence=HIGH) def register(linter: PyLinter) -> None: linter.register_checker(BadChainedComparisonChecker(linter)) diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt index 5a1eb6f9c7..601540c769 100644 --- a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt @@ -1,11 +1,11 @@ -bad-chained-comparison:21:7:21:36:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:23:7:23:44:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:28:7:28:33:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:30:9:30:39:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:32:9:32:58:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:34:9:34:57:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:39:7:39:33:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:41:9:41:39:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:43:9:43:62:id_member_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:48:7:48:56:complex_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED -bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:UNDEFINED \ No newline at end of file +bad-chained-comparison:21:7:21:36:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:23:7:23:44:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:28:7:28:33:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:30:9:30:39:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:32:9:32:58:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:34:9:34:57:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:39:7:39:33:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:41:9:41:39:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:43:9:43:62:id_member_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:48:7:48:56:complex_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH \ No newline at end of file From 8d3691dfffb1fbebad71e3818751112147ca3517 Mon Sep 17 00:00:00 2001 From: Zen Date: Wed, 28 Dec 2022 13:59:32 +0800 Subject: [PATCH 08/21] Reassign message id to bad chained comparison --- doc/user_guide/checkers/extensions.rst | 2 +- pylint/extensions/bad_chained_comparison.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/user_guide/checkers/extensions.rst b/doc/user_guide/checkers/extensions.rst index 6a01c231b0..0e0be34326 100644 --- a/doc/user_guide/checkers/extensions.rst +++ b/doc/user_guide/checkers/extensions.rst @@ -48,7 +48,7 @@ Verbatim name of the checker is ``bad-chained-comparison``. Bad-Chained-Comparison checker Messages ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -:bad-chained-comparison (W2401): *Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.* +:bad-chained-comparison (W3501): *Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.* Used when there is a chained comparison where one expression is part of two comparisons that belong to different groups. diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py index 517c1f3bdd..d151ad93b8 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/extensions/bad_chained_comparison.py @@ -18,7 +18,7 @@ class BadChainedComparisonChecker(BaseChecker): """Checks for unintentional usage of chained comparison""" name = "bad-chained-comparison" msgs = { - "W2401": ( + "W3501": ( "Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.", "bad-chained-comparison", "Used when there is a chained comparison where one expression is part of two comparisons that belong to different groups.", From 24c3351b358db74966d726127279cb41d22a8562 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 27 Dec 2022 07:03:05 +0000 Subject: [PATCH 09/21] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- doc/data/messages/b/bad-chained-comparison/pylintrc | 2 +- doc/data/messages/b/bad-chained-comparison/related.rst | 2 +- doc/whatsnew/fragments/6559.new_check | 2 +- pylint/extensions/bad_chained_comparison.py | 5 ++++- .../ext/bad_chained_comparison/bad_chained_comparison.rc | 2 +- .../ext/bad_chained_comparison/bad_chained_comparison.txt | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/doc/data/messages/b/bad-chained-comparison/pylintrc b/doc/data/messages/b/bad-chained-comparison/pylintrc index 1ad1f67a16..2eb553ad79 100644 --- a/doc/data/messages/b/bad-chained-comparison/pylintrc +++ b/doc/data/messages/b/bad-chained-comparison/pylintrc @@ -1,2 +1,2 @@ [MAIN] -load-plugins=pylint.extensions.bad_chained_comparison \ No newline at end of file +load-plugins=pylint.extensions.bad_chained_comparison diff --git a/doc/data/messages/b/bad-chained-comparison/related.rst b/doc/data/messages/b/bad-chained-comparison/related.rst index 3cf876f33a..620ba6df35 100644 --- a/doc/data/messages/b/bad-chained-comparison/related.rst +++ b/doc/data/messages/b/bad-chained-comparison/related.rst @@ -1 +1 @@ -- `Comparison Chaining `_ \ No newline at end of file +- `Comparison Chaining `_ diff --git a/doc/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check index 72d7d092d4..9d88c80004 100644 --- a/doc/whatsnew/fragments/6559.new_check +++ b/doc/whatsnew/fragments/6559.new_check @@ -1,4 +1,4 @@ Add a ``bad-chained-comparison`` extension that emits a warning when there is a chained comparison where one expression is part of two comparisons that belong to different groups. -Closes #6559 \ No newline at end of file +Closes #6559 diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py index d151ad93b8..a0151d7bbe 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/extensions/bad_chained_comparison.py @@ -14,8 +14,10 @@ IDENTITY_OP = frozenset(("is", "is not")) MEMBERSHIP_OP = frozenset(("in", "not in")) + class BadChainedComparisonChecker(BaseChecker): - """Checks for unintentional usage of chained comparison""" + """Checks for unintentional usage of chained comparison.""" + name = "bad-chained-comparison" msgs = { "W3501": ( @@ -38,5 +40,6 @@ def visit_compare(self, node: nodes.Compare) -> None: num_pieces = f"{len(node.ops)}" self.add_message("bad-chained-comparison", node=node, args=(num_pieces), confidence=HIGH) + def register(linter: PyLinter) -> None: linter.register_checker(BadChainedComparisonChecker(linter)) diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc index 1ad1f67a16..2eb553ad79 100644 --- a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc @@ -1,2 +1,2 @@ [MAIN] -load-plugins=pylint.extensions.bad_chained_comparison \ No newline at end of file +load-plugins=pylint.extensions.bad_chained_comparison diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt index 601540c769..65ceb6c9d3 100644 --- a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt @@ -8,4 +8,4 @@ bad-chained-comparison:39:7:39:33:id_member_invalid:Expression gets interpreted bad-chained-comparison:41:9:41:39:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH bad-chained-comparison:43:9:43:62:id_member_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH bad-chained-comparison:48:7:48:56:complex_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH \ No newline at end of file +bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH From dd7f524e73bd881322240d29545fb841a6276059 Mon Sep 17 00:00:00 2001 From: Zen Date: Thu, 29 Dec 2022 11:30:45 +0800 Subject: [PATCH 10/21] Improve message clarity of bad chained comparison --- doc/whatsnew/fragments/6559.new_check | 4 ++-- pylint/extensions/bad_chained_comparison.py | 21 ++++++++++++------ .../bad_chained_comparison.txt | 22 +++++++++---------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/doc/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check index 9d88c80004..30f6b850c0 100644 --- a/doc/whatsnew/fragments/6559.new_check +++ b/doc/whatsnew/fragments/6559.new_check @@ -1,4 +1,4 @@ Add a ``bad-chained-comparison`` extension that emits a warning when -there is a chained comparison where one expression is part of two comparisons that belong to different groups. +there is a chained comparison where one expression is semantically incompatible with the other. -Closes #6559 +Closes #6559 diff --git a/pylint/extensions/bad_chained_comparison.py b/pylint/extensions/bad_chained_comparison.py index a0151d7bbe..3694898a1c 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/extensions/bad_chained_comparison.py @@ -21,24 +21,31 @@ class BadChainedComparisonChecker(BaseChecker): name = "bad-chained-comparison" msgs = { "W3501": ( - "Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.", + "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 groups.", + "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 as mistake)." , ) } - def _has_diff_comparison_groups(self, node: nodes.Compare) -> bool: + def _get_distinct_operators(self, node: nodes.Compare) -> list: operators = [op[0] for op in node.ops] - # Check if comparison operators are in the same group + return list(dict.fromkeys(operators)) + + + def _has_diff_semantic_groups(self, operators: list) -> bool: + # Check if comparison operators are in the same semantic group for comparison_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP): if operators[0] in comparison_group: group = comparison_group return not all(o in group for o in operators) def visit_compare(self, node: nodes.Compare) -> None: - if self._has_diff_comparison_groups(node): - num_pieces = f"{len(node.ops)}" - self.add_message("bad-chained-comparison", node=node, args=(num_pieces), confidence=HIGH) + operators = self._get_distinct_operators(node) + 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: diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt index 65ceb6c9d3..3a89faa679 100644 --- a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt +++ b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt @@ -1,11 +1,11 @@ -bad-chained-comparison:21:7:21:36:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:23:7:23:44:id_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:28:7:28:33:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:30:9:30:39:member_comparison_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:32:9:32:58:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:34:9:34:57:member_comparison_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:39:7:39:33:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:41:9:41:39:id_member_invalid:Expression gets interpreted as a 3-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:43:9:43:62:id_member_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:48:7:48:56:complex_invalid:Expression gets interpreted as a 5-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH -bad-chained-comparison:50:9:50:53:complex_invalid:Expression gets interpreted as a 6-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.:HIGH +bad-chained-comparison:21:7:21:36:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('is' and '!='):HIGH +bad-chained-comparison:23:7:23:44:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('is not' and '=='):HIGH +bad-chained-comparison:28:7:28:33:member_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('in' and '=='):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 ('not in', '==', 'in' and '!='):HIGH +bad-chained-comparison:34:9:34:57:member_comparison_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('not in', '<=', '>' and '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 ('not in', 'is', 'in' and 'is not'):HIGH +bad-chained-comparison:48:7:48:56:complex_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('in', '==', 'not in' and 'is not'):HIGH +bad-chained-comparison:50:9:50:53:complex_invalid:Suspicious 6-part chained comparison using semantically incompatible operators ('<', '<=', '!=', 'not in', 'is' and 'in'):HIGH From b3f06a3c9d6bf2a14302136dc059300787e2901e Mon Sep 17 00:00:00 2001 From: Zen Date: Thu, 29 Dec 2022 11:58:20 +0800 Subject: [PATCH 11/21] Make bad chained comparison a default check --- doc/user_guide/checkers/extensions.rst | 16 ---------------- doc/user_guide/checkers/features.rst | 13 +++++++++++++ doc/whatsnew/fragments/6559.new_check | 2 +- .../bad_chained_comparison.py | 2 +- 4 files changed, 15 insertions(+), 18 deletions(-) rename pylint/{extensions => checkers}/bad_chained_comparison.py (94%) diff --git a/doc/user_guide/checkers/extensions.rst b/doc/user_guide/checkers/extensions.rst index 0e0be34326..563633e62b 100644 --- a/doc/user_guide/checkers/extensions.rst +++ b/doc/user_guide/checkers/extensions.rst @@ -7,7 +7,6 @@ Optional checkers Pylint provides the following optional plugins: - :ref:`pylint.extensions.bad_builtin` -- :ref:`pylint.extensions.bad_chained_comparison` - :ref:`pylint.extensions.broad_try_clause` - :ref:`pylint.extensions.check_elif` - :ref:`pylint.extensions.code_style` @@ -38,21 +37,6 @@ You can activate any or all of these extensions by adding a ``load-plugins`` lin load-plugins=pylint.extensions.docparams,pylint.extensions.docstyle -.. _pylint.extensions.bad_chained_comparison: - -Bad-Chained-Comparison checker -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -This checker is provided by ``pylint.extensions.bad_chained_comparison``. -Verbatim name of the checker is ``bad-chained-comparison``. - -Bad-Chained-Comparison checker Messages -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -:bad-chained-comparison (W3501): *Expression gets interpreted as a %s-part chained comparison which straddles comparison groups. If this is not the intent, please parenthesize.* - Used when there is a chained comparison where one expression is part of two - comparisons that belong to different groups. - - .. _pylint.extensions.broad_try_clause: Broad Try Clause checker diff --git a/doc/user_guide/checkers/features.rst b/doc/user_guide/checkers/features.rst index bbcd0ae58f..177f1f7f14 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/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check index 30f6b850c0..52b47075e1 100644 --- a/doc/whatsnew/fragments/6559.new_check +++ b/doc/whatsnew/fragments/6559.new_check @@ -1,4 +1,4 @@ -Add a ``bad-chained-comparison`` extension that emits a warning when +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/extensions/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py similarity index 94% rename from pylint/extensions/bad_chained_comparison.py rename to pylint/checkers/bad_chained_comparison.py index 3694898a1c..352af2a7db 100644 --- a/pylint/extensions/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -24,7 +24,7 @@ class BadChainedComparisonChecker(BaseChecker): "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 as mistake)." , + " (\"<\" does not mean the same thing as \"is\", chaining them in \"0 < x is None\" is probably a mistake)." , ) } From dbbe982ea2040722892b0f1ec3bda2fd870bab60 Mon Sep 17 00:00:00 2001 From: Zen Date: Thu, 29 Dec 2022 13:31:56 +0800 Subject: [PATCH 12/21] Move bad chained comparison functional tests to the correct dir --- .../{ext/bad_chained_comparison => b}/bad_chained_comparison.py | 0 .../bad_chained_comparison => b}/bad_chained_comparison.txt | 0 tests/functional/c/consider/consider_iterating_dictionary.py | 2 +- .../ext/bad_chained_comparison/bad_chained_comparison.rc | 2 -- tests/functional/ext/set_membership/use_set_membership.py | 2 +- 5 files changed, 2 insertions(+), 4 deletions(-) rename tests/functional/{ext/bad_chained_comparison => b}/bad_chained_comparison.py (100%) rename tests/functional/{ext/bad_chained_comparison => b}/bad_chained_comparison.txt (100%) delete mode 100644 tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.py b/tests/functional/b/bad_chained_comparison.py similarity index 100% rename from tests/functional/ext/bad_chained_comparison/bad_chained_comparison.py rename to tests/functional/b/bad_chained_comparison.py diff --git a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt b/tests/functional/b/bad_chained_comparison.txt similarity index 100% rename from tests/functional/ext/bad_chained_comparison/bad_chained_comparison.txt rename to tests/functional/b/bad_chained_comparison.txt diff --git a/tests/functional/c/consider/consider_iterating_dictionary.py b/tests/functional/c/consider/consider_iterating_dictionary.py index 8c75b4e3ef..91402bf772 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/bad_chained_comparison/bad_chained_comparison.rc b/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc deleted file mode 100644 index 2eb553ad79..0000000000 --- a/tests/functional/ext/bad_chained_comparison/bad_chained_comparison.rc +++ /dev/null @@ -1,2 +0,0 @@ -[MAIN] -load-plugins=pylint.extensions.bad_chained_comparison diff --git a/tests/functional/ext/set_membership/use_set_membership.py b/tests/functional/ext/set_membership/use_set_membership.py index 7872d7f98f..df5cd961b7 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}) From be54dba0f872ce6dd41406cc915bfb5f76d87d96 Mon Sep 17 00:00:00 2001 From: Zen Date: Thu, 29 Dec 2022 14:08:39 +0800 Subject: [PATCH 13/21] Remove redundant rc file for documentation --- doc/data/messages/b/bad-chained-comparison/pylintrc | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 doc/data/messages/b/bad-chained-comparison/pylintrc diff --git a/doc/data/messages/b/bad-chained-comparison/pylintrc b/doc/data/messages/b/bad-chained-comparison/pylintrc deleted file mode 100644 index 2eb553ad79..0000000000 --- a/doc/data/messages/b/bad-chained-comparison/pylintrc +++ /dev/null @@ -1,2 +0,0 @@ -[MAIN] -load-plugins=pylint.extensions.bad_chained_comparison From f82a0b0927c4240685acf7fb790c5e304aacf424 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 29 Dec 2022 05:34:32 +0000 Subject: [PATCH 14/21] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/bad_chained_comparison.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index 352af2a7db..f214fae927 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -24,7 +24,7 @@ class BadChainedComparisonChecker(BaseChecker): "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)." , + ' ("<" does not mean the same thing as "is", chaining them in "0 < x is None" is probably a mistake).', ) } @@ -32,7 +32,6 @@ def _get_distinct_operators(self, node: nodes.Compare) -> list: operators = [op[0] for op in node.ops] return list(dict.fromkeys(operators)) - def _has_diff_semantic_groups(self, operators: list) -> bool: # Check if comparison operators are in the same semantic group for comparison_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP): @@ -44,8 +43,15 @@ def visit_compare(self, node: nodes.Compare) -> None: operators = self._get_distinct_operators(node) 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) + 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: From 47efafe98732fb08d102ffda321a6f3dcef4d5d0 Mon Sep 17 00:00:00 2001 From: Zen Date: Fri, 30 Dec 2022 14:51:14 +0800 Subject: [PATCH 15/21] Rename variable for consistency in bad chained comparison --- pylint/checkers/bad_chained_comparison.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index f214fae927..2eead481b1 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -34,9 +34,9 @@ def _get_distinct_operators(self, node: nodes.Compare) -> list: def _has_diff_semantic_groups(self, operators: list) -> bool: # Check if comparison operators are in the same semantic group - for comparison_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP): - if operators[0] in comparison_group: - group = comparison_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: From dc9f6c0981237fc38bf84d4dee64eb2413c68584 Mon Sep 17 00:00:00 2001 From: Zen Date: Sat, 31 Dec 2022 01:27:16 +0800 Subject: [PATCH 16/21] Shorten long line length in bad chained comparisons --- pylint/checkers/bad_chained_comparison.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index 2eead481b1..5c051a2a3b 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -23,8 +23,10 @@ class BadChainedComparisonChecker(BaseChecker): "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).', + "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).', ) } From 3ea3c6dc53b1b3aba6b407ef1fea73479c4cd5bd Mon Sep 17 00:00:00 2001 From: Zen Date: Sat, 31 Dec 2022 03:49:22 +0800 Subject: [PATCH 17/21] Refactor bad chained comparison to use sort --- pylint/checkers/bad_chained_comparison.py | 6 +----- tests/functional/b/bad_chained_comparison.txt | 18 +++++++++--------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index 5c051a2a3b..7a840ce78b 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -30,10 +30,6 @@ class BadChainedComparisonChecker(BaseChecker): ) } - def _get_distinct_operators(self, node: nodes.Compare) -> list: - operators = [op[0] for op in node.ops] - return list(dict.fromkeys(operators)) - def _has_diff_semantic_groups(self, operators: list) -> bool: # Check if comparison operators are in the same semantic group for semantic_group in (COMPARISON_OP, IDENTITY_OP, MEMBERSHIP_OP): @@ -42,7 +38,7 @@ def _has_diff_semantic_groups(self, operators: list) -> bool: return not all(o in group for o in operators) def visit_compare(self, node: nodes.Compare) -> None: - operators = self._get_distinct_operators(node) + operators = sorted(set(op[0] for op in node.ops)) if self._has_diff_semantic_groups(operators): num_parts = f"{len(node.ops)}" incompatibles = ( diff --git a/tests/functional/b/bad_chained_comparison.txt b/tests/functional/b/bad_chained_comparison.txt index 3a89faa679..3ed65a31b9 100644 --- a/tests/functional/b/bad_chained_comparison.txt +++ b/tests/functional/b/bad_chained_comparison.txt @@ -1,11 +1,11 @@ -bad-chained-comparison:21:7:21:36:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('is' and '!='):HIGH -bad-chained-comparison:23:7:23:44:id_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('is not' and '=='):HIGH -bad-chained-comparison:28:7:28:33:member_comparison_invalid:Suspicious 3-part chained comparison using semantically incompatible operators ('in' and '=='):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 ('not in', '==', 'in' and '!='):HIGH -bad-chained-comparison:34:9:34:57:member_comparison_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('not in', '<=', '>' and 'in'):HIGH +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 ('not in', 'is', 'in' and 'is not'):HIGH -bad-chained-comparison:48:7:48:56:complex_invalid:Suspicious 5-part chained comparison using semantically incompatible operators ('in', '==', 'not in' and 'is not'):HIGH -bad-chained-comparison:50:9:50:53:complex_invalid:Suspicious 6-part chained comparison using semantically incompatible operators ('<', '<=', '!=', 'not in', 'is' and '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 From 943ee69c722df7b2fcb90206e9cef721eb045e5e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 30 Dec 2022 17:30:14 +0000 Subject: [PATCH 18/21] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/bad_chained_comparison.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index 7a840ce78b..a18c721372 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -25,7 +25,7 @@ class BadChainedComparisonChecker(BaseChecker): "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 ' + '("<" does not mean the same thing as "is", chaining them in ' '"0 < x is None" is probably a mistake).', ) } From f4ddd8b1bfd62bab6f641bed4905ada2935ba54a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 30 Dec 2022 19:52:06 +0000 Subject: [PATCH 19/21] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/checkers/bad_chained_comparison.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index a18c721372..e759562df0 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -38,7 +38,7 @@ def _has_diff_semantic_groups(self, operators: list) -> bool: return not all(o in group for o in operators) def visit_compare(self, node: nodes.Compare) -> None: - operators = sorted(set(op[0] for op in node.ops)) + operators = sorted({op[0] for op in node.ops}) if self._has_diff_semantic_groups(operators): num_parts = f"{len(node.ops)}" incompatibles = ( From 6e004e26c8e5ab47185bc2bf3546d8434b6c75be Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 30 Dec 2022 22:19:43 +0100 Subject: [PATCH 20/21] Apply suggestions from code review --- pylint/checkers/bad_chained_comparison.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pylint/checkers/bad_chained_comparison.py b/pylint/checkers/bad_chained_comparison.py index e759562df0..e6f0c445fd 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -1,3 +1,7 @@ +# 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 @@ -30,7 +34,8 @@ class BadChainedComparisonChecker(BaseChecker): ) } - def _has_diff_semantic_groups(self, operators: list) -> bool: + 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: From 05ae84a883d8cb7cf9e320a225fc74a0fe24b33f Mon Sep 17 00:00:00 2001 From: Zen Date: Sun, 1 Jan 2023 09:21:21 +0800 Subject: [PATCH 21/21] Change line endings from CRLF to LF --- .../messages/b/bad-chained-comparison/bad.py | 6 +- .../messages/b/bad-chained-comparison/good.py | 6 +- doc/whatsnew/fragments/6559.new_check | 8 +- pylint/checkers/bad_chained_comparison.py | 122 +++++++++--------- tests/functional/b/bad_chained_comparison.py | 102 +++++++-------- tests/functional/b/bad_chained_comparison.txt | 22 ++-- 6 files changed, 133 insertions(+), 133 deletions(-) diff --git a/doc/data/messages/b/bad-chained-comparison/bad.py b/doc/data/messages/b/bad-chained-comparison/bad.py index 62e83401c6..eeca75f13e 100644 --- a/doc/data/messages/b/bad-chained-comparison/bad.py +++ b/doc/data/messages/b/bad-chained-comparison/bad.py @@ -1,3 +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.') +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 index 412a716773..115f4a9db0 100644 --- a/doc/data/messages/b/bad-chained-comparison/good.py +++ b/doc/data/messages/b/bad-chained-comparison/good.py @@ -1,3 +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.') +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/whatsnew/fragments/6559.new_check b/doc/whatsnew/fragments/6559.new_check index 52b47075e1..fa4f146e3d 100644 --- a/doc/whatsnew/fragments/6559.new_check +++ b/doc/whatsnew/fragments/6559.new_check @@ -1,4 +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 +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 index e6f0c445fd..fa75c52b2a 100644 --- a/pylint/checkers/bad_chained_comparison.py +++ b/pylint/checkers/bad_chained_comparison.py @@ -1,61 +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)) +# 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 index df30401742..c1bdab9809 100644 --- a/tests/functional/b/bad_chained_comparison.py +++ b/tests/functional/b/bad_chained_comparison.py @@ -1,51 +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 +# 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 index 3ed65a31b9..276a1debce 100644 --- a/tests/functional/b/bad_chained_comparison.txt +++ b/tests/functional/b/bad_chained_comparison.txt @@ -1,11 +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 +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