Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the handling of # fmt: skip when it's at a colon line. #3148

Merged
merged 7 commits into from Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -10,6 +10,7 @@

<!-- Changes that affect Black's stable style -->

- Fix incorrect handling of `# fmt: skip` on colon `:` lines. (#3148)
- Comments are no longer deleted when a line had spaces removed around power operators
(#2874)

Expand Down
74 changes: 54 additions & 20 deletions src/black/comments.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -211,26 +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
# 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
):
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
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
Expand All @@ -246,6 +235,51 @@ 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 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
):
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`.
Expand Down
4 changes: 3 additions & 1 deletion src/black/linegen.py
Expand Up @@ -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)
Expand Down
62 changes: 62 additions & 0 deletions 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 sure 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")