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

Remove unnecessary parentheses from tuple unpacking in for loops #2945

Merged
merged 4 commits into from Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -15,6 +15,7 @@
<!-- Changes that affect Black's preview style -->

- Code cell separators `#%%` are now standardised to `# %%` (#2919)
- Remove unnecessary parentheses from tuple unpacking in `for` loops (#2945)
- Avoid magic-trailing-comma in single-element subscripts (#2942)

### _Blackd_
Expand Down
26 changes: 22 additions & 4 deletions src/black/linegen.py
Expand Up @@ -841,7 +841,11 @@ def normalize_invisible_parens(

if check_lpar:
if child.type == syms.atom:
if maybe_make_parens_invisible_in_atom(child, parent=node):
if maybe_make_parens_invisible_in_atom(
child,
parent=node,
preview=preview,
):
wrap_in_parentheses(node, child, visible=False)
elif is_one_tuple(child):
wrap_in_parentheses(node, child, visible=True)
Expand All @@ -865,21 +869,35 @@ def normalize_invisible_parens(
check_lpar = isinstance(child, Leaf) and child.value in parens_after


def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool:
def maybe_make_parens_invisible_in_atom(
node: LN,
parent: LN,
preview: 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`
as they are redundant.

Returns whether the node should itself be wrapped in invisible parentheses.

"""
if (
preview
and parent.type == syms.for_stmt
and isinstance(node.prev_sibling, Leaf)
and node.prev_sibling.type == token.NAME
and node.prev_sibling.value == "for"
):
for_stmt_check = False
else:
for_stmt_check = True

if (
node.type != syms.atom
or is_empty_tuple(node)
or is_one_tuple(node)
or (is_yield(node) and parent.type != syms.expr_stmt)
or max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY
or (max_delimiter_priority_in_atom(node) >= COMMA_PRIORITY and for_stmt_check)
):
return False

Expand All @@ -902,7 +920,7 @@ def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool:
# make parentheses invisible
first.value = ""
last.value = ""
maybe_make_parens_invisible_in_atom(middle, parent=parent)
maybe_make_parens_invisible_in_atom(middle, parent=parent, preview=preview)

if is_atom_with_invisible_parens(middle):
# Strip the invisible parens from `middle` by replacing
Expand Down
1 change: 1 addition & 0 deletions src/black/mode.py
Expand Up @@ -127,6 +127,7 @@ class Preview(Enum):
"""Individual preview style features."""

string_processing = auto()
remove_redundant_parens = auto()
one_element_subscript = auto()


Expand Down
12 changes: 6 additions & 6 deletions src/black/trans.py
Expand Up @@ -365,7 +365,7 @@ def do_match(self, line: Line) -> TMatchResult:

is_valid_index = is_valid_index_factory(LL)

for (i, leaf) in enumerate(LL):
for i, leaf in enumerate(LL):
if (
leaf.type == token.STRING
and is_valid_index(i + 1)
Expand Down Expand Up @@ -570,7 +570,7 @@ def make_naked(string: str, string_prefix: str) -> str:

# Build the final line ('new_line') that this method will later return.
new_line = line.clone()
for (i, leaf) in enumerate(LL):
for i, leaf in enumerate(LL):
if i == string_idx:
new_line.append(string_leaf)

Expand Down Expand Up @@ -691,7 +691,7 @@ def do_match(self, line: Line) -> TMatchResult:

is_valid_index = is_valid_index_factory(LL)

for (idx, leaf) in enumerate(LL):
for idx, leaf in enumerate(LL):
# Should be a string...
if leaf.type != token.STRING:
continue
Expand Down Expand Up @@ -1713,7 +1713,7 @@ def _assert_match(LL: List[Leaf]) -> Optional[int]:
if parent_type(LL[0]) == syms.assert_stmt and LL[0].value == "assert":
is_valid_index = is_valid_index_factory(LL)

for (i, leaf) in enumerate(LL):
for i, leaf in enumerate(LL):
# We MUST find a comma...
if leaf.type == token.COMMA:
idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1
Expand Down Expand Up @@ -1751,7 +1751,7 @@ def _assign_match(LL: List[Leaf]) -> Optional[int]:
):
is_valid_index = is_valid_index_factory(LL)

for (i, leaf) in enumerate(LL):
for i, leaf in enumerate(LL):
# We MUST find either an '=' or '+=' symbol...
if leaf.type in [token.EQUAL, token.PLUSEQUAL]:
idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1
Expand Down Expand Up @@ -1794,7 +1794,7 @@ def _dict_match(LL: List[Leaf]) -> Optional[int]:
if syms.dictsetmaker in [parent_type(LL[0]), parent_type(LL[0].parent)]:
is_valid_index = is_valid_index_factory(LL)

for (i, leaf) in enumerate(LL):
for i, leaf in enumerate(LL):
# We MUST find a colon...
if leaf.type == token.COLON:
idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1
Expand Down
2 changes: 1 addition & 1 deletion tests/data/long_strings__regression.py
Expand Up @@ -599,7 +599,7 @@ def foo():


def foo(xxxx):
for (xxx_xxxx, _xxx_xxx, _xxx_xxxxx, xxx_xxxx) in xxxx:
for xxx_xxxx, _xxx_xxx, _xxx_xxxxx, xxx_xxxx in xxxx:
for xxx in xxx_xxxx:
assert ("x" in xxx) or (xxx in xxx_xxx_xxxxx), (
"{0} xxxxxxx xx {1}, xxx {1} xx xxx xx xxxx xx xxx xxxx: xxx xxxx {2}"
Expand Down
18 changes: 18 additions & 0 deletions tests/data/remove_for_brackets.py
@@ -0,0 +1,18 @@
# Only remove tuple brackets after `for`
for (k, v) in d.items():
jpy-git marked this conversation as resolved.
Show resolved Hide resolved
print(k, v)

# Don't touch tuple brackets after `in`
for module in (core, _unicodefun):
if hasattr(module, "_verify_python3_env"):
module._verify_python3_env = lambda: None

# output
# Only remove tuple brackets after `for`
for k, v in d.items():
print(k, v)

# Don't touch tuple brackets after `in`
for module in (core, _unicodefun):
if hasattr(module, "_verify_python3_env"):
module._verify_python3_env = lambda: None
1 change: 1 addition & 0 deletions tests/test_format.py
Expand Up @@ -80,6 +80,7 @@
"long_strings__edge_case",
"long_strings__regression",
"percent_precedence",
"remove_for_brackets",
"one_element_subscript",
]

Expand Down