Work in progress merging of use-implicit-booleaness checker

Author: Pierre-Sassoulas

doc/whatsnew/fragments/6871.internal

@@ -0,0 +1,8 @@
+* The compare to empty string checker (``pylint.extensions.emptystring``) has been removed
+  as it is now part of the implicit booleaness checker, a default check.
+
+  - ``compare-to-zero`` was renamed ``use-implicit-booleaness-not-comparison-to-zero`` but is still an extension.
+  - ``compare-to-empty-string`` was merged with ``use-implicit-booleaness-not-comparison``.
+  - Messages related to implicit booleaness were made more explicit and actionable.
+
+Closes #6871

pylint/checkers/refactoring/implicit_booleaness_checker.py

@@ -4,6 +4,8 @@
 
 from __future__ import annotations
 
+import itertools
+
 import astroid
 from astroid import bases, nodes
 
@@ -50,7 +52,6 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
     * comparison such as variable != empty_literal:
     """
 
-    # configuration section name
     name = "refactoring"
     msgs = {
         "C1802": (
@@ -64,11 +65,16 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
             {"old_names": [("C1801", "len-as-condition")]},
         ),
         "C1803": (
-            "'%s' can be simplified to '%s' as an empty sequence is falsey",
+            "'%s' can be simplified to '%s' as %s is falsey",
             "use-implicit-booleaness-not-comparison",
             "Used when Pylint detects that collection literal comparison is being "
             "used to check for emptiness; Use implicit booleaness instead "
             "of a collection classes; empty collections are considered as false",
+            {
+                "old_names": [
+                    ("C1901", "compare-to-empty-string"),
+                ]
+            },
         ),
     }
 
@@ -139,6 +145,52 @@ def visit_unaryop(self, node: nodes.UnaryOp) -> None:
     @utils.only_required_for_messages("use-implicit-booleaness-not-comparison")
     def visit_compare(self, node: nodes.Compare) -> None:
         self._check_use_implicit_booleaness_not_comparison(node)
+        self._check_compare_to_empty_string(node)
+
+    def _check_compare_to_empty_string(self, node: nodes.Compare) -> None:
+        """Checks for comparisons to empty string.
+
+        Most of the time you should use the fact that empty strings are false.
+        An exception to this rule is when an empty string value is allowed in the program
+        and has a different meaning than None!
+        """
+        _operators = {"!=", "==", "is not", "is"}
+        # note: astroid.Compare has the left most operand in node.left
+        # while the rest are a list of tuples in node.ops
+        # the format of the tuple is ('compare operator sign', node)
+        # here we squash everything into `ops` to make it easier for processing later
+        ops: list[tuple[str, nodes.NodeNG | None]] = [("", node.left)]
+        ops.extend(node.ops)
+        iter_ops = iter(ops)
+        ops = list(itertools.chain(*iter_ops))  # type: ignore[arg-type]
+        for ops_idx in range(len(ops) - 2):
+            op_1: nodes.NodeNG | None = ops[ops_idx]
+            op_2: str = ops[ops_idx + 1]  # type: ignore[assignment]
+            op_3: nodes.NodeNG | None = ops[ops_idx + 2]
+            error_detected = False
+            if op_1 is None or op_3 is None or op_2 not in _operators:
+                continue
+            if isinstance(node.parent, nodes.Assert) and op_2 == "==":
+                # Comparing equality to an empty string directly make more
+                # sense in tests where you want to make sure you don't just
+                # have a falsey value but an empty string
+                continue
+            # x ?? ""
+            if utils.is_empty_str_literal(op_1):
+                error_detected = True
+                node_name = op_3.as_string()
+            # '' ?? X
+            elif utils.is_empty_str_literal(op_3):
+                error_detected = True
+                node_name = op_1.as_string()
+            if error_detected:
+                suggestion = f"not {node_name}" if op_2 in {"==", "is"} else node_name
+
+                self.add_message(
+                    "compare-to-empty-string",
+                    args=(node.as_string(), suggestion, "an empty string"),
+                    node=node,
+                )
 
     def _check_use_implicit_booleaness_not_comparison(
         self, node: nodes.Compare
@@ -177,11 +229,11 @@ def _check_use_implicit_booleaness_not_comparison(
 
                 # No need to check for operator when visiting compare node
                 if operator in {"==", "!=", ">=", ">", "<=", "<"}:
-                    collection_literal = "{}"
+                    collection_literal, sequence_type = "{}", "dict"
                     if isinstance(literal_node, nodes.List):
-                        collection_literal = "[]"
+                        collection_literal, sequence_type = "[]", "list"
                     if isinstance(literal_node, nodes.Tuple):
-                        collection_literal = "()"
+                        collection_literal, sequence_type = "()", "tuple"
 
                     instance_name = "x"
                     if isinstance(target_node, nodes.Call) and target_node.func:
@@ -202,6 +254,7 @@ def _check_use_implicit_booleaness_not_comparison(
                         args=(
                             original_comparison,
                             suggestion,
+                            f"an empty {sequence_type}",
                         ),
                         node=node,
                     )

pylint/checkers/strings.py

@@ -494,7 +494,7 @@ def _check_new_format(self, node: nodes.Call, func: bases.BoundMethod) -> None:
 
         check_args = False
         # Consider "{[0]} {[1]}" as num_args.
-        num_args += sum(1 for field in named_fields if field == "")
+        num_args += sum(1 for field in named_fields if not field)
         if named_fields:
             for field in named_fields:
                 if field and field not in named_arguments:
@@ -509,7 +509,7 @@ def _check_new_format(self, node: nodes.Call, func: bases.BoundMethod) -> None:
             # num_args can be 0 if manual_pos is not.
             num_args = num_args or manual_pos
             if positional_arguments or num_args:
-                empty = any(field == "" for field in named_fields)
+                empty = not all(field for field in named_fields)
                 if named_arguments or empty:
                     # Verify the required number of positional arguments
                     # only if the .format got at least one keyword argument.
@@ -546,7 +546,7 @@ def _check_new_format_specifiers(
         for key, specifiers in fields:
             # Obtain the argument. If it can't be obtained
             # or inferred, skip this check.
-            if key == "":
+            if not key:
                 # {[0]} will have an unnamed argument, defaulting
                 # to 0. It will not be present in `named`, so use the value
                 # 0 for it.

pylint/config/_pylint_config/utils.py

@@ -83,7 +83,7 @@ def validate_yes_no(question: str, default: Literal["yes", "no"] | None) -> bool
     # pylint: disable-next=bad-builtin
     answer = input(question).lower()
 
-    if answer == "" and default:
+    if not answer and default:
         answer = default
 
     if answer not in YES_NO_ANSWERS:

pylint/extensions/comparetozero.py

@@ -34,10 +34,15 @@ class CompareToZeroChecker(checkers.BaseChecker):
     # configuration section name
     name = "compare-to-zero"
     msgs = {
-        "C2001": (
+        "C2002": (
             "Avoid comparisons to zero",
-            "compare-to-zero",
+            "use-implicit-booleaness-not-comparison-to-zero",
             "Used when Pylint detects comparison to a 0 constant.",
+            {
+                "old_names": [
+                    ("C2001", "compare-to-zero"),
+                ]
+            },
         )
     }
 

pylint/extensions/emptystring.py

@@ -63,15 +63,14 @@ def _create_comment(
             comment += self._create_comment_for_package(
                 package, new_messages, missing_messages
             )
-        if comment == "":
-            comment = (
+        comment = (
+            f"🤖 **Effect of this PR on checked open source code:** 🤖\n\n{comment}"
+            if comment
+            else (
                 "🤖 According to the primer, this change has **no effect** on the"
                 " checked open source code. 🤖🎉\n\n"
             )
-        else:
-            comment = (
-                f"🤖 **Effect of this PR on checked open source code:** 🤖\n\n{comment}"
-            )
+        )
         return self._truncate_comment(comment)
 
     def _create_comment_for_package(

pylint/testutils/_primer/primer_compare_command.py

@@ -93,7 +93,7 @@ def create_files(paths: list[str], chroot: str = ".") -> None:
         path = os.path.join(chroot, path)
         filename = os.path.basename(path)
         # path is a directory path
-        if filename == "":
+        if not filename:
             dirs.add(path)
         # path is a filename path
         else:

pylint/testutils/utils.py

@@ -36,12 +36,7 @@ def changelog_insert_empty_lines(file_content: str, subtitle_text: str) -> str:
     for i, line in enumerate(lines):
         if line.startswith(subtitle_text):
             subtitle_count += 1
-            if (
-                subtitle_count == 1
-                or i < 2
-                or lines[i - 1] == ""
-                and lines[i - 2] == ""
-            ):
+            if subtitle_count == 1 or i < 2 or not lines[i - 1] and not lines[i - 2]:
                 continue
             lines.insert(i, "")
     return "\n".join(lines)

script/fix_documentation.py

@@ -0,0 +1,28 @@
+# pylint: disable=literal-comparison,missing-docstring
+
+X = 123
+Y = len('test')
+
+if X is 0:  # [use-implicit-booleaness-not-comparison-to-zero]
+    pass
+
+if Y is not 0:  # [use-implicit-booleaness-not-comparison-to-zero]
+    pass
+
+if X == 0:  # [use-implicit-booleaness-not-comparison-to-zero]
+    pass
+
+if Y != 0:  # [use-implicit-booleaness-not-comparison-to-zero]
+    pass
+
+if X > 0:
+    pass
+
+if X < 0:
+    pass
+
+if 0 < X:
+    pass
+
+if 0 > X:
+    pass

tests/functional/ext/comparetozero/compare_to_zero.py

@@ -0,0 +1,4 @@
+use-implicit-booleaness-not-comparison-to-zero:6:3:6:9::Avoid comparisons to zero:UNDEFINED
+use-implicit-booleaness-not-comparison-to-zero:9:3:9:13::Avoid comparisons to zero:UNDEFINED
+use-implicit-booleaness-not-comparison-to-zero:12:3:12:9::Avoid comparisons to zero:UNDEFINED
+use-implicit-booleaness-not-comparison-to-zero:15:3:15:9::Avoid comparisons to zero:UNDEFINED