Add improve-conditionals check in the CodeStyle extension

doc/data/messages/i/improve-conditionals/bad.py

@@ -0,0 +1,4 @@
+def func(expr, node_cls):
+    # +1:[improve-conditional]
+    if not isinstance(expr, node_cls) or expr.attrname != "__init__":
+        ...

doc/data/messages/i/improve-conditionals/good.py

@@ -0,0 +1,3 @@
+def func(expr, node_cls):
+    if not (isinstance(expr, node_cls) and expr.attrname == "__init__"):
+        ...

doc/user_guide/checkers/extensions.rst

@@ -88,6 +88,8 @@ Code Style checker Messages
   'typing.NamedTuple' uses the well-known 'class' keyword with type-hints for
   readability (it's also faster as it avoids an internal exec call). Disabled
   by default!
+:improve-conditionals (R6106): *Rewrite conditional expression to '%s'*
+  Rewrite negated if expressions to improve readability.
 
 
 .. _pylint.extensions.comparison_placement:

doc/user_guide/messages/messages_overview.rst

@@ -515,6 +515,7 @@ All messages in the refactor category:
    refactor/duplicate-code
    refactor/else-if-used
    refactor/empty-comment
+   refactor/improve-conditionals
    refactor/inconsistent-return-statements
    refactor/literal-comparison
    refactor/magic-value-comparison

doc/whatsnew/fragments/10600.new_check

@@ -0,0 +1,3 @@
+Add :ref:`improve-conditionals` check to the Code Style extension.
+
+Refs #10600

pylint/checkers/base/name_checker/checker.py

@@ -99,7 +99,7 @@ def _redefines_import(node: nodes.AssignName) -> bool:
     current = node
     while current and not isinstance(current.parent, nodes.ExceptHandler):
         current = current.parent
-    if not current or not utils.error_of_type(current.parent, ImportError):
+    if not (current and utils.error_of_type(current.parent, ImportError)):
         return False
     try_block = current.parent.parent
     for import_node in try_block.nodes_of_class((nodes.ImportFrom, nodes.Import)):
@@ -160,7 +160,7 @@ def _is_multi_naming_match(
         match is not None
         and match.lastgroup is not None
         and match.lastgroup not in EXEMPT_NAME_CATEGORIES
-        and (node_type != "method" or confidence != interfaces.INFERENCE_FAILURE)
+        and not (node_type == "method" and confidence == interfaces.INFERENCE_FAILURE)
     )
 
 

pylint/checkers/classes/class_checker.py

@@ -186,10 +186,10 @@ def _is_trivial_super_delegation(function: nodes.FunctionDef) -> bool:
     # Should be a super call with the MRO pointer being the
     # current class and the type being the current instance.
     current_scope = function.parent.scope()
-    if (
-        super_call.mro_pointer != current_scope
-        or not isinstance(super_call.type, astroid.Instance)
-        or super_call.type.name != current_scope.name
+    if not (
+        super_call.mro_pointer == current_scope
+        and isinstance(super_call.type, astroid.Instance)
+        and super_call.type.name == current_scope.name
     ):
         return False
 
@@ -1139,8 +1139,9 @@ def _check_unused_private_variables(self, node: nodes.ClassDef) -> None:
 
     def _check_unused_private_attributes(self, node: nodes.ClassDef) -> None:
         for assign_attr in node.nodes_of_class(nodes.AssignAttr):
-            if not is_attr_private(assign_attr.attrname) or not isinstance(
-                assign_attr.expr, nodes.Name
+            if not (
+                is_attr_private(assign_attr.attrname)
+                and isinstance(assign_attr.expr, nodes.Name)
             ):
                 continue
 
@@ -1928,7 +1929,7 @@ def _check_protected_attribute_access(
         callee = node.expr.as_string()
         parents_callee = callee.split(".")
         for callee in reversed(parents_callee):
-            if not outer_klass or callee != outer_klass.name:
+            if not (outer_klass and callee == outer_klass.name):
                 inside_klass = False
                 break
 

pylint/checkers/dataclass_checker.py

@@ -82,7 +82,7 @@ def _check_invalid_field_call(self, node: nodes.Call) -> None:
             self._check_invalid_field_call_within_call(node, scope_node)
             return
 
-        if not scope_node or not scope_node.is_dataclass:
+        if not (scope_node and scope_node.is_dataclass):
             self.add_message(
                 "invalid-field-call",
                 node=node,

pylint/checkers/design_analysis.py

@@ -660,8 +660,8 @@ def visit_if(self, node: nodes.If) -> None:
         self._check_boolean_expressions(node)
         branches = 1
         # don't double count If nodes coming from some 'elif'
-        if node.orelse and (
-            len(node.orelse) > 1 or not isinstance(node.orelse[0], nodes.If)
+        if node.orelse and not (
+            len(node.orelse) == 1 and isinstance(node.orelse[0], nodes.If)
         ):
             branches += 1
         self._inc_branch(node, branches)

pylint/checkers/exceptions.py

@@ -348,7 +348,7 @@ def _check_misplaced_bare_raise(self, node: nodes.Raise) -> None:
             current = current.parent
 
         expected = (nodes.ExceptHandler,)
-        if not current or not isinstance(current.parent, expected):
+        if not (current and isinstance(current.parent, expected)):
             self.add_message("misplaced-bare-raise", node=node, confidence=HIGH)
 
     def _check_bad_exception_cause(self, node: nodes.Raise) -> None:

pylint/checkers/imports.py

@@ -1172,10 +1172,13 @@ def _report_dependencies_graph(
     ) -> None:
         """Write dependencies as a dot (graphviz) file."""
         dep_info = self.linter.stats.dependencies
-        if not dep_info or not (
-            self.linter.config.import_graph
-            or self.linter.config.ext_import_graph
-            or self.linter.config.int_import_graph
+        if not (
+            dep_info
+            and (
+                self.linter.config.import_graph
+                or self.linter.config.ext_import_graph
+                or self.linter.config.int_import_graph
+            )
         ):
             raise EmptyReportError()
         filename = self.linter.config.import_graph

pylint/checkers/refactoring/recommendation_checker.py

@@ -281,10 +281,10 @@ def _check_consider_using_dict_items(self, node: nodes.For) -> None:
                     continue
 
                 value = subscript.slice
-                if (
-                    not isinstance(value, nodes.Name)
-                    or value.name != node.target.name
-                    or iterating_object_name != subscript.value.as_string()
+                if not (
+                    isinstance(value, nodes.Name)
+                    and value.name == node.target.name
+                    and iterating_object_name == subscript.value.as_string()
                 ):
                     continue
                 last_definition_lineno = value.lookup(value.name)[1][-1].lineno
@@ -334,10 +334,10 @@ def _check_consider_using_dict_items_comprehension(
                     continue
 
                 value = subscript.slice
-                if (
-                    not isinstance(value, nodes.Name)
-                    or value.name != node.target.name
-                    or iterating_object_name != subscript.value.as_string()
+                if not (
+                    isinstance(value, nodes.Name)
+                    and value.name == node.target.name
+                    and iterating_object_name == subscript.value.as_string()
                 ):
                     continue
 
@@ -428,8 +428,9 @@ def _detect_replacable_format_call(self, node: nodes.Const) -> None:
                 return
 
             # If % applied to another type than str, it's modulo and can't be replaced by formatting
-            if not hasattr(node.parent.left, "value") or not isinstance(
-                node.parent.left.value, str
+            if not (
+                hasattr(node.parent.left, "value")
+                and isinstance(node.parent.left.value, str)
             ):
                 return
 

pylint/checkers/refactoring/refactoring_checker.py

@@ -634,7 +634,7 @@ def _check_simplifiable_if(self, node: nodes.If) -> None:
                     for target in else_branch.targets
                     if isinstance(target, nodes.AssignName)
                 ]
-                if not first_branch_targets or not else_branch_targets:
+                if not (first_branch_targets and else_branch_targets):
                     return
                 if sorted(first_branch_targets) != sorted(else_branch_targets):
                     return
@@ -645,7 +645,7 @@ def _check_simplifiable_if(self, node: nodes.If) -> None:
             case _:
                 return
 
-        if not first_branch_is_bool or not else_branch_is_bool:
+        if not (first_branch_is_bool and else_branch_is_bool):
             return
         if not first_branch.value.value:
             # This is a case that can't be easily simplified and
@@ -1053,7 +1053,7 @@ def visit_raise(self, node: nodes.Raise) -> None:
     def _check_stop_iteration_inside_generator(self, node: nodes.Raise) -> None:
         """Check if an exception of type StopIteration is raised inside a generator."""
         frame = node.frame()
-        if not isinstance(frame, nodes.FunctionDef) or not frame.is_generator():
+        if not (isinstance(frame, nodes.FunctionDef) and frame.is_generator()):
             return
         if utils.node_ignores_exception(node, StopIteration):
             return
@@ -1319,11 +1319,11 @@ def _duplicated_isinstance_types(node: nodes.BoolOp) -> dict[str, set[str]]:
         all_types: collections.defaultdict[str, set[str]] = collections.defaultdict(set)
 
         for call in node.values:
-            if not isinstance(call, nodes.Call) or len(call.args) != 2:
+            if not (isinstance(call, nodes.Call) and len(call.args) == 2):
                 continue
 
             inferred = utils.safe_infer(call.func)
-            if not inferred or not utils.is_builtin_object(inferred):
+            if not (inferred and utils.is_builtin_object(inferred)):
                 continue
 
             if inferred.name != "isinstance":
@@ -1365,7 +1365,7 @@ def _check_consider_merging_isinstance(self, node: nodes.BoolOp) -> None:
     def _check_consider_using_in(self, node: nodes.BoolOp) -> None:
         allowed_ops = {"or": "==", "and": "!="}
 
-        if node.op not in allowed_ops or len(node.values) < 2:
+        if not (node.op in allowed_ops and len(node.values) >= 2):
             return
 
         for value in node.values:
@@ -1416,7 +1416,7 @@ def _check_chained_comparison(self, node: nodes.BoolOp) -> None:
 
         Care is taken to avoid simplifying a < b < c and b < d.
         """
-        if node.op != "and" or len(node.values) < 2:
+        if not (node.op == "and" and len(node.values) >= 2):
             return
 
         def _find_lower_upper_bounds(
@@ -1566,7 +1566,7 @@ def _is_simple_assignment(node: nodes.NodeNG | None) -> bool:
         return False
 
     def _check_swap_variables(self, node: nodes.Return | nodes.Assign) -> None:
-        if not node.next_sibling() or not node.next_sibling().next_sibling():
+        if not (node.next_sibling() and node.next_sibling().next_sibling()):
             return
         assignments = [node, node.next_sibling(), node.next_sibling().next_sibling()]
         if not all(self._is_simple_assignment(node) for node in assignments):
@@ -1643,10 +1643,10 @@ def _append_context_managers_to_stack(self, node: nodes.Assign) -> None:
             if not isinstance(value, nodes.Call):
                 continue
             inferred = utils.safe_infer(value.func)
-            if (
-                not inferred
-                or inferred.qname() not in CALLS_RETURNING_CONTEXT_MANAGERS
-                or not isinstance(assignee, (nodes.AssignName, nodes.AssignAttr))
+            if not (
+                inferred
+                and inferred.qname() in CALLS_RETURNING_CONTEXT_MANAGERS
+                and isinstance(assignee, (nodes.AssignName, nodes.AssignAttr))
             ):
                 continue
             stack = self._consider_using_with_stack.get_stack_for_frame(node.frame())
@@ -1685,8 +1685,11 @@ def _check_consider_using_with(self, node: nodes.Call) -> None:
             # checked when leaving the scope.
             return
         inferred = utils.safe_infer(node.func)
-        if not inferred or not isinstance(
-            inferred, (nodes.FunctionDef, nodes.ClassDef, bases.BoundMethod)
+        if not (
+            inferred
+            and isinstance(
+                inferred, (nodes.FunctionDef, nodes.ClassDef, bases.BoundMethod)
+            )
         ):
             return
         could_be_used_in_with = (
@@ -2178,12 +2181,12 @@ def _check_unnecessary_dict_index_lookup(
 
                 # Case where .items is assigned to k,v (i.e., for k, v in d.items())
                 if isinstance(value, nodes.Name):
-                    if (
-                        not isinstance(node.target, nodes.Tuple)
+                    if not (
+                        isinstance(node.target, nodes.Tuple)
                         # Ignore 1-tuples: for k, in d.items()
-                        or len(node.target.elts) < 2
-                        or value.name != node.target.elts[0].name
-                        or iterating_object_name != subscript.value.as_string()
+                        and len(node.target.elts) >= 2
+                        and value.name == node.target.elts[0].name
+                        and iterating_object_name == subscript.value.as_string()
                     ):
                         continue
 
@@ -2213,11 +2216,11 @@ def _check_unnecessary_dict_index_lookup(
 
                 # Case where .items is assigned to single var (i.e., for item in d.items())
                 elif isinstance(value, nodes.Subscript):
-                    if (
-                        not isinstance(node.target, nodes.AssignName)
-                        or not isinstance(value.value, nodes.Name)
-                        or node.target.name != value.value.name
-                        or iterating_object_name != subscript.value.as_string()
+                    if not (
+                        isinstance(node.target, nodes.AssignName)
+                        and isinstance(value.value, nodes.Name)
+                        and node.target.name == value.value.name
+                        and iterating_object_name == subscript.value.as_string()
                     ):
                         continue
 
@@ -2234,7 +2237,7 @@ def _check_unnecessary_dict_index_lookup(
 
                     # check if subscripted by 0 (key)
                     inferred = utils.safe_infer(value.slice)
-                    if not isinstance(inferred, nodes.Const) or inferred.value != 0:
+                    if not (isinstance(inferred, nodes.Const) and inferred.value == 0):
                         continue
 
                     if has_nested_loops:
@@ -2350,9 +2353,9 @@ def _check_unnecessary_list_index_lookup(
 
                 index = subscript.slice
                 if isinstance(index, nodes.Name):
-                    if (
-                        index.name != name1
-                        or iterating_object_name != subscript.value.as_string()
+                    if not (
+                        index.name == name1
+                        and iterating_object_name == subscript.value.as_string()
                     ):
                         continue
 

pylint/checkers/spelling.py

@@ -161,11 +161,11 @@ def next(self) -> tuple[str, int]:
             pre_text, post_text = self._text.split("/", 1)
             self._text = post_text
             self._offset = 0
-            if (
-                not pre_text
-                or not post_text
-                or not pre_text[-1].isalpha()
-                or not post_text[0].isalpha()
+            if not (
+                pre_text
+                and post_text
+                and pre_text[-1].isalpha()
+                and post_text[0].isalpha()
             ):
                 self._text = ""
                 self._offset = 0
@@ -177,9 +177,9 @@ def _next(self) -> tuple[str, Literal[0]]:
             if "/" not in self._text:
                 return self._text, 0
             pre_text, post_text = self._text.split("/", 1)
-            if not pre_text or not post_text:
+            if not (pre_text and post_text):
                 break
-            if not pre_text[-1].isalpha() or not post_text[0].isalpha():
+            if not (pre_text[-1].isalpha() and post_text[0].isalpha()):
                 raise StopIteration()
             self._text = pre_text + " " + post_text
         raise StopIteration()

pylint/checkers/stdlib.py

@@ -641,7 +641,7 @@ def _check_bad_thread_instantiation(self, node: nodes.Call) -> None:
         if "target" in func_kwargs:
             return
 
-        if len(node.args) < 2 and (not node.kwargs or "target" not in func_kwargs):
+        if len(node.args) < 2 and not (node.kwargs and "target" in func_kwargs):
             self.add_message(
                 "bad-thread-instantiation", node=node, confidence=interfaces.HIGH
             )
@@ -885,7 +885,7 @@ def _check_open_call(
 
         if not mode_arg or (
             isinstance(mode_arg, nodes.Const)
-            and (not mode_arg.value or "b" not in str(mode_arg.value))
+            and not (mode_arg.value and "b" in str(mode_arg.value))
         ):
             confidence = HIGH
             try: