From 2cf85576d3696177114e980ad3d7a37b24a76529 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 30 Jun 2022 13:42:19 -0700 Subject: [PATCH 1/6] Fix the handling of `# fmt: skip` when it's at a colon line. When the Leaf node with `# fmt: skip` is a NEWLINE inside a `suite` Node, the nodes to ignore should be from the siblings of the parent `suite` Node. There is a also a special case for the ASYNC token, where it expands to the grandparent Node where the ASYNC token is. This fixes #2646, #3126, #2680, #2421, #2339, #2138. --- src/black/comments.py | 59 ++++++++++++++++++++------- src/black/linegen.py | 4 +- tests/data/simple_cases/fmtskip8.py | 62 +++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 tests/data/simple_cases/fmtskip8.py diff --git a/src/black/comments.py b/src/black/comments.py index 23bf87fca7c..10f35ea7026 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -9,7 +9,7 @@ else: from typing_extensions import Final -from blib2to3.pytree import Node, Leaf +from blib2to3.pytree import Node, Leaf, type_repr from blib2to3.pgen2 import token from black.nodes import first_leaf_column, preceding_leaf, container_of @@ -174,6 +174,11 @@ def convert_one_fmt_off_pair(node: Node, *, preview: bool) -> bool: first.prefix = prefix[comment.consumed :] if comment.value in FMT_SKIP: first.prefix = "" + standalone_comment_prefix = prefix + else: + standalone_comment_prefix = ( + prefix[:previous_consumed] + "\n" * comment.newlines + ) hidden_value = "".join(str(n) for n in ignored_nodes) if comment.value in FMT_OFF: hidden_value = comment.value + "\n" + hidden_value @@ -195,7 +200,7 @@ def convert_one_fmt_off_pair(node: Node, *, preview: bool) -> bool: Leaf( STANDALONE_COMMENT, hidden_value, - prefix=prefix[:previous_consumed] + "\n" * comment.newlines, + prefix=standalone_comment_prefix, ), ) return True @@ -214,22 +219,48 @@ def generate_ignored_nodes( container: Optional[LN] = container_of(leaf) if comment.value in FMT_SKIP: prev_sibling = leaf.prev_sibling + parent = leaf.parent # Need to properly format the leaf prefix to compare it to comment.value, # which is also formatted comments = list_comments(leaf.prefix, is_endmarker=False, preview=preview) - if comments and comment.value == comments[0].value and prev_sibling is not None: - leaf.prefix = "" - siblings = [prev_sibling] - while ( - "\n" not in prev_sibling.prefix - and prev_sibling.prev_sibling is not None + if comments and comment.value == comments[0].value: + if prev_sibling is not None: + leaf.prefix = "" + siblings = [prev_sibling] + while ( + "\n" not in prev_sibling.prefix + and prev_sibling.prev_sibling is not None + ): + prev_sibling = prev_sibling.prev_sibling + siblings.insert(0, prev_sibling) + for sibling in siblings: + yield sibling + elif ( + parent is not None + and type_repr(parent.type) == "suite" + and leaf.type == token.NEWLINE ): - prev_sibling = prev_sibling.prev_sibling - siblings.insert(0, prev_sibling) - for sibling in siblings: - yield sibling - elif leaf.parent is not None: - yield leaf.parent + # The `# fmt: skip` is on the colon line of the if/while/def/class/... + # statements. The ignored nodes should be previous siblings of the + # parent suite node. + leaf.prefix = "" + ignored_nodes = [] + parent_sibling = parent.prev_sibling + while ( + parent_sibling is not None + and type_repr(parent_sibling.type) != "suite" + ): + ignored_nodes.insert(0, parent_sibling) + parent_sibling = parent_sibling.prev_sibling + # Sspecial case for `async_stmt` where the ASYNC token is on the grandparent node. + grandparent = parent.parent + if ( + grandparent is not None + and grandparent.prev_sibling is not None + and grandparent.prev_sibling.type == token.ASYNC + ): + ignored_nodes.insert(0, grandparent.prev_sibling) + yield from iter(ignored_nodes) return while container is not None and container.type != token.ENDMARKER: if is_fmt_on(container, preview=preview): diff --git a/src/black/linegen.py b/src/black/linegen.py index ff54e05c4e6..a776f4cfdd3 100644 --- a/src/black/linegen.py +++ b/src/black/linegen.py @@ -220,7 +220,9 @@ def visit_async_stmt(self, node: Node) -> Iterator[Line]: for child in children: yield from self.visit(child) - if child.type == token.ASYNC: + if child.type == token.ASYNC or child.type == STANDALONE_COMMENT: + # STANDALONE_COMMENT happens when `# fmt: skip` is applied on the async + # line. break internal_stmt = next(children) diff --git a/tests/data/simple_cases/fmtskip8.py b/tests/data/simple_cases/fmtskip8.py new file mode 100644 index 00000000000..6d73c4223fd --- /dev/null +++ b/tests/data/simple_cases/fmtskip8.py @@ -0,0 +1,62 @@ +# Make sure a leading comment is not removed. +def some_func( unformatted, args ): # fmt: skip + print("I am some_func") + return 0 + # Make this this comment is not removed. + + +# Make sure a leading comment is not removed. +async def some_async_func( unformatted, args): # fmt: skip + print("I am some_async_func") + await asyncio.sleep(1) + + +# Make sure a leading comment is not removed. +class SomeClass( Unformatted, SuperClasses ): # fmt: skip + def some_method( self, unformatted, args ): # fmt: skip + print("I am some_method") + return 0 + + async def some_async_method( self, unformatted, args ): # fmt: skip + print("I am some_async_method") + await asyncio.sleep(1) + + +# Make sure a leading comment is not removed. +if unformatted_call( args ): # fmt: skip + print("First branch") + # Make sure this is not removed. +elif another_unformatted_call( args ): # fmt: skip + print("Second branch") +else : # fmt: skip + print("Last branch") + + +while some_condition( unformatted, args ): # fmt: skip + print("Do something") + + +for i in some_iter( unformatted, args ): # fmt: skip + print("Do something") + + +async def test_async_for(): + async for i in some_async_iter( unformatted, args ): # fmt: skip + print("Do something") + + +try : # fmt: skip + some_call() +except UnformattedError as ex: # fmt: skip + handle_exception() +finally : # fmt: skip + finally_call() + + +with give_me_context( unformatted, args ): # fmt: skip + print("Do something") + + +async def test_async_with(): + async with give_me_async_context( unformatted, args ): # fmt: skip + print("Do something") From e42a8774aa9274cf0a459b0144c5a48647cb43b9 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 30 Jun 2022 13:43:52 -0700 Subject: [PATCH 2/6] Add an entry to the changelog. --- CHANGES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 1d30045f48b..fbf8b22a3bf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,8 @@ +- Fix incorrect handling of `# fmt: skip` on colon `:` lines. (#3148) + ### Preview style From 94a6ff0193b148d62a4fe15317a77d2a6c011697 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Thu, 30 Jun 2022 13:47:24 -0700 Subject: [PATCH 3/6] Fix lint errors, mypy comments, and typo. --- src/black/comments.py | 5 +++-- tests/data/simple_cases/fmtskip8.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 10f35ea7026..06f64cd8db2 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -244,7 +244,7 @@ def generate_ignored_nodes( # statements. The ignored nodes should be previous siblings of the # parent suite node. leaf.prefix = "" - ignored_nodes = [] + ignored_nodes: List[LN] = [] parent_sibling = parent.prev_sibling while ( parent_sibling is not None @@ -252,7 +252,8 @@ def generate_ignored_nodes( ): ignored_nodes.insert(0, parent_sibling) parent_sibling = parent_sibling.prev_sibling - # Sspecial case for `async_stmt` where the ASYNC token is on the grandparent node. + # Sspecial case for `async_stmt` where the ASYNC token is on the + # grandparent node. grandparent = parent.parent if ( grandparent is not None diff --git a/tests/data/simple_cases/fmtskip8.py b/tests/data/simple_cases/fmtskip8.py index 6d73c4223fd..38e9c2a9f47 100644 --- a/tests/data/simple_cases/fmtskip8.py +++ b/tests/data/simple_cases/fmtskip8.py @@ -2,7 +2,7 @@ def some_func( unformatted, args ): # fmt: skip print("I am some_func") return 0 - # Make this this comment is not removed. + # Make sure this comment is not removed. # Make sure a leading comment is not removed. From 6c9215a11dea8d8f5d658dedf324a801492b220c Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Fri, 1 Jul 2022 11:52:48 -0700 Subject: [PATCH 4/6] Fix typo in the comment. --- src/black/comments.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/black/comments.py b/src/black/comments.py index 06f64cd8db2..37fc131cd03 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -252,7 +252,7 @@ def generate_ignored_nodes( ): ignored_nodes.insert(0, parent_sibling) parent_sibling = parent_sibling.prev_sibling - # Sspecial case for `async_stmt` where the ASYNC token is on the + # Special case for `async_stmt` where the ASYNC token is on the # grandparent node. grandparent = parent.parent if ( From 92b4a4baa9c9554c91a495a961ec16b2a0231df4 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 19 Jul 2022 11:04:04 -0700 Subject: [PATCH 5/6] Extract a separate function to handle the `# fmt: skip` comment. --- src/black/comments.py | 96 +++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 45 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 37fc131cd03..076e4a8f71f 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -216,53 +216,10 @@ def generate_ignored_nodes( If comment is skip, returns leaf only. Stops at the end of the block. """ - container: Optional[LN] = container_of(leaf) if comment.value in FMT_SKIP: - prev_sibling = leaf.prev_sibling - parent = leaf.parent - # Need to properly format the leaf prefix to compare it to comment.value, - # which is also formatted - comments = list_comments(leaf.prefix, is_endmarker=False, preview=preview) - if comments and comment.value == comments[0].value: - if prev_sibling is not None: - leaf.prefix = "" - siblings = [prev_sibling] - while ( - "\n" not in prev_sibling.prefix - and prev_sibling.prev_sibling is not None - ): - prev_sibling = prev_sibling.prev_sibling - siblings.insert(0, prev_sibling) - for sibling in siblings: - yield sibling - elif ( - parent is not None - and type_repr(parent.type) == "suite" - and leaf.type == token.NEWLINE - ): - # The `# fmt: skip` is on the colon line of the if/while/def/class/... - # statements. The ignored nodes should be previous siblings of the - # parent suite node. - leaf.prefix = "" - ignored_nodes: List[LN] = [] - parent_sibling = parent.prev_sibling - while ( - parent_sibling is not None - and type_repr(parent_sibling.type) != "suite" - ): - ignored_nodes.insert(0, parent_sibling) - parent_sibling = parent_sibling.prev_sibling - # Special case for `async_stmt` where the ASYNC token is on the - # grandparent node. - grandparent = parent.parent - if ( - grandparent is not None - and grandparent.prev_sibling is not None - and grandparent.prev_sibling.type == token.ASYNC - ): - ignored_nodes.insert(0, grandparent.prev_sibling) - yield from iter(ignored_nodes) + yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment, preview=preview) return + container: Optional[LN] = container_of(leaf) while container is not None and container.type != token.ENDMARKER: if is_fmt_on(container, preview=preview): return @@ -278,6 +235,55 @@ def generate_ignored_nodes( container = container.next_sibling +def _generate_ignored_nodes_from_fmt_skip( + leaf: Leaf, comment: ProtoComment, *, preview: bool +) -> Iterator[LN]: + """Generate all leaves that should be ignored by the `# fmt: skip` from `leaf`.""" + prev_sibling = leaf.prev_sibling + parent = leaf.parent + # Need to properly format the leaf prefix to compare it to comment.value, + # which is also formatted + comments = list_comments(leaf.prefix, is_endmarker=False, preview=preview) + if comments and comment.value == comments[0].value: + if prev_sibling is not None: + leaf.prefix = "" + siblings = [prev_sibling] + while ( + "\n" not in prev_sibling.prefix + and prev_sibling.prev_sibling is not None + ): + prev_sibling = prev_sibling.prev_sibling + siblings.insert(0, prev_sibling) + for sibling in siblings: + yield sibling + elif ( + parent is not None + and type_repr(parent.type) == "suite" + and leaf.type == token.NEWLINE + ): + # The `# fmt: skip` is on the colon line of the if/while/def/class/... + # statements. The ignored nodes should be previous siblings of the + # parent suite node. + leaf.prefix = "" + ignored_nodes: List[LN] = [] + parent_sibling = parent.prev_sibling + while ( + parent_sibling is not None and type_repr(parent_sibling.type) != "suite" + ): + ignored_nodes.insert(0, parent_sibling) + parent_sibling = parent_sibling.prev_sibling + # Special case for `async_stmt` where the ASYNC token is on the + # grandparent node. + grandparent = parent.parent + if ( + grandparent is not None + and grandparent.prev_sibling is not None + and grandparent.prev_sibling.type == token.ASYNC + ): + ignored_nodes.insert(0, grandparent.prev_sibling) + yield from iter(ignored_nodes) + + def is_fmt_on(container: LN, preview: bool) -> bool: """Determine whether formatting is switched on within a container. Determined by whether the last `# fmt:` comment is `on` or `off`. From 6551e0162629ce03df3152d180cb0ec259c481f4 Mon Sep 17 00:00:00 2001 From: Yilei Yang Date: Tue, 19 Jul 2022 11:10:02 -0700 Subject: [PATCH 6/6] Return early in the _generate_ignored_nodes_from_fmt_skip function. --- src/black/comments.py | 70 ++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/src/black/comments.py b/src/black/comments.py index 076e4a8f71f..522c1a7b88c 100644 --- a/src/black/comments.py +++ b/src/black/comments.py @@ -244,44 +244,40 @@ def _generate_ignored_nodes_from_fmt_skip( # Need to properly format the leaf prefix to compare it to comment.value, # which is also formatted comments = list_comments(leaf.prefix, is_endmarker=False, preview=preview) - if comments and comment.value == comments[0].value: - if prev_sibling is not None: - leaf.prefix = "" - siblings = [prev_sibling] - while ( - "\n" not in prev_sibling.prefix - and prev_sibling.prev_sibling is not None - ): - prev_sibling = prev_sibling.prev_sibling - siblings.insert(0, prev_sibling) - for sibling in siblings: - yield sibling - elif ( - parent is not None - and type_repr(parent.type) == "suite" - and leaf.type == token.NEWLINE + if not comments or comment.value != comments[0].value: + return + if prev_sibling is not None: + leaf.prefix = "" + siblings = [prev_sibling] + while "\n" not in prev_sibling.prefix and prev_sibling.prev_sibling is not None: + prev_sibling = prev_sibling.prev_sibling + siblings.insert(0, prev_sibling) + for sibling in siblings: + yield sibling + elif ( + parent is not None + and type_repr(parent.type) == "suite" + and leaf.type == token.NEWLINE + ): + # The `# fmt: skip` is on the colon line of the if/while/def/class/... + # statements. The ignored nodes should be previous siblings of the + # parent suite node. + leaf.prefix = "" + ignored_nodes: List[LN] = [] + parent_sibling = parent.prev_sibling + while parent_sibling is not None and type_repr(parent_sibling.type) != "suite": + ignored_nodes.insert(0, parent_sibling) + parent_sibling = parent_sibling.prev_sibling + # Special case for `async_stmt` where the ASYNC token is on the + # grandparent node. + grandparent = parent.parent + if ( + grandparent is not None + and grandparent.prev_sibling is not None + and grandparent.prev_sibling.type == token.ASYNC ): - # The `# fmt: skip` is on the colon line of the if/while/def/class/... - # statements. The ignored nodes should be previous siblings of the - # parent suite node. - leaf.prefix = "" - ignored_nodes: List[LN] = [] - parent_sibling = parent.prev_sibling - while ( - parent_sibling is not None and type_repr(parent_sibling.type) != "suite" - ): - ignored_nodes.insert(0, parent_sibling) - parent_sibling = parent_sibling.prev_sibling - # Special case for `async_stmt` where the ASYNC token is on the - # grandparent node. - grandparent = parent.parent - if ( - grandparent is not None - and grandparent.prev_sibling is not None - and grandparent.prev_sibling.type == token.ASYNC - ): - ignored_nodes.insert(0, grandparent.prev_sibling) - yield from iter(ignored_nodes) + ignored_nodes.insert(0, grandparent.prev_sibling) + yield from iter(ignored_nodes) def is_fmt_on(container: LN, preview: bool) -> bool: