Skip to content

Commit

Permalink
[refactor] Create files for comparison checker in pylint.checker.base
Browse files Browse the repository at this point in the history
  • Loading branch information
@Pierre-Sassoulas
Pierre-Sassoulas committed Mar 24, 2022
1 parent ddfca0c commit 977b08d
Show file tree
Hide file tree
Showing 2 changed files with 297 additions and 285 deletions.
287 changes: 2 additions & 285 deletions pylint/checkers/base/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from pylint import utils as lint_utils
from pylint.checkers import utils
from pylint.checkers.base.basic_checker import _BasicChecker
from pylint.checkers.base.comparison_checker import ComparisonChecker
from pylint.checkers.utils import (
infer_all,
is_overload_stub,
Expand Down Expand Up @@ -135,10 +136,7 @@ class AnyStyle(NamingStyle):
REVERSED_PROTOCOL_METHOD = "__reversed__"
SEQUENCE_PROTOCOL_METHODS = ("__getitem__", "__len__")
REVERSED_METHODS = (SEQUENCE_PROTOCOL_METHODS, (REVERSED_PROTOCOL_METHOD,))
TYPECHECK_COMPARISON_OPERATORS = frozenset(("is", "is not", "==", "!="))
LITERAL_NODE_TYPES = (nodes.Const, nodes.Dict, nodes.List, nodes.Set)
UNITTEST_CASE = "unittest.case"
TYPE_QNAME = "builtins.type"
ABC_METACLASSES = {"_py_abc.ABCMeta", "abc.ABCMeta"} # Python 3.7+,

# Name categories that are always consistent with all naming conventions.
Expand All @@ -165,7 +163,7 @@ class AnyStyle(NamingStyle):
},
)

COMPARISON_OPERATORS = frozenset(("==", "!=", "<", ">", "<=", ">="))

# List of methods which can be redefined
REDEFINABLE_METHODS = frozenset(("__module__",))
TYPING_FORWARD_REF_QNAME = "typing.ForwardRef"
Expand Down Expand Up @@ -2333,13 +2331,6 @@ def visit_pass(self, node: nodes.Pass) -> None:
self.add_message("unnecessary-pass", node=node)


def _is_one_arg_pos_call(call):
"""Is this a call with exactly 1 argument,
where that argument is positional?
"""
return isinstance(call, nodes.Call) and len(call.args) == 1 and not call.keywords


def _infer_dunder_doc_attribute(node):
# Try to see if we have a `__doc__` attribute.
try:
Expand All @@ -2355,280 +2346,6 @@ def _infer_dunder_doc_attribute(node):
return docstring.value


class ComparisonChecker(_BasicChecker):
"""Checks for comparisons.
- singleton comparison: 'expr == True', 'expr == False' and 'expr == None'
- yoda condition: 'const "comp" right' where comp can be '==', '!=', '<',
'<=', '>' or '>=', and right can be a variable, an attribute, a method or
a function
"""

msgs = {
"C0121": (
"Comparison %s should be %s",
"singleton-comparison",
"Used when an expression is compared to singleton "
"values like True, False or None.",
),
"C0123": (
"Use isinstance() rather than type() for a typecheck.",
"unidiomatic-typecheck",
"The idiomatic way to perform an explicit typecheck in "
"Python is to use isinstance(x, Y) rather than "
"type(x) == Y, type(x) is Y. Though there are unusual "
"situations where these give different results.",
{"old_names": [("W0154", "old-unidiomatic-typecheck")]},
),
"R0123": (
"Comparison to literal",
"literal-comparison",
"Used when comparing an object to a literal, which is usually "
"what you do not want to do, since you can compare to a different "
"literal than what was expected altogether.",
),
"R0124": (
"Redundant comparison - %s",
"comparison-with-itself",
"Used when something is compared against itself.",
),
"W0143": (
"Comparing against a callable, did you omit the parenthesis?",
"comparison-with-callable",
"This message is emitted when pylint detects that a comparison with a "
"callable was made, which might suggest that some parenthesis were omitted, "
"resulting in potential unwanted behaviour.",
),
"W0177": (
"Comparison %s should be %s",
"nan-comparison",
"Used when an expression is compared to NaN"
"values like numpy.NaN and float('nan')",
),
}

def _check_singleton_comparison(
self, left_value, right_value, root_node, checking_for_absence: bool = False
):
"""Check if == or != is being used to compare a singleton value."""
singleton_values = (True, False, None)

def _is_singleton_const(node) -> bool:
return isinstance(node, nodes.Const) and any(
node.value is value for value in singleton_values
)

if _is_singleton_const(left_value):
singleton, other_value = left_value.value, right_value
elif _is_singleton_const(right_value):
singleton, other_value = right_value.value, left_value
else:
return

singleton_comparison_example = {False: "'{} is {}'", True: "'{} is not {}'"}

# True/False singletons have a special-cased message in case the user is
# mistakenly using == or != to check for truthiness
if singleton in {True, False}:
suggestion_template = (
"{} if checking for the singleton value {}, or {} if testing for {}"
)
truthiness_example = {False: "not {}", True: "{}"}
truthiness_phrase = {True: "truthiness", False: "falsiness"}

# Looks for comparisons like x == True or x != False
checking_truthiness = singleton is not checking_for_absence

suggestion = suggestion_template.format(
singleton_comparison_example[checking_for_absence].format(
left_value.as_string(), right_value.as_string()
),
singleton,
(
"'bool({})'"
if not utils.is_test_condition(root_node) and checking_truthiness
else "'{}'"
).format(
truthiness_example[checking_truthiness].format(
other_value.as_string()
)
),
truthiness_phrase[checking_truthiness],
)
else:
suggestion = singleton_comparison_example[checking_for_absence].format(
left_value.as_string(), right_value.as_string()
)
self.add_message(
"singleton-comparison",
node=root_node,
args=(f"'{root_node.as_string()}'", suggestion),
)

def _check_nan_comparison(
self, left_value, right_value, root_node, checking_for_absence: bool = False
):
def _is_float_nan(node):
try:
if isinstance(node, nodes.Call) and len(node.args) == 1:
if (
node.args[0].value.lower() == "nan"
and node.inferred()[0].pytype() == "builtins.float"
):
return True
return False
except AttributeError:
return False

def _is_numpy_nan(node):
if isinstance(node, nodes.Attribute) and node.attrname == "NaN":
if isinstance(node.expr, nodes.Name):
return node.expr.name in {"numpy", "nmp", "np"}
return False

def _is_nan(node) -> bool:
return _is_float_nan(node) or _is_numpy_nan(node)

nan_left = _is_nan(left_value)
if not nan_left and not _is_nan(right_value):
return

absence_text = ""
if checking_for_absence:
absence_text = "not "
if nan_left:
suggestion = f"'{absence_text}math.isnan({right_value.as_string()})'"
else:
suggestion = f"'{absence_text}math.isnan({left_value.as_string()})'"
self.add_message(
"nan-comparison",
node=root_node,
args=(f"'{root_node.as_string()}'", suggestion),
)

def _check_literal_comparison(self, literal, node: nodes.Compare):
"""Check if we compare to a literal, which is usually what we do not want to do."""
is_other_literal = isinstance(literal, (nodes.List, nodes.Dict, nodes.Set))
is_const = False
if isinstance(literal, nodes.Const):
if isinstance(literal.value, bool) or literal.value is None:
# Not interested in these values.
return
is_const = isinstance(literal.value, (bytes, str, int, float))

if is_const or is_other_literal:
self.add_message("literal-comparison", node=node)

def _check_logical_tautology(self, node: nodes.Compare):
"""Check if identifier is compared against itself.
:param node: Compare node
:Example:
val = 786
if val == val: # [comparison-with-itself]
pass
"""
left_operand = node.left
right_operand = node.ops[0][1]
operator = node.ops[0][0]
if isinstance(left_operand, nodes.Const) and isinstance(
right_operand, nodes.Const
):
left_operand = left_operand.value
right_operand = right_operand.value
elif isinstance(left_operand, nodes.Name) and isinstance(
right_operand, nodes.Name
):
left_operand = left_operand.name
right_operand = right_operand.name

if left_operand == right_operand:
suggestion = f"{left_operand} {operator} {right_operand}"
self.add_message("comparison-with-itself", node=node, args=(suggestion,))

def _check_callable_comparison(self, node):
operator = node.ops[0][0]
if operator not in COMPARISON_OPERATORS:
return

bare_callables = (nodes.FunctionDef, astroid.BoundMethod)
left_operand, right_operand = node.left, node.ops[0][1]
# this message should be emitted only when there is comparison of bare callable
# with non bare callable.
number_of_bare_callables = 0
for operand in left_operand, right_operand:
inferred = utils.safe_infer(operand)
# Ignore callables that raise, as well as typing constants
# implemented as functions (that raise via their decorator)
if (
isinstance(inferred, bare_callables)
and "typing._SpecialForm" not in inferred.decoratornames()
and not any(isinstance(x, nodes.Raise) for x in inferred.body)
):
number_of_bare_callables += 1
if number_of_bare_callables == 1:
self.add_message("comparison-with-callable", node=node)

@utils.check_messages(
"singleton-comparison",
"unidiomatic-typecheck",
"literal-comparison",
"comparison-with-itself",
"comparison-with-callable",
)
def visit_compare(self, node: nodes.Compare) -> None:
self._check_callable_comparison(node)
self._check_logical_tautology(node)
self._check_unidiomatic_typecheck(node)
# NOTE: this checker only works with binary comparisons like 'x == 42'
# but not 'x == y == 42'
if len(node.ops) != 1:
return

left = node.left
operator, right = node.ops[0]

if operator in {"==", "!="}:
self._check_singleton_comparison(
left, right, node, checking_for_absence=operator == "!="
)

if operator in {"==", "!=", "is", "is not"}:
self._check_nan_comparison(
left, right, node, checking_for_absence=operator in {"!=", "is not"}
)
if operator in {"is", "is not"}:
self._check_literal_comparison(right, node)

def _check_unidiomatic_typecheck(self, node):
operator, right = node.ops[0]
if operator in TYPECHECK_COMPARISON_OPERATORS:
left = node.left
if _is_one_arg_pos_call(left):
self._check_type_x_is_y(node, left, operator, right)

def _check_type_x_is_y(self, node, left, operator, right):
"""Check for expressions like type(x) == Y."""
left_func = utils.safe_infer(left.func)
if not (
isinstance(left_func, nodes.ClassDef) and left_func.qname() == TYPE_QNAME
):
return

if operator in {"is", "is not"} and _is_one_arg_pos_call(right):
right_func = utils.safe_infer(right.func)
if (
isinstance(right_func, nodes.ClassDef)
and right_func.qname() == TYPE_QNAME
):
# type(x) == type(a)
right_arg = utils.safe_infer(right.args[0])
if not isinstance(right_arg, LITERAL_NODE_TYPES):
# not e.g. type(x) == type([])
return
self.add_message("unidiomatic-typecheck", node=node)


def register(linter: "PyLinter") -> None:
linter.register_checker(BasicErrorChecker(linter))
linter.register_checker(BasicChecker(linter))
Expand Down
Loading

0 comments on commit 977b08d

Please sign in to comment.