From 01d79168185dcdb2330e78ed5dc698ec611134fc Mon Sep 17 00:00:00 2001 From: Chris Rebert Date: Sat, 21 Feb 2015 02:42:59 -0800 Subject: Fix #299: Warn about `type(x) is/== Y` --- ChangeLog | 5 ++ pylint/checkers/stdlib.py | 45 +++++++++++++- pylint/test/functional/unidiomatic_typecheck.py | 76 ++++++++++++++++++++++++ pylint/test/functional/unidiomatic_typecheck.txt | 18 ++++++ 4 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 pylint/test/functional/unidiomatic_typecheck.py create mode 100644 pylint/test/functional/unidiomatic_typecheck.txt diff --git a/ChangeLog b/ChangeLog index e32e165..c361c10 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,6 +36,11 @@ ChangeLog for Pylint appropriate built-in is not used in an iterating context (semantics taken from 2to3). + * Add a new warning, 'unidiomatic-typecheck', emitted when an explicit + typecheck uses type() instead of isinstance(). For example, + `type(x) == Y` instead of `isinstance(x, Y)`. Patch by Chris Rebert. + Closes issue #299. + 2015-01-16 -- 1.4.1 diff --git a/pylint/checkers/stdlib.py b/pylint/checkers/stdlib.py index ac8b876..5331704 100644 --- a/pylint/checkers/stdlib.py +++ b/pylint/checkers/stdlib.py @@ -27,10 +27,15 @@ from pylint.checkers import BaseChecker from pylint.checkers import utils +TYPECHECK_COMPARISON_OPERATORS = frozenset(('is', 'is not', '==', '!=', 'in', 'not in')) +LITERAL_NODE_TYPES = (astroid.Const, astroid.Dict, astroid.List, astroid.Set) + if sys.version_info >= (3, 0): OPEN_MODULE = '_io' + TYPE_QNAME = 'builtins.type' else: OPEN_MODULE = '__builtin__' + TYPE_QNAME = '__builtin__.type' def _check_mode_str(mode): @@ -76,6 +81,15 @@ def _check_mode_str(mode): return True +def _is_one_arg_pos_call(call): + """Is this a call with exactly 1 argument, + where that argument is positional? + """ + return (isinstance(call, astroid.CallFunc) + and len(call.args) == 1 + and not isinstance(call.args[0], astroid.Keyword)) + + class StdlibChecker(BaseChecker): __implements__ = (IAstroidChecker,) name = 'stdlib' @@ -99,7 +113,13 @@ class StdlibChecker(BaseChecker): 'The first argument of assertTrue and assertFalse is ' 'a condition. If a constant is passed as parameter, that ' 'condition will be always true. In this case a warning ' - 'should be emitted.') + 'should be emitted.'), + 'W1504': ('Using type() instead of isinstance() 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.') } @utils.check_messages('bad-open-mode', 'redundant-unittest-assert') @@ -132,6 +152,14 @@ class StdlibChecker(BaseChecker): for value in node.values: self._check_datetime(value) + @utils.check_messages('unidiomatic-typecheck') + def visit_compare(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_redundant_assert(self, node, infer): if (isinstance(infer, astroid.BoundMethod) and node.args and isinstance(node.args[0], astroid.Const) and @@ -168,6 +196,21 @@ class StdlibChecker(BaseChecker): args=mode_arg.value) + 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 isinstance(left_func, astroid.Class) and left_func.qname() == TYPE_QNAME: + if operator in ('is', 'is not') and _is_one_arg_pos_call(right): + right_func = utils.safe_infer(right.func) + if isinstance(right_func, astroid.Class) 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): """required method to auto register this checker """ linter.register_checker(StdlibChecker(linter)) diff --git a/pylint/test/functional/unidiomatic_typecheck.py b/pylint/test/functional/unidiomatic_typecheck.py new file mode 100644 index 0000000..008456b --- /dev/null +++ b/pylint/test/functional/unidiomatic_typecheck.py @@ -0,0 +1,76 @@ +"""Warnings for using type(x) == Y or type(x) is Y instead of isinstance(x, Y).""" +# pylint: disable=missing-docstring,expression-not-assigned,redefined-builtin,invalid-name + +def simple_positives(): + type(42) is int # [unidiomatic-typecheck] + type(42) is not int # [unidiomatic-typecheck] + type(42) == int # [unidiomatic-typecheck] + type(42) != int # [unidiomatic-typecheck] + type(42) in [int] # [unidiomatic-typecheck] + type(42) not in [int] # [unidiomatic-typecheck] + +def simple_inference_positives(): + alias = type + alias(42) is int # [unidiomatic-typecheck] + alias(42) is not int # [unidiomatic-typecheck] + alias(42) == int # [unidiomatic-typecheck] + alias(42) != int # [unidiomatic-typecheck] + alias(42) in [int] # [unidiomatic-typecheck] + alias(42) not in [int] # [unidiomatic-typecheck] + +def type_creation_negatives(): + type('Q', (object,), dict(a=1)) is int + type('Q', (object,), dict(a=1)) is not int + type('Q', (object,), dict(a=1)) == int + type('Q', (object,), dict(a=1)) != int + type('Q', (object,), dict(a=1)) in [int] + type('Q', (object,), dict(a=1)) not in [int] + +def invalid_type_call_negatives(**kwargs): + type(bad=7) is int + type(bad=7) is not int + type(bad=7) == int + type(bad=7) != int + type(bad=7) in [int] + type(bad=7) not in [int] + type('bad', 7) is int + type('bad', 7) is not int + type('bad', 7) == int + type('bad', 7) != int + type('bad', 7) in [int] + type('bad', 7) not in [int] + type(**kwargs) is int + type(**kwargs) is not int + type(**kwargs) == int + type(**kwargs) != int + type(**kwargs) in [int] + type(**kwargs) not in [int] + +def local_var_shadowing_inference_negatives(): + type = lambda dummy: 7 + type(42) is int + type(42) is not int + type(42) == int + type(42) != int + type(42) in [int] + type(42) not in [int] + +def parameter_shadowing_inference_negatives(type): + type(42) is int + type(42) is not int + type(42) == int + type(42) != int + type(42) in [int] + type(42) not in [int] + +def deliberate_subclass_check_negatives(b): + type(42) is type(b) + type(42) is not type(b) + +def type_of_literals_positives(a): + type(a) is type([]) # [unidiomatic-typecheck] + type(a) is not type([]) # [unidiomatic-typecheck] + type(a) is type({}) # [unidiomatic-typecheck] + type(a) is not type({}) # [unidiomatic-typecheck] + type(a) is type("") # [unidiomatic-typecheck] + type(a) is not type("") # [unidiomatic-typecheck] diff --git a/pylint/test/functional/unidiomatic_typecheck.txt b/pylint/test/functional/unidiomatic_typecheck.txt new file mode 100644 index 0000000..23c0f6f --- /dev/null +++ b/pylint/test/functional/unidiomatic_typecheck.txt @@ -0,0 +1,18 @@ +unidiomatic-typecheck:5:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:6:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:7:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:8:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:9:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:10:simple_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:14:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:15:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:16:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:17:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:18:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:19:simple_inference_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:71:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:72:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:73:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:74:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:75:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. +unidiomatic-typecheck:76:type_of_literals_positives:Using type() instead of isinstance() for a typecheck. \ No newline at end of file -- cgit v1.2.1