Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/data/messages/c/consider-rewriting-conditional/bad.py
Original file line number Diff line number Diff line change
@@ -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()
)
3 changes: 3 additions & 0 deletions doc/data/messages/c/consider-rewriting-conditional/good.py
Original file line number Diff line number Diff line change
@@ -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())
2 changes: 2 additions & 0 deletions doc/data/messages/c/consider-rewriting-conditional/pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[MAIN]
load-plugins=pylint.extensions.code_style
4 changes: 4 additions & 0 deletions doc/user_guide/checkers/extensions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion doc/user_guide/configuration/all-options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []

Expand Down
1 change: 1 addition & 0 deletions doc/user_guide/messages/messages_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions doc/whatsnew/fragments/10600.new_check
Original file line number Diff line number Diff line change
@@ -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
103 changes: 102 additions & 1 deletion pylint/extensions/code_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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 = (
(
Expand Down Expand Up @@ -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))
1 change: 1 addition & 0 deletions pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
Original file line number Diff line number Diff line change
@@ -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]
...
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[MAIN]
load-plugins=pylint.extensions.code_style
enable=consider-rewriting-conditional
Original file line number Diff line number Diff line change
@@ -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