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

Strip even more redundant for parentheses #3243

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 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 @@ -28,6 +28,8 @@
subscript expressions with more than 1 element (#3209)
- Fix a string merging/split issue when a comment is present in the middle of implicitly
concatenated strings on its own line (#3227)
- All unnecessary parentheses in `for` assignments are now removed, previously certain
redundant parentheses were still kept (#3243)

### _Blackd_

Expand Down
49 changes: 34 additions & 15 deletions src/black/linegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def visit_funcdef(self, node: Node) -> Iterator[Line]:
if maybe_make_parens_invisible_in_atom(
child,
parent=node,
remove_brackets_around_comma=False,
remove_parens_around_comma=False,
):
wrap_in_parentheses(node, child, visible=False)
else:
Expand Down Expand Up @@ -951,18 +951,14 @@ def normalize_invisible_parens(
if check_lpar:
if (
preview
and child.type == syms.atom
and isinstance(child, Node)
and child.type in (syms.atom, syms.exprlist)
and node.type == syms.for_stmt
and isinstance(child.prev_sibling, Leaf)
and child.prev_sibling.type == token.NAME
and child.prev_sibling.value == "for"
):
if maybe_make_parens_invisible_in_atom(
child,
parent=node,
remove_brackets_around_comma=True,
):
wrap_in_parentheses(node, child, visible=False)
remove_for_target_parens(child, node)
elif preview and isinstance(child, Node) and node.type == syms.with_stmt:
remove_with_parens(child, node)
elif child.type == syms.atom:
Expand Down Expand Up @@ -1015,7 +1011,7 @@ def remove_await_parens(node: Node) -> None:
if maybe_make_parens_invisible_in_atom(
node.children[1],
parent=node,
remove_brackets_around_comma=True,
remove_parens_around_comma=True,
):
wrap_in_parentheses(node, node.children[1], visible=False)

Expand All @@ -1042,6 +1038,28 @@ def remove_await_parens(node: Node) -> None:
remove_await_parens(bracket_contents)


def remove_for_target_parens(node: Node, parent: Node) -> None:
"""Recursively hide optional parens in `for` statements."""
# The main goal is to run `maybe_make_parens_invisible_in_atom` on every atom Node
# between "for" and "in".
if node.type == syms.atom:
# Parenthesized group of nodes/leaves, eg. `(x, y)`
# First try removing the group's surrounding parentheses.
if maybe_make_parens_invisible_in_atom(
node, parent, remove_parens_around_comma=(parent.type == syms.for_stmt)
):
wrap_in_parentheses(parent, node, visible=False)
# Then check if this atom could contain more atoms.
middle = node.children[1]
if isinstance(middle, Node):
remove_for_target_parens(middle, node)
elif node.type in (syms.exprlist, syms.testlist_gexp):
# A series of nodes/leaves separated by commas, eg. `(x), (y)`
for c in node.children:
if isinstance(c, Node) and c.type == syms.atom:
remove_for_target_parens(c, node)


def remove_with_parens(node: Node, parent: Node) -> None:
"""Recursively hide optional parens in `with` statements."""
# Removing all unnecessary parentheses in with statements in one pass is a tad
Expand All @@ -1064,7 +1082,7 @@ def remove_with_parens(node: Node, parent: Node) -> None:
if maybe_make_parens_invisible_in_atom(
node,
parent=parent,
remove_brackets_around_comma=True,
remove_parens_around_comma=True,
):
wrap_in_parentheses(parent, node, visible=False)
if isinstance(node.children[1], Node):
Expand All @@ -1079,15 +1097,16 @@ def remove_with_parens(node: Node, parent: Node) -> None:
if maybe_make_parens_invisible_in_atom(
node.children[0],
parent=node,
remove_brackets_around_comma=True,
remove_parens_around_comma=True,
):
wrap_in_parentheses(node, node.children[0], visible=False)


def maybe_make_parens_invisible_in_atom(
node: LN,
parent: LN,
remove_brackets_around_comma: bool = False,
*,
remove_parens_around_comma: bool = False,
) -> bool:
"""If it's safe, make the parens in the atom `node` invisible, recursively.
Additionally, remove repeated, adjacent invisible parens from the atom `node`
Expand All @@ -1101,10 +1120,10 @@ def maybe_make_parens_invisible_in_atom(
or is_one_tuple(node)
or (is_yield(node) and parent.type != syms.expr_stmt)
or (
# This condition tries to prevent removing non-optional brackets
# This condition tries to prevent removing non-optional parentheses
# around a tuple, however, can be a bit overzealous so we provide
# and option to skip this check for `for` and `with` statements.
not remove_brackets_around_comma
not remove_parens_around_comma
and max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY
)
):
Expand Down Expand Up @@ -1132,7 +1151,7 @@ def maybe_make_parens_invisible_in_atom(
maybe_make_parens_invisible_in_atom(
middle,
parent=parent,
remove_brackets_around_comma=remove_brackets_around_comma,
remove_parens_around_comma=remove_parens_around_comma,
)

if is_atom_with_invisible_parens(middle):
Expand Down
83 changes: 83 additions & 0 deletions tests/data/preview/remove_for_brackets.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
for (((x))) in points:
ichard26 marked this conversation as resolved.
Show resolved Hide resolved
pass

# Only remove tuple brackets after `for`
for (k, v) in d.items():
print(k, v)
Expand All @@ -18,7 +21,47 @@
for (((((k, v))))) in d.items():
print(k, v)

# One-tuple
for (x,) in points:
pass

# A series of atoms that each need their brackets removed
for (x), (y) in points:
pass

# A mix of "simple" atoms and atoms that contain more atoms
for ((x), (y)) in points:
pass

for ((((x)), (((y))))) in points:
pass

# Mixed again; some of these brackets matter.
for ((x), (y)), z in points:
pass

for ((x, y), z) in points:
pass

for ((x, (((y)))), (((z)))) in points:
pass

for (((x,), (y)), ((z)),) in points:
pass

for ((a, b)), ((c, d)) in points:
pass

for ((((a), (b))), (((c), (d)))) in points:
pass

for (a, (b, (c, (d)))) in points:
pass

# output
for x in points:
pass

# Only remove tuple brackets after `for`
for k, v in d.items():
print(k, v)
Expand Down Expand Up @@ -46,3 +89,43 @@
# Test deeply nested brackets
for k, v in d.items():
print(k, v)

# One-tuple
for (x,) in points:
pass

# A series of atoms that each need their brackets removed
for x, y in points:
pass

# A mix of "simple" atoms and atoms that contain more atoms
for x, y in points:
pass

for x, y in points:
pass

# Mixed again; some of these brackets matter.
for (x, y), z in points:
pass

for (x, y), z in points:
pass

for (x, y), z in points:
pass

for (
((x,), y),
z,
) in points:
pass

for (a, b), (c, d) in points:
pass

for (a, b), (c, d) in points:
pass

for a, (b, (c, d)) in points:
pass