Repository: google/yapf

Issue:
mistaken added indentation to comment when previous block ended in a comment
The specific issue is the final change, adding a level of indentation of the comment "attached" to the return statement, presumably because the previous block ended with a comment.  While this is flipping where the comment was attached (eg: end of block, then before block), I think that it's worth preserving the original location of that final comment because of the explicit newline, outdent, code at outdent pattern.

``` diff
         except Exception as ex:
             self.logger.exception(
                 "Caught exception while fetching livehoststatus. "
-                "Falling back to old API: %s",
-                ex
+                "Falling back to old API: %s", ex
             )
             # @todo dpittman 2015-09-25: I feel like this is way too broad a
             # catch statement for the failure mode assumption, but it matches
             # the original.  We should probably fix that.

-        # ...guess we try the old fashioned way.
+            # ...guess we try the old fashioned way.
         return self.get_bad_hosts_with_live_maint_status(maint, stage)

     def get_bad_hosts_with_live_host_status(self, maint, stage):
```


Discussion:

Patch:
diff --git a/CHANGELOG b/CHANGELOG
index 270d30c..e90c25b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,8 @@
 ## [0.17.1] UNRELEASED
 ### Fixed
 - Allow semicolons if the line is disabled.
+- Fix issue where subsequent comments at decreasing levels of indentation
+  were improperly aligned and/or caused output with invalid syntax.
 
 ## [0.17.0] 2017-08-20
 ### Added
diff --git a/yapf/yapflib/comment_splicer.py b/yapf/yapflib/comment_splicer.py
index 7c79e80..32997b7 100644
--- a/yapf/yapflib/comment_splicer.py
+++ b/yapf/yapflib/comment_splicer.py
@@ -71,8 +71,7 @@ def SpliceComments(tree):
             # We can't just insert it before the NEWLINE node, because as a
             # result of the way pytrees are organized, this node can be under
             # an inappropriate parent.
-            comment_column -= len(comment_prefix)
-            comment_column += len(comment_prefix) - len(comment_prefix.lstrip())
+            comment_column -= len(comment_prefix.lstrip())
             pytree_utils.InsertNodesAfter(
                 _CreateCommentsFromPrefix(
                     comment_prefix,
@@ -83,63 +82,53 @@ def SpliceComments(tree):
             # Comment prefixes on DEDENT nodes also deserve special treatment,
             # because their final placement depends on their prefix.
             # We'll look for an ancestor of this child with a matching
-            # indentation, and insert the comment after it.
-            ancestor_at_indent = _FindAncestorAtIndent(child, prefix_indent)
-            if ancestor_at_indent.type == token.DEDENT:
-              comments = comment_prefix.split('\n')
-
-              # lib2to3 places comments that should be separated into the same
-              # DEDENT node. For example, "comment 1" and "comment 2" will be
-              # combined.
-              #
-              #   def _():
-              #     for x in y:
-              #       pass
-              #       # comment 1
-              #
-              #     # comment 2
-              #     pass
-              #
-              # In this case, we need to split them up ourselves.
-              before = []
-              after = []
-              after_lineno = comment_lineno
-
-              index = 0
-              while index < len(comments):
-                cmt = comments[index]
-                if not cmt.strip() or cmt.startswith(prefix_indent + '#'):
-                  before.append(cmt)
-                else:
-                  after_lineno += index
-                  after.extend(comments[index:])
-                  break
-                index += 1
-
-              # Special case where the comment is inserted in the same
-              # indentation level as the DEDENT it was originally attached to.
-              pytree_utils.InsertNodesBefore(
-                  _CreateCommentsFromPrefix(
-                      '\n'.join(before) + '\n',
-                      comment_lineno,
-                      comment_column,
-                      standalone=True), ancestor_at_indent)
-              if after:
-                after_column = len(after[0]) - len(after[0].lstrip())
-                comment_column -= comment_column - after_column
-                pytree_utils.InsertNodesAfter(
-                    _CreateCommentsFromPrefix(
-                        '\n'.join(after) + '\n',
-                        after_lineno,
-                        comment_column,
-                        standalone=True), _FindNextAncestor(ancestor_at_indent))
-            else:
-              pytree_utils.InsertNodesAfter(
+            # indentation, and insert the comment before it if the ancestor is
+            # on a DEDENT node and after it otherwise.
+            #
+            # lib2to3 places comments that should be separated into the same
+            # DEDENT node. For example, "comment 1" and "comment 2" will be
+            # combined.
+            #
+            #   def _():
+            #     for x in y:
+            #       pass
+            #       # comment 1
+            #
+            #     # comment 2
+            #     pass
+            #
+            # In this case, we need to split them up ourselves.
+
+            # Split into groups of comments at decreasing levels of indentation
+            comment_groups = []
+            comment_column = None
+            for cmt in comment_prefix.split('\n'):
+              col = cmt.find('#')
+              if col < 0:
+                if comment_column is None:
+                  # Skip empty lines at the top of the first comment group
+                  comment_lineno += 1
+                  continue
+              elif comment_column is None or col < comment_column:
+                comment_column = col
+                comment_indent = cmt[:comment_column]
+                comment_groups.append((comment_column, comment_indent, []))
+              comment_groups[-1][-1].append(cmt)
+
+            # Insert a node for each group
+            for comment_column, comment_indent, comment_group in comment_groups:
+              ancestor_at_indent = _FindAncestorAtIndent(child, comment_indent)
+              if ancestor_at_indent.type == token.DEDENT:
+                InsertNodes = pytree_utils.InsertNodesBefore
+              else:
+                InsertNodes = pytree_utils.InsertNodesAfter
+              InsertNodes(
                   _CreateCommentsFromPrefix(
-                      comment_prefix,
+                      '\n'.join(comment_group) + '\n',
                       comment_lineno,
                       comment_column,
                       standalone=True), ancestor_at_indent)
+              comment_lineno += len(comment_group)
           else:
             # Otherwise there are two cases.
             #
@@ -337,16 +326,6 @@ def _FindAncestorAtIndent(node, indent):
     return _FindAncestorAtIndent(node.parent, indent)
 
 
-def _FindNextAncestor(node):
-  if node.parent is None:
-    return node
-
-  if node.parent.next_sibling is not None:
-    return node.parent.next_sibling
-
-  return _FindNextAncestor(node.parent)
-
-
 def _AnnotateIndents(tree):
   """Annotate the tree with child_indent annotations.
 


Test patch:
diff --git a/yapftests/comment_splicer_test.py b/yapftests/comment_splicer_test.py
index ee0527e..5abe450 100644
--- a/yapftests/comment_splicer_test.py
+++ b/yapftests/comment_splicer_test.py
@@ -206,6 +206,33 @@ class CommentSplicerTest(unittest.TestCase):
     self._AssertNodeIsComment(if_suite.children[-2])
     self._AssertNodeType('DEDENT', if_suite.children[-1])
 
+  def testCommentBeforeDedentThreeLevel(self):
+    code = textwrap.dedent(r'''
+      if foo:
+        if bar:
+          z = 1
+          # comment 2
+        # comment 1
+      # comment 0
+      j = 2
+      ''')
+    tree = pytree_utils.ParseCodeToTree(code)
+    comment_splicer.SpliceComments(tree)
+
+    # comment 0 should go under the tree root
+    self._AssertNodeIsComment(tree.children[1], '# comment 0')
+
+    # comment 1 is in the first if_suite, right before the DEDENT
+    if_suite_1 = self._FindNthChildNamed(tree, 'suite', n=1)
+    self._AssertNodeIsComment(if_suite_1.children[-2], '# comment 1')
+    self._AssertNodeType('DEDENT', if_suite_1.children[-1])
+
+    # comment 2 is in if_suite nested under the first if suite,
+    # right before the DEDENT
+    if_suite_2 = self._FindNthChildNamed(tree, 'suite', n=2)
+    self._AssertNodeIsComment(if_suite_2.children[-2], '# comment 2')
+    self._AssertNodeType('DEDENT', if_suite_2.children[-1])
+
   def testCommentsInClass(self):
     code = textwrap.dedent(r'''
       class Foo:
diff --git a/yapftests/reformatter_buganizer_test.py b/yapftests/reformatter_buganizer_test.py
index 2d0cf9f..996a0a3 100644
--- a/yapftests/reformatter_buganizer_test.py
+++ b/yapftests/reformatter_buganizer_test.py
@@ -501,6 +501,18 @@ class BuganizerFixes(yapf_test_helper.YAPFTest):
     uwlines = yapf_test_helper.ParseAndUnwrap(code)
     self.assertCodeEqual(code, reformatter.Reformat(uwlines))
 
+  def testB30087363(self):
+    code = textwrap.dedent("""\
+        if False:
+          bar()
+          # This is a comment
+        # This is another comment
+        elif True:
+          foo()
+        """)
+    uwlines = yapf_test_helper.ParseAndUnwrap(code)
+    self.assertCodeEqual(code, reformatter.Reformat(uwlines))
+
   def testB29093579(self):
     unformatted_code = textwrap.dedent("""\
         def _():