Skip to content

Commit

Permalink
Merge branch 'master' into map-as-iterating-context
Browse files Browse the repository at this point in the history
  • Loading branch information
Pierre-Sassoulas committed Feb 19, 2021
2 parents fb12243 + bad6eec commit e23c7dc
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 4 deletions.
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ What's New in Pylint 2.7.0?

* Python 3.6+ is now required.

* Fix false positive for ``builtin-not-iterating`` when ``zip`` receives iterable

* Add `nan-comparison` check for NaN comparisons

* Bug fix for empty-comment message line number.

Closes #4009
Expand Down
4 changes: 3 additions & 1 deletion doc/whatsnew/2.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Summary -- Release highlights
New checkers
============

* Add `nan-comparison` check for comparison of NaN values

* Add support to ``ignored-argument-names`` in DocstringParameterChecker and
adds `useless-param-doc` and `useless-type-doc` messages.

Expand All @@ -27,7 +29,7 @@ New checkers
Other Changes
=============

* Fix false positive for ``builtin-not-iterating`` when ``map`` receives iterable
* Fix false positive for ``builtin-not-iterating`` when ``map`` or `zip` receives iterable

* Fix linter multiprocessing pool shutdown which triggered warnings when runned in parallels with other pytest plugins.

Expand Down
59 changes: 57 additions & 2 deletions pylint/checkers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,6 @@ def visit_for(self, node):
"inlinevar",
}


HUMAN_READABLE_TYPES = {
"module": "module",
"const": "constant",
Expand Down Expand Up @@ -1724,7 +1723,6 @@ def _create_naming_options():


class NameChecker(_BasicChecker):

msgs = {
"C0102": (
'Black listed name "%s"',
Expand Down Expand Up @@ -2342,6 +2340,12 @@ class ComparisonChecker(_BasicChecker):
"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(
Expand Down Expand Up @@ -2402,6 +2406,52 @@ def _is_singleton_const(node) -> bool:
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, astroid.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, astroid.Attribute) and node.attrname == "NaN":
if isinstance(node.expr, astroid.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 = "'{}math.isnan({})'".format(
absence_text, right_value.as_string()
)
else:
suggestion = "'{}math.isnan({})'".format(
absence_text, left_value.as_string()
)

self.add_message(
"nan-comparison",
node=root_node,
args=("'{}'".format(root_node.as_string()), suggestion),
)

def _check_literal_comparison(self, literal, node):
"""Check if we compare to a literal, which is usually what we do not want to do."""
nodes = (astroid.List, astroid.Tuple, astroid.Dict, astroid.Set)
Expand Down Expand Up @@ -2495,6 +2545,11 @@ def visit_compare(self, node):
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)

Expand Down
1 change: 1 addition & 0 deletions pylint/checkers/python3.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def _is_builtin(node):
"frozenset",
"OrderedDict",
"map",
"zip",
}
ATTRIBUTES_ACCEPTS_ITERATOR = {"join", "from_iterable"}
_BUILTIN_METHOD_ACCEPTS_ITERATOR = {
Expand Down
37 changes: 37 additions & 0 deletions tests/checkers/unittest_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,43 @@ def test_comparison(self):
with self.assertAddsMessages(message):
self.checker.visit_compare(node)

node = astroid.extract_node("foo is float('nan')")
message = Message(
"nan-comparison",
node=node,
args=("'foo is float('nan')'", "'math.isnan(foo)'"),
)
with self.assertAddsMessages(message):
self.checker.visit_compare(node)

node = astroid.extract_node(
"""
import numpy
foo != numpy.NaN
"""
)
message = Message(
"nan-comparison",
node=node,
args=("'foo != numpy.NaN'", "'not math.isnan(foo)'"),
)
with self.assertAddsMessages(message):
self.checker.visit_compare(node)

node = astroid.extract_node(
"""
import numpy as nmp
foo is not nmp.NaN
"""
)
message = Message(
"nan-comparison",
node=node,
args=("'foo is not nmp.NaN'", "'not math.isnan(foo)'"),
)
with self.assertAddsMessages(message):
self.checker.visit_compare(node)

node = astroid.extract_node("True == foo")
messages = (
Message("misplaced-comparison-constant", node=node, args=("foo == True",)),
Expand Down
7 changes: 6 additions & 1 deletion tests/checkers/unittest_python3.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ def as_argument_to_map_test(self, fxn):
module = astroid.parse(f"list(map(__, {fxn}()))")
with self.assertNoMessages():
self.walk(module)

def as_argument_to_zip_test(self, fxn):
module = astroid.parse(f"list(zip({fxn}))")
with self.assertNoMessages():
self.walk(module)

def as_iterable_in_unpacking(self, fxn):
node = astroid.extract_node(
Expand Down Expand Up @@ -224,7 +229,7 @@ def iterating_context_tests(self, fxn):
self.as_iterable_in_starred_context(fxn)
self.as_argument_to_itertools_functions(fxn)
self.as_argument_to_map_test(fxn)

self.as_argument_to_zip_test(fxn)
for func in (
"iter",
"list",
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/n/nan_comparison_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# pylint: disable=missing-docstring, invalid-name, misplaced-comparison-constant,literal-comparison,comparison-with-itself, import-error
"""Test detection of NaN value comparison."""

import numpy
x = 42
a = x is numpy.NaN # [nan-comparison]
b = x == numpy.NaN # [nan-comparison]
c = x == float('nan') # [nan-comparison]
e = numpy.NaN == numpy.NaN # [nan-comparison]
f = x is 1
g = 123 is "123"
h = numpy.NaN is not x # [nan-comparison]
i = numpy.NaN != x # [nan-comparison]

j = x != numpy.NaN # [nan-comparison]
j1 = x != float('nan') # [nan-comparison]
assert x == numpy.NaN # [nan-comparison]
assert x is not float('nan') # [nan-comparison]
if x == numpy.NaN: # [nan-comparison]
pass
z = bool(x is numpy.NaN) # [nan-comparison]
12 changes: 12 additions & 0 deletions tests/functional/n/nan_comparison_check.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
nan-comparison:6:4::Comparison 'x is numpy.NaN' should be 'math.isnan(x)'
nan-comparison:7:4::Comparison 'x == numpy.NaN' should be 'math.isnan(x)'
nan-comparison:8:4::Comparison 'x == float('nan')' should be 'math.isnan(x)'
nan-comparison:9:4::Comparison 'numpy.NaN == numpy.NaN' should be 'math.isnan(numpy.NaN)'
nan-comparison:12:4::Comparison 'numpy.NaN is not x' should be 'not math.isnan(x)'
nan-comparison:13:4::Comparison 'numpy.NaN != x' should be 'not math.isnan(x)'
nan-comparison:15:4::Comparison 'x != numpy.NaN' should be 'not math.isnan(x)'
nan-comparison:16:5::Comparison 'x != float('nan')' should be 'not math.isnan(x)'
nan-comparison:17:7::Comparison 'x == numpy.NaN' should be 'math.isnan(x)'
nan-comparison:18:7::Comparison 'x is not float('nan')' should be 'not math.isnan(x)'
nan-comparison:19:3::Comparison 'x == numpy.NaN' should be 'math.isnan(x)'
nan-comparison:21:9::Comparison 'x is numpy.NaN' should be 'math.isnan(x)'

0 comments on commit e23c7dc

Please sign in to comment.