diff --git a/slicker/inputs.py b/slicker/inputs.py index cdeb57d..57f608a 100644 --- a/slicker/inputs.py +++ b/slicker/inputs.py @@ -30,15 +30,17 @@ def _expand_and_normalize_one(project_root, old_fullname, new_fullname, - path_filter=khodemod.default_path_filter()): + path_filter=None): """See expand_and_normalize.__doc__.""" def filename_for(mod): return os.path.join(project_root, util.filename_for_module_name(mod)) - def _assert_exists(module, error_prefix): - if not os.path.exists(filename_for(module)): - raise ValueError("%s: %s not found" - % (error_prefix, filename_for(module))) + def _assert_exists_as_file(module, error_prefix): + fname = filename_for(module) + if not os.path.exists(fname): + raise ValueError("%s: %s not found" % (error_prefix, fname)) + if os.path.islink(fname): + raise ValueError("%s: %s is a symlink" % (error_prefix, fname)) def _normalize_fullname_and_get_type(fullname): # Check the cases that fullname is a file or a directory. @@ -70,7 +72,11 @@ def _normalize_fullname_and_get_type(fullname): def _modules_under(package_name): """Yield module-names relative to package_name-root.""" package_dir = os.path.dirname(filename_for(package_name + '.__init__')) - for path in khodemod.resolve_paths(path_filter, root=package_dir): + if path_filter is None: + this_path_filter = khodemod.default_path_filter(package_dir) + else: + this_path_filter = path_filter + for path in khodemod.resolve_paths(this_path_filter, root=package_dir): yield util.module_name_for_filename(path) (old_fullname, old_type) = _normalize_fullname_and_get_type(old_fullname) @@ -85,7 +91,7 @@ def _modules_under(package_name): if old_type == "symbol": (module, symbol) = old_fullname.rsplit('.', 1) - _assert_exists(module, "Cannot move %s" % old_fullname) + _assert_exists_as_file(module, "Cannot move %s" % old_fullname) # TODO(csilvers): check that the 2nd element of the return-value # doesn't refer to a symbol that already exists. @@ -107,7 +113,7 @@ def _modules_under(package_name): yield (old_fullname, '%s.%s' % (new_fullname, symbol), True) elif old_type == "module": - _assert_exists(old_fullname, "Cannot move %s" % old_fullname) + _assert_exists_as_file(old_fullname, "Cannot move %s" % old_fullname) if new_type == "symbol": raise ValueError("Cannot move a module '%s' to a symbol (%s)" % (old_fullname, new_fullname)) @@ -129,8 +135,8 @@ def _modules_under(package_name): yield (old_fullname, new_fullname, False) elif old_type == "package": - _assert_exists(old_fullname + '.__init__', - "Cannot move %s" % old_fullname) + _assert_exists_as_file(old_fullname + '.__init__', + "Cannot move %s" % old_fullname) if new_type in ("symbol", "module"): raise ValueError("Cannot move a package '%s' into a %s (%s)" % (old_fullname, new_type, new_fullname)) @@ -162,7 +168,7 @@ def _modules_under(package_name): def expand_and_normalize(project_root, old_fullnames, new_fullname, - path_filter=khodemod.default_path_filter()): + path_filter=None): """Return a list of old-new-info triples that effect the requested rename. In the simple case old_fullname is a module and new_fullname is a diff --git a/slicker/khodemod.py b/slicker/khodemod.py index 4f5611a..5f9a714 100644 --- a/slicker/khodemod.py +++ b/slicker/khodemod.py @@ -156,6 +156,10 @@ def dotfiles_path_filter(): for part in os.path.split(path)) +def symlink_path_filter(root): + return lambda path: not os.path.islink(os.path.join(root, path)) + + def exclude_paths_filter(exclude_paths): return lambda path: not any(part in exclude_paths for part in path.split(os.path.sep)) @@ -165,12 +169,14 @@ def and_filters(filters): return lambda item: all(f(item) for f in filters) -def default_path_filter(extensions=DEFAULT_EXTENSIONS, +def default_path_filter(root, + extensions=DEFAULT_EXTENSIONS, include_extensionless=False, exclude_paths=DEFAULT_EXCLUDE_PATHS): return and_filters([ extensions_path_filter(extensions, include_extensionless), dotfiles_path_filter(), + symlink_path_filter(root), exclude_paths_filter(exclude_paths), ]) @@ -377,9 +383,10 @@ def run_suggestor_on_files(self, suggestor, filenames, root='.'): for filename in self.progress_bar(filenames): self._run_suggestor_on_file(suggestor, filename, root) - def run_suggestor(self, suggestor, - path_filter=default_path_filter(), root='.'): + def run_suggestor(self, suggestor, path_filter=None, root='.'): """Run the suggestor on all files matching the path_filter.""" + if path_filter is None: + path_filter = default_path_filter(root) self.run_suggestor_on_files( suggestor, resolve_paths(path_filter, root), root) diff --git a/tests/test_inputs.py b/tests/test_inputs.py index cf48806..d8330c9 100644 --- a/tests/test_inputs.py +++ b/tests/test_inputs.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +import os import unittest from slicker import inputs @@ -148,6 +149,26 @@ def test_symbol_to_existing_symbol(self): "'bar' already defines a symbol named 'myfunc'.") self.assert_fails('foo.myfunc', 'bar', error) + def test_symlink_for_file(self): + self.write_file('bar.py', 'def myfunc(): return 4\n') + os.symlink('bar.py', self.join('sym.py')) + error = 'Cannot move sym: %s is a symlink' % self.join('sym.py') + self.assert_fails('sym', 'newdir', error) + + def test_symlink_for_symbol(self): + self.write_file('bar.py', 'def myfunc(): return 4\n') + os.symlink('bar.py', self.join('sym.py')) + error = 'Cannot move sym.myfunc: %s is a symlink' % self.join('sym.py') + self.assert_fails('sym.myfunc', 'newdir', error) + + def test_symlink_for_package(self): + self.write_file('dir_with_symlink/bar.py', 'def myfunc(): return 4\n') + self.write_file('dir_with_symlink/__init__.py', '') + os.symlink('bar.py', self.join('dir_with_symlink/sym.py')) + self._assert('dir_with_symlink', 'newdir', + [('dir_with_symlink.bar', 'newdir.bar', False), + ('dir_with_symlink.__init__', 'newdir.__init__', False)]) + class ManyInputsTest(base.TestBase): def setUp(self): diff --git a/tests/test_khodemod.py b/tests/test_khodemod.py index 27c6ee7..f228601 100644 --- a/tests/test_khodemod.py +++ b/tests/test_khodemod.py @@ -19,13 +19,14 @@ def test_resolve_paths(self): self.assertItemsEqual( khodemod.resolve_paths( - khodemod.default_path_filter(), + khodemod.default_path_filter(self.tmpdir), root=self.tmpdir), ['foo.py', 'bar/baz.py', 'build/qux.py']) self.assertItemsEqual( khodemod.resolve_paths( khodemod.default_path_filter( + self.tmpdir, exclude_paths=('genfiles', 'build')), root=self.tmpdir), ['foo.py', 'bar/baz.py']) @@ -33,6 +34,7 @@ def test_resolve_paths(self): self.assertItemsEqual( khodemod.resolve_paths( khodemod.default_path_filter( + self.tmpdir, extensions=('js', 'css'), include_extensionless=True), root=self.tmpdir), ['foo_extensionless_py', 'foo.js', 'foo.css']) diff --git a/tests/test_slicker.py b/tests/test_slicker.py index 39a5d93..6bff366 100644 --- a/tests/test_slicker.py +++ b/tests/test_slicker.py @@ -337,6 +337,16 @@ def test_repeated_name(self): 'repeated_name', 'foo.foo', 'bar.foo.foo') + def test_ignore_uses_in_symlink(self): + self.create_module('foo') + os.symlink('simple_in.py', self.join('sym.py')) + self.run_test( + 'simple', + 'foo.some_function', 'bar.new_name') + # We shouldn't have rewritten the symlink, so it should still be + # a symlink. + self.assertTrue(os.path.islink(self.join('sym.py'))) + class AliasTest(base.TestBase): def assert_(self, old_module, new_module, alias,