Fix #299: Warn about `type(x) is/== Y`impl-299

4 files changed, 143 insertions, 1 deletions

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