Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge use implicit booleaness like checks #8630

Merged
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
8 changes: 0 additions & 8 deletions doc/data/messages/c/compare-to-empty-string/bad.py

This file was deleted.

8 changes: 0 additions & 8 deletions doc/data/messages/c/compare-to-empty-string/good.py

This file was deleted.

2 changes: 0 additions & 2 deletions doc/data/messages/c/compare-to-empty-string/pylintrc

This file was deleted.

8 changes: 0 additions & 8 deletions doc/data/messages/c/compare-to-zero/bad.py

This file was deleted.

8 changes: 0 additions & 8 deletions doc/data/messages/c/compare-to-zero/good.py

This file was deleted.

2 changes: 0 additions & 2 deletions doc/data/messages/c/compare-to-zero/pylintrc

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def important_string_manipulation(x: str, y: str) -> None:
if x == "": # [use-implicit-booleaness-not-comparison-to-string]
print("x is an empty string")

if y != "": # [use-implicit-booleaness-not-comparison-to-string]
print("y is not an empty string")
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Following this check blindly in weakly typed code base can create hard to debug issues. If the value
can be something else that is falsey but not a string (for example ``None``, an empty sequence, or ``0``),
the code will not be equivalent.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def important_string_manipulation(x: str, y: str) -> None:
if not x:
print("x is an empty string")

if y:
print("y is not an empty string")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[main]
enable=use-implicit-booleaness-not-comparison-to-string
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def important_math(x: int, y: int) -> None:
if x == 0: # [use-implicit-booleaness-not-comparison-to-zero]
print("x is equal to zero")

if y != 0: # [use-implicit-booleaness-not-comparison-to-zero]
print("y is not equal to zero")
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Following this check blindly in weakly typed code base can create hard to debug issues. If the value
can be something else that is falsey but not an ``int`` (for example ``None``, an empty sequence,
or an empty string), the code will not be equivalent.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
def important_math(x: int, y: int) -> None:
if not x:
print("x is equal to zero")

if y:
print("y is not equal to zero")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[main]
enable=use-implicit-booleaness-not-comparison-to-zero
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Following this check blindly in weakly typed code base can create hard to debug issues. If the value
can be something else that is falsey but not a sequence (for example ``None``, an empty string, or ``0``),
the code will not be equivalent.
30 changes: 0 additions & 30 deletions doc/user_guide/checkers/extensions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Pylint provides the following optional plugins:
- :ref:`pylint.extensions.broad_try_clause`
- :ref:`pylint.extensions.check_elif`
- :ref:`pylint.extensions.code_style`
- :ref:`pylint.extensions.comparetozero`
- :ref:`pylint.extensions.comparison_placement`
- :ref:`pylint.extensions.confusing_elif`
- :ref:`pylint.extensions.consider_refactoring_into_while_condition`
Expand All @@ -20,7 +19,6 @@ Pylint provides the following optional plugins:
- :ref:`pylint.extensions.docstyle`
- :ref:`pylint.extensions.dunder`
- :ref:`pylint.extensions.empty_comment`
- :ref:`pylint.extensions.emptystring`
- :ref:`pylint.extensions.eq_without_hash`
- :ref:`pylint.extensions.for_any_all`
- :ref:`pylint.extensions.magic_value`
Expand Down Expand Up @@ -88,34 +86,6 @@ Code Style checker Messages
to. This can be changed to be an augmented assign. Disabled by default!


.. _pylint.extensions.emptystring:

Compare-To-Empty-String checker
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This checker is provided by ``pylint.extensions.emptystring``.
Verbatim name of the checker is ``compare-to-empty-string``.

Compare-To-Empty-String checker Messages
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
:compare-to-empty-string (C1901): *"%s" can be simplified to "%s" as an empty string is falsey*
Used when Pylint detects comparison to an empty string constant.


.. _pylint.extensions.comparetozero:

Compare-To-Zero checker
~~~~~~~~~~~~~~~~~~~~~~~

This checker is provided by ``pylint.extensions.comparetozero``.
Verbatim name of the checker is ``compare-to-zero``.

Compare-To-Zero checker Messages
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
:compare-to-zero (C2001): *"%s" can be simplified to "%s" as 0 is falsey*
Used when Pylint detects comparison to a 0 constant.


.. _pylint.extensions.comparison_placement:

Comparison-Placement checker
Expand Down
4 changes: 4 additions & 0 deletions doc/user_guide/checkers/features.rst
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,10 @@ Refactoring checker Messages
Emitted when a single "return" or "return None" statement is found at the end
of function or method definition. This statement can safely be removed
because Python will implicitly return None
:use-implicit-booleaness-not-comparison-to-zero (C1805): *"%s" can be simplified to "%s" as 0 is falsey*
Used when Pylint detects comparison to a 0 constant.
:use-implicit-booleaness-not-comparison-to-string (C1804): *"%s" can be simplified to "%s" as an empty string is falsey*
Used when Pylint detects comparison to an empty string constant.
:use-implicit-booleaness-not-comparison (C1803): *'%s' can be simplified to '%s' as an empty %s is falsey*
Used when Pylint detects that collection literal comparison is being used to
check for emptiness; Use implicit booleaness instead of a collection classes;
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 = ["raw-checker-failed", "bad-inline-option", "locally-disabled", "file-ignored", "suppressed-message", "useless-suppression", "deprecated-pragma", "use-symbolic-message-instead", "consider-using-augmented-assign"]
disable = ["raw-checker-failed", "bad-inline-option", "locally-disabled", "file-ignored", "suppressed-message", "useless-suppression", "deprecated-pragma", "use-implicit-booleaness-not-comparison-to-string", "use-implicit-booleaness-not-comparison-to-zero", "use-symbolic-message-instead", "consider-using-augmented-assign"]

enable = []

Expand Down
6 changes: 4 additions & 2 deletions doc/user_guide/messages/messages_overview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,6 @@ All messages in the convention category:
convention/bad-file-encoding
convention/bad-mcs-classmethod-argument
convention/bad-mcs-method-argument
convention/compare-to-empty-string
convention/compare-to-zero
convention/consider-iterating-dictionary
convention/consider-using-any-or-all
convention/consider-using-dict-items
Expand Down Expand Up @@ -433,6 +431,8 @@ All messages in the convention category:
convention/unnecessary-lambda-assignment
convention/unneeded-not
convention/use-implicit-booleaness-not-comparison
convention/use-implicit-booleaness-not-comparison-to-string
convention/use-implicit-booleaness-not-comparison-to-zero
convention/use-implicit-booleaness-not-len
convention/use-maxsplit-arg
convention/use-sequence-for-iteration
Expand All @@ -449,6 +449,8 @@ All renamed messages in the convention category:
:titlesonly:

convention/blacklisted-name
convention/compare-to-empty-string
convention/compare-to-zero
convention/len-as-condition
convention/missing-docstring
convention/old-misplaced-comparison-constant
Expand Down
15 changes: 15 additions & 0 deletions doc/whatsnew/fragments/6871.user_action
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
The compare to empty string checker (``pylint.extensions.emptystring``) and the compare to
zero checker (``pylint.extensions.compare-to-zero``) have been removed and their checks are
now part of the implicit booleaness checker:

``compare-to-zero`` was renamed ``use-implicit-booleaness-not-comparison-to-zero`` and
``compare-to-empty-string`` was renamed ``use-implicit-booleaness-not-comparison-to-string``
and they now need to be enabled explicitly.

The `pylint.extensions.emptystring`` and ``pylint.extensions.compare-to-zero`` extensions
no longer exists and needs to be removed from the ``load-plugins`` option.

This permits to make their likeness explicit and will provide better performance as they share most of their
conditions to be raised.

Refs #6871
4 changes: 3 additions & 1 deletion examples/pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,15 @@ disable=raw-checker-failed,
suppressed-message,
useless-suppression,
deprecated-pragma,
use-implicit-booleaness-not-comparison-to-string,
use-implicit-booleaness-not-comparison-to-zero,
use-symbolic-message-instead

# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
# 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=c-extension-no-member
enable=


[METHOD_ARGS]
Expand Down
4 changes: 2 additions & 2 deletions examples/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -365,13 +365,13 @@ confidence = ["HIGH", "CONTROL_FLOW", "INFERENCE", "INFERENCE_FAILURE", "UNDEFIN
# --enable=similarities". If you want to run only the classes checker, but have
# no Warning level messages displayed, use "--disable=all --enable=classes
# --disable=W".
disable = ["raw-checker-failed", "bad-inline-option", "locally-disabled", "file-ignored", "suppressed-message", "useless-suppression", "deprecated-pragma", "use-symbolic-message-instead"]
disable = ["raw-checker-failed", "bad-inline-option", "locally-disabled", "file-ignored", "suppressed-message", "useless-suppression", "deprecated-pragma", "use-implicit-booleaness-not-comparison-to-string", "use-implicit-booleaness-not-comparison-to-zero", "use-symbolic-message-instead"]

# Enable the message, report, category or checker with the given id(s). You can
# either give multiple identifier separated by comma (,) or put this option
# 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 = ["c-extension-no-member"]
# enable =

[tool.pylint.method_args]
# List of qualified names (i.e., library.method) which require a timeout
Expand Down
115 changes: 114 additions & 1 deletion pylint/checkers/refactoring/implicit_booleaness_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from __future__ import annotations

import itertools

import astroid
from astroid import bases, nodes, util

Expand All @@ -12,6 +14,14 @@
from pylint.interfaces import HIGH, INFERENCE


def _is_constant_zero(node: str | nodes.NodeNG) -> bool:
# We have to check that node.value is not False because node.value == 0 is True
# when node.value is False
return (
isinstance(node, astroid.Const) and node.value == 0 and node.value is not False
)


class ImplicitBooleanessChecker(checkers.BaseChecker):
"""Checks for incorrect usage of comparisons or len() inside conditions.

Expand Down Expand Up @@ -70,6 +80,21 @@ class ImplicitBooleanessChecker(checkers.BaseChecker):
"used to check for emptiness; Use implicit booleaness instead "
"of a collection classes; empty collections are considered as false",
),
"C1804": (
'"%s" can be simplified to "%s" as an empty string is falsey',
"use-implicit-booleaness-not-comparison-to-string",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"use-implicit-booleaness-not-comparison-to-string",
"consider-implicit-booleaness-str",

"Used when Pylint detects comparison to an empty string constant.",
{
"default_enabled": False,
"old_names": [("C1901", "compare-to-empty-string")],
},
),
"C1805": (
'"%s" can be simplified to "%s" as 0 is falsey',
"use-implicit-booleaness-not-comparison-to-zero",
"Used when Pylint detects comparison to a 0 constant.",
{"default_enabled": False, "old_names": [("C2001", "compare-to-zero")]},
),
}

options = ()
Expand Down Expand Up @@ -146,9 +171,97 @@ def visit_unaryop(self, node: nodes.UnaryOp) -> None:
"use-implicit-booleaness-not-len", node=node, confidence=HIGH
)

@utils.only_required_for_messages("use-implicit-booleaness-not-comparison")
@utils.only_required_for_messages(
"use-implicit-booleaness-not-comparison",
"use-implicit-booleaness-not-comparison-to-string",
"use-implicit-booleaness-not-comparison-to-zero",
)
def visit_compare(self, node: nodes.Compare) -> None:
self._check_use_implicit_booleaness_not_comparison(node)
self._check_compare_to_zero(node)
self._check_compare_to_string(node)

def _check_compare_to_zero(self, node: nodes.Compare) -> None:
# pylint: disable=duplicate-code
_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]] = [("", node.left)]
ops.extend(node.ops)
iter_ops = iter(ops)
all_ops = list(itertools.chain(*iter_ops))

for ops_idx in range(len(all_ops) - 2):
op_1 = all_ops[ops_idx]
op_2 = all_ops[ops_idx + 1]
op_3 = all_ops[ops_idx + 2]
error_detected = False

# 0 ?? X
if _is_constant_zero(op_1) and op_2 in _operators:
error_detected = True
op = op_3
# X ?? 0
elif op_2 in _operators and _is_constant_zero(op_3):
error_detected = True
op = op_1

if error_detected:
original = f"{op_1.as_string()} {op_2} {op_3.as_string()}"
suggestion = (
op.as_string()
if op_2 in {"!=", "is not"}
else f"not {op.as_string()}"
)
self.add_message(
"compare-to-zero",
args=(original, suggestion),
node=node,
confidence=HIGH,
)

def _check_compare_to_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
node_name = ""
# 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),
node=node,
confidence=HIGH,
)

def _check_use_implicit_booleaness_not_comparison(
self, node: nodes.Compare
Expand Down
Loading