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 bug where # fmt: skip is not being respected with one-liner functions #4552

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
- Collapse multiple empty lines after an import into one (#4489)
- Prevent `string_processing` and `wrap_long_dict_values_in_parens` from removing
parentheses around long dictionary values (#4377)
- Fix a bug where one-liner functions/conditionals marked with `# fmt: skip`
would still be formatted (#4552)

### Configuration

Expand Down
3 changes: 3 additions & 0 deletions docs/the_black_code_style/future_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Currently, the following features are included in the preview style:
cases)
- `always_one_newline_after_import`: Always force one blank line after import
statements, except when the line after the import is a comment or an import statement
- `fix_fmt_skip_in_one_liners`: Fix `# fmt: skip` behaviour on one-liner declarations,
such as `def foo(): return "mock" # fmt: skip`, where previously the declaration
would have been incorrectly collapsed.

(labels/unstable-features)=

Expand Down
59 changes: 51 additions & 8 deletions src/black/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ def generate_ignored_nodes(
Stops at the end of the block.
"""
if _contains_fmt_skip_comment(comment.value, mode):
yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment)
yield from _generate_ignored_nodes_from_fmt_skip(leaf, comment, mode)
return
container: Optional[LN] = container_of(leaf)
while container is not None and container.type != token.ENDMARKER:
Expand Down Expand Up @@ -313,31 +313,74 @@ def generate_ignored_nodes(


def _generate_ignored_nodes_from_fmt_skip(
leaf: Leaf, comment: ProtoComment
leaf: Leaf, comment: ProtoComment, mode: Mode
) -> Iterator[LN]:
"""Generate all leaves that should be ignored by the `# fmt: skip` from `leaf`."""
prev_sibling = leaf.prev_sibling
parent = leaf.parent
ignored_nodes: list[LN] = []
# 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)
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)
yield from siblings

if Preview.multiline_string_handling not in mode:
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)
yield from siblings
return

# Generates the nodes to be ignored by `fmt: skip`.

# Nodes to ignore are the ones on the same line as the
# `# fmt: skip` comment, excluding the `# fmt: skip`
# node itself.

# Traversal process (starting at the `# fmt: skip` node):
# 1. Move to the `prev_sibling` of the current node.
# 2. If `prev_sibling` has children, go to its rightmost leaf.
# 3. If there’s no `prev_sibling`, move up to the parent
# node and repeat.
# 4. Continue until:
# a. You encounter an `INDENT` or `NEWLINE` node (indicates
# start of the line).
# b. You reach the root node.

# Include all visited LEAVES in the ignored list, except INDENT
# or NEWLINE leaves.

current_node = prev_sibling
ignored_nodes = [current_node]
if current_node.prev_sibling is None and current_node.parent is not None:
current_node = current_node.parent
while "\n" not in current_node.prefix and current_node.prev_sibling is not None:
leaf_nodes = list(current_node.prev_sibling.leaves())
current_node = leaf_nodes[-1] if leaf_nodes else current_node

if current_node.type in (token.NEWLINE, token.INDENT):
current_node.prefix = ""
break

ignored_nodes.insert(0, current_node)

if current_node.prev_sibling is None and current_node.parent is not None:
current_node = current_node.parent
yield from ignored_nodes
elif (
parent is not None and parent.type == syms.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 parent_sibling.type != syms.suite:
ignored_nodes.insert(0, parent_sibling)
Expand Down
1 change: 1 addition & 0 deletions src/black/mode.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class Preview(Enum):
remove_lone_list_item_parens = auto()
pep646_typed_star_arg_type_var_tuple = auto()
always_one_newline_after_import = auto()
fix_fmt_skip_in_one_liners = auto()


UNSTABLE_FEATURES: set[Preview] = {
Expand Down
3 changes: 2 additions & 1 deletion src/black/resources/black.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@
"parens_for_long_if_clauses_in_case_block",
"remove_lone_list_item_parens",
"pep646_typed_star_arg_type_var_tuple",
"always_one_newline_after_import"
"always_one_newline_after_import",
"fix_fmt_skip_in_one_liners"
]
},
"description": "Enable specific features included in the `--unstable` style. Requires `--preview`. No compatibility guarantees are provided on the behavior or existence of any unstable features."
Expand Down
9 changes: 9 additions & 0 deletions tests/data/cases/fmtskip10.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# flags: --unstable
def foo(): return "mock" # fmt: skip
if True: print("yay") # fmt: skip
for i in range(10): print(i) # fmt: skip

j = 1 # fmt: skip
while j < 10: j += 1 # fmt: skip

b = [c for c in "A very long string that would normally generate some kind of collapse, since it is this long"] # fmt: skip