Merge with defaultiterable-checker

23 files changed, 478 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index 1036a31..b25533b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,11 @@ ChangeLog for Pylint
 --------------------
 
 --
+    * Add new errors, 'not-an-iterable', emitted when non-iterable value
+      is used in an iterating context (starargs, for-statement,
+      comprehensions, etc), and 'not-a-mapping', emitted when non-mapping
+      value is used in a mapping context. Closes issue #563.
+
     * Make 'no-self-use' checker not emit a warning if there is a 'super()'
       call inside the method.
       Closes issue #667.
diff --git a/pylint/checkers/typecheck.py b/pylint/checkers/typecheck.py
index 66ac05b..80b51b5 100644
--- a/pylint/checkers/typecheck.py
+++ b/pylint/checkers/typecheck.py
@@ -44,6 +44,10 @@ _ZOPE_DEPRECATED = (
 )
 BUILTINS = six.moves.builtins.__name__
 STR_FORMAT = "%s.str.format" % BUILTINS
+ITER_METHOD = '__iter__'
+NEXT_METHOD = 'next' if six.PY2 else '__next__'
+GETITEM_METHOD = '__getitem__'
+KEYS_METHOD = 'keys'
 
 
 def _unflatten(iterable):
@@ -84,6 +88,44 @@ def _is_owner_ignored(owner, name, ignored_classes, ignored_modules):
     return any(name == ignore or qname == ignore for ignore in ignored_classes)
 
 
+def _hasattr(value, attr):
+    try:
+        value.getattr(attr)
+        return True
+    except astroid.NotFoundError:
+        return False
+
+def _is_comprehension(node):
+    comprehensions = (astroid.ListComp,
+                      astroid.SetComp,
+                      astroid.DictComp)
+    return isinstance(node, comprehensions)
+
+
+def _is_iterable(value):
+    # '__iter__' is for standard iterables
+    # '__getitem__' is for strings and other old-style iterables
+    return _hasattr(value, ITER_METHOD) or _hasattr(value, GETITEM_METHOD)
+
+
+def _is_iterator(value):
+    return _hasattr(value, NEXT_METHOD) and _hasattr(value, ITER_METHOD)
+
+
+def _is_mapping(value):
+    return _hasattr(value, GETITEM_METHOD) and _hasattr(value, KEYS_METHOD)
+
+
+def _is_inside_mixin_declaration(node):
+    while node is not None:
+        if isinstance(node, astroid.ClassDef):
+            name = getattr(node, 'name', None)
+            if name is not None and name.lower().endswith("mixin"):
+                return True
+        node = node.parent
+    return False
+
+
 MSGS = {
     'E1101': ('%s %r has no %r member',
               'no-member',
@@ -791,6 +833,124 @@ 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 _check_iterable(self, node, root_node):
+        # for/set/dict-comprehensions can't be infered with astroid
+        # so we have to check for them explicitly
+        if _is_comprehension(node) or _is_inside_mixin_declaration(node):
+            return
+
+        infered = helpers.safe_infer(node)
+        if infered is None or infered is astroid.YES:
+            return
+
+        if isinstance(infered, astroid.ClassDef):
+            if not helpers.has_known_bases(infered):
+                return
+            # classobj can only be iterable if it has an iterable metaclass
+            meta = infered.metaclass()
+            if meta is not None:
+                if _is_iterable(meta):
+                    return
+                if _is_iterator(meta):
+                    return
+
+        if isinstance(infered, astroid.Instance):
+            if not helpers.has_known_bases(infered):
+                return
+            if _is_iterable(infered) or _is_iterator(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) or _is_inside_mixin_declaration(node):
+            return
+
+        infered = helpers.safe_infer(node)
+        if infered is None or infered is astroid.YES:
+            return
+
+        if isinstance(infered, astroid.ClassDef):
+            if not helpers.has_known_bases(infered):
+                return
+            meta = infered.metaclass()
+            if meta is not None and _is_mapping(meta):
+                return
+
+        if isinstance(infered, astroid.Instance):
+            if not helpers.has_known_bases(infered):
+                return
+            if _is_mapping(infered):
+                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/functional/iterable_context.py b/pylint/test/functional/iterable_context.py
new file mode 100644
index 0000000..8dfcbbe
--- /dev/null
+++ b/pylint/test/functional/iterable_context.py
@@ -0,0 +1,141 @@
+"""
+Checks that primitive values are not used in an
+iterating/mapping context.
+"""
+# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-init,no-self-use,import-error,unused-argument,bad-mcs-method-argument
+from __future__ import print_function
+
+# primitives
+numbers = [1, 2, 3]
+
+for i in numbers:
+    pass
+
+for i in iter(numbers):
+    pass
+
+for i in "123":
+    pass
+
+for i in u"123":
+    pass
+
+for i in b"123":
+    pass
+
+for i in bytearray(b"123"):
+    pass
+
+for i in set(numbers):
+    pass
+
+for i in frozenset(numbers):
+    pass
+
+for i in dict(a=1, b=2):
+    pass
+
+# comprehensions
+for i in [x for x in range(10)]:
+    pass
+
+for i in {x for x in range(1, 100, 2)}:
+    pass
+
+for i in {x: 10 - x for x in range(10)}:
+    pass
+
+# generators
+def powers_of_two():
+    k = 0
+    while k < 10:
+        yield 2 ** k
+        k += 1
+
+for i in powers_of_two():
+    pass
+
+for i in powers_of_two:  # [not-an-iterable]
+    pass
+
+# check for custom iterators
+class A(object):
+    pass
+
+class B(object):
+    def __iter__(self):
+        return self
+
+    def __next__(self):
+        return 1
+
+    def next(self):
+        return 1
+
+class C(object):
+    "old-style iterator"
+    def __getitem__(self, k):
+        if k > 10:
+            raise IndexError
+        return k + 1
+
+    def __len__(self):
+        return 10
+
+for i in C():
+    print(i)
+
+
+def test(*args):
+    print(args)
+
+
+test(*A())  # [not-an-iterable]
+test(*B())
+test(*B)  # [not-an-iterable]
+for i in A():  # [not-an-iterable]
+    pass
+for i in B():
+    pass
+for i in B:  # [not-an-iterable]
+    pass
+
+for i in range:  # [not-an-iterable]
+    pass
+
+# check that primitive non-iterable types are catched
+for i in True:  # [not-an-iterable]
+    pass
+
+for i in None:  # [not-an-iterable]
+    pass
+
+for i in 8.5:  # [not-an-iterable]
+    pass
+
+for i in 10:  # [not-an-iterable]
+    pass
+
+
+# skip uninferable instances
+from some_missing_module import Iterable
+
+class MyClass(Iterable):
+    pass
+
+m = MyClass()
+for i in m:
+    print(i)
+
+# skip checks if statement is inside mixin class
+class ManagedAccessViewMixin(object):
+    access_requirements = None
+
+    def get_access_requirements(self):
+        return self.access_requirements
+
+    def dispatch(self, *_args, **_kwargs):
+        klasses = self.get_access_requirements()
+
+        for requirement in klasses:
+            print(requirement)
diff --git a/pylint/test/functional/iterable_context.txt b/pylint/test/functional/iterable_context.txt
new file mode 100644
index 0000000..fbe1433
--- /dev/null
+++ b/pylint/test/functional/iterable_context.txt
@@ -0,0 +1,10 @@
+not-an-iterable:58::Non-iterable value powers_of_two is used in an iterating context
+not-an-iterable:93::Non-iterable value A() is used in an iterating context
+not-an-iterable:95::Non-iterable value B is used in an iterating context
+not-an-iterable:96::Non-iterable value A() is used in an iterating context
+not-an-iterable:100::Non-iterable value B is used in an iterating context
+not-an-iterable:103::Non-iterable value range is used in an iterating context
+not-an-iterable:107::Non-iterable value True is used in an iterating context
+not-an-iterable:110::Non-iterable value None is used in an iterating context
+not-an-iterable:113::Non-iterable value 8.5 is used in an iterating context
+not-an-iterable:116::Non-iterable value 10 is used in an iterating context
diff --git a/pylint/test/functional/iterable_context_py2.py b/pylint/test/functional/iterable_context_py2.py
new file mode 100644
index 0000000..8687f84
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py2.py
@@ -0,0 +1,18 @@
+"""
+Checks that iterable metaclasses are recognized by pylint.
+"""
+# pylint: disable=missing-docstring,too-few-public-methods,no-init,no-self-use,unused-argument,bad-mcs-method-argument
+
+# metaclasses as iterables
+class Meta(type):
+    def __iter__(self):
+        return iter((1, 2, 3))
+
+class SomeClass(object):
+    __metaclass__ = Meta
+
+
+for i in SomeClass:
+    print i
+for i in SomeClass():  # [not-an-iterable]
+    print i
diff --git a/pylint/test/functional/iterable_context_py2.rc b/pylint/test/functional/iterable_context_py2.rc
new file mode 100644
index 0000000..61e01ea
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py2.rc
@@ -0,0 +1,3 @@
+[testoptions]
+max_pyver=2.7
+
diff --git a/pylint/test/functional/iterable_context_py2.txt b/pylint/test/functional/iterable_context_py2.txt
new file mode 100644
index 0000000..8de579a
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py2.txt
@@ -0,0 +1 @@
+not-an-iterable:17::Non-iterable value SomeClass() is used in an iterating context
diff --git a/pylint/test/functional/iterable_context_py3.py b/pylint/test/functional/iterable_context_py3.py
new file mode 100644
index 0000000..cb2a505
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py3.py
@@ -0,0 +1,18 @@
+"""
+Checks that iterable metaclasses are recognized by pylint.
+"""
+# pylint: disable=missing-docstring,too-few-public-methods,no-init,no-self-use,unused-argument,bad-mcs-method-argument
+
+# metaclasses as iterables
+class Meta(type):
+    def __iter__(self):
+        return iter((1, 2, 3))
+
+class SomeClass(metaclass=Meta):
+    pass
+
+
+for i in SomeClass:
+    print(i)
+for i in SomeClass():  # [not-an-iterable]
+    print(i)
diff --git a/pylint/test/functional/iterable_context_py3.rc b/pylint/test/functional/iterable_context_py3.rc
new file mode 100644
index 0000000..9bf6df0
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py3.rc
@@ -0,0 +1,3 @@
+[testoptions]
+min_pyver=3.0
+
diff --git a/pylint/test/functional/iterable_context_py3.txt b/pylint/test/functional/iterable_context_py3.txt
new file mode 100644
index 0000000..8de579a
--- /dev/null
+++ b/pylint/test/functional/iterable_context_py3.txt
@@ -0,0 +1 @@
+not-an-iterable:17::Non-iterable value SomeClass() is used in an iterating context
diff --git a/pylint/test/functional/mapping_context.py b/pylint/test/functional/mapping_context.py
new file mode 100644
index 0000000..cfab8dc
--- /dev/null
+++ b/pylint/test/functional/mapping_context.py
@@ -0,0 +1,59 @@
+"""
+Checks that only valid values are used in a mapping context.
+"""
+# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use,import-error
+from __future__ import print_function
+
+
+def test(**kwargs):
+    print(kwargs)
+
+
+# dictionary value/comprehension
+dict_value = dict(a=1, b=2, c=3)
+dict_comp = {chr(x): x for x in range(256)}
+test(**dict_value)
+test(**dict_comp)
+
+
+# in order to be used in kwargs custom mapping class should define
+# __iter__(), __getitem__(key) and keys().
+class CustomMapping(object):
+    def __init__(self):
+        self.data = dict(a=1, b=2, c=3, d=4, e=5)
+
+    def __getitem__(self, key):
+        return self.data[key]
+
+    def keys(self):
+        return self.data.keys()
+
+test(**CustomMapping())
+test(**CustomMapping)  # [not-a-mapping]
+
+class NotMapping(object):
+    pass
+
+test(**NotMapping())  # [not-a-mapping]
+
+# skip checks if statement is inside mixin class
+class SomeMixin(object):
+    kwargs = None
+
+    def get_kwargs(self):
+        return self.kwargs
+
+    def run(self, **kwargs):
+        print(kwargs)
+
+    def dispatch(self):
+        kws = self.get_kwargs()
+        self.run(**kws)
+
+# skip uninferable instances
+from some_missing_module import Mapping
+
+class MyClass(Mapping):
+    pass
+
+test(**MyClass())
diff --git a/pylint/test/functional/mapping_context.txt b/pylint/test/functional/mapping_context.txt
new file mode 100644
index 0000000..201da1a
--- /dev/null
+++ b/pylint/test/functional/mapping_context.txt
@@ -0,0 +1,2 @@
+not-a-mapping:32::Non-mapping value CustomMapping is used in a mapping context
+not-a-mapping:37::Non-mapping value NotMapping() is used in a mapping context
diff --git a/pylint/test/functional/mapping_context_py2.py b/pylint/test/functional/mapping_context_py2.py
new file mode 100644
index 0000000..afe4400
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py2.py
@@ -0,0 +1,19 @@
+# pylint: disable=missing-docstring,invalid-name,too-few-public-methods
+from __future__ import print_function
+
+
+def test(**kwargs):
+    print(kwargs)
+
+# metaclasses as mappings
+class Meta(type):
+    def __getitem__(self, key):
+        return ord(key)
+    def keys(self):
+        return ['a', 'b', 'c']
+
+class SomeClass(object):
+    __metaclass__ = Meta
+
+test(**SomeClass)
+test(**SomeClass())  # [not-a-mapping]
diff --git a/pylint/test/functional/mapping_context_py2.rc b/pylint/test/functional/mapping_context_py2.rc
new file mode 100644
index 0000000..61e01ea
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py2.rc
@@ -0,0 +1,3 @@
+[testoptions]
+max_pyver=2.7
+
diff --git a/pylint/test/functional/mapping_context_py2.txt b/pylint/test/functional/mapping_context_py2.txt
new file mode 100644
index 0000000..59cca6c
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py2.txt
@@ -0,0 +1 @@
+not-a-mapping:19::Non-mapping value SomeClass() is used in a mapping context
diff --git a/pylint/test/functional/mapping_context_py3.py b/pylint/test/functional/mapping_context_py3.py
new file mode 100644
index 0000000..042d4d0
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py3.py
@@ -0,0 +1,19 @@
+# pylint: disable=missing-docstring,invalid-name,too-few-public-methods,no-self-use
+from __future__ import print_function
+
+def test(**kwargs):
+    print(kwargs)
+
+# metaclasses as mappings
+class Meta(type):
+    def __getitem__(cls, key):
+        return ord(key)
+
+    def keys(cls):
+        return ['a', 'b', 'c']
+
+class SomeClass(metaclass=Meta):
+    pass
+
+test(**SomeClass)
+test(**SomeClass())  # [not-a-mapping]
diff --git a/pylint/test/functional/mapping_context_py3.rc b/pylint/test/functional/mapping_context_py3.rc
new file mode 100644
index 0000000..9bf6df0
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py3.rc
@@ -0,0 +1,3 @@
+[testoptions]
+min_pyver=3.0
+
diff --git a/pylint/test/functional/mapping_context_py3.txt b/pylint/test/functional/mapping_context_py3.txt
new file mode 100644
index 0000000..59cca6c
--- /dev/null
+++ b/pylint/test/functional/mapping_context_py3.txt
@@ -0,0 +1 @@
+not-a-mapping:19::Non-mapping value SomeClass() is used in a mapping context
diff --git a/pylint/test/functional/yield_from_iterable_py33.py b/pylint/test/functional/yield_from_iterable_py33.py
new file mode 100644
index 0000000..7803936
--- /dev/null
+++ b/pylint/test/functional/yield_from_iterable_py33.py
@@ -0,0 +1,7 @@
+"""
+Check that `yield from`-statement takes an iterable.
+"""
+# pylint: disable=missing-docstring
+
+def to_ten():
+    yield from 10  # [not-an-iterable]
diff --git a/pylint/test/functional/yield_from_iterable_py33.rc b/pylint/test/functional/yield_from_iterable_py33.rc
new file mode 100644
index 0000000..3330edd
--- /dev/null
+++ b/pylint/test/functional/yield_from_iterable_py33.rc
@@ -0,0 +1,2 @@
+[testoptions]
+min_pyver=3.3
\ No newline at end of file
diff --git a/pylint/test/functional/yield_from_iterable_py33.txt b/pylint/test/functional/yield_from_iterable_py33.txt
new file mode 100644
index 0000000..906ee93
--- /dev/null
+++ b/pylint/test/functional/yield_from_iterable_py33.txt
@@ -0,0 +1 @@
+not-an-iterable:7:to_ten:Non-iterable value 10 is used in an iterating context

diff --git a/pylint/test/unittest_checker_base.py b/pylint/test/unittest_checker_base.py
index c68f379..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
diff --git a/pylint/test/unittest_checker_typecheck.py b/pylint/test/unittest_checker_typecheck.py
index b7135ea..0aaa8a5 100644
--- a/pylint/test/unittest_checker_typecheck.py
+++ b/pylint/test/unittest_checker_typecheck.py
@@ -5,6 +5,7 @@ from astroid import test_utils
 from pylint.checkers import typecheck
 from pylint.testutils import CheckerTestCase, Message, set_config
 
+
 class TypeCheckerTest(CheckerTestCase):
     "Tests for pylint.checkers.typecheck"
     CHECKER_CLASS = typecheck.TypeChecker