Ignore symlinks. You can't move them, and we don't modify them.

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

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)
 

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):

tests/test_khodemod.py

@@ -19,20 +19,22 @@ 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'])
 
         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'])

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,