diff --git a/doc/data/messages/c/consider-rewriting-conditional/bad.py b/doc/data/messages/c/consider-rewriting-conditional/bad.py new file mode 100644 index 0000000000..e498b6cb7c --- /dev/null +++ b/doc/data/messages/c/consider-rewriting-conditional/bad.py @@ -0,0 +1,7 @@ +def is_penguin(animal): + # Penguins are the only flightless, kneeless sea birds + return animal.is_seabird() and ( + # +1: [consider-rewriting-conditional] + not animal.can_fly() + or not animal.has_visible_knee() + ) diff --git a/doc/data/messages/c/consider-rewriting-conditional/good.py b/doc/data/messages/c/consider-rewriting-conditional/good.py new file mode 100644 index 0000000000..4fa3a1d8ae --- /dev/null +++ b/doc/data/messages/c/consider-rewriting-conditional/good.py @@ -0,0 +1,3 @@ +def is_penguin(animal): + # Penguins are the only flightless, kneeless sea birds + return animal.is_seabird() and not (animal.can_fly() or animal.has_visible_knee()) diff --git a/doc/data/messages/c/consider-rewriting-conditional/pylintrc b/doc/data/messages/c/consider-rewriting-conditional/pylintrc new file mode 100644 index 0000000000..8663ab085d --- /dev/null +++ b/doc/data/messages/c/consider-rewriting-conditional/pylintrc @@ -0,0 +1,2 @@ +[MAIN] +load-plugins=pylint.extensions.code_style diff --git a/doc/user_guide/checkers/extensions.rst b/doc/user_guide/checkers/extensions.rst index c9dead7ca3..07d3588a29 100644 --- a/doc/user_guide/checkers/extensions.rst +++ b/doc/user_guide/checkers/extensions.rst @@ -92,6 +92,10 @@ Code Style checker Messages Using math.inf or math.nan permits to benefit from typing and it is up to 4 times faster than a float call (after the initial import of math). This check also catches typos in float calls as a side effect. +:consider-rewriting-conditional (R6107): *Consider rewriting conditional expression to '%s'* + Rewrite negated if expressions to improve readability. This style is simpler + and also permits converting long if/elif chains to match case with more ease. + Disabled by default! .. _pylint.extensions.comparison_placement: diff --git a/doc/user_guide/configuration/all-options.rst b/doc/user_guide/configuration/all-options.rst index 309714a0de..084388d6a2 100644 --- a/doc/user_guide/configuration/all-options.rst +++ b/doc/user_guide/configuration/all-options.rst @@ -233,7 +233,7 @@ Standard Checkers confidence = ["HIGH", "CONTROL_FLOW", "INFERENCE", "INFERENCE_FAILURE", "UNDEFINED"] - disable = ["bad-inline-option", "consider-using-augmented-assign", "deprecated-pragma", "file-ignored", "locally-disabled", "prefer-typing-namedtuple", "raw-checker-failed", "suppressed-message", "use-implicit-booleaness-not-comparison-to-string", "use-implicit-booleaness-not-comparison-to-zero", "use-symbolic-message-instead", "useless-suppression"] + disable = ["bad-inline-option", "consider-rewriting-conditional", "consider-using-augmented-assign", "deprecated-pragma", "file-ignored", "locally-disabled", "prefer-typing-namedtuple", "raw-checker-failed", "suppressed-message", "use-implicit-booleaness-not-comparison-to-string", "use-implicit-booleaness-not-comparison-to-zero", "use-symbolic-message-instead", "useless-suppression"] enable = [] diff --git a/doc/user_guide/messages/messages_overview.rst b/doc/user_guide/messages/messages_overview.rst index 87685835bb..4112ca47a3 100644 --- a/doc/user_guide/messages/messages_overview.rst +++ b/doc/user_guide/messages/messages_overview.rst @@ -494,6 +494,7 @@ All messages in the refactor category: refactor/consider-math-not-float refactor/consider-merging-isinstance refactor/consider-refactoring-into-while-condition + refactor/consider-rewriting-conditional refactor/consider-swap-variables refactor/consider-using-alias refactor/consider-using-assignment-expr diff --git a/doc/whatsnew/fragments/10600.new_check b/doc/whatsnew/fragments/10600.new_check new file mode 100644 index 0000000000..135951842b --- /dev/null +++ b/doc/whatsnew/fragments/10600.new_check @@ -0,0 +1,6 @@ +Add ``consider-rewriting-conditional`` check to the Code Style extension to suggest +``not (x and y)`` instead of ``not x or not y`` in order to facilitate the detection +of match case refactors. The code style extension must be enabled and +``consider-rewriting-conditional`` itself needs to be explicitly enabled. + +Refs #10600 diff --git a/pylint/extensions/code_style.py b/pylint/extensions/code_style.py index d8ea869cb9..0005084c63 100644 --- a/pylint/extensions/code_style.py +++ b/pylint/extensions/code_style.py @@ -5,18 +5,26 @@ from __future__ import annotations import difflib +from copy import copy +from enum import IntFlag, auto from typing import TYPE_CHECKING, TypeGuard, cast from astroid import nodes from pylint.checkers import BaseChecker, utils from pylint.checkers.utils import only_required_for_messages, safe_infer -from pylint.interfaces import INFERENCE +from pylint.interfaces import HIGH, INFERENCE if TYPE_CHECKING: from pylint.lint import PyLinter +class InvertibleValues(IntFlag): + NO = 0 + YES = auto() + EXPLICIT_NEGATION = auto() + + class CodeStyleChecker(BaseChecker): """Checkers that can improve code consistency. @@ -82,6 +90,16 @@ class CodeStyleChecker(BaseChecker): "to 4 times faster than a float call (after the initial import of math). " "This check also catches typos in float calls as a side effect.", ), + "R6107": ( + "Consider rewriting conditional expression to '%s'", + "consider-rewriting-conditional", + "Rewrite negated if expressions to improve readability. This style is simpler " + "and also permits converting long if/elif chains to match case with more ease.\n" + "Disabled by default!", + { + "default_enabled": False, + }, + ), } options = ( ( @@ -356,6 +374,89 @@ def visit_assign(self, node: nodes.Assign) -> None: confidence=INFERENCE, ) + @staticmethod + def _can_be_inverted(values: list[nodes.NodeNG]) -> InvertibleValues: + invertible = InvertibleValues.NO + for node in values: + match node: + case nodes.UnaryOp(op="not"): + invertible |= InvertibleValues.EXPLICIT_NEGATION + case nodes.Compare(ops=[("!=" | "not in" | "is not" as op, n)]) if not ( + op == "is not" and isinstance(n, nodes.Const) and n.value is None + ): + invertible |= InvertibleValues.EXPLICIT_NEGATION + case nodes.Compare( + ops=[("<" | "<=" | ">" | ">=", nodes.Const(value=int()))] + ): + invertible |= InvertibleValues.YES + case _: + return InvertibleValues.NO + return invertible + + @staticmethod + def _invert_node(node: nodes.NodeNG) -> nodes.NodeNG: + match node: + case nodes.UnaryOp(op="not"): + new_node = copy(node.operand) + new_node.parent = node + return new_node + case nodes.Compare(left=left, ops=[(op, n)]): + new_node = copy(node) + match op: + case "!=": + new_op = "==" + case "not in": + new_op = "in" + case "is not": + new_op = "is" + case "<": + new_op = ">=" + case "<=": + new_op = ">" + case ">": + new_op = "<=" + case ">=": + new_op = "<" + case _: # pragma: no cover + raise AssertionError + new_node.postinit(left=left, ops=[(new_op, n)]) + return new_node + case _: # pragma: no cover + raise AssertionError + + @only_required_for_messages("consider-rewriting-conditional") + def visit_boolop(self, node: nodes.BoolOp) -> None: + if ( + node.op == "or" + and (invertible := self._can_be_inverted(node.values)) + and invertible & InvertibleValues.EXPLICIT_NEGATION + ): + new_boolop = copy(node) + new_boolop.op = "and" + new_boolop.postinit([self._invert_node(val) for val in node.values]) + + if isinstance(node.parent, nodes.UnaryOp) and node.parent.op == "not": + target_node = node.parent + new_node = new_boolop + else: + target_node = node + new_node = nodes.UnaryOp( + op="not", + lineno=0, + col_offset=0, + end_lineno=None, + end_col_offset=None, + parent=node.parent, + ) + new_node.postinit(operand=new_boolop) + + self.add_message( + "consider-rewriting-conditional", + node=target_node, + args=(new_node.as_string(),), + confidence=HIGH, + ) + def register(linter: PyLinter) -> None: linter.register_checker(CodeStyleChecker(linter)) diff --git a/pylintrc b/pylintrc index fe653a092c..43fba9fa4c 100644 --- a/pylintrc +++ b/pylintrc @@ -81,6 +81,7 @@ clear-cache-post-run=no # multiple time (only on the command line, not in the configuration file where # it should appear only once). See also the "--disable" option for examples. enable= + consider-rewriting-conditional, use-symbolic-message-instead, useless-suppression, diff --git a/tests/functional/ext/code_style/cs_consider_rewriting_conditional.py b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.py new file mode 100644 index 0000000000..3e72d62a4b --- /dev/null +++ b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.py @@ -0,0 +1,41 @@ +# pylint: disable=missing-docstring,too-many-branches + +def f1(expr, node_cls, x, y, z): + if isinstance(expr, node_cls) and expr.attrname == "__init__": + ... + elif isinstance(expr, node_cls) or expr.attrname == "__init__": + ... + elif not isinstance(expr, node_cls) and expr.attrname == "__init__": + ... + elif not isinstance(expr, node_cls) and expr.attrname != "__init__": + ... + elif not isinstance(expr, node_cls) or expr.attrname == "__init__": + ... + + if x < 0 or x > 100: + ... + elif x > 0 or y >= 1: + ... + elif x < 0 or y <= 1: + ... + + if x is not None or y is not None: + ... + elif not isinstance(expr, node_cls) or x is not None: + ... + + # +1: [consider-rewriting-conditional] + if not isinstance(expr, node_cls) or expr.attrname != "__init__": + ... + elif not x or y not in z: # [consider-rewriting-conditional] + ... + elif not x or y is not z: # [consider-rewriting-conditional] + ... + elif not (not x or not y): # [consider-rewriting-conditional] + ... + elif x and (not y or not z): # [consider-rewriting-conditional] + ... + elif not x or y < 0 or z <= 0: # [consider-rewriting-conditional] + ... + elif not x or y > 0 or z >= 0: # [consider-rewriting-conditional] + ... diff --git a/tests/functional/ext/code_style/cs_consider_rewriting_conditional.rc b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.rc new file mode 100644 index 0000000000..794559fb63 --- /dev/null +++ b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.rc @@ -0,0 +1,3 @@ +[MAIN] +load-plugins=pylint.extensions.code_style +enable=consider-rewriting-conditional diff --git a/tests/functional/ext/code_style/cs_consider_rewriting_conditional.txt b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.txt new file mode 100644 index 0000000000..fb1c1e026c --- /dev/null +++ b/tests/functional/ext/code_style/cs_consider_rewriting_conditional.txt @@ -0,0 +1,7 @@ +consider-rewriting-conditional:28:7:28:68:f1:Consider rewriting conditional expression to 'not (isinstance(expr, node_cls) and expr.attrname == '__init__')':HIGH +consider-rewriting-conditional:30:9:30:28:f1:Consider rewriting conditional expression to 'not (x and y in z)':HIGH +consider-rewriting-conditional:32:9:32:28:f1:Consider rewriting conditional expression to 'not (x and y is z)':HIGH +consider-rewriting-conditional:34:9:34:29:f1:Consider rewriting conditional expression to 'x and y':HIGH +consider-rewriting-conditional:36:16:36:30:f1:Consider rewriting conditional expression to 'not (y and z)':HIGH +consider-rewriting-conditional:38:9:38:33:f1:Consider rewriting conditional expression to 'not (x and y >= 0 and z > 0)':HIGH +consider-rewriting-conditional:40:9:40:33:f1:Consider rewriting conditional expression to 'not (x and y <= 0 and z < 0)':HIGH