merge with default

2 files changed, 199 insertions, 38 deletions

diff --git a/pylint/checkers/python3.py b/pylint/checkers/python3.py
index 880d5c5..33689a9 100644
--- a/pylint/checkers/python3.py
+++ b/pylint/checkers/python3.py
@@ -12,7 +12,7 @@
 # this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 """Check Python 2 code for Python 2/3 source-compatible issues."""
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function
 
 import re
 import tokenize
@@ -46,6 +46,40 @@ def _check_dict_node(node):
     return (not inferred_types
             or any(isinstance(x, astroid.Dict) for x in inferred_types))
 
+def _is_builtin(node):
+    return getattr(node, 'name', None) in ('__builtin__', 'builtins')
+
+_accepts_iterator = {'iter', 'list', 'tuple', 'sorted', 'set', 'sum', 'any',
+                     'all', 'enumerate'}
+
+def _in_iterating_context(node):
+    """Check if the node is being used as an iterator.
+
+    Definition is taken from lib2to3.fixer_util.in_special_context().
+    """
+    parent = node.parent
+    # Since a call can't be the loop variant we only need to know if the node's
+    # parent is a 'for' loop to know it's being used as the iterator for the
+    # loop.
+    if isinstance(parent, astroid.For):
+        return True
+    # Need to make sure the use of the node is in the iterator part of the
+    # comprehension.
+    elif isinstance(parent, astroid.Comprehension):
+        if parent.iter == node:
+            return True
+    # Various built-ins can take in an iterable or list and lead to the same
+    # value.
+    elif isinstance(parent, astroid.CallFunc):
+        if isinstance(parent.func, astroid.Name):
+            parent_scope = parent.func.lookup(parent.func.name)[0]
+            if _is_builtin(parent_scope) and parent.func.name in _accepts_iterator:
+                return True
+        elif isinstance(parent.func, astroid.Getattr):
+            if parent.func.attrname == 'join':
+                return True
+    return False
+
 
 class Python3Checker(checkers.BaseChecker):
 
@@ -244,14 +278,7 @@ class Python3Checker(checkers.BaseChecker):
                   'Used when a __cmp__ method is defined '
                   '(method is not used by Python 3)',
                   {'maxversion': (3, 0)}),
-        'W1631': ('map is used as implicitly evaluated call',
-                  'implicit-map-evaluation',
-                  'Used when the map builtin is used as implicitly '
-                  'evaluated call, as in "map(func, args)" on a single line. '
-                  'This behaviour will not work in Python 3, where '
-                  'map is a generator and must be evaluated. '
-                  'Prefer a for-loop as alternative.',
-                  {'maxversion': (3, 0)}),
+        # 'W1631': 'implicit-map-evaluation' superceded by map-builtin-not-iterating.
         'W1632': ('input built-in referenced',
                   'input-builtin',
                   'Used when the input built-in is referenced '
@@ -272,6 +299,26 @@ class Python3Checker(checkers.BaseChecker):
                   'Used when the unichr built-in is referenced '
                   '(Use chr in Python 3)',
                   {'maxversion': (3, 0)}),
+        'W1636': ('map built-in referenced when not iterating',
+                  'map-builtin-not-iterating',
+                  'Used when the map built-in is referenced in a non-iterating '
+                  'context (returns an iterator in Python 3)',
+                  {'maxversion': (3, 0)}),
+        'W1637': ('zip built-in referenced when not iterating',
+                  'zip-builtin-not-iterating',
+                  'Used when the zip built-in is referenced in a non-iterating '
+                  'context (returns an iterator in Python 3)',
+                  {'maxversion': (3, 0)}),
+        'W1638': ('range built-in referenced when not iterating',
+                  'range-builtin-not-iterating',
+                  'Used when the range built-in is referenced in a non-iterating '
+                  'context (returns an iterator in Python 3)',
+                  {'maxversion': (3, 0)}),
+        'W1639': ('filter built-in referenced when not iterating',
+                  'filter-builtin-not-iterating',
+                  'Used when the filter built-in is referenced in a non-iterating '
+                  'context (returns an iterator in Python 3)',
+                  {'maxversion': (3, 0)}),
     }
 
     _bad_builtins = frozenset([
@@ -324,19 +371,19 @@ class Python3Checker(checkers.BaseChecker):
             if isinstance(arg, astroid.Tuple):
                 self.add_message('parameter-unpacking', node=arg)
 
-    @utils.check_messages('implicit-map-evaluation')
+    @utils.check_messages('map-builtin-not-iterating')
     def visit_discard(self, node):
         if (isinstance(node.value, astroid.CallFunc) and
                 isinstance(node.value.func, astroid.Name) and
                 node.value.func.name == 'map'):
             module = node.value.func.lookup('map')[0]
-            if getattr(module, 'name', None) == '__builtin__':
-                self.add_message('implicit-map-evaluation', node=node)
+            if _is_builtin(module):
+                self.add_message('map-builtin-not-iterating', node=node)
 
     def visit_name(self, node):
         """Detect when a "bad" built-in is referenced."""
         found_node = node.lookup(node.name)[0]
-        if getattr(found_node, 'name', None) == '__builtin__':
+        if _is_builtin(found_node):
             if node.name in self._bad_builtins:
                 message = node.name.lower() + '-builtin'
                 self.add_message(message, node=node)
@@ -375,22 +422,26 @@ class Python3Checker(checkers.BaseChecker):
             else:
                 self.add_message('old-division', node=node)
 
-    @utils.check_messages('next-method-called',
-                          'dict-iter-method',
-                          'dict-view-method')
     def visit_callfunc(self, node):
-        if not isinstance(node.func, astroid.Getattr):
-            return
-        if any([node.args, node.starargs, node.kwargs]):
-            return
-        if node.func.attrname == 'next':
-            self.add_message('next-method-called', node=node)
-        else:
-            if _check_dict_node(node.func.expr):
-                if node.func.attrname in ('iterkeys', 'itervalues', 'iteritems'):
-                    self.add_message('dict-iter-method', node=node)
-                elif node.func.attrname in ('viewkeys', 'viewvalues', 'viewitems'):
-                    self.add_message('dict-view-method', node=node)
+        if isinstance(node.func, astroid.Getattr):
+            if any([node.args, node.starargs, node.kwargs]):
+                return
+            if node.func.attrname == 'next':
+                self.add_message('next-method-called', node=node)
+            else:
+                if _check_dict_node(node.func.expr):
+                    if node.func.attrname in ('iterkeys', 'itervalues', 'iteritems'):
+                        self.add_message('dict-iter-method', node=node)
+                    elif node.func.attrname in ('viewkeys', 'viewvalues', 'viewitems'):
+                        self.add_message('dict-view-method', node=node)
+        elif isinstance(node.func, astroid.Name):
+            found_node = node.func.lookup(node.func.name)[0]
+            if _is_builtin(found_node):
+                if node.func.name in ('filter', 'map', 'range', 'zip'):
+                    if not _in_iterating_context(node):
+                        checker = '{}-builtin-not-iterating'.format(node.func.name)
+                        self.add_message(checker, node=node)
+
 
     @utils.check_messages('indexing-exception')
     def visit_subscript(self, node):
diff --git a/pylint/test/unittest_checker_python3.py b/pylint/test/unittest_checker_python3.py
index a7d31ba..357065a 100644
--- a/pylint/test/unittest_checker_python3.py
+++ b/pylint/test/unittest_checker_python3.py
@@ -18,6 +18,7 @@ import sys
 import unittest
 import textwrap
 
+import astroid
 from astroid import test_utils
 
 from pylint import testutils
@@ -63,7 +64,116 @@ class Python3CheckerTest(testutils.CheckerTestCase):
         for builtin in builtins:
             self.check_bad_builtin(builtin)
 
-    def _test_defined_method(self, method, warning):
+    def as_iterable_in_for_loop_test(self, fxn):
+        code = "for x in {}(): pass".format(fxn)
+        module = test_utils.build_module(code)
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def as_used_by_iterable_in_for_loop_test(self, fxn):
+        checker = '{}-builtin-not-iterating'.format(fxn)
+        node = test_utils.extract_node("""
+        for x in (whatever(
+            {}() #@
+        )):
+            pass
+        """.format(fxn))
+        message = testutils.Message(checker, node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_callfunc(node)
+
+    def as_iterable_in_genexp_test(self, fxn):
+        code = "x = (x for x in {}())".format(fxn)
+        module = test_utils.build_module(code)
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def as_iterable_in_listcomp_test(self, fxn):
+        code = "x = [x for x in {}(None, [1])]".format(fxn)
+        module = test_utils.build_module(code)
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def as_used_in_variant_in_genexp_test(self, fxn):
+        checker = '{}-builtin-not-iterating'.format(fxn)
+        node = test_utils.extract_node("""
+        list(
+            {}(x) #@
+            for x in [1]
+        )
+        """.format(fxn))
+        # For some reason extract_node won't grab the map() call.
+        assert isinstance(node, astroid.GenExpr)
+        node = node.elt
+        message = testutils.Message(checker, node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_callfunc(node)
+
+    def as_used_in_variant_in_listcomp_test(self, fxn):
+        checker = '{}-builtin-not-iterating'.format(fxn)
+        node = test_utils.extract_node("""
+        [
+            {}(None, x) #@
+        for x in [[1]]]
+        """.format(fxn))
+        # For some reason extract_node won't grab the map() call.
+        node = node.elt
+        message = testutils.Message(checker, node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_callfunc(node)
+
+    def as_argument_to_list_constructor_test(self, fxn):
+        module = test_utils.build_module("x = list({}())".format(fxn))
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def as_argument_to_random_fxn_test(self, fxn):
+        checker = '{}-builtin-not-iterating'.format(fxn)
+        node = test_utils.extract_node("""
+        y(
+            {}() #@
+        )
+        """.format(fxn))
+        message = testutils.Message(checker, node=node)
+        with self.assertAddsMessages(message):
+            self.checker.visit_callfunc(node)
+
+    def as_argument_to_str_join_test(self, fxn):
+        code = "x = ''.join({}())".format(fxn)
+        module = test_utils.build_module(code)
+        with self.assertNoMessages():
+            self.walk(module)
+
+    def iterating_context_tests(self, fxn):
+        """Helper for verifying a function isn't used as an iterator."""
+        self.as_iterable_in_for_loop_test(fxn)
+        self.as_used_by_iterable_in_for_loop_test(fxn)
+        self.as_iterable_in_genexp_test(fxn)
+        self.as_iterable_in_listcomp_test(fxn)
+        self.as_used_in_variant_in_genexp_test(fxn)
+        self.as_used_in_variant_in_listcomp_test(fxn)
+        self.as_argument_to_list_constructor_test(fxn)
+        self.as_argument_to_random_fxn_test(fxn)
+        self.as_argument_to_str_join_test(fxn)
+
+    @python2_only
+    def test_map_in_iterating_context(self):
+        self.iterating_context_tests('map')
+
+    @python2_only
+    def test_zip_in_iterating_context(self):
+        self.iterating_context_tests('zip')
+
+    @python2_only
+    def test_range_in_iterating_context(self):
+        self.iterating_context_tests('range')
+
+    @python2_only
+    def test_filter_in_iterating_context(self):
+        self.iterating_context_tests('filter')
+
+    def defined_method_test(self, method, warning):
+        """Helper for verifying that a certain method is not defined."""
         node = test_utils.extract_node("""
             class Foo(object):
                 def __{0}__(self, other):  #@
@@ -73,28 +183,28 @@ class Python3CheckerTest(testutils.CheckerTestCase):
             self.checker.visit_function(node)
 
     def test_delslice_method(self):
-        self._test_defined_method('delslice', 'delslice-method')
+        self.defined_method_test('delslice', 'delslice-method')
 
     def test_getslice_method(self):
-        self._test_defined_method('getslice', 'getslice-method')
+        self.defined_method_test('getslice', 'getslice-method')
 
     def test_setslice_method(self):
-        self._test_defined_method('setslice', 'setslice-method')
+        self.defined_method_test('setslice', 'setslice-method')
 
     def test_coerce_method(self):
-        self._test_defined_method('coerce', 'coerce-method')
+        self.defined_method_test('coerce', 'coerce-method')
 
     def test_oct_method(self):
-        self._test_defined_method('oct', 'oct-method')
+        self.defined_method_test('oct', 'oct-method')
 
     def test_hex_method(self):
-        self._test_defined_method('hex', 'hex-method')
+        self.defined_method_test('hex', 'hex-method')
 
     def test_nonzero_method(self):
-        self._test_defined_method('nonzero', 'nonzero-method')
+        self.defined_method_test('nonzero', 'nonzero-method')
 
     def test_cmp_method(self):
-        self._test_defined_method('cmp', 'cmp-method')
+        self.defined_method_test('cmp', 'cmp-method')
 
     @python2_only
     def test_print_statement(self):
@@ -208,7 +318,7 @@ class Python3CheckerTest(testutils.CheckerTestCase):
     def test_implicit_map_evaluation(self):
         node = test_utils.extract_node('map(str, [1, 2, 3])')
         discard = node.parent
-        message = testutils.Message('implicit-map-evaluation', node=discard)
+        message = testutils.Message('map-builtin-not-iterating', node=discard)
         with self.assertAddsMessages(message):
             # Use node.parent because extract_node returns the value
             # of a discard node, not the discard itself.