Repository: google/yapf
Pull request #548 (2018-03-31 02:22:01)

Bug: Does not exclude hidden folders in a relative context
This bug arises when the root directory is specified as a relative path, and when you are trying to exclude an immediate child directory (or file) which starts with a period. The exclusion pattern is fine, but the file path(s) it is being compared against are each over-stripped so that the period is no longer present.

In this example, the exclusion filter has failed to prevent changing `dependency.py`

    user@host:/var/project# file ./.serverless
    ./.serverless: directory

    user@host:/var/project# yapf -vv --recursive --exclude '*/.serverless/*' -i .
    Reformatting ./.serverless/requirements/dependency.py
    [--snip--]

For anyone discovering this PR who has the same problem, a workaround is to use an absolute path to the current directory instead:

    user@host:/var/project# yapf -vv --recursive --exclude '*/.serverless/*' -i `pwd`
    Reformatting /var/project/acme/exceptions.py
    [--snip--]


Patch:
diff --git a/CHANGELOG b/CHANGELOG
index 6970b8d..7dc793c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,8 +19,6 @@
 - Attempt to determine if long lambdas are allowed. This can be done on a
   case-by-case basis with a "pylint" disable comment.
 - A comment before a decorator isn't part of the decorator's line.
-- Only force a new wrapped line after a comment in a decorator when it's the
-  first token in the decorator.
 
 ## [0.21.0] 2018-03-18
 ### Added
diff --git a/yapf/yapflib/blank_line_calculator.py b/yapf/yapflib/blank_line_calculator.py
index c239ee7..0a4fcc2 100644
--- a/yapf/yapflib/blank_line_calculator.py
+++ b/yapf/yapflib/blank_line_calculator.py
@@ -22,6 +22,8 @@ Annotations:
   newlines: The number of newlines required before the node.
 """
 
+from lib2to3 import pytree
+
 from yapf.yapflib import py3compat
 from yapf.yapflib import pytree_utils
 from yapf.yapflib import pytree_visitor
diff --git a/yapf/yapflib/file_resources.py b/yapf/yapflib/file_resources.py
index 5468ced..67eea61 100644
--- a/yapf/yapflib/file_resources.py
+++ b/yapf/yapflib/file_resources.py
@@ -148,7 +148,10 @@ def _FindPythonFiles(filenames, recursive, exclude):
 
 def IsIgnored(path, exclude):
   """Return True if filename matches any patterns in exclude."""
-  return any(fnmatch.fnmatch(path.lstrip('./'), e.rstrip('/')) for e in exclude)
+  path = path.lstrip("/")
+  while path.startswith("./"):
+    path = path[2:]
+  return any(fnmatch.fnmatch(path, e.rstrip('/')) for e in exclude)
 
 
 def IsPythonFile(filename):
diff --git a/yapf/yapflib/identify_container.py b/yapf/yapflib/identify_container.py
index 5c5fc5b..23630c5 100644
--- a/yapf/yapflib/identify_container.py
+++ b/yapf/yapflib/identify_container.py
@@ -19,6 +19,8 @@ to the opening bracket and vice-versa.
   IdentifyContainers(): the main function exported by this module.
 """
 
+from lib2to3 import pytree
+
 from yapf.yapflib import pytree_utils
 from yapf.yapflib import pytree_visitor
 
diff --git a/yapf/yapflib/pytree_unwrapper.py b/yapf/yapflib/pytree_unwrapper.py
index 550bc89..049dbbb 100644
--- a/yapf/yapflib/pytree_unwrapper.py
+++ b/yapf/yapflib/pytree_unwrapper.py
@@ -225,8 +225,7 @@ class PyTreeUnwrapper(pytree_visitor.PyTreeVisitor):
   def Visit_decorator(self, node):  # pylint: disable=invalid-name
     for child in node.children:
       self.Visit(child)
-      if (pytree_utils.NodeName(child) == 'COMMENT' and
-          child == node.children[0]):
+      if pytree_utils.NodeName(child) == 'COMMENT':
         self._StartNewLine()
 
   def Visit_decorators(self, node):  # pylint: disable=invalid-name


Test patch:
diff --git a/yapftests/file_resources_test.py b/yapftests/file_resources_test.py
index 4440e07..5f06e3e 100644
--- a/yapftests/file_resources_test.py
+++ b/yapftests/file_resources_test.py
@@ -14,6 +14,7 @@
 # limitations under the License.
 """Tests for yapf.file_resources."""
 
+import contextlib
 import os
 import shutil
 import tempfile
@@ -26,6 +27,15 @@ from yapf.yapflib import py3compat
 from yapftests import utils
 
 
+@contextlib.contextmanager
+def _restore_working_dir():
+  curdir = os.getcwd()
+  try:
+    yield
+  finally:
+    os.chdir(curdir)
+
+
 class GetDefaultStyleForDirTest(unittest.TestCase):
 
   def setUp(self):
@@ -138,6 +148,68 @@ class GetCommandLineFilesTest(unittest.TestCase):
             os.path.join(tdir2, 'testfile2.py'),
         ]))
 
+  def test_find_with_excluded_hidden_dirs(self):
+    tdir1 = self._make_test_dir('.test1')
+    tdir2 = self._make_test_dir('test_2')
+    tdir3 = self._make_test_dir('test.3')
+    files = [
+        os.path.join(tdir1, 'testfile1.py'),
+        os.path.join(tdir2, 'testfile2.py'),
+        os.path.join(tdir3, 'testfile3.py'),
+    ]
+    _touch_files(files)
+
+    actual = file_resources.GetCommandLineFiles(
+        [self.test_tmpdir], recursive=True, exclude=['*.test1*'])
+
+    self.assertEqual(
+        sorted(actual),
+        sorted([
+            os.path.join(tdir2, 'testfile2.py'),
+            os.path.join(tdir3, 'testfile3.py'),
+        ]))
+
+  def test_find_with_excluded_hidden_dirs_relative(self):
+    """
+    A regression test against a specific case where a hidden directory (one
+    beginning with a period) is being excluded, but it is also an immediate
+    child of the current directory which has been specified in a relative
+    manner.
+
+    At its core, the bug has to do with overzelous stripping of "./foo" so that
+    it removes too much from "./.foo" .
+    """
+    tdir1 = self._make_test_dir('.test1')
+    tdir2 = self._make_test_dir('test_2')
+    tdir3 = self._make_test_dir('test.3')
+    files = [
+        os.path.join(tdir1, 'testfile1.py'),
+        os.path.join(tdir2, 'testfile2.py'),
+        os.path.join(tdir3, 'testfile3.py'),
+    ]
+    _touch_files(files)
+
+    # We must temporarily change the current directory, so that we test against
+    # patterns like ./.test1/file instead of /tmp/foo/.test1/file
+    with _restore_working_dir():
+
+      os.chdir(self.test_tmpdir)
+      actual = file_resources.GetCommandLineFiles(
+          [os.path.relpath(self.test_tmpdir)],
+          recursive=True,
+          exclude=['*.test1*'])
+
+      self.assertEqual(
+          sorted(actual),
+          sorted([
+              os.path.join(
+                  os.path.relpath(self.test_tmpdir), os.path.basename(tdir2),
+                  'testfile2.py'),
+              os.path.join(
+                  os.path.relpath(self.test_tmpdir), os.path.basename(tdir3),
+                  'testfile3.py'),
+          ]))
+
   def test_find_with_excluded_dirs(self):
     tdir1 = self._make_test_dir('test1')
     tdir2 = self._make_test_dir('test2/testinner/')
diff --git a/yapftests/reformatter_buganizer_test.py b/yapftests/reformatter_buganizer_test.py
index 519b0f1..ffb6ded 100644
--- a/yapftests/reformatter_buganizer_test.py
+++ b/yapftests/reformatter_buganizer_test.py
@@ -28,25 +28,7 @@ class BuganizerFixes(yapf_test_helper.YAPFTest):
   def setUpClass(cls):
     style.SetGlobalStyle(style.CreateChromiumStyle())
 
-  def testB77329955(self):
-    code = """\
-class _():
-
-  @parameterized.named_parameters(
-      ('ReadyExpiredSuccess', True, True, True, None, None),
-      ('SpannerUpdateFails', True, False, True, None, None),
-      ('ReadyNotExpired', False, True, True, True, None),
-      # ('ReadyNotExpiredNotHealthy', False, True, True, False, True),
-      # ('ReadyNotExpiredNotHealthyErrorFails', False, True, True, False, False
-      # ('ReadyNotExpiredNotHealthyUpdateFails', False, False, True, False, True
-  )
-  def _():
-    pass
-"""
-    uwlines = yapf_test_helper.ParseAndUnwrap(code)
-    self.assertCodeEqual(code, reformatter.Reformat(uwlines))
-
-  def testB65197969(self):
+  def testB65546221(self):
     unformatted_code = """\
 class _():