diff --git a/Lib/lib2to3/fixer_util.py b/Lib/lib2to3/fixer_util.py index d19635fda6c..fefa61239d2 100644 --- a/Lib/lib2to3/fixer_util.py +++ b/Lib/lib2to3/fixer_util.py @@ -222,6 +222,29 @@ def in_special_context(node): return True return False +def is_probably_builtin(node): + """ + Check that something isn't an attribute or function name etc. + """ + prev = node.get_prev_sibling() + if prev is not None and prev.type == token.DOT: + # Attribute lookup. + return False + parent = node.parent + if parent.type in (syms.funcdef, syms.classdef): + return False + if parent.type == syms.expr_stmt and parent.children[0] is node: + # Assignment. + return False + if parent.type == syms.parameters or \ + (parent.type == syms.typedargslist and ( + (prev is not None and prev.type == token.COMMA) or + parent.children[0] is node + )): + # The name of an argument. + return False + return True + ########################################################### ### The following functions are to find bindings in a suite ########################################################### diff --git a/Lib/lib2to3/fixes/fix_execfile.py b/Lib/lib2to3/fixes/fix_execfile.py index 5854900d2fa..f7a7a700147 100644 --- a/Lib/lib2to3/fixes/fix_execfile.py +++ b/Lib/lib2to3/fixes/fix_execfile.py @@ -7,9 +7,9 @@ This converts usages of the execfile function into calls to the built-in exec() function. """ -from .. import pytree from .. import fixer_base -from ..fixer_util import Comma, Name, Call, LParen, RParen, Dot +from ..fixer_util import (Comma, Name, Call, LParen, RParen, Dot, Node, + ArgList, String, syms) class FixExecfile(fixer_base.BaseFix): @@ -22,16 +22,30 @@ class FixExecfile(fixer_base.BaseFix): def transform(self, node, results): assert results - syms = self.syms filename = results["filename"] globals = results.get("globals") locals = results.get("locals") - args = [Name('open'), LParen(), filename.clone(), RParen(), Dot(), - Name('read'), LParen(), RParen()] - args[0].set_prefix("") + + # Copy over the prefix from the right parentheses end of the execfile + # call. + execfile_paren = node.children[-1].children[-1].clone() + # Construct open().read(). + open_args = ArgList([filename.clone()], rparen=execfile_paren) + open_call = Node(syms.power, [Name("open"), open_args]) + read = [Node(syms.trailer, [Dot(), Name('read')]), + Node(syms.trailer, [LParen(), RParen()])] + open_expr = [open_call] + read + # Wrap the open call in a compile call. This is so the filename will be + # preserved in the execed code. + filename_arg = filename.clone() + filename_arg.set_prefix(" ") + exec_str = String("'exec'", " ") + compile_args = open_expr + [Comma(), filename_arg, Comma(), exec_str] + compile_call = Call(Name("compile"), compile_args, "") + # Finally, replace the execfile call with an exec call. + args = [compile_call] if globals is not None: args.extend([Comma(), globals.clone()]) if locals is not None: args.extend([Comma(), locals.clone()]) - return Call(Name("exec"), args, prefix=node.get_prefix()) diff --git a/Lib/lib2to3/fixes/fix_import.py b/Lib/lib2to3/fixes/fix_import.py index c065f705440..4c751337dc4 100644 --- a/Lib/lib2to3/fixes/fix_import.py +++ b/Lib/lib2to3/fixes/fix_import.py @@ -13,55 +13,78 @@ Becomes: # Local imports from .. import fixer_base from os.path import dirname, join, exists, pathsep -from ..fixer_util import FromImport, syms +from ..fixer_util import FromImport, syms, token + + +def traverse_imports(names): + """ + Walks over all the names imported in a dotted_as_names node. + """ + pending = [names] + while pending: + node = pending.pop() + if node.type == token.NAME: + yield node.value + elif node.type == syms.dotted_name: + yield "".join([ch.value for ch in node.children]) + elif node.type == syms.dotted_as_name: + pending.append(node.children[0]) + elif node.type == syms.dotted_as_names: + pending.extend(node.children[::-2]) + else: + raise AssertionError("unkown node type") + class FixImport(fixer_base.BaseFix): PATTERN = """ - import_from< type='from' imp=any 'import' ['('] any [')'] > + import_from< 'from' imp=any 'import' ['('] any [')'] > | - import_name< type='import' imp=any > + import_name< 'import' imp=any > """ def transform(self, node, results): imp = results['imp'] - mod_name = str(imp.children[0] if imp.type == syms.dotted_as_name \ - else imp) - - if str(imp).startswith('.'): - # Already a new-style import - return - - if not probably_a_local_import(str(mod_name), self.filename): - # I guess this is a global import -- skip it! - return - - if results['type'].value == 'from': + if node.type == syms.import_from: # Some imps are top-level (eg: 'import ham') # some are first level (eg: 'import ham.eggs') # some are third level (eg: 'import ham.eggs as spam') # Hence, the loop while not hasattr(imp, 'value'): imp = imp.children[0] - imp.value = "." + imp.value - node.changed() + if self.probably_a_local_import(imp.value): + imp.value = "." + imp.value + imp.changed() + return node else: - new = FromImport('.', getattr(imp, 'content', None) or [imp]) - new.set_prefix(node.get_prefix()) - node = new - return node + have_local = False + have_absolute = False + for mod_name in traverse_imports(imp): + if self.probably_a_local_import(mod_name): + have_local = True + else: + have_absolute = True + if have_absolute: + if have_local: + # We won't handle both sibling and absolute imports in the + # same statement at the moment. + self.warning(node, "absolute and local imports together") + return -def probably_a_local_import(imp_name, file_path): - # Must be stripped because the right space is included by the parser - imp_name = imp_name.split('.', 1)[0].strip() - base_path = dirname(file_path) - base_path = join(base_path, imp_name) - # If there is no __init__.py next to the file its not in a package - # so can't be a relative import. - if not exists(join(dirname(base_path), '__init__.py')): + new = FromImport('.', [imp]) + new.set_prefix(node.get_prefix()) + return new + + def probably_a_local_import(self, imp_name): + imp_name = imp_name.split('.', 1)[0] + base_path = dirname(self.filename) + base_path = join(base_path, imp_name) + # If there is no __init__.py next to the file its not in a package + # so can't be a relative import. + if not exists(join(dirname(base_path), '__init__.py')): + return False + for ext in ['.py', pathsep, '.pyc', '.so', '.sl', '.pyd']: + if exists(base_path + ext): + return True return False - for ext in ['.py', pathsep, '.pyc', '.so', '.sl', '.pyd']: - if exists(base_path + ext): - return True - return False diff --git a/Lib/lib2to3/fixes/fix_imports.py b/Lib/lib2to3/fixes/fix_imports.py index 75770c915f9..98d440698c8 100644 --- a/Lib/lib2to3/fixes/fix_imports.py +++ b/Lib/lib2to3/fixes/fix_imports.py @@ -118,7 +118,7 @@ class FixImports(fixer_base.BaseFix): def transform(self, node, results): import_mod = results.get("module_name") if import_mod: - new_name = self.mapping[(import_mod or mod_name).value] + new_name = self.mapping[import_mod.value] import_mod.replace(Name(new_name, prefix=import_mod.get_prefix())) if "name_import" in results: # If it's not a "from x import x, y" or "import x as y" import, @@ -129,10 +129,8 @@ class FixImports(fixer_base.BaseFix): # line (e.g., "import StringIO, urlparse"). The problem is that I # can't figure out an easy way to make a pattern recognize the # keys of MAPPING randomly sprinkled in an import statement. - while True: - results = self.match(node) - if not results: - break + results = self.match(node) + if results: self.transform(node, results) else: # Replace usage of the module. diff --git a/Lib/lib2to3/fixes/fix_long.py b/Lib/lib2to3/fixes/fix_long.py index 5fd6af55145..873ecf1b9ea 100644 --- a/Lib/lib2to3/fixes/fix_long.py +++ b/Lib/lib2to3/fixes/fix_long.py @@ -5,20 +5,18 @@ """ # Local imports -from .. import pytree from .. import fixer_base -from ..fixer_util import Name, Number +from ..fixer_util import Name, Number, is_probably_builtin class FixLong(fixer_base.BaseFix): PATTERN = "'long'" - static_long = Name("long") static_int = Name("int") def transform(self, node, results): - assert node == self.static_long, node - new = self.static_int.clone() - new.set_prefix(node.get_prefix()) - return new + if is_probably_builtin(node): + new = self.static_int.clone() + new.set_prefix(node.get_prefix()) + return new diff --git a/Lib/lib2to3/main.py b/Lib/lib2to3/main.py index a73149a0854..d37fe9e0f3e 100644 --- a/Lib/lib2to3/main.py +++ b/Lib/lib2to3/main.py @@ -41,7 +41,7 @@ class StdoutRefactoringTool(refactor.RefactoringTool): super(StdoutRefactoringTool, self).write_file(new_text, filename, old_text) if not self.nobackups: - shutil.copymode(filename, backup) + shutil.copymode(backup, filename) def print_output(self, lines): for line in lines: diff --git a/Lib/lib2to3/refactor.py b/Lib/lib2to3/refactor.py index 5e386de5143..c0ba196b74c 100755 --- a/Lib/lib2to3/refactor.py +++ b/Lib/lib2to3/refactor.py @@ -287,17 +287,13 @@ class RefactoringTool(object): Returns: True if the tree was modified, False otherwise. """ - # Two calls to chain are required because pre_order.values() - # will be a list of lists of fixers: - # [[, ], []] - all_fixers = chain(self.pre_order, self.post_order) - for fixer in all_fixers: + for fixer in chain(self.pre_order, self.post_order): fixer.start_tree(tree, name) self.traverse_by(self.pre_order_heads, tree.pre_order()) self.traverse_by(self.post_order_heads, tree.post_order()) - for fixer in all_fixers: + for fixer in chain(self.pre_order, self.post_order): fixer.finish_tree(tree, name) return tree.was_changed diff --git a/Lib/lib2to3/tests/data/py3_test_grammar.py b/Lib/lib2to3/tests/data/py3_test_grammar.py index 9a8846307ec..eadf1db7ab3 100644 --- a/Lib/lib2to3/tests/data/py3_test_grammar.py +++ b/Lib/lib2to3/tests/data/py3_test_grammar.py @@ -485,6 +485,14 @@ class GrammarTests(unittest.TestCase): global a, b global one, two, three, four, five, six, seven, eight, nine, ten + def testNonlocal(self): + # 'nonlocal' NAME (',' NAME)* + x = 0 + y = 0 + def f(): + nonlocal x + nonlocal x, y + def testAssert(self): # assert_stmt: 'assert' test [',' test] assert 1 diff --git a/Lib/lib2to3/tests/test_fixers.py b/Lib/lib2to3/tests/test_fixers.py index 17baaed725e..3dcabf9625c 100755 --- a/Lib/lib2to3/tests/test_fixers.py +++ b/Lib/lib2to3/tests/test_fixers.py @@ -1073,11 +1073,72 @@ class Test_long(FixerTestCase): a = """z = type(x) in (int, int)""" self.check(b, a) + def test_unchanged(self): + s = """long = True""" + self.unchanged(s) + + s = """s.long = True""" + self.unchanged(s) + + s = """def long(): pass""" + self.unchanged(s) + + s = """class long(): pass""" + self.unchanged(s) + + s = """def f(long): pass""" + self.unchanged(s) + + s = """def f(g, long): pass""" + self.unchanged(s) + + s = """def f(x, long=True): pass""" + self.unchanged(s) + def test_prefix_preservation(self): b = """x = long( x )""" a = """x = int( x )""" self.check(b, a) + +class Test_execfile(FixerTestCase): + fixer = "execfile" + + def test_conversion(self): + b = """execfile("fn")""" + a = """exec(compile(open("fn").read(), "fn", 'exec'))""" + self.check(b, a) + + b = """execfile("fn", glob)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), glob)""" + self.check(b, a) + + b = """execfile("fn", glob, loc)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), glob, loc)""" + self.check(b, a) + + b = """execfile("fn", globals=glob)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), globals=glob)""" + self.check(b, a) + + b = """execfile("fn", locals=loc)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), locals=loc)""" + self.check(b, a) + + b = """execfile("fn", globals=glob, locals=loc)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), globals=glob, locals=loc)""" + self.check(b, a) + + def test_spacing(self): + b = """execfile( "fn" )""" + a = """exec(compile(open( "fn" ).read(), "fn", 'exec'))""" + self.check(b, a) + + b = """execfile("fn", globals = glob)""" + a = """exec(compile(open("fn").read(), "fn", 'exec'), globals = glob)""" + self.check(b, a) + + class Test_isinstance(FixerTestCase): fixer = "isinstance" @@ -3466,11 +3527,30 @@ class Test_import(FixerTestCase): a = "from . import foo, bar" self.check_both(b, a) + b = "import foo, bar, x" + a = "from . import foo, bar, x" + self.check_both(b, a) + + b = "import x, y, z" + a = "from . import x, y, z" + self.check_both(b, a) + def test_import_as(self): b = "import foo as x" a = "from . import foo as x" self.check_both(b, a) + b = "import a as b, b as c, c as d" + a = "from . import a as b, b as c, c as d" + self.check_both(b, a) + + def test_local_and_absolute(self): + self.always_exists = False + self.present_files = set(["foo.py", "__init__.py"]) + + s = "import foo, bar" + self.warns_unchanged(s, "absolute and local imports together") + def test_dotted_import(self): b = "import foo.bar" a = "from . import foo.bar"