diff --git a/Lib/lib2to3/fixer_util.py b/Lib/lib2to3/fixer_util.py index e6f7cdff801..1fb3742fa3e 100644 --- a/Lib/lib2to3/fixer_util.py +++ b/Lib/lib2to3/fixer_util.py @@ -1,6 +1,8 @@ """Utility functions, node construction macros, etc.""" # Author: Collin Winter +from itertools import islice + # Local imports from .pgen2 import token from .pytree import Leaf, Node @@ -14,7 +16,7 @@ from . import patcomp def KeywordArg(keyword, value): return Node(syms.argument, - [keyword, Leaf(token.EQUAL, '='), value]) + [keyword, Leaf(token.EQUAL, "="), value]) def LParen(): return Leaf(token.LPAR, "(") @@ -76,9 +78,9 @@ def Number(n, prefix=None): def Subscript(index_node): """A numeric or string subscript""" - return Node(syms.trailer, [Leaf(token.LBRACE, '['), + return Node(syms.trailer, [Leaf(token.LBRACE, "["), index_node, - Leaf(token.RBRACE, ']')]) + Leaf(token.RBRACE, "]")]) def String(string, prefix=None): """A string leaf""" @@ -120,9 +122,9 @@ def FromImport(package_name, name_leafs): # Pull the leaves out of their old tree leaf.remove() - children = [Leaf(token.NAME, 'from'), + children = [Leaf(token.NAME, "from"), Leaf(token.NAME, package_name, prefix=" "), - Leaf(token.NAME, 'import', prefix=" "), + Leaf(token.NAME, "import", prefix=" "), Node(syms.import_as_names, name_leafs)] imp = Node(syms.import_from, children) return imp @@ -245,6 +247,16 @@ def is_probably_builtin(node): return False return True +def find_indentation(node): + """Find the indentation of *node*.""" + while node is not None: + if node.type == syms.suite and len(node.children) > 2: + indent = node.children[1] + if indent.type == token.INDENT: + return indent.value + node = node.parent + return "" + ########################################################### ### The following functions are to find bindings in a suite ########################################################### @@ -314,11 +326,11 @@ def touch_import(package, name, node): if package is None: import_ = Node(syms.import_name, [ - Leaf(token.NAME, 'import'), - Leaf(token.NAME, name, prefix=' ') + Leaf(token.NAME, "import"), + Leaf(token.NAME, name, prefix=" ") ]) else: - import_ = FromImport(package, [Leaf(token.NAME, name, prefix=' ')]) + import_ = FromImport(package, [Leaf(token.NAME, name, prefix=" ")]) children = [import_, Newline()] root.insert_child(insert_pos, Node(syms.simple_stmt, children)) @@ -404,7 +416,7 @@ def _is_import_binding(node, name, package=None): if package and str(node.children[1]).strip() != package: return None n = node.children[3] - if package and _find('as', n): + if package and _find("as", n): # See test_from_import_as for explanation return None elif n.type == syms.import_as_names and _find(name, n): diff --git a/Lib/lib2to3/fixes/fix_itertools_imports.py b/Lib/lib2to3/fixes/fix_itertools_imports.py index f4b4ead5fea..a1702ba29c8 100644 --- a/Lib/lib2to3/fixes/fix_itertools_imports.py +++ b/Lib/lib2to3/fixes/fix_itertools_imports.py @@ -43,8 +43,8 @@ class FixItertoolsImports(fixer_base.BaseFix): else: remove_comma ^= True - if children[-1].type == token.COMMA: - children[-1].remove() + while children and children[-1].type == token.COMMA: + children.pop().remove() # If there are no imports left, just get rid of the entire statement if (not (imports.children or getattr(imports, 'value', None)) or diff --git a/Lib/lib2to3/fixes/fix_operator.py b/Lib/lib2to3/fixes/fix_operator.py index ded9eee099a..c393f1e7690 100644 --- a/Lib/lib2to3/fixes/fix_operator.py +++ b/Lib/lib2to3/fixes/fix_operator.py @@ -1,40 +1,89 @@ -"""Fixer for operator.{isCallable,sequenceIncludes} +"""Fixer for operator functions. -operator.isCallable(obj) -> hasattr(obj, '__call__') +operator.isCallable(obj) -> hasattr(obj, '__call__') operator.sequenceIncludes(obj) -> operator.contains(obj) +operator.isSequenceType(obj) -> isinstance(obj, collections.Sequence) +operator.isMappingType(obj) -> isinstance(obj, collections.Mapping) +operator.isNumberType(obj) -> isinstance(obj, numbers.Number) +operator.repeat(obj, n) -> operator.mul(obj, n) +operator.irepeat(obj, n) -> operator.imul(obj, n) """ +import collections + # Local imports -from .. import fixer_base -from ..fixer_util import Call, Name, String +from lib2to3 import fixer_base +from lib2to3.fixer_util import Call, Name, String, touch_import + class FixOperator(fixer_base.BaseFix): - methods = "method=('isCallable'|'sequenceIncludes')" - func = "'(' func=any ')'" + methods = """ + method=('isCallable'|'sequenceIncludes' + |'isSequenceType'|'isMappingType'|'isNumberType' + |'repeat'|'irepeat') + """ + obj = "'(' obj=any ')'" PATTERN = """ power< module='operator' - trailer< '.' %(methods)s > trailer< %(func)s > > + trailer< '.' %(methods)s > trailer< %(obj)s > > | - power< %(methods)s trailer< %(func)s > > - """ % dict(methods=methods, func=func) + power< %(methods)s trailer< %(obj)s > > + """ % dict(methods=methods, obj=obj) def transform(self, node, results): - method = results["method"][0] + method = self._check_method(node, results) + if method is not None: + return method(node, results) - if method.value == "sequenceIncludes": - if "module" not in results: - # operator may not be in scope, so we can't make a change. - self.warning(node, "You should use operator.contains here.") + def _sequenceIncludes(self, node, results): + """operator.contains(%s)""" + return self._handle_rename(node, results, "contains") + + def _isCallable(self, node, results): + """hasattr(%s, '__call__')""" + obj = results["obj"] + args = [obj.clone(), String(", "), String("'__call__'")] + return Call(Name("hasattr"), args, prefix=node.prefix) + + def _repeat(self, node, results): + """operator.mul(%s)""" + return self._handle_rename(node, results, "mul") + + def _irepeat(self, node, results): + """operator.imul(%s)""" + return self._handle_rename(node, results, "imul") + + def _isSequenceType(self, node, results): + """isinstance(%s, collections.Sequence)""" + return self._handle_type2abc(node, results, "collections", "Sequence") + + def _isMappingType(self, node, results): + """isinstance(%s, collections.Mapping)""" + return self._handle_type2abc(node, results, "collections", "Mapping") + + def _isNumberType(self, node, results): + """isinstance(%s, numbers.Number)""" + return self._handle_type2abc(node, results, "numbers", "Number") + + def _handle_rename(self, node, results, name): + method = results["method"][0] + method.value = name + method.changed() + + def _handle_type2abc(self, node, results, module, abc): + touch_import(None, module, node) + obj = results["obj"] + args = [obj.clone(), String(", " + ".".join([module, abc]))] + return Call(Name("isinstance"), args, prefix=node.prefix) + + def _check_method(self, node, results): + method = getattr(self, "_" + results["method"][0].value) + if isinstance(method, collections.Callable): + if "module" in results: + return method else: - method.value = "contains" - method.changed() - elif method.value == "isCallable": - if "module" not in results: - self.warning(node, - "You should use hasattr(%s, '__call__') here." % - results["func"].value) - else: - func = results["func"] - args = [func.clone(), String(", "), String("'__call__'")] - return Call(Name("hasattr"), args, prefix=node.prefix) + sub = (str(results["obj"]),) + invocation_str = str(method.__doc__) % sub + self.warning(node, "You should use '%s' here." % invocation_str) + return None diff --git a/Lib/lib2to3/fixes/fix_urllib.py b/Lib/lib2to3/fixes/fix_urllib.py index db18ca84eef..87ccec0b413 100644 --- a/Lib/lib2to3/fixes/fix_urllib.py +++ b/Lib/lib2to3/fixes/fix_urllib.py @@ -5,39 +5,40 @@ # Author: Nick Edds # Local imports -from .fix_imports import alternates, FixImports -from .. import fixer_base -from ..fixer_util import Name, Comma, FromImport, Newline, attr_chain +from lib2to3.fixes.fix_imports import alternates, FixImports +from lib2to3 import fixer_base +from lib2to3.fixer_util import (Name, Comma, FromImport, Newline, + find_indentation) -MAPPING = {'urllib': [ - ('urllib.request', - ['URLOpener', 'FancyURLOpener', 'urlretrieve', - '_urlopener', 'urlopen', 'urlcleanup', - 'pathname2url', 'url2pathname']), - ('urllib.parse', - ['quote', 'quote_plus', 'unquote', 'unquote_plus', - 'urlencode', 'splitattr', 'splithost', 'splitnport', - 'splitpasswd', 'splitport', 'splitquery', 'splittag', - 'splittype', 'splituser', 'splitvalue', ]), - ('urllib.error', - ['ContentTooShortError'])], - 'urllib2' : [ - ('urllib.request', - ['urlopen', 'install_opener', 'build_opener', - 'Request', 'OpenerDirector', 'BaseHandler', - 'HTTPDefaultErrorHandler', 'HTTPRedirectHandler', - 'HTTPCookieProcessor', 'ProxyHandler', - 'HTTPPasswordMgr', - 'HTTPPasswordMgrWithDefaultRealm', - 'AbstractBasicAuthHandler', - 'HTTPBasicAuthHandler', 'ProxyBasicAuthHandler', - 'AbstractDigestAuthHandler', - 'HTTPDigestAuthHandler', 'ProxyDigestAuthHandler', - 'HTTPHandler', 'HTTPSHandler', 'FileHandler', - 'FTPHandler', 'CacheFTPHandler', - 'UnknownHandler']), - ('urllib.error', - ['URLError', 'HTTPError']), +MAPPING = {"urllib": [ + ("urllib.request", + ["URLOpener", "FancyURLOpener", "urlretrieve", + "_urlopener", "urlopen", "urlcleanup", + "pathname2url", "url2pathname"]), + ("urllib.parse", + ["quote", "quote_plus", "unquote", "unquote_plus", + "urlencode", "splitattr", "splithost", "splitnport", + "splitpasswd", "splitport", "splitquery", "splittag", + "splittype", "splituser", "splitvalue", ]), + ("urllib.error", + ["ContentTooShortError"])], + "urllib2" : [ + ("urllib.request", + ["urlopen", "install_opener", "build_opener", + "Request", "OpenerDirector", "BaseHandler", + "HTTPDefaultErrorHandler", "HTTPRedirectHandler", + "HTTPCookieProcessor", "ProxyHandler", + "HTTPPasswordMgr", + "HTTPPasswordMgrWithDefaultRealm", + "AbstractBasicAuthHandler", + "HTTPBasicAuthHandler", "ProxyBasicAuthHandler", + "AbstractDigestAuthHandler", + "HTTPDigestAuthHandler", "ProxyDigestAuthHandler", + "HTTPHandler", "HTTPSHandler", "FileHandler", + "FTPHandler", "CacheFTPHandler", + "UnknownHandler"]), + ("urllib.error", + ["URLError", "HTTPError"]), ] } @@ -78,7 +79,7 @@ class FixUrllib(FixImports): import name with a comma separated list of its replacements. """ - import_mod = results.get('module') + import_mod = results.get("module") pref = import_mod.prefix names = [] @@ -94,9 +95,9 @@ class FixUrllib(FixImports): the module to be imported from with the appropriate new module. """ - mod_member = results.get('mod_member') + mod_member = results.get("mod_member") pref = mod_member.prefix - member = results.get('member') + member = results.get("member") # Simple case with only a single member being imported if member: @@ -111,19 +112,18 @@ class FixUrllib(FixImports): if new_name: mod_member.replace(Name(new_name, prefix=pref)) else: - self.cannot_convert(node, - 'This is an invalid module element') + self.cannot_convert(node, "This is an invalid module element") # Multiple members being imported else: # a dictionary for replacements, order matters modules = [] mod_dict = {} - members = results.get('members') + members = results["members"] for member in members: member = member.value # we only care about the actual members - if member != ',': + if member != ",": for change in MAPPING[mod_member.value]: if member in change[1]: if change[0] in mod_dict: @@ -133,13 +133,19 @@ class FixUrllib(FixImports): modules.append(change[0]) new_nodes = [] + indentation = find_indentation(node) + first = True for module in modules: elts = mod_dict[module] names = [] for elt in elts[:-1]: names.extend([Name(elt, prefix=pref), Comma()]) names.append(Name(elts[-1], prefix=pref)) - new_nodes.append(FromImport(module, names)) + new = FromImport(module, names) + if not first or node.parent.prefix.endswith(indentation): + new.prefix = indentation + new_nodes.append(new) + first = False if new_nodes: nodes = [] for new_node in new_nodes[:-1]: @@ -147,12 +153,12 @@ class FixUrllib(FixImports): nodes.append(new_nodes[-1]) node.replace(nodes) else: - self.cannot_convert(node, 'All module elements are invalid') + self.cannot_convert(node, "All module elements are invalid") def transform_dot(self, node, results): """Transform for calls to module members in code.""" - module_dot = results.get('bare_with_attr') - member = results.get('member') + module_dot = results.get("bare_with_attr") + member = results.get("member") new_name = None if isinstance(member, list): member = member[0] @@ -164,17 +170,17 @@ class FixUrllib(FixImports): module_dot.replace(Name(new_name, prefix=module_dot.prefix)) else: - self.cannot_convert(node, 'This is an invalid module element') + self.cannot_convert(node, "This is an invalid module element") def transform(self, node, results): - if results.get('module'): + if results.get("module"): self.transform_import(node, results) - elif results.get('mod_member'): + elif results.get("mod_member"): self.transform_member(node, results) - elif results.get('bare_with_attr'): + elif results.get("bare_with_attr"): self.transform_dot(node, results) # Renaming and star imports are not supported for these modules. - elif results.get('module_star'): - self.cannot_convert(node, 'Cannot handle star imports.') - elif results.get('module_as'): - self.cannot_convert(node, 'This module is now multiple modules') + elif results.get("module_star"): + self.cannot_convert(node, "Cannot handle star imports.") + elif results.get("module_as"): + self.cannot_convert(node, "This module is now multiple modules") diff --git a/Lib/lib2to3/pytree.py b/Lib/lib2to3/pytree.py index c502771faaa..38fda0c5592 100644 --- a/Lib/lib2to3/pytree.py +++ b/Lib/lib2to3/pytree.py @@ -286,7 +286,7 @@ class Node(Base): """Return a pre-order iterator for the tree.""" yield self for child in self.children: - for node in child.post_order(): + for node in child.pre_order(): yield node def _prefix_getter(self): diff --git a/Lib/lib2to3/refactor.py b/Lib/lib2to3/refactor.py index 3f4246073a0..4d83d201b87 100644 --- a/Lib/lib2to3/refactor.py +++ b/Lib/lib2to3/refactor.py @@ -517,7 +517,7 @@ class RefactoringTool(object): try: tree = self.parse_block(block, lineno, indent) except Exception as err: - if self.log.isEnabledFor(logging.DEBUG): + if self.logger.isEnabledFor(logging.DEBUG): for line in block: self.log_debug("Source: %s", line.rstrip("\n")) self.log_error("Can't parse docstring in %s line %s: %s: %s", diff --git a/Lib/lib2to3/tests/test_fixers.py b/Lib/lib2to3/tests/test_fixers.py index 91853a4a127..eca099d9267 100644 --- a/Lib/lib2to3/tests/test_fixers.py +++ b/Lib/lib2to3/tests/test_fixers.py @@ -1818,6 +1818,33 @@ class Test_urllib(FixerTestCase): s = "from %s import *" % old self.warns_unchanged(s, "Cannot handle star imports") + def test_indented(self): + b = """ +def foo(): + from urllib import urlencode, urlopen +""" + a = """ +def foo(): + from urllib.parse import urlencode + from urllib.request import urlopen +""" + self.check(b, a) + + b = """ +def foo(): + other() + from urllib import urlencode, urlopen +""" + a = """ +def foo(): + other() + from urllib.parse import urlencode + from urllib.request import urlopen +""" + self.check(b, a) + + + def test_import_module_usage(self): for old, changes in self.modules.items(): for new, members in changes: @@ -3623,6 +3650,10 @@ class Test_itertools_imports(FixerTestCase): a = "from itertools import bar, foo" self.check(b, a) + b = "from itertools import chain, imap, izip" + a = "from itertools import chain" + self.check(b, a) + def test_comments(self): b = "#foo\nfrom itertools import imap, izip" a = "#foo\n" @@ -4303,13 +4334,89 @@ class Test_operator(FixerTestCase): a = "operator.contains(x, y)" self.check(b, a) + b = "operator .sequenceIncludes(x, y)" + a = "operator .contains(x, y)" + self.check(b, a) + + b = "operator. sequenceIncludes(x, y)" + a = "operator. contains(x, y)" + self.check(b, a) + + def test_operator_isSequenceType(self): + b = "operator.isSequenceType(x)" + a = "import collections\nisinstance(x, collections.Sequence)" + self.check(b, a) + + def test_operator_isMappingType(self): + b = "operator.isMappingType(x)" + a = "import collections\nisinstance(x, collections.Mapping)" + self.check(b, a) + + def test_operator_isNumberType(self): + b = "operator.isNumberType(x)" + a = "import numbers\nisinstance(x, numbers.Number)" + self.check(b, a) + + def test_operator_repeat(self): + b = "operator.repeat(x, n)" + a = "operator.mul(x, n)" + self.check(b, a) + + b = "operator .repeat(x, n)" + a = "operator .mul(x, n)" + self.check(b, a) + + b = "operator. repeat(x, n)" + a = "operator. mul(x, n)" + self.check(b, a) + + def test_operator_irepeat(self): + b = "operator.irepeat(x, n)" + a = "operator.imul(x, n)" + self.check(b, a) + + b = "operator .irepeat(x, n)" + a = "operator .imul(x, n)" + self.check(b, a) + + b = "operator. irepeat(x, n)" + a = "operator. imul(x, n)" + self.check(b, a) + def test_bare_isCallable(self): s = "isCallable(x)" - self.warns_unchanged(s, "You should use hasattr(x, '__call__') here.") + t = "You should use 'hasattr(x, '__call__')' here." + self.warns_unchanged(s, t) def test_bare_sequenceIncludes(self): s = "sequenceIncludes(x, y)" - self.warns_unchanged(s, "You should use operator.contains here.") + t = "You should use 'operator.contains(x, y)' here." + self.warns_unchanged(s, t) + + def test_bare_operator_isSequenceType(self): + s = "isSequenceType(z)" + t = "You should use 'isinstance(z, collections.Sequence)' here." + self.warns_unchanged(s, t) + + def test_bare_operator_isMappingType(self): + s = "isMappingType(x)" + t = "You should use 'isinstance(x, collections.Mapping)' here." + self.warns_unchanged(s, t) + + def test_bare_operator_isNumberType(self): + s = "isNumberType(y)" + t = "You should use 'isinstance(y, numbers.Number)' here." + self.warns_unchanged(s, t) + + def test_bare_operator_repeat(self): + s = "repeat(x, n)" + t = "You should use 'operator.mul(x, n)' here." + self.warns_unchanged(s, t) + + def test_bare_operator_irepeat(self): + s = "irepeat(y, 187)" + t = "You should use 'operator.imul(y, 187)' here." + self.warns_unchanged(s, t) class Test_exitfunc(FixerTestCase): diff --git a/Lib/lib2to3/tests/test_pytree.py b/Lib/lib2to3/tests/test_pytree.py index d31f67debdc..53695f5083e 100644 --- a/Lib/lib2to3/tests/test_pytree.py +++ b/Lib/lib2to3/tests/test_pytree.py @@ -181,14 +181,18 @@ class TestNodes(support.TestCase): def test_post_order(self): l1 = pytree.Leaf(100, "foo") l2 = pytree.Leaf(100, "bar") - n1 = pytree.Node(1000, [l1, l2]) - self.assertEqual(list(n1.post_order()), [l1, l2, n1]) + l3 = pytree.Leaf(100, "fooey") + c1 = pytree.Node(1000, [l1, l2]) + n1 = pytree.Node(1000, [c1, l3]) + self.assertEqual(list(n1.post_order()), [l1, l2, c1, l3, n1]) def test_pre_order(self): l1 = pytree.Leaf(100, "foo") l2 = pytree.Leaf(100, "bar") - n1 = pytree.Node(1000, [l1, l2]) - self.assertEqual(list(n1.pre_order()), [n1, l1, l2]) + l3 = pytree.Leaf(100, "fooey") + c1 = pytree.Node(1000, [l1, l2]) + n1 = pytree.Node(1000, [c1, l3]) + self.assertEqual(list(n1.pre_order()), [n1, c1, l1, l2, l3]) def test_changed(self): l1 = pytree.Leaf(100, "f") diff --git a/Lib/lib2to3/tests/test_util.py b/Lib/lib2to3/tests/test_util.py index 6186b4ff74a..0ab753767e5 100644 --- a/Lib/lib2to3/tests/test_util.py +++ b/Lib/lib2to3/tests/test_util.py @@ -575,3 +575,20 @@ class Test_touch_import(support.TestCase): node = parse('bar()') fixer_util.touch_import(None, "cgi", node) self.assertEqual(str(node), 'import cgi\nbar()\n\n') + +class Test_find_indentation(support.TestCase): + + def test_nothing(self): + fi = fixer_util.find_indentation + node = parse("node()") + self.assertEqual(fi(node), "") + node = parse("") + self.assertEqual(fi(node), "") + + def test_simple(self): + fi = fixer_util.find_indentation + node = parse("def f():\n x()") + self.assertEqual(fi(node), "") + self.assertEqual(fi(node.children[0].children[4].children[2]), " ") + node = parse("def f():\n x()\n y()") + self.assertEqual(fi(node.children[0].children[4].children[4]), " ")