Remove unnecessary parentheses from tuple unpacking in `for` loops (#2945)

diff --git a/CHANGES.md b/CHANGES.md
index d753a24ff772e2dad8f97e44b9a1249b1f01f562..b2325b648f9713634100f9eab01cc3af074d90fc 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -15,6 +15,7 @@
 <!-- Changes that affect Black's preview style -->
 
 - Code cell separators `#%%` are now standardised to `# %%` (#2919)
 <!-- 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_
 - Avoid magic-trailing-comma in single-element subscripts (#2942)
 
 ### _Blackd_

diff --git a/src/black/linegen.py b/src/black/linegen.py
index 5d92011da9ab1c5de432cc3a1f68421b1dd68cf1..cb605ee8be484e1e9db4f004d45444a776753a18 100644 (file)
--- a/src/black/linegen.py
+++ b/src/black/linegen.py
@@ -841,7 +841,11 @@ def normalize_invisible_parens(
 
         if check_lpar:
             if child.type == syms.atom:
 
         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)
                     wrap_in_parentheses(node, child, visible=False)
             elif is_one_tuple(child):
                 wrap_in_parentheses(node, child, visible=True)


@@ -865,7 +869,11 @@ def normalize_invisible_parens(
         check_lpar = isinstance(child, Leaf) and child.value in parens_after
 
 
         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.
     """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.


@@ -873,13 +881,23 @@ def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool:
     Returns whether the node should itself be wrapped in invisible parentheses.
 
     """
     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)
 
     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
 
     ):
         return False
 


@@ -902,7 +920,7 @@ def maybe_make_parens_invisible_in_atom(node: LN, parent: LN) -> bool:
         # make parentheses invisible
         first.value = ""
         last.value = ""
         # 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
 
         if is_atom_with_invisible_parens(middle):
             # Strip the invisible parens from `middle` by replacing

diff --git a/src/black/mode.py b/src/black/mode.py
index 77b1cabfcbcd87895427f186fbecd453eb768ece..6b74c14b6de42063f3c2555a068767a4461cd9bb 100644 (file)
--- a/src/black/mode.py
+++ b/src/black/mode.py
@@ -127,6 +127,7 @@ class Preview(Enum):
     """Individual preview style features."""
 
     string_processing = auto()
     """Individual preview style features."""
 
     string_processing = auto()

+    remove_redundant_parens = auto()

     one_element_subscript = auto()
 
 
     one_element_subscript = auto()
 
 

diff --git a/src/black/trans.py b/src/black/trans.py
index 74d052fe2dcbd0854d11bee16417777e28ed26eb..01aa80eaaf84760f765ae34eef651d7336e28c09 100644 (file)
--- a/src/black/trans.py
+++ b/src/black/trans.py
@@ -365,7 +365,7 @@ class StringMerger(StringTransformer, CustomSplitMapMixin):
 
         is_valid_index = is_valid_index_factory(LL)
 
 
         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)
             if (
                 leaf.type == token.STRING
                 and is_valid_index(i + 1)


@@ -570,7 +570,7 @@ class StringMerger(StringTransformer, CustomSplitMapMixin):
 
         # Build the final line ('new_line') that this method will later return.
         new_line = line.clone()
 
         # 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)
 
             if i == string_idx:
                 new_line.append(string_leaf)
 


@@ -691,7 +691,7 @@ class StringParenStripper(StringTransformer):
 
         is_valid_index = is_valid_index_factory(LL)
 
 
         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
             # Should be a string...
             if leaf.type != token.STRING:
                 continue


@@ -1713,7 +1713,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
         if parent_type(LL[0]) == syms.assert_stmt and LL[0].value == "assert":
             is_valid_index = is_valid_index_factory(LL)
 
         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
                 # We MUST find a comma...
                 if leaf.type == token.COMMA:
                     idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1


@@ -1751,7 +1751,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
         ):
             is_valid_index = is_valid_index_factory(LL)
 
         ):
             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
                 # 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


@@ -1794,7 +1794,7 @@ class StringParenWrapper(BaseStringSplitter, CustomSplitMapMixin):
         if syms.dictsetmaker in [parent_type(LL[0]), parent_type(LL[0].parent)]:
             is_valid_index = is_valid_index_factory(LL)
 
         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
                 # We MUST find a colon...
                 if leaf.type == token.COLON:
                     idx = i + 2 if is_empty_par(LL[i + 1]) else i + 1

diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py
index 36f323e04d6de18e7ca0041200951706c001b695..58ccc4ac0b1613ef8cd6f2eec4cb16e8ef5a8135 100644 (file)
--- a/tests/data/long_strings__regression.py
+++ b/tests/data/long_strings__regression.py
@@ -599,7 +599,7 @@ class A:
 
 
 def foo(xxxx):
 
 
 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}"
         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}"

diff --git a/tests/data/remove_for_brackets.py b/tests/data/remove_for_brackets.py
new file mode 100644 (file)
index 0000000..c8d88ab
--- /dev/null
+++ b/tests/data/remove_for_brackets.py
@@ -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)

diff --git a/tests/test_format.py b/tests/test_format.py
index 4de3170026835b181958f217518ac6116514ee46..b7446853e7b8b1e25f3096b999946441d291bd84 100644 (file)
--- a/tests/test_format.py
+++ b/tests/test_format.py
@@ -80,6 +80,7 @@ PREVIEW_CASES: List[str] = [
     "long_strings__edge_case",
     "long_strings__regression",
     "percent_precedence",
     "long_strings__edge_case",
     "long_strings__regression",
     "percent_precedence",

+    "remove_for_brackets",

     "one_element_subscript",
 ]
 
     "one_element_subscript",
 ]