Skip to content

Commit

Permalink
Remove unnecessary parentheses from tuple unpacking in for loops (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jpy-git authored Mar 24, 2022
1 parent 3800ebd commit 14e5ce5
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
40 changes: 40 additions & 0 deletions tests/data/remove_for_brackets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# 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

# Brackets remain for long for loop lines
for (why_would_anyone_choose_to_name_a_loop_variable_with_a_name_this_long, i_dont_know_but_we_should_still_check_the_behaviour_if_they_do) in d.items():
print(k, v)

for (k, v) in dfkasdjfldsjflkdsjflkdsjfdslkfjldsjfgkjdshgkljjdsfldgkhsdofudsfudsofajdslkfjdslkfjldisfjdffjsdlkfjdlkjjkdflskadjldkfjsalkfjdasj.items():
print(k, v)

# 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

# Brackets remain for long for loop lines
for (
why_would_anyone_choose_to_name_a_loop_variable_with_a_name_this_long,
i_dont_know_but_we_should_still_check_the_behaviour_if_they_do,
) in d.items():
print(k, v)

for (
k,
v,
) in (
dfkasdjfldsjflkdsjflkdsjfdslkfjldsjfgkjdshgkljjdsfldgkhsdofudsfudsofajdslkfjdslkfjldisfjdffjsdlkfjdlkjjkdflskadjldkfjsalkfjdasj.items()
):
print(k, v)
1 change: 1 addition & 0 deletions tests/test_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"long_strings__edge_case",
"long_strings__regression",
"percent_precedence",
"remove_for_brackets",
"one_element_subscript",
]

Expand Down

0 comments on commit 14e5ce5

Please sign in to comment.