Move iterable checker to typecheck module and alter inference strategy

4 files changed, 264 insertions, 223 deletions

diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index b68b082..30e337b 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1476,84 +1476,6 @@ class ComparisonChecker(_BasicChecker):
                 self.check_singleton_comparison(right, node)
 
 
-class IterableChecker(_BasicChecker):
-    """
-    Checks for non-iterables used in an iterable context.
-    Contexts include:
-    - for-statement
-    - starargs in function call
-    - `yield from`-statement
-    - list, dict and set comprehensions
-    - generator expressions
-    Also checks for non-mappings in function call kwargs.
-    """
-    msgs = {'E0118': ('Non-iterable value %s is used in an iterating context',
-                      'not-an-iterable',
-                      'Used when a non-iterable value is used in place where'
-                      'iterable is expected'),
-            'E0119': ('Non-mapping value %s is used in a mapping context',
-                      'not-a-mapping',
-                      'Used when a non-mapping value is used in place where'
-                      'mapping is expected'),
-           }
-
-    def _not_an_iterable(self, node):
-        is_const = isinstance(node, astroid.Const)
-        is_string = is_const and isinstance(node.value, six.string_types)
-        return is_const and not is_string
-
-    def _check_iterable(self, element, root_node):
-        infered = helpers.safe_infer(element)
-        if infered is not None:
-            if self._not_an_iterable(infered):
-                self.add_message('not-an-iterable',
-                                 args=element.as_string(),
-                                 node=root_node)
-
-    def _check_mapping(self, element, root_node):
-        infered = helpers.safe_infer(element)
-        if infered is not None:
-            if isinstance(infered, astroid.Const):
-                self.add_message('not-a-mapping',
-                                 args=element.as_string(),
-                                 node=root_node)
-
-    @check_messages('not-an-iterable')
-    def visit_for(self, node):
-        self._check_iterable(node.iter, node)
-
-    @check_messages('not-an-iterable')
-    def visit_yieldfrom(self, node):
-        self._check_iterable(node.value, node)
-
-    @check_messages('not-an-iterable', 'not-a-mapping')
-    def visit_call(self, node):
-        for stararg in node.starargs:
-            self._check_iterable(stararg.value, node)
-        for kwarg in node.kwargs:
-            self._check_mapping(kwarg.value, node)
-
-    @check_messages('not-an-iterable')
-    def visit_listcomp(self, node):
-        for gen in node.generators:
-            self._check_iterable(gen.iter, node)
-
-    @check_messages('not-an-iterable')
-    def visit_dictcomp(self, node):
-        for gen in node.generators:
-            self._check_iterable(gen.iter, node)
-
-    @check_messages('not-an-iterable')
-    def visit_setcomp(self, node):
-        for gen in node.generators:
-            self._check_iterable(gen.iter, node)
-
-    @check_messages('not-an-iterable')
-    def visit_generatorexp(self, node):
-        for gen in node.generators:
-            self._check_iterable(gen.iter, node)
-
-
 def register(linter):
     """required method to auto register this checker"""
     linter.register_checker(BasicErrorChecker(linter))
@@ -1563,4 +1485,3 @@ def register(linter):
     linter.register_checker(PassChecker(linter))
     linter.register_checker(LambdaForComprehensionChecker(linter))
     linter.register_checker(ComparisonChecker(linter))
-    linter.register_checker(IterableChecker(linter))
diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index 5f2bb23..f7df240 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -794,6 +794,126 @@ accessed. Python regular expressions are accepted.'}
                              args=str(error), node=node)
 
 
+class IterableChecker(BaseChecker):
+    """
+    Checks for non-iterables used in an iterable context.
+    Contexts include:
+    - for-statement
+    - starargs in function call
+    - `yield from`-statement
+    - list, dict and set comprehensions
+    - generator expressions
+    Also checks for non-mappings in function call kwargs.
+    """
+
+    __implements__ = (IAstroidChecker,)
+    name = 'iterable_check'
+
+    msgs = {'E1132': ('Non-iterable value %s is used in an iterating context',
+                      'not-an-iterable',
+                      'Used when a non-iterable value is used in place where'
+                      'iterable is expected'),
+            'E1133': ('Non-mapping value %s is used in a mapping context',
+                      'not-a-mapping',
+                      'Used when a non-mapping value is used in place where'
+                      'mapping is expected'),
+           }
+
+    def _is_comprehension(self, node):
+        comprehensions = (astroid.ListComp,
+                          astroid.SetComp,
+                          astroid.DictComp)
+        return isinstance(node, comprehensions)
+
+    def _is_string_value(self, node):
+        return isinstance(node, astroid.Const) and isinstance(node.value, six.string_types)
+
+    def _is_iterable_value(self, value):
+        try:
+            value.getattr('__iter__')
+            return True
+        except astroid.NotFoundError:
+            return False
+
+    def _is_old_style_iterator(self, value):
+        try:
+            value.getattr('__next__')
+            value.getattr('__len__')
+            return True
+        except astroid.NotFoundError:
+            return False
+
+    def _check_iterable(self, node, root_node):
+        # for/set/dict-comprehensions can't be infered with astroid,
+        # so we check for them before the inference
+        if self._is_comprehension(node):
+            return
+        infered = helpers.safe_infer(node)
+        if infered is None:
+            return
+        if infered is astroid.YES:
+            return
+        if self._is_iterable_value(infered):
+            return
+        if self._is_old_style_iterator(infered):
+            return
+        if self._is_string_value(infered):
+            return
+        self.add_message('not-an-iterable',
+                         args=node.as_string(),
+                         node=root_node)
+
+    def _check_mapping(self, node, root_node):
+        if isinstance(node, astroid.DictComp):
+            return
+        infered = helpers.safe_infer(node)
+        if infered is None:
+            return
+        if infered is astroid.YES:
+            return
+        if isinstance(infered, astroid.Dict):
+            return
+        self.add_message('not-a-mapping',
+                         args=node.as_string(),
+                         node=root_node)
+
+    @check_messages('not-an-iterable')
+    def visit_for(self, node):
+        self._check_iterable(node.iter, node)
+
+    @check_messages('not-an-iterable')
+    def visit_yieldfrom(self, node):
+        self._check_iterable(node.value, node)
+
+    @check_messages('not-an-iterable', 'not-a-mapping')
+    def visit_call(self, node):
+        for stararg in node.starargs:
+            self._check_iterable(stararg.value, node)
+        for kwarg in node.kwargs:
+            self._check_mapping(kwarg.value, node)
+
+    @check_messages('not-an-iterable')
+    def visit_listcomp(self, node):
+        for gen in node.generators:
+            self._check_iterable(gen.iter, node)
+
+    @check_messages('not-an-iterable')
+    def visit_dictcomp(self, node):
+        for gen in node.generators:
+            self._check_iterable(gen.iter, node)
+
+    @check_messages('not-an-iterable')
+    def visit_setcomp(self, node):
+        for gen in node.generators:
+            self._check_iterable(gen.iter, node)
+
+    @check_messages('not-an-iterable')
+    def visit_generatorexp(self, node):
+        for gen in node.generators:
+            self._check_iterable(gen.iter, node)
+
+
 def register(linter):
     """required method to auto register this checker """
     linter.register_checker(TypeChecker(linter))
+    linter.register_checker(IterableChecker(linter))
diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py
index 0b1cddd..2094314 100644
--- a/pylint/test/unittest_checker_base.py
+++ b/pylint/test/unittest_checker_base.py
@@ -1,7 +1,6 @@
 """Unittest for the base checker."""
 
 import re
-import sys
 import unittest
 
 import astroid
@@ -10,13 +9,6 @@ from pylint.checkers import base
 from pylint.testutils import CheckerTestCase, Message, set_config
 
 
-def python33_and_newer(test):
-    """
-    Decorator for any tests that will fail if launched not with Python 3.3+.
-    """
-    return unittest.skipIf(sys.version_info < (3, 3),
-                           'Python 3.2 and older')(test)
-
 class DocstringTest(CheckerTestCase):
     CHECKER_CLASS = base.DocStringChecker
 
@@ -300,141 +292,5 @@ class ComparisonTest(CheckerTestCase):
             self.checker.visit_compare(node)
 
 
-class IterableTest(CheckerTestCase):
-    CHECKER_CLASS = base.IterableChecker
-
-    def test_non_iterable_in_for(self):
-        node = test_utils.extract_node("""
-        for i in 42:
-            print(i)
-        """)
-        message = Message('not-an-iterable', node=node, args='42')
-        with self.assertAddsMessages(message):
-            self.checker.visit_for(node)
-
-        node = test_utils.extract_node("""
-        for i in [1,2,3]:
-            print(i)
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_for(node)
-
-        node = test_utils.extract_node("""
-        def count():
-            i = 0
-            while True:
-                yield i
-                i += 1
-
-        for i in count():
-            print(i)
-        """)
-        with self.assertNoMessages():
-            self.walk(node.root())
-
-        node = test_utils.extract_node("""
-        for i in "aeiou":
-            print(i)
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_for(node)
-
-    @python33_and_newer
-    def test_non_iterable_in_yield_from(self):
-        node = test_utils.extract_node("""
-        yield from 42
-        """)
-        message = Message('not-an-iterable', node=node, args='42')
-        with self.assertAddsMessages(message):
-            self.checker.visit_yieldfrom(node)
-
-        node = test_utils.extract_node("""
-        yield from [1,2,3]
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_yieldfrom(node)
-
-    def test_non_iterable_in_funcall_starargs(self):
-        node = test_utils.extract_node("""
-        foo(*123)
-        """)
-        message = Message('not-an-iterable', node=node, args='123')
-        with self.assertAddsMessages(message):
-            self.checker.visit_call(node)
-
-        node = test_utils.extract_node("""
-        stuff = [1,2,3]
-        foo(*stuff)
-        """)
-        with self.assertNoMessages():
-            self.walk(node.root())
-
-    def test_non_mapping_in_funcall_kwargs(self):
-        node = test_utils.extract_node("""
-        foo(**123)
-        """)
-        message = Message('not-a-mapping', node=node, args='123')
-        with self.assertAddsMessages(message):
-            self.checker.visit_call(node)
-
-        node = test_utils.extract_node("""
-        foo(**retdict())
-        """)
-        with self.assertNoMessages():
-            self.walk(node.root())
-
-    def test_non_iterable_in_listcomp(self):
-        node = test_utils.extract_node("""
-        [x ** 2 for x in 10]
-        """)
-        message = Message('not-an-iterable', node=node, args='10')
-        with self.assertAddsMessages(message):
-            self.checker.visit_listcomp(node)
-
-        node = test_utils.extract_node("""
-        [x ** 2 for x in range(10)]
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_listcomp(node)
-
-    def test_non_iterable_in_dictcomp(self):
-        node = test_utils.extract_node("""
-        {x: chr(x) for x in 256}
-        """)
-        message = Message('not-an-iterable', node=node, args='256')
-        with self.assertAddsMessages(message):
-            self.checker.visit_dictcomp(node)
-
-        node = test_utils.extract_node("""
-        {ord(x): x for x in "aoeui"}
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_dictcomp(node)
-
-    def test_non_iterable_in_setcomp(self):
-        node = test_utils.extract_node("""
-        {2 ** x for x in 10}
-        """)
-        message = Message('not-an-iterable', node=node, args='10')
-        with self.assertAddsMessages(message):
-            self.checker.visit_setcomp(node)
-
-        node = test_utils.extract_node("""
-        {2 ** x for x in range(10)}
-        """)
-        with self.assertNoMessages():
-            self.checker.visit_setcomp(node)
-
-    def test_non_iterable_in_generator(self):
-        node = test_utils.extract_node("__(x for x in 123)")
-        message = Message('not-an-iterable', node=node, args='123')
-        with self.assertAddsMessages(message):
-            self.walk(node.root())
-
-        node = test_utils.extract_node("__(chr(x) for x in range(25))")
-        with self.assertNoMessages():
-            self.walk(node.root())
-
-
 if __name__ == '__main__':
     unittest.main()
diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py
index b7135ea..a2f3598 100644
--- a/pylint/test/unittest_checker_typecheck.py
+++ b/pylint/test/unittest_checker_typecheck.py
@@ -1,10 +1,19 @@
 """Unittest for the type checker."""
 import unittest
+import sys
 
 from astroid import test_utils
 from pylint.checkers import typecheck
 from pylint.testutils import CheckerTestCase, Message, set_config
 
+
+def python33_and_newer(test):
+    """
+    Decorator for any tests that will fail if launched not with Python 3.3+.
+    """
+    return unittest.skipIf(sys.version_info < (3, 3),
+                           'Python 3.2 and older')(test)
+
 class TypeCheckerTest(CheckerTestCase):
     "Tests for pylint.checkers.typecheck"
     CHECKER_CLASS = typecheck.TypeChecker
@@ -88,6 +97,141 @@ class TypeCheckerTest(CheckerTestCase):
         with self.assertNoMessages():
             self.checker.visit_attribute(node)
 
+class IterableTest(CheckerTestCase):
+    CHECKER_CLASS = typecheck.IterableChecker
+
+    def test_non_iterable_in_for(self):
+        node = test_utils.extract_node("""
+        for i in 42:
+            print(i)
+        """)
+        message = Message('not-an-iterable', node=node, args='42')
+        with self.assertAddsMessages(message):
+            self.checker.visit_for(node)
+
+        node = test_utils.extract_node("""
+        for i in [1,2,3]:
+            print(i)
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+        node = test_utils.extract_node("""
+        def count():
+            i = 0
+            while True:
+                yield i
+                i += 1
+
+        for i in count():
+            print(i)
+        """)
+        with self.assertNoMessages():
+            self.walk(node.root())
+
+        node = test_utils.extract_node("""
+        for i in "aeiou":
+            print(i)
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_for(node)
+
+    @python33_and_newer
+    def test_non_iterable_in_yield_from(self):
+        node = test_utils.extract_node("""
+        yield from 42
+        """)
+        message = Message('not-an-iterable', node=node, args='42')
+        with self.assertAddsMessages(message):
+            self.checker.visit_yieldfrom(node)
+
+        node = test_utils.extract_node("""
+        yield from [1,2,3]
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_yieldfrom(node)
+
+    def test_non_iterable_in_funcall_starargs(self):
+        node = test_utils.extract_node("""
+        foo(*123)
+        """)
+        message = Message('not-an-iterable', node=node, args='123')
+        with self.assertAddsMessages(message):
+            self.checker.visit_call(node)
+
+        node = test_utils.extract_node("""
+        stuff = [1,2,3]
+        foo(*stuff)
+        """)
+        with self.assertNoMessages():
+            self.walk(node.root())
+
+    def test_non_mapping_in_funcall_kwargs(self):
+        node = test_utils.extract_node("""
+        foo(**123)
+        """)
+        message = Message('not-a-mapping', node=node, args='123')
+        with self.assertAddsMessages(message):
+            self.checker.visit_call(node)
+
+        node = test_utils.extract_node("""
+        foo(**retdict())
+        """)
+        with self.assertNoMessages():
+            self.walk(node.root())
+
+    def test_non_iterable_in_listcomp(self):
+        node = test_utils.extract_node("""
+        [x ** 2 for x in 10]
+        """)
+        message = Message('not-an-iterable', node=node, args='10')
+        with self.assertAddsMessages(message):
+            self.checker.visit_listcomp(node)
+
+        node = test_utils.extract_node("""
+        [x ** 2 for x in range(10)]
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_listcomp(node)
+
+    def test_non_iterable_in_dictcomp(self):
+        node = test_utils.extract_node("""
+        {x: chr(x) for x in 256}
+        """)
+        message = Message('not-an-iterable', node=node, args='256')
+        with self.assertAddsMessages(message):
+            self.checker.visit_dictcomp(node)
+
+        node = test_utils.extract_node("""
+        {ord(x): x for x in "aoeui"}
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_dictcomp(node)
+
+    def test_non_iterable_in_setcomp(self):
+        node = test_utils.extract_node("""
+        {2 ** x for x in 10}
+        """)
+        message = Message('not-an-iterable', node=node, args='10')
+        with self.assertAddsMessages(message):
+            self.checker.visit_setcomp(node)
+
+        node = test_utils.extract_node("""
+        {2 ** x for x in range(10)}
+        """)
+        with self.assertNoMessages():
+            self.checker.visit_setcomp(node)
+
+    def test_non_iterable_in_generator(self):
+        node = test_utils.extract_node("__(x for x in 123)")
+        message = Message('not-an-iterable', node=node, args='123')
+        with self.assertAddsMessages(message):
+            self.walk(node.root())
+
+        node = test_utils.extract_node("__(chr(x) for x in range(25))")
+        with self.assertNoMessages():
+            self.walk(node.root())
+
 
 if __name__ == '__main__':
     unittest.main()